Fix replayed satellite-DB packets masquerading as fresh live traffic - #11014
Conversation
📝 WalkthroughWalkthroughPhoneAPI replay packet construction now derives stable identifiers from node records and computes hop fields from stored node information for Position, Telemetry, Environment, and Status packets. ChangesReplay packet metadata
Estimated code review effort: 3 (Moderate) | ~15–30 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Warning Tools execution failed with the following error: Failed to run tools: 13 INTERNAL: Received RST_STREAM with code 2 (Internal server error) 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 |
⚡ Try this PR in the Web Flasher
Warning This is an automated, unreviewed CI test build. Back up your device configuration Supported boards built by this PR (28)
Build artifacts expire on 2026-08-14. Updated for |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (3)
src/mesh/PhoneAPI.cpp (3)
1241-1242: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRemove the redundant comment.
This comment restates what the subsequent
makeReplayPacketIdcall obviously does, which violates the project's coding guidelines. As per coding guidelines: "Keep code comments minimal... only when the reason is not obvious... or restate what the next line does."♻️ Proposed refactor
- // Stable per-node/per-fix id: replaying the same unchanged history on every - // reconnect must not look like a brand new packet to the phone's history/dedup.🤖 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/PhoneAPI.cpp` around lines 1241 - 1242, Remove the redundant comment immediately preceding the makeReplayPacketId call in the replay packet handling code, leaving the call and surrounding implementation unchanged.Source: Coding guidelines
1206-1211: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCondense the multi-line explanatory comment.
This 6-line comment violates the project's coding guidelines. As per coding guidelines: "Keep code comments minimal: use at most one or two lines, only when the reason is not obvious; do not add multi-paragraph explanatory comments".
♻️ Proposed refactor
-/// Derive a stable id for a replayed satellite-DB record. Unchanged history replayed on -/// every reconnect must not look like a new packet to the phone's history/dedup - the id -/// only changes when the underlying data (its timestamp) actually changes. -// `kind` only needs to distinguish the record types that can otherwise collide (e.g. device -// vs. environment metrics both replay as TELEMETRY_APP with the same node/last_heard) - pass -// the payload's own variant/port constant. +/// Derive a stable id for a replayed record based on node, timestamp, and kind.🤖 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/PhoneAPI.cpp` around lines 1206 - 1211, Condense the explanatory comment above the stable replayed-record ID logic into at most one or two lines, retaining only the essential purpose: the ID remains stable across unchanged replays and uses the payload variant to distinguish record types.Source: Coding guidelines
1233-1235: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCondense the multi-line explanatory comment.
This 3-line comment violates the project's coding guidelines. As per coding guidelines: "Keep code comments minimal: use at most one or two lines, only when the reason is not obvious; do not add multi-paragraph explanatory comments".
♻️ Proposed refactor
- // Shape this exactly like a fresh live broadcast Position from the peer so the - // phone runs it through its normal "live position broadcast" handler path. - // to=ourNum would read as a DM-from-peer and never lands in node detail UI. + // Shape as a fresh broadcast so the phone routes it to the node detail UI.🤖 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/PhoneAPI.cpp` around lines 1233 - 1235, Condense the three-line comment in the live broadcast position handling near the shown code into one or two concise lines, retaining only the essential reason for shaping the message like a peer’s fresh live broadcast and avoiding DM routing.Source: Coding guidelines
🤖 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.
Nitpick comments:
In `@src/mesh/PhoneAPI.cpp`:
- Around line 1241-1242: Remove the redundant comment immediately preceding the
makeReplayPacketId call in the replay packet handling code, leaving the call and
surrounding implementation unchanged.
- Around line 1206-1211: Condense the explanatory comment above the stable
replayed-record ID logic into at most one or two lines, retaining only the
essential purpose: the ID remains stable across unchanged replays and uses the
payload variant to distinguish record types.
- Around line 1233-1235: Condense the three-line comment in the live broadcast
position handling near the shown code into one or two concise lines, retaining
only the essential reason for shaping the message like a peer’s fresh live
broadcast and avoiding DM routing.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 3f6c1c7c-c20f-43cc-b0ee-fdfb475fe86f
📒 Files selected for processing (1)
src/mesh/PhoneAPI.cpp
There was a problem hiding this comment.
Pull request overview
This PR adjusts how PhoneAPI replays satellite-DB history packets (position/telemetry/environment/status) to connected clients so they no longer appear as newly received over-the-air traffic on every reconnect. It does this by stabilizing replay packet IDs and by populating link metadata (SNR and hop fields) using the node’s last-known RF characteristics.
Changes:
- Introduces a deterministic replay packet ID generator (
makeReplayPacketId) so unchanged replayed history deduplicates cleanly across reconnects. - Populates
rx_snrand deriveshop_start/hop_limitfromNodeInfoLite(viasetReplayHopFields) to preserve realistic “heard” metadata for replayed packets. - Applies the above consistently across replayed position, device metrics telemetry, environment metrics telemetry, and node status packets.
Position/telemetry/environment/status records replayed to the phone after config_complete_id were built to look exactly like a live over-the-air broadcast (fresh random packet id, zeroed rx_snr, hop_start == hop_limit), so on every reconnect the client logged them as brand-new packets and reset the node's rssi/snr/hops-away/last-heard as if just received directly.
Populate rx_snr and derive hop_start/hop_limit from the node's real last-known snr/hops_away, and use a stable id derived from (node, timestamp, record kind) so unchanged history replayed on reconnect no longer reads as new.
🤝 Attestations
Pro-micro DIY
Summary by CodeRabbit