Field import: udp_bridge stale-packet gate (2026-06-29) + tests#33
Merged
Conversation
Under a saturated link the resend protocol can deliver a previously- missing packet after newer packets for the same destination topic have already been published, so a consumer sees the topic's stamp jump backward (observed on heartbeat, video, and costmap during the 2026-06-29 BizzyBoat deployment). Add a per-(remote-node, destination-topic) high-water mark on the wrapped packet_number: decodeData drops a decoded message whose packet_number is older than the newest already published for its destination topic. The number is threaded from UDPBridge::unwrap via SourceInfo; the high-water marks live in RemoteNode and reset together with received_packet_times_ on remote-restart detection, so the post- restart packet_number reset to 0 is not mistaken for stale. Gated by a new drop_stale_packets parameter (default true) so a deployment can disable it for topics that need every message regardless of order. Throttled INFO logs the cumulative drop count. Receiver-side only; the resend protocol and duplicate filtering are unchanged. Unit tests for admitForPublish to follow. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The field commit (b7ae191) deferred unit tests for the new RemoteNode::admitForPublish to wrap-up per live-ops pace. This adds them. Six cases pin the documented contract: first message for a topic admitted; strictly-older packet_number dropped (the late-resend case); equal number admitted (exact dupes are filtered upstream, so == is not a duplicate); newer number advances the high-water mark; high-water is per-destination-topic and independent; and — the load-bearing case — the marks reset on remote-restart detection so a post-restart packet #0 is admitted rather than dropped (without that clear the link would go silent after a remote udp_bridge restart). Full suite: 109 tests, 0 failures. Part of #32
Owner
Author
|
Operator decision (2026-06-30): keep the gate as-is for now — Authored-By: |
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.
Field import: udp_bridge stale-packet gate (+ tests)
Imports field commit
b7ae191(pushed via gitcloud) and adds the unit teststhe field deferred to wrap-up.
What the gate does
On the 2026-06-29 BizzyBoat deployment the operator saw old packets republished
after newer ones (heartbeat/video/costmap), so the topic stamp jumped backward
— the resend protocol delivering a late-requested packet out of order. The new
gate keeps a per-(remote-node, destination-topic) high-water mark of the wrapped
packet_number(threaded viaSourceInfofromUDPBridge::unwrap) and drops adecoded message whose number is strictly below it. The mark lives in
RemoteNodeand clears withreceived_packet_times_on remote-restartdetection. New param
drop_stale_packets(defaulttrue). Field-verifiedfixing the backwards-jumping stamps.
Added in this PR (beyond the field commit)
test/test_stale_packet_gate.cpp— 6 GTest cases forRemoteNode::admitForPublish:first-admit, strict-older-drop, equal-admit (exact dupes are filtered upstream),
newer-advances-high-water, per-topic independence, and the remote-restart
reset (the load-bearing one — without the clear, a post-restart packet #0 is
dropped and the link goes silent). Full suite: 109 tests, 0 failures.
The field RCA (salmon log, 18:04 / 18:11) found the gate is global default-ON
and keyed per-topic, which is correct only for single-stream latest-wins topics
(it fixed heartbeat/video/costmap). It is wrong for:
sonar_image_*, pointclouds) — eachping is unique, so an out-of-order drop is a gap, not a dedup;
/tf— one topic carries many frames; a late/tffor frame A is dropped because a newer
/tffor frame B advanced thehigh-water, dropping valid transforms.
~677k cumulative drops were logged including
sonar_image_*,/tf, cameras,mavros, odom. Live-display only — recorded survey data is boat-side and
complete. Field recommendation: make the gate opt-in per latest-wins topic
(allowlist) or exclude sonar/tf/pointcloud; stopgap
-p drop_stale_packets:=false.Reviewer decision needed: merge as-is + open the opt-in follow-up issue, or
flip the default to opt-in within this PR. I did not redesign the gate here —
the field explicitly scoped the redesign to a follow-up, so this PR keeps the
field behavior + adds test coverage and surfaces the concern.
Field SHA
b7ae191preserved (fast-forward import). See unh_echoboats #356 forthe deployment context.
Closes #32
Authored-By:
Claude Code AgentModel:
Claude Opus 4.8 (1M context)