#bugfix phantom/false "Direct Connections" and 0 SNR/RSSI node list corru… - #11208
#bugfix phantom/false "Direct Connections" and 0 SNR/RSSI node list corru…#11208Folex-Fire wants to merge 2 commits into
Conversation
…ption caused by stripped sync/forwarded payloads
Problem Description / Findings: In Meshtastic firmware versions 2.8+, an issue occurs where remote nodes are incorrectly classified as 0-hop "Direct Connections" with 0 SNR and 0 RSSI in the node list, signal graph, and apps. This corrupts local mesh topology tracking.
Root Cause:This behavior is triggered by historical database syncs or specific store-and-forward position payloads received over the air (TRANSPORT_LORA) where via_mqtt is evaluated as false. Because these are stored data records rather than real-time over-the-air origin transmissions, their physical hop and radio headers are stripped or uninitialized. When NodeDB.cpp processes these packets getHopsAway(mp) evaluates mathematically to 0. The logic block if (hopsAway >= 0) accepts the 0 value without verifying physical hardware metrics.The UI and database interpret 0 hops as a direct neighbor, overwriting healthy historical RF signal stats with absolute 0 placeholders, and displays nodes that are not directly connected as a "Good" 0 RSSI 0 SNR direct connection.
The fix enforces a physical validation rule in NodeDB.cpp: a packet can only be evaluated as a 0-hop (Direct) neighbor if the local radio hardware actually measured an active RF signal during arrival. If hopsAway == 0 but both rx_snr and rx_rssi are exactly 0, the packet is flagged as a phantom sync payload and blocked from registering as a local neighbor.
Code Modification Applied (in NodeDB.cpp lines 3500 - 3520):
// If hopStart was set and there wasn't someone messing with the limit in the middle, add hopsAway
const int8_t hopsAway = getHopsAway(mp);
// Gating rule: 0-hop payloads must have real hardware metrics to be treated as a direct link.
// Prevent uninitialized historical sync / forwarded payloads from pretending to be local neighbors.
bool is_phantom_packet = (hopsAway == 0 && mp.rx_snr == 0.0f && mp.rx_rssi == 0);
if (hopsAway >= 0 && !is_phantom_packet) {
info->has_hops_away = true;
info->hops_away = hopsAway;
} else if (is_phantom_packet) {
info->has_hops_away = false; // Protect topology; drop direct neighbor flag
}
sortMeshDB();
Impact: Preserves Mesh Map Integrity: Stops phantom nodes from hijacking the direct neighbor list and signal graphs.Safe for Low Signals: Genuine weak packets still carry real hardware register readings (e.g., negative RSSI/SNR) and remain perfectly tracked, while mathematical absolute zeros from uninitialized structs are safely isolated.
assisted by Google Gemini
📝 WalkthroughWalkthrough
ChangesNodeDB topology update
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested labels: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/mesh/NodeDB.cpp`:
- Around line 3503-3514: Move the phantom-packet predicate calculation before
the samplePacketForHistogram() call, then skip histogram sampling when the
packet is phantom. Reuse the existing hopsAway value and predicate in the later
has_hops_away gating logic, avoiding duplicate calculations while preserving
sampling for valid packets.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 3a8d77d8-9ea0-42dc-9f53-dffd22372285
📒 Files selected for processing (1)
src/mesh/NodeDB.cpp
| // If hopStart was set and there wasn't someone messing with the limit in the middle, add hopsAway | ||
| const int8_t hopsAway = getHopsAway(mp); | ||
| if (hopsAway >= 0) { | ||
|
|
||
| // Gating rule: 0-hop payloads must have real hardware metrics to be treated as a direct link. | ||
| // Prevent uninitialized historical sync / forwarded payloads from pretending to be local neighbors. | ||
| bool is_phantom_packet = (hopsAway == 0 && mp.rx_snr == 0.0f && mp.rx_rssi == 0); | ||
|
|
||
| if (hopsAway >= 0 && !is_phantom_packet) { | ||
| info->has_hops_away = true; | ||
| info->hops_away = hopsAway; | ||
| } else if (is_phantom_packet) { | ||
| info->has_hops_away = false; // Protect topology; drop direct neighbor flag |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Apply the phantom filter before hop-scaling sampling.
At Lines 3496-3499, samplePacketForHistogram() runs before this gate. Phantom packets therefore contribute a hop-0 sample and can skew variable-hop recommendations even though has_hops_away is later cleared. Compute the predicate earlier and skip histogram sampling for phantom packets, reusing hopsAway.
Proposed fix
+ const int8_t hopsAway = getHopsAway(mp);
+ const bool isPhantomPacket = (hopsAway == 0 && mp.rx_snr == 0.0f && mp.rx_rssi == 0);
`#if` HAS_VARIABLE_HOPS
if (mp.transport_mechanism == meshtastic_MeshPacket_TransportMechanism_TRANSPORT_LORA && !mp.via_mqtt &&
- hopScalingModule) {
- uint8_t hopCount = std::max(int8_t(0), getHopsAway(mp));
+ hopScalingModule && !isPhantomPacket) {
+ uint8_t hopCount = std::max(int8_t(0), hopsAway);
hopScalingModule->samplePacketForHistogram(mp.from, hopCount);
}
`#endif`
- const int8_t hopsAway = getHopsAway(mp);
- bool is_phantom_packet = (hopsAway == 0 && mp.rx_snr == 0.0f && mp.rx_rssi == 0);
-
- if (hopsAway >= 0 && !is_phantom_packet) {
+ if (hopsAway >= 0 && !isPhantomPacket) {
info->has_hops_away = true;
info->hops_away = hopsAway;
- } else if (is_phantom_packet) {
+ } else if (isPhantomPacket) {
info->has_hops_away = false;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // If hopStart was set and there wasn't someone messing with the limit in the middle, add hopsAway | |
| const int8_t hopsAway = getHopsAway(mp); | |
| if (hopsAway >= 0) { | |
| // Gating rule: 0-hop payloads must have real hardware metrics to be treated as a direct link. | |
| // Prevent uninitialized historical sync / forwarded payloads from pretending to be local neighbors. | |
| bool is_phantom_packet = (hopsAway == 0 && mp.rx_snr == 0.0f && mp.rx_rssi == 0); | |
| if (hopsAway >= 0 && !is_phantom_packet) { | |
| info->has_hops_away = true; | |
| info->hops_away = hopsAway; | |
| } else if (is_phantom_packet) { | |
| info->has_hops_away = false; // Protect topology; drop direct neighbor flag | |
| const int8_t hopsAway = getHopsAway(mp); | |
| const bool isPhantomPacket = (hopsAway == 0 && mp.rx_snr == 0.0f && mp.rx_rssi == 0); | |
| `#if` HAS_VARIABLE_HOPS | |
| if (mp.transport_mechanism == meshtastic_MeshPacket_TransportMechanism_TRANSPORT_LORA && !mp.via_mqtt && | |
| hopScalingModule && !isPhantomPacket) { | |
| uint8_t hopCount = std::max(int8_t(0), hopsAway); | |
| hopScalingModule->samplePacketForHistogram(mp.from, hopCount); | |
| } | |
| `#endif` | |
| // If hopStart was set and there wasn't someone messing with the limit in the middle, add hopsAway | |
| if (hopsAway >= 0 && !isPhantomPacket) { | |
| info->has_hops_away = true; | |
| info->hops_away = hopsAway; | |
| } else if (isPhantomPacket) { | |
| info->has_hops_away = false; // Protect topology; drop direct neighbor flag |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/mesh/NodeDB.cpp` around lines 3503 - 3514, Move the phantom-packet
predicate calculation before the samplePacketForHistogram() call, then skip
histogram sampling when the packet is phantom. Reuse the existing hopsAway value
and predicate in the later has_hops_away gating logic, avoiding duplicate
calculations while preserving sampling for valid packets.
There was a problem hiding this comment.
@NomDeTom, glad this helped. Filtering before histogram sampling will keep phantom zero-hop records from affecting both direct-neighbor topology and variable-hop recommendations.
🐇👍
There was a problem hiding this comment.
I will test out the coderabitai suggestion and report back the results
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
@Folex-Fire, 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 😄 |
Formatting
Folex-Fire
changed the title
|
Immediately after updating to 2.8 a few weeks ago I noticed this. I updated to the new Android pre release as well and thought initially it was an app issue. Later I went back to the stable version of the app and it still persisted. My situation seems to be a perfect storm for this oddball . After digging through packets , and mqqt traffic on the local mesh, What I have deduced is happening is : mostly position packets are sent initially from node A over lora, then node B sends it out over mqtt with 0 rssi 0 snr 0 hop and since pretty much my only node in range is node C with up and down kink, it is broadcasting those packets to me as a mqtt= false 0 snr 0 rssi 0 hop packets, the original code would interpret that as a direct connection node and list these in my nodedb and show a bunch of 0 rasi signals on the graph. After the new firmware with the proposed change I have no issues with this. I did not want to wait forever for the node list to eventually overwrite as nodes become aged , so I did have to reset my node DB, to see immediate results. When doing so I preserved the 1 local node and one non local. To verify the fix, the local node is reporting signal normally again, and the distant non local node is still reporting 0 0, but I have not received any update info packets from it yet. 
|
Folex-Fire
changed the title
Summary: 0 SNR/RSSI "phantom direct connection" investigationStarted as one bug report (nodes showing 0 SNR / 0 RSSI while flagged as a direct/0-hop Issue 1:
|
|
@Folex-Fire it's never simple, is it? |
Nope, never is! Especially when it initially looks or sounds simple! I Mostly deal with PostgreSQL and Java still getting used to C So far adding looks to be working for the position packets but its too early to tell since i have to sit and wait for them. at least i have 2 nodes going and 1 with serial logging going to help see what is happening. got 2 packets so far that should have caused the issue and didn't on my v3, but we will see. one thing at a time. |
|
hahahah you know what(a big part of) it is?! its the phone API.cpp that was changed so that packets replay as TRANSOPRT_LORA with 0 for all the info because they wanted to be able to load historical position data on re-connection to the phone. so on android it sees all these as 0 hop 0 snr 0 rssi direct nodes and displays them as such. What the heck! wow! |
|
Hmmm... worth having a look at #11014 - replayed packets are given the same IDs, so at least they're deduped after the first go-around. |
|
The DB does have error checking, and relatively corruption/powerloss resistant methods. We just need to fix this issue of them ending up as phantom links. |
3 participants
…ption caused by stripped sync/forwarded payloads
Problem Description / Findings: In Meshtastic firmware versions 2.8+, an issue occurs where remote nodes are incorrectly classified as 0-hop "Direct Connections" with 0 SNR and 0 RSSI in the node list, signal graph, and apps. This corrupts local mesh topology tracking.
Root Cause:This behavior is triggered by historical database syncs or specific store-and-forward position payloads received over the air (TRANSPORT_LORA) where via_mqtt is evaluated as false. Because these are stored data records rather than real-time over-the-air origin transmissions, their physical hop and radio headers are stripped or uninitialized. When NodeDB.cpp processes these packets getHopsAway(mp) evaluates mathematically to 0. The logic block if (hopsAway >= 0) accepts the 0 value without verifying physical hardware metrics.The UI and database interpret 0 hops as a direct neighbor, overwriting healthy historical RF signal stats with absolute 0 placeholders, and displays nodes that are not directly connected as a "Good" 0 RSSI 0 SNR direct connection.
The fix enforces a physical validation rule in NodeDB.cpp: a packet can only be evaluated as a 0-hop (Direct) neighbor if the local radio hardware actually measured an active RF signal during arrival. If hopsAway == 0 but both rx_snr and rx_rssi are exactly 0, the packet is flagged as a phantom sync payload and blocked from registering as a local neighbor.
Code Modification Applied (in NodeDB.cpp lines 3500 - 3520):
Impact: Preserves Mesh Map Integrity: Stops phantom nodes from hijacking the direct neighbor list and signal graphs.Safe for Low Signals: Genuine weak packets still carry real hardware register readings (e.g., negative RSSI/SNR) and remain perfectly tracked, while mathematical absolute zeros from uninitialized structs are safely isolated.
assisted by Google Gemini
rak3401-1watt
Summary by CodeRabbit
Summary by CodeRabbit