fix(position): halve the stationary/fixed-position broadcast floor to 6h - #11606
Conversation
The 12h floor introduced with traffic management was too aggressive: a fixed_position or stationary node goes quiet for half a day after its boot broadcast, so anything that missed that one packet - a node that joined later, or one that restarted - shows it with no position until the next refresh. Drop the floor to 6h, and drop the traffic-management identical-position dedup window from 11h to 5h with it. The two are a pair: the dedup window was deliberately sized just under the broadcast floor so a stationary node's periodic refresh clears its neighbours' window instead of being dropped as a duplicate. Leaving it at 11h would have made the extra broadcast pure airtime - aired, then discarded by every receiver - so the mesh would still have seen a 12h refresh. Role caps are unchanged and still bind: tracker 1h, lost-and-found 15m. Both remain shorter than the new 5h default, so those exceptions apply exactly as before. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
⚡ Try this PR in the Web FlasherNote Building this pull request… the flash button, badges and supported-board |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
Included review availability: Your plan provides up to 8 included reviews per hour; 7 remain after this review. 📝 WalkthroughWalkthroughThe default stationary broadcast interval changes from 12 hours to 6 hours. The traffic-management position interval changes from 11 hours to 5 hours. Related comments and test descriptions are updated to match these values. ChangesPosition and traffic-management interval updates
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: ⚪ Minimal · up to The change shortens stationary-position refresh timing from 12 hours to 6 hours and aligns deduplication timing accordingly; the reported tests pass, and no actionable merge-blocking risk remains. Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Description checkExplanation The description explains the motivation, changed constants, preserved behavior, role caps, cache sizing, tests, and issue references. It is substantially complete, although it does not include the template's device-specific attestation checkboxes. ✨ Finishing Touches 💡 2📝 Generate docstrings 💡
🛠️ Fix failing CI checks 💡
🧪 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 |
12 hours was too aggressive.
The stationary/fixed-position broadcast floor added in #10706 means a
fixed_positionnode (or any node that hasn't moved beyond the broadcast precision) sends once at boot and then goes silent for half a day. Anything that missed that one packet — a node that joined the mesh later, or one that rebooted — shows it with no position until the next refresh. Two reports of exactly that shape: #11198 and #11547.Changes
default_position_stationary_broadcast_secsdefault_traffic_mgmt_position_min_interval_secsThe two move together on purpose. The dedup window was sized just under the broadcast floor so a stationary node's periodic refresh clears its neighbours' window instead of being dropped as a duplicate. Halving only the floor would have left every second broadcast landing inside an 11 h dedup window — aired on the channel, then dropped by every receiver in
shouldDropPosition()before it reached NodeDB or the phone. Same effective refresh rate as today, double the airtime. So the dedup window comes down with it and keeps the same 1 h margin.Role caps are untouched and still bind, since both are shorter than the new 5 h default:
TRACKER/TAK_TRACKER— 1 hLOST_AND_FOUND— 15 minCache sizing stays comfortably inside its bounds at the new value: the dedup window is 50 pos-ticks (6 min/tick,
uint8period 25.6 h) and the entry TTL is 200 ticks ≈ 20 h.Not changed
position_broadcast_secs— the floor still only raises intervals shorter than itself, and a configured interval already longer than 6 h still wins.Testing
test_traffic_managementandtest_position_moduleboth green — 101 test cases. The dedup tests reference the default symbolically, so they follow the new value; comments citing "11 h" updated to match.Refs #11198, #11547
Summary by CodeRabbit