fix(PlanManager): resend last mission waypoint in case of final ack timeout - #14895
fix(PlanManager): resend last mission waypoint in case of final ack timeout#14895MaEtUgR wants to merge 1 commit into
Conversation
…imeout PX4 does resend the final ack upon receiving the last waypoint again. See https://github.com/PX4/PX4-Autopilot/blob/cda3c3603f0b0c52e878ed1852c1a80131bddf01/src/modules/mavlink/mavlink_mission.cpp#L1164
There was a problem hiding this comment.
Pull request overview
Adds recovery for lost final mission acknowledgements by resending the last requested mission item.
Changes:
- Retries final acknowledgements with bounded resend attempts.
- Extracts mission-item transmission into a reusable helper.
- Adds mock-link behavior and test coverage for a dropped first final ACK.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
src/MissionManager/PlanManager.cc |
Implements final-ACK recovery and item resend helper. |
src/MissionManager/PlanManager.h |
Declares the resend helper. |
src/Comms/MockLink/MockLinkMissionItemHandler.cc |
Simulates dropping the first final ACK. |
src/Comms/MockLink/MockLinkMissionItemHandler.h |
Adds failure-mode state. |
test/MissionManager/MissionManagerTest.cc |
Adds the successful retry test case. |
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
| if (_failureMode == FailWriteFinalAckFirstResponse && _failWriteFinalAckFirstResponse) { | ||
| _failWriteFinalAckFirstResponse = false; | ||
| qCDebug(MockLinkMissionItemHandlerLog) << "not sending final ack due to failure mode FailWriteFinalAckFirstResponse, first response"; | ||
| return; | ||
| } |
|
|
||
| MissionItem* item = _writeMissionItems[missionRequestSeq]; | ||
| qCDebug(PlanManagerLog) << QStringLiteral("_handleMissionRequest %1 sequenceNumber:command").arg(_planTypeString()) << missionRequestSeq << item->command(); | ||
| _sendMissionItem(missionRequestSeq); |
Build ResultsPlatform Status
All builds passed. Pre-commit
Pre-commit hooks: 2 passed, 75 failed, 7 skipped. Test Resultslinux-coverage-integration: 48 passed, 0 skipped Code CoverageCoverage: 70.2% No baseline available for comparison Artifact Sizes
Updated: 2026-08-20 17:17:53 UTC • Commit: 9acece2 • Triggered by: Linux |
|
This pull request has been mentioned on Dronecode Forum | Open Source Drone Development. There might be relevant details there: https://discuss.px4.io/t/px4-dev-call-aug-19-2026-team-sync-and-community-q-a/49378/1 |
|
FYI: I traced how ArduPilot reacts to this retry mechanism, since the recovery relies on the vehicle re-acking a duplicate final item: ArduPilot does not resend the final ack. When it sends the final if (!prot->receiving) {
send_mission_ack(msg, type, MAV_MISSION_ERROR);
return;
}So ArduPilot replies with a Net effect on ArduPilot: same false "write failed" outcome as today when the final ack is lost (the mission is actually stored fine), just reached via one retransmit and an error ack instead of a timeout. No regression and no added delay — but the recovery in this PR only benefits PX4. Per the MAVLink mission protocol spec the vehicle is supposed to resend the ack in this situation, so this is arguably an ArduPilot compliance gap worth an upstream issue. |
|
@hamishwillee The spec for mission upload seems silent on what to do if the GCS doesn't receive the final item ACK. With this change PX4 kinda makes an assumption on that which ArduPilot doesn't also assume. |
But that isn't really how upload works. The vehicle side is in control. The GCS side does no retry if it's in the middle I believe. The protocol doesn't really work that way. |
Problem
If the final mission ack is lost on the link the user gets the message "Mission write failed, vehicle failed to send final ack." even though the mission was uploaded succesfully. Based on the message he'll manually reupload the entire mission again.
Solution
PX4 does resend the final ack upon receiving the last waypoint again. So in case the last mission ack times out, we should not immediately give up but retransmit the last waypoint like we do if a waypoint or an ack in the middle of the mission transfer goes missing.
Type of Change
Testing
Platforms Tested
Flight Stacks Tested
Checklist
By submitting this pull request, I confirm that my contribution is made under the terms of the project's dual license (Apache 2.0 and GPL v3).