Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
108 changes: 96 additions & 12 deletions src/main/mavlink/mavlink_mission.c
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,19 @@ static bool mavlinkMissionTargetIsLocal(uint8_t targetSystem, uint8_t targetComp
(targetComponent == 0 || targetComponent == mavComponentId);
}

// A mission edit (upload or clear) is refused while the WP mission is being
// executed: WP mode active, the mission's own RTH leg running (the land/loiter
// decision at home reads the live list), or the on-the-fly mission planner
// writing the same list. This matches the MSP policy from #10273 combined
// with the updateWpMissionPlanner() guard. The ARMED term makes the disarmed
// path provably unchanged, including the short window right after disarming
// before the nav FSM drops out of WP mode.
static bool mavlinkMissionEditBlocked(void)
{
return ARMING_FLAG(ARMED) &&
(FLIGHT_MODE(NAV_WP_MODE) || isWaypointMissionRTHActive() || isWpMissionPlannerActive());
Comment on lines +101 to +102

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Action required

1. Stale planner corrupts missions 🐞 Bug ≡ Correctness

The new gate permits armed MAVLink upload or clear after the planner is disabled, but those
operations do not reset wpPlannerActiveWPIndex, which planner deactivation also preserves.
Reactivating the planner then writes at the stale index and derives the mission count from it,
resurrecting cleared waypoints or overwriting/truncating the newly uploaded mission.
Agent Prompt
## Issue description
Armed MAVLink mission upload and clear are now allowed after the on-the-fly planner is disabled, but the planner's progress index remains stale. Reset planner progress whenever an external operation replaces or clears the waypoint list so later planner activation starts from a consistent list.

## Issue Context
Planner deactivation does not clear `wpPlannerActiveWPIndex`. MAVLink clear and commit reset the mission metadata without resetting that index, while `missionPlannerSetWaypoint()` uses any nonzero index directly and then sets the mission count from it.

## Fix Focus Areas
- src/main/mavlink/mavlink_mission.c[203-219]
- src/main/mavlink/mavlink_mission.c[446-477]
- src/main/navigation/navigation.c[5502-5514]
- src/main/navigation/navigation.c[6440-6480]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

}

static bool mavlinkMissionSenderOwnsTransfer(void)
{
return mavlinkContext.recvMsg.sysid == mavMissionTransfer.partnerSystem &&
Expand Down Expand Up @@ -194,7 +207,11 @@ static bool mavlinkClearPersistedMission(void)

resetWaypointList();
mavlinkContext.missionCompleted = false;
if (mavlinkPersistMission()) {
// While armed the clear applies to RAM only, mirroring the MSP policy:
// saveNonVolatileWaypointList() refuses to run while armed, and a flash
// write mid-flight would stall the main loop anyway. The persisted
// mission is left untouched until the next clear or upload on the ground.
if (ARMING_FLAG(ARMED) || mavlinkPersistMission()) {
return true;
}

Expand Down Expand Up @@ -400,6 +417,26 @@ static bool mavlinkResolveUploadedMissionJumps(void)
return false;
}

// For a mission committed in flight, enforce the same JUMP rules as
// the arm-time validation in navigationIsBlockingArming(), which an
// in-flight upload bypasses entirely: a JUMP cannot be the first
// mission item, cannot target itself or an immediately adjacent
// item, must have a sane repeat count and must target a
// geo-referenced item. Ground uploads are left to the arm-time
// check, keeping disarmed behaviour unchanged.
if (ARMING_FLAG(ARMED)) {
const int targetIndex = targetWaypointNumber - 1;
const navWaypoint_t *target = &mavlinkMissionUploadWaypoints[targetIndex];
if (i == 0 ||
wp->p2 < -1 ||
(targetIndex >= (int)i - 1 && targetIndex <= (int)i + 1) ||
!(target->action == NAV_WP_ACTION_WAYPOINT ||
target->action == NAV_WP_ACTION_HOLD_TIME ||
target->action == NAV_WP_ACTION_LAND)) {
return false;
}
}

wp->p1 = targetWaypointNumber;
}

Expand Down Expand Up @@ -427,7 +464,11 @@ static bool mavlinkCommitMissionUpload(void)
setWaypoint(i + 1, &mavlinkMissionUploadWaypoints[i]);
}

