Reprocess repeated packets and deduplicate logic#8216
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the packet routing logic to unify the handling of upgraded packets and rebroadcasting between FloodingRouter and NextHopRouter, eliminating code duplication while extending reprocessing logic to repeated packets for improved traceroute functionality.
- Consolidates duplicate packet upgrade handling logic into shared methods
- Refactors NextHopRouter to use the same rebroadcasting interface as FloodingRouter
- Extends packet reprocessing to repeated packets to fix traceroute "unknown" node issues
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/mesh/PacketHistory.cpp | Removes unnecessary seenRecently = false assignment for upgraded packets |
| src/mesh/NextHopRouter.h | Changes method signature from perhapsRelay() to perhapsRebroadcast() with override |
| src/mesh/NextHopRouter.cpp | Refactors packet handling to use shared upgrade logic and adds reprocessing for repeated packets |
| src/mesh/FloodingRouter.h | Makes perhapsRebroadcast() pure virtual and adds new shared methods for packet handling |
| src/mesh/FloodingRouter.cpp | Extracts shared upgrade handling logic and removes duplicate rebroadcast implementation |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
thebentern
approved these changes
Oct 5, 2025
8 tasks
3 tasks
jeek
pushed a commit
to jeek/Meshtastic-Exploiteers-Hacker-Pager
that referenced
this pull request
Jun 30, 2026
Reprocess repeated packets and deduplicate logic
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This unifies the logic for upgraded packets and rebroadcasting to avoid duplication.
@h3lix1 I created a new
perhapsHandleUpgradedPacket()method. Instead of settingwasSeenRecentlyagain when we did not remove the pending Tx packet, I just removed thewasSeenRecently = falsefrom the PacketHistory, as we don't need it, because we would already return before that is checked.Originally NextHopRouter had a
perhapsRelay()method that was almost the same asperhapsRebroadcast(), except that it needed to check whether the next-hop was set to us. Since for the FloodingRouter in this case it'sNO_NEXT_HOP_PREFERENCE, we can use the same logic, so in FloodingRouter its now a pure virtual function. Then when sending, it either callsFloodingRouter::send()orNextHopRouter::send().Furthermore, with this, we apply the "reprocessing" logic also to repeated packets (when the same node retransmits), in order to update the traceroute result also. Currently, this would result in "unknown" nodes (
0xffffff).