Provide relay node info for outbound packets#10767
Conversation
@alecperkins, Welcome to Meshtastic!Thanks for opening your first pull request. We really appreciate it. We discuss work as a team in discord, please join us in the #firmware channel. Welcome to the team 😄 |
github-actions
Bot
added
first-contribution
needs-review
There was a problem hiding this comment.
Pull request overview
This PR extends the routing/phone notification path so clients can learn which nodes relayed a packet (including subsequent relays), by attaching relay identity + link metrics to implicit ACKs for outbound packets and by forwarding relay-info-only duplicates to the phone.
Changes:
- Extend ACK/NAK allocation/sending APIs to optionally carry relay metadata from an observed packet.
- Generate implicit ACK notifications for each overheard rebroadcast of our outbound packets, carrying the relayer + RSSI/SNR + hop fields.
- When a duplicate incoming packet is ignored (not rebroadcast), forward a decoded copy to the phone so it can learn additional relayers.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/modules/RoutingModule.h | Extends sendAckNak/allocAckNak to accept an optional relay-source packet. |
| src/modules/RoutingModule.cpp | Threads the new relaySource parameter through to MeshModule::allocAckNak(). |
| src/mesh/Router.h | Extends sendAckNak signature and adds notifyPhoneOfRelayDupe() API for relay-only dupe forwarding. |
| src/mesh/Router.cpp | Implements relay-dupe forwarding by copying, decoding, and sending the packet to the phone. |
| src/mesh/ReliableRouter.cpp | Emits implicit-ack notifications on every overheard rebroadcast of our outbound packets (including later relays). |
| src/mesh/NextHopRouter.cpp | Calls notifyPhoneOfRelayDupe() on duplicate packets so the phone can learn additional relayers. |
| src/mesh/MeshModule.h | Extends allocAckNak() to accept an optional relay-source packet. |
| src/mesh/MeshModule.cpp | Copies relay identity + RSSI/SNR + hop fields into the locally-delivered implicit ACK when relaySource is provided. |
| src/mesh/FloodingRouter.cpp | Calls notifyPhoneOfRelayDupe() on duplicate packets so the phone can learn additional relayers. |
| // Use the channel of our original pending packet when we still have it. Otherwise (a plain | ||
| // broadcast with no pending retransmission, or a subsequent relayer after we already cleared it) | ||
| // the overheard copy only carries the channel hash, not a usable index, so fall back to primary. | ||
| ChannelIndex ackChannel = old ? old->packet->channel : channels.getPrimaryIndex(); |
|
Not sure I love this approach, sending the same packet over the phone api multiple times seems like a problematic pattern, especially for diagnostic only data. |
|
Is there a better way to provide it to the phone? It's info the node already has, that clients may find useful or can ignore if they don't. I wouldn't call it diagnostic exactly, more a revelation of how the mesh functions at a fundamental level. "Who is hearing me?" and "How am I hearing this?" are useful information for users as they move about, or as the mesh changes around them. And it's a FAQ from those getting set up that's often hard to answer. The apps even already seem to expect the outbound relay, it just wasn't working before. Edit: and traceroutes are a special case where the packets are not duplicates. Each transmission carries different and useful information. Path diversity is a critical concept for making a well-functioning mesh, so it's helpful to illustrate it rather than obscure it. |
garthvh
left a comment
garthvh
left a comment
There was a problem hiding this comment.
The need to immediately patch every client to prevent duplicate messages is a no go, this needs to be rethought. I would suggest that this is maybe a routing packet.
@garthvh without the dupe notifications, is the addition of relay characteristics to the ack message acceptable? Being able to known outbound relays is the more important and more frequent feature request. |
|
We can't send duplicate messages, that approach is not going to work. |
When we overhear another node rebroadcast one of our own packets, we generate an implicit-ack ROUTING packet for the local sending process. That ack is delivered locally to the phone, so pass the overheard rebroadcast as a relay source and copy its relay_node and the rx_rssi / rx_snr we heard it at onto the ack. This lets the connected client see which node relayed our packet and the link quality, instead of only learning that the packet was repeated. allocAckNak / sendAckNak gain an optional relaySource parameter. The ack is sent to ourselves (to == us), so Router::send() is bypassed and does not overwrite these fields with our own. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
📝 WalkthroughWalkthroughAdds an optional ChangesRelay source metadata in ACK/NAK packets
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Comment |
alecperkins
changed the title
Right, I mean the addition of the relay info to the existing implicit ack notice, so the clients can know who gave them the implicit ack. I've stripped out the duplicate portion so this change is only that addition: 6555721. Though I will note clients are already expecting to be notified about duplicates, because it’s a reasonable assumption, and already dedupe by mesh packet ID. Some of them even are relying on this in their UI, so arguably the current firmware behavior is defective from their perspective. For example, the stock Android and Apple apps connected to a node running the now-removed changes: 
Versus the stock Apple and 2.7.15 fw: 
Even though that packet was definitely RX'd at this location multiple times: 
The only change the Apple app needed for the multiple notices was a fix to prevent duplicate push notifications, it otherwise already tolerated and assumed it might hear the same packet repeatedly. |
GUVWAF
left a comment
GUVWAF
left a comment
There was a problem hiding this comment.
The current implementation without duplicated packets looks fine to me.
Some of them even are relying on this in their UI, so arguably the current firmware behavior is defective from their perspective.
This is true, currently you only get the implicit ACK when it's still in the list of "pending packets" (i.e. retransmissions), where it will be deleted after the first rebroadcast is overheard:
firmware/src/mesh/ReliableRouter.cpp
Lines 52 to 54 in 1e7cf4e
This provides the relay info in the implicit ack response to phones so the phone can know who relayed the node's own outbound packets.
Apps can then use this to show more details about which infrastructure is servicing the user’s node.
The stock 2.7.15 (2074) meshtastic/Meshtastic-Apple app, for instance:
Client compatibility
🤝 Attestations