if (!isWaypointListValid() || !mavlinkPersistMission()) {
// While armed the uploaded mission lives in RAM only, mirroring the MSP
// in-flight upload policy (see #10273): saveNonVolatileWaypointList()
// refuses to run while armed, and a flash write mid-flight would stall
// the main loop anyway. On the ground the mission is persisted as before.
if (!isWaypointListValid() || (!ARMING_FLAG(ARMED) && !mavlinkPersistMission())) {
mavlinkRestoreMission(&previousMission);
return false;
}
Expand Down Expand Up @@ -950,8 +991,16 @@ bool mavlinkHandleIncomingMissionClearAll(void)
mavlinkSendMissionAckTo(mavlinkContext.recvMsg.sysid, mavlinkContext.recvMsg.compid, MAV_MISSION_UNSUPPORTED);
return true;
}
if (ARMING_FLAG(ARMED)) {
mavlinkSendMissionAckTo(mavlinkContext.recvMsg.sysid, mavlinkContext.recvMsg.compid, MAV_MISSION_DENIED);
// Clearing is allowed while merely armed (RAM only, see
// mavlinkClearPersistedMission), but not while the WP mission is being
// executed. If the refused sender owns a receiving transfer, abort it so
// the retry engine stops soliciting items from a partner we just denied.
if (mavlinkMissionEditBlocked()) {
if (mavMissionTransfer.state == MAVLINK_MISSION_TRANSFER_RECEIVING && mavlinkMissionSenderOwnsTransfer()) {
mavlinkAbortMissionUpload(MAV_MISSION_DENIED);
} else {
mavlinkSendMissionAckTo(mavlinkContext.recvMsg.sysid, mavlinkContext.recvMsg.compid, MAV_MISSION_DENIED);
}
return true;
}
if (mavMissionTransfer.state != MAVLINK_MISSION_TRANSFER_IDLE && !mavlinkMissionSenderOwnsTransfer()) {
Expand Down Expand Up @@ -983,8 +1032,17 @@ bool mavlinkHandleIncomingMissionCount(void)
}
return true;
}
if (ARMING_FLAG(ARMED)) {
mavlinkSendMissionAckTo(mavlinkContext.recvMsg.sysid, mavlinkContext.recvMsg.compid, MAV_MISSION_DENIED);
// Starting an upload is allowed while merely armed (the commit stays in
// RAM, see mavlinkCommitMissionUpload), but not while the WP mission is
// being executed. If the refused sender owns a receiving transfer (e.g.
// a retry after WP mode engaged mid-upload), abort it so the retry
// engine stops soliciting items from a partner we just denied.
if (mavlinkMissionEditBlocked()) {
if (mavMissionTransfer.state == MAVLINK_MISSION_TRANSFER_RECEIVING && mavlinkMissionSenderOwnsTransfer()) {
mavlinkAbortMissionUpload(MAV_MISSION_DENIED);
} else {
mavlinkSendMissionAckTo(mavlinkContext.recvMsg.sysid, mavlinkContext.recvMsg.compid, MAV_MISSION_DENIED);
}
return true;
}
if (mavMissionTransfer.state != MAVLINK_MISSION_TRANSFER_IDLE && !mavlinkMissionSenderOwnsTransfer()) {
Expand Down Expand Up @@ -1049,14 +1107,27 @@ bool mavlinkHandleIncomingMissionItem(void)
}

if (ARMING_FLAG(ARMED)) {
if (msg.command == MAV_CMD_NAV_WAYPOINT) {
// Guided fly-to-here (current == 2) and altitude-target (current == 3)
// items are identified by the item itself - no legitimate upload item
// carries these current values - so a guided click is never absorbed
// into a running upload transfer.
if (msg.command == MAV_CMD_NAV_WAYPOINT && (msg.current == 2 || msg.current == 3)) {
return mavlinkHandleArmedGuidedMissionItem(msg.current, msg.frame,
MAV_FRAME_SUPPORTED_GLOBAL | MAV_FRAME_SUPPORTED_GLOBAL_RELATIVE_ALT,
(int32_t)lrintf(msg.x * 1e7f), (int32_t)lrintf(msg.y * 1e7f), msg.z);
}

mavlinkSendMissionAckTo(mavlinkContext.recvMsg.sysid, mavlinkContext.recvMsg.compid, MAV_MISSION_ERROR);
return true;
// Upload items are accepted while merely armed (in-flight upload,
// matching the MSP policy from #10273), but not while the WP mission
// is being executed.
if (mavlinkMissionEditBlocked()) {
if (mavMissionTransfer.state == MAVLINK_MISSION_TRANSFER_RECEIVING && mavlinkMissionSenderOwnsTransfer()) {
mavlinkAbortMissionUpload(MAV_MISSION_DENIED);
} else {
mavlinkSendMissionAckTo(mavlinkContext.recvMsg.sysid, mavlinkContext.recvMsg.compid, MAV_MISSION_DENIED);
}
return true;
}
}

return mavlinkHandleMissionItemCommon(false, msg.frame, msg.command, msg.current, msg.autocontinue, msg.seq,
Expand Down Expand Up @@ -1252,14 +1323,27 @@ bool mavlinkHandleIncomingMissionItemInt(void)
}

if (ARMING_FLAG(ARMED)) {
if (msg.command == MAV_CMD_NAV_WAYPOINT) {
// Guided fly-to-here (current == 2) and altitude-target (current == 3)
// items are identified by the item itself - no legitimate upload item
// carries these current values - so a guided click is never absorbed
// into a running upload transfer.
if (msg.command == MAV_CMD_NAV_WAYPOINT && (msg.current == 2 || msg.current == 3)) {
return mavlinkHandleArmedGuidedMissionItem(msg.current, msg.frame,
MAV_FRAME_SUPPORTED_GLOBAL_INT | MAV_FRAME_SUPPORTED_GLOBAL_RELATIVE_ALT_INT,
msg.x, msg.y, msg.z);
}

mavlinkSendMissionAckTo(mavlinkContext.recvMsg.sysid, mavlinkContext.recvMsg.compid, MAV_MISSION_ERROR);
return true;
// Upload items are accepted while merely armed (in-flight upload,
// matching the MSP policy from #10273), but not while the WP mission
// is being executed.
if (mavlinkMissionEditBlocked()) {
if (mavMissionTransfer.state == MAVLINK_MISSION_TRANSFER_RECEIVING && mavlinkMissionSenderOwnsTransfer()) {
mavlinkAbortMissionUpload(MAV_MISSION_DENIED);
} else {
mavlinkSendMissionAckTo(mavlinkContext.recvMsg.sysid, mavlinkContext.recvMsg.compid, MAV_MISSION_DENIED);
}
return true;
}
}

return mavlinkHandleMissionItemCommon(true, msg.frame, msg.command, msg.current, msg.autocontinue, msg.seq,
Expand Down
10 changes: 8 additions & 2 deletions src/main/navigation/navigation.c
Original file line number Diff line number Diff line change
Expand Up @@ -5488,8 +5488,9 @@ void setWaypoint(uint8_t wpNumber, const navWaypoint_t * wpData)
posControl.geoWaypointCount = posControl.waypointCount - nonGeoWaypointCount;
if (posControl.waypointListValid) {
nonGeoWaypointCount = 0;
// If active WP index is bigger than total mission WP number, reset active WP index (Mission Upload mid flight with interrupted mission) if RESUME is enabled
if (posControl.activeWaypointIndex > posControl.waypointCount) {
// If active WP index is beyond the new mission, reset active WP index (Mission Upload mid flight with interrupted mission) if RESUME is enabled.
// activeWaypointIndex is 0-based, so an index equal to waypointCount is already out of range.
if (posControl.activeWaypointIndex >= posControl.waypointCount) {
posControl.activeWaypointIndex = 0;
}
}
Expand Down Expand Up @@ -5517,6 +5518,11 @@ bool isWaypointListValid(void)
return posControl.waypointListValid;
}

bool isWpMissionPlannerActive(void)
{
return posControl.flags.wpMissionPlannerActive;
}

int getWaypointCount(void)
{
uint8_t waypointCount = posControl.waypointCount;
Expand Down
1 change: 1 addition & 0 deletions src/main/navigation/navigation.h
Original file line number Diff line number Diff line change
Expand Up @@ -848,6 +848,7 @@ bool navCanSetHome(void);
*/
bool navigationRTHAllowsLanding(void);
bool isWaypointMissionRTHActive(void);
bool isWpMissionPlannerActive(void);
#ifdef USE_AUTO_TRANSITION
navVtolTransitionOsdState_e navigationVtolTransitionOsdState(void);
#endif
Expand Down
Loading