Skip to content

MAVLink: refine reconnect STATUSTEXT (per-peer tracking, snapshot rate limit)#11731

Open
xznhj8129 wants to merge 1 commit into
iNavFlight:maintenance-10.xfrom
xznhj8129:mav/reconnect-refine
Open

MAVLink: refine reconnect STATUSTEXT (per-peer tracking, snapshot rate limit)#11731
xznhj8129 wants to merge 1 commit into
iNavFlight:maintenance-10.xfrom
xznhj8129:mav/reconnect-refine

Conversation

@xznhj8129

Copy link
Copy Markdown
Contributor

Follow-up to the reconnect STATUSTEXT handling merged in #11715, addressing review feedback on the reconnect-detection design.

The original commit tracked heartbeat presence with a single timestamp per physical port and sent an arming-reason snapshot on every judged reconnect. Review raised two soundness issues with that:

  • a steady peer on a port masks a newly joining peer behind the same port (their heartbeats keep the single timestamp fresh), and a peer failing over to another port is never noticed;
  • MAVLink leaves heartbeat rate channel-dependent, so a peer heartbeating slower than the 5 s gap threshold is classified as reconnecting on every beat, resending the arming notice each time.

Changes

  • Per-peer tracking. Heartbeat presence now lives in the route-table entry (sysid/compid), not per port. A steady peer no longer masks a new one; a peer moving to another port registers as a reconnect there. If the route table is full the new peer degrades gracefully to the pre-existing broadcast-only behaviour.
  • Snapshot rate limit. A 10 s per-port floor on arming snapshots, so a slow-heartbeat peer can't elicit a resend on every beat.
  • Dropped the enable-transition snapshot. It fired into a port nobody had connected to yet (boot) and duplicated the first-heartbeat snapshot on shared-port enable. Reconnect is now driven purely by the heartbeat signal; the enable transition still resets per-port one-shot state via the port reset path.

Testing

  • SITL builds clean with warnings-as-errors; mavlink_unittest 38/38 at this level (no new unit tests — behaviour is timing/multi-peer and covered by the live rig below).
  • Live SITL + pymavlink: existing reconnect regression 6/6, plus new cases — a 6.5 s-period peer is capped to 3 arming sends across 5 beats (was 5), and a second sysid joining a busy port gets its own snapshot while the steady peer produces none.

Built on the mav/04-streams-protocol content already in maintenance-10.x via #11715, so this shows a single commit.

Review feedback on the reconnect STATUSTEXT commit:

- Track heartbeat presence per peer in the route table instead of one
  timestamp per port, so a steady peer cannot mask a newly joining one
  behind the same port, and a peer failing over to another port
  registers as a reconnect there. If the route table is full the new
  peer degrades to broadcast-only behavior.
- Rate-limit arming snapshots per port so a peer heartbeating slower
  than the gap threshold cannot elicit a resend on every beat.
- Drop the enable-transition snapshot: it fired into a port nobody had
  connected to yet (boot) and duplicated the first-heartbeat snapshot on
  shared-port enable. Reconnect is now driven purely by the heartbeat
  signal; the enable transition still resets per-port one-shot state via
  the port reset path.
@qodo-code-review

Copy link
Copy Markdown
Contributor

Qodo reviews are paused for this user.

Troubleshooting steps vary by plan Learn more →

On a Teams plan?
Reviews resume once this user has a paid seat and their Git account is linked in Qodo.
Link Git account →

Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center?
These require an Enterprise plan - Contact us
Contact us →

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant