MAVLink mission transfer state machine

[xznhj8129]

⚠ Part 5/7 of the mavlink_multiport2 split — depends on #11715.
GitHub can't base an upstream PR on a fork branch, so this targets maintenance-10.x and its diff also contains the earlier parts of the stack. Review only this part's own commits:

• `98a301e71` Add MAVLink mission transfer state machine
• `6857a38d6` Add MAVLink SITL sanity harness

This part in isolation: xznhj8129/inav@mav/04-streams-protocol...mav/05-mission
Merge order: parts 1 → 7 in sequence.

Part 5/7 of the mavlink_multiport2 stack.

Rewritten MAVLink mission protocol, replacing the old ad-hoc counter-based implementation (absent since part 3/7) with an owned transfer state machine. The transfer design is adapted from Betaflight's GPLv3 mavlink_mission.c (commit 87d4bd63) and reshaped for INAV's routed multi-port runtime.

Transfer robustness

• Transfers track the initiating system, component, and ingress port; only that partner may continue a transfer.
• Uploads retry the outstanding request every 1.5 s and abort after five retries. Downloads expire after 5 s of inactivity and accept one-item retransmission.
• Out-of-sequence upload items re-request the expected item; valid out-of-order download requests are served; explicit MAV_MISSION_OPERATION_CANCELLED returns the transfer to idle.
• Legacy MISSION_REQUEST downloads are answered with MISSION_ITEM_INT, per the current MAVLink mission service.
• Protocol-specific failure ACKs throughout.

Translation (MAVLink ↔ INAV waypoints)

• Uploads are staged: the full MAVLink mission is received and translated into a temporary INAV waypoint list, validated, and committed only after the transfer succeeds — a failed upload never leaves a half-written mission.
• QGC planned-home item 0 is skipped (INAV stores home separately); MAVLink sequence 1 becomes INAV waypoint 1. A real current first waypoint is preserved, not mistaken for planned home.
• QGC-style NaN/unused waypoint params are tolerated.
• Modifier items fold onto the correct INAV waypoint instead of consuming slots: DO_CHANGE_SPEED → pending leg speed; CONDITION_DELAY → previous waypoint becomes/updates POSHOLD_TIME; altitude modifiers update the previous geographic waypoint while preserving INAV's p3 bitfield semantics (bit 0 = AMSL reference, bits 1–4 = user actions).
• NAV_WAYPOINT → WAYPOINT or POSHOLD_TIME (hold time set), NAV_LOITER_TIME → POSHOLD_TIME, NAV_LAND → LAND (retaining supplied coordinates), DO_JUMP → JUMP with remapped target.

Persistence and reporting

• Successful uploads and mission clears update nonvolatile waypoint storage; on persistence failure the previous mission is restored in RAM instead of leaving it cleared or half-written.
• 1 Hz MISSION_CURRENT (count, current item, execution mode, state); active-item flags on downloads.
• MISSION_ITEM_REACHED broadcast to all active MAVLink ports from a navigation-side reached latch (hold-time waypoints, altitude-enforced holds, and waypoint-next transitions).

Caveats

Mission translation to INAV's MSP waypoint model is lossy (leg speed, LAND elevation, RTH land flag, p3 user-action bits). MAVLink mission downloads are best-effort reconstruction from the stored MSP waypoint list — not a canonical mission backup. Documented in docs/Mavlink.md (part 7/7).

Testing

• Unit slice: 58/58 passing (mavlink_unittest).
• Includes the live SITL mission rig (src/test/mavlink/missions/, 8 scenario cases incl. planned-home skip, modifier folding, JUMP remap, failed-upload preservation, legacy download) and the routing compliance harness (src/test/mavlink/routing/).
• Also includes the standalone SITL sanity harness (src/test/mavlink/sanity/): 8-check pass/fail rig covering MSP, MAVLink telemetry, RC override, GCS heartbeat, mission upload/readback, and stream/message-rate control.
• Full SITL build, warnings-as-errors, clean.
• X-Plane and QGC SITL testing successful; proprely made missions (without takeoff) load and fly accurately

[qodo-code-review]

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 →

[sensei-hacker]

This is an AI-generated draft of things to check on — flagged for the author's judgment, not a formal review verdict.

Reviewed this part in isolation (pr-11715-head..pr-11716-head, commits 98a301e718/6857a38d6f), and independently verified the SITL/hardware builds and the mavlink_unittest 58/58 result — all check out cleanly. Architecturally this is solid: the staged-upload → validate → commit-with-rollback flow genuinely protects against a half-written mission on failure, and the DO_JUMP target remapping correctly rejects jumps into folded modifier items. A few things worth a look before/around merge:

1. LAND-terminated missions may never report completion. This PR wires the new "waypoint reached" latch (navMarkWaypointReached/navigationConsumeWaypointReached) into three places: the HOLD_TIME case in NAV_STATE_WAYPOINT_REACHED, the altitude-reached branch in NAV_STATE_WAYPOINT_HOLD_TIME, and the catch-all in NAV_STATE_WAYPOINT_NEXT's entry. But NAV_STATE_WAYPOINT_RTH_LAND's entry (unchanged by this PR) routes success straight to RTH_FINISHING/RTH_FINISHED → WAYPOINT_FINISHED, bypassing WAYPOINT_NEXT entirely. So a mission ending in NAV_WP_ACTION_LAND — the normal terminal action for fixed-wing/rover — looks like it would never fire MISSION_ITEM_REACHED for that item, and MISSION_CURRENT would report MISSION_STATE_NOT_STARTED indefinitely after landing rather than MISSION_STATE_COMPLETE. Did your SITL/QGC testing happen to cover a LAND-ending mission specifically, or mostly HOLD_TIME/WAYPOINT-ending ones? If this is real it'd be worth a fix and a test that drives the actual FSM rather than stubbing navigationConsumeWaypointReached() directly.

2. MISSION_CLEAR_ALL doesn't check transfer ownership. MISSION_COUNT and MISSION_REQUEST_LIST both deny a non-owning partner while a transfer is active (mavMissionTransfer.state != IDLE && !mavlinkMissionSenderOwnsTransfer()). mavlinkHandleIncomingMissionClearAll doesn't have that check — any local-target sender can cancel another partner's in-progress upload/download. Intentional (CLEAR_ALL as a "global" op), or worth the same guard for consistency with "only that partner may continue a transfer"?

3. Legacy MISSION_REQUEST (non-INT) download path looks unreachable. Both mavlinkHandleIncomingMissionRequest and mavlinkHandleIncomingMissionRequestInt call mavlinkSendMissionItemResponse(seq, true) — hardcoded true, so the useIntMessages == false branch (the mavlink_msg_mission_item_pack float encoder) can never execute. If INT-encoding is intentionally used unconditionally per the mission microservice spec, a one-line comment at both call sites would save a future reader from "fixing" what looks like a bug.

4. mavlinkHandleMissionItemCommon is ~356 lines, and the mission_type != MAV_MISSION_TYPE_MISSION rejection boilerplate repeats near-verbatim across five handlers. Not blocking, but the duplication is plausibly related to #3 above (a decision applied in one copy, missed in a sibling) — might be worth splitting per-command validate/build helpers at some point given parts 6/7 build on top of this file.

Minor, bundled:

• Lines ~609-617: eight UNUSED(...) calls on parameters that are used elsewhere in the same function — safe to delete.
• mavlinkHandleArmedGuidedMissionItem's current == 2/current == 3 are magic numbers for guided fly-to vs. guided altitude-change — a short comment would help.
• The upload-commit snapshot (mavlinkMissionSnapshot_t previousMission) is stack-allocated in two places while the staging buffer is a static global elsewhere in the file — not a safety issue given the module's non-reentrancy, just a style inconsistency.

Aside, spanning the stack rather than specific to this PR: by #11717 there are now four independent static bool mavlinkIsLocalTarget(...)-shaped helpers doing the same "is this addressed to my system/component" check — one each in mavlink_command.c, mavlink_guided.c, mavlink_streams.c (all literally named mavlinkIsLocalTarget), plus mavlinkMissionTargetIsLocal here in mavlink_mission.c. None are declared in a shared header. Not urgent, but flagging now in case it's easier to consolidate before the stack finalizes than after.