Report: MAVLink commands that silently produce wrong behavior despite accepting the upload

I’ve been reviewing PX4’s MAVLink command handling code recently (v1.16.1) and noticed several commands that accept a mission upload with no error, then at execution time produce behavior that doesn’t match what the operator asked for. Parameters get dropped during parsing, fields get overloaded with multiple conflicting meanings, or the format the command arrives in silently determines whether it works at all. The operator sees a clean upload ACK the whole time.

A quick note on process: LOITER_TIME param4 overloading (B1 below) was already filed as a GitHub issue and a maintainer confirmed it as non-standard behavior. ARM_DISARM param2 dropped (A1) and NAV_DELAY zero duration (A2) have also been filed but haven’t gotten a response yet. Rather than keep opening individual issues and cluttering the tracker, I’m posting the rest here on the discussion board first.

I’d like to get your take on whether these are considered bugs worth fixing, or if some of the behavior is intentional.


A. Parser stores the command but skips specific fields

These commands have explicit case branches in mavlink_mission.cpp, but the branch only sets nav_cmd and forgets to copy one or more MAVLink parameters into the mission_item struct.

A1. MAV_CMD_COMPONENT_ARM_DISARM — param2 (force flag) dropped

Sending COMMAND_LONG with param1=0 (disarm) and param2=21196 triggers a forced disarm that bypasses the “not landed” guard in Commander.cpp:1012-1022. The mission path stores only param1:

// mavlink_mission.cpp (non-MAV_FRAME_MISSION branch)
case MAV_CMD_COMPONENT_ARM_DISARM:
    mission_item->nav_cmd = (NAV_CMD)mavlink_mission_item->command;
    mission_item->params[0] = (uint16_t)mavlink_mission_item->param1;
    break;
    // params[1] (the force flag) is never assigned

When Navigator republishes the item as a vehicle_command at mission_block.cpp:536-539:

vehicle_command.param1 = item.params[0];
vehicle_command.param2 = item.params[1];  // uninitialized, always 0

Commander checks cmd.param2 == 21196 to decide whether the disarm is forced (Commander.cpp:1012). Since param2 is always 0, a mission-embedded forced disarm degrades to a normal disarm. If the vehicle is airborne, Commander denies it with “Disarming denied: not landed.”

The mission author gets no feedback at upload that the force flag was thrown away.


A2. MAV_CMD_NAV_DELAY — delay duration never reaches time_inside

MAV_CMD_NAV_DELAY with param1 set to N seconds should hold the vehicle at the waypoint for N seconds before advancing. The mission parser at mavlink_mission.cpp:1629-1638 falls into a generic passthrough branch:

// mavlink_mission.cpp:1629-1638
case MAV_CMD_NAV_DELAY:  // falls through to generic branch
    // ...
    mission_item->nav_cmd = (NAV_CMD)mavlink_mission_item->command;
    // param1 (delay seconds) never copied to mission_item->time_inside
    break;

At execution, mission_block.cpp:564-573 reads time_inside to decide when the delay is over:

// mission_block.cpp:564-573
const float time_inside = mission_item->time_inside;  // always 0.0f
if (time_inside > FLT_EPSILON && elapsed < time_inside) {
    return false;  // not yet reached
}
return true;  // immediately complete

time_inside was never set, so it stays 0.0f. Every NAV_DELAY mission item completes instantly. A 30-second hold proceeds without pausing.


A3. MAV_CMD_DO_SET_ROI_WPNEXT_OFFSET — all three angular offsets lost

This command lets the operator set pitch, roll, and yaw offsets from the next waypoint for gimbal targeting. In MAV_FRAME_MISSION, these are param5 (pitch), param6 (roll), param7 (yaw). The parser at mavlink_mission.cpp:1631 uses the same generic branch as A2:

// mavlink_mission.cpp:1631
case MAV_CMD_DO_SET_ROI_WPNEXT_OFFSET:
    mission_item->nav_cmd = (NAV_CMD)mavlink_mission_item->command;
    // param5/6/7 never extracted
    break;

At execution, mission_block.cpp:734-738 reads from the item’s stored fields:

// mission_block.cpp:734-738
const float pitch_offset = 0.0f;   // hardcoded
const float roll_offset  = 0.0f;   // hardcoded
const float yaw_offset   = mission_item->yaw;  // from item, default 0

The gimbal ends up pointing straight at the next waypoint with zero correction. The three offsets are gone at upload time and cannot be recovered from mission readback.


B. Protocol encoding mismatches

B1. MAV_CMD_NAV_LOITER_TIME — param4 overloaded with two meanings; mission round-trip destroys stored yaw

Per MAVLink spec, param4 for LOITER_TIME is “Exit location: 0 for center of loiter radius, 1 for exit location” — a boolean xtrack flag. PX4’s mission parser at mavlink_mission.cpp:1461-1469 stores it as both the xtrack flag AND a yaw angle:

// mavlink_mission.cpp:1461-1469
case MAV_CMD_NAV_LOITER_TIME:
    mission_item->time_inside = mavlink_mission_item->param1;
    mission_item->force_heading = (mavlink_mission_item->param2 > 0);
    mission_item->loiter_radius = mavlink_mission_item->param3;
    mission_item->loiter_exit_xtrack = (mavlink_mission_item->param4 > 0);
    mission_item->yaw = wrap_2pi(math::radians(mavlink_mission_item->param4));
    break;

Two problems come out of this.

First, the double meaning. Any positive param4 intended to enable xtrack exit also sets yaw to that value in degrees. For rotary-wing vehicles, mission_block.cpp:392-415 checks whether yaw acceptance is required and can delay loiter completion until the vehicle reaches the unintended heading:

// mission_block.cpp:392-415 (rotary-wing yaw acceptance)
if (_navigator->get_yaw_to_be_accepted(_mission_item.yaw)) {
    // vehicle waits for heading alignment before loiter timer can complete
}

An operator who sets param4=1 for xtrack exit gets two side effects: the vehicle tries to face 1 degree, and if force_heading was also set via param2, the loiter may timeout trying to reach that heading.

Second, mission round-trip is broken. The formatter at mavlink_mission.cpp:1797-1802 serializes param4 back as the boolean flag only:

// mavlink_mission.cpp:1797-1802 (mission download)
case NAV_CMD_LOITER_TIME_LIMIT:
    mavlink_mission_item->param1 = mission_item->time_inside;
    mavlink_mission_item->param2 = mission_item->force_heading;
    mavlink_mission_item->param3 = mission_item->loiter_radius;
    mavlink_mission_item->param4 = mission_item->loiter_exit_xtrack;  // 0 or 1
    break;

Other commands with yaw explicitly serialize it to param4 (lines 1788, 1793, 1807, 1811), but LOITER_TIME does not. The stored yaw is permanently lost on mission download. A GCS that uploads, downloads, edits, and re-uploads a mission will see the yaw collapse to 0 or 1 degree.


B2. MAV_CMD_DO_REPOSITION — COMMAND_LONG version silently drops position

MAV_CMD_DO_REPOSITION should move the vehicle to specified coordinates regardless of message format. Commander accepts and ACKs the request at Commander.cpp:806-807. Then Navigator at navigator_main.cpp:273-274 reads the coordinates from the command packet:

// navigator_main.cpp:273-274 (COMMAND_LONG path)
const double lat = cmd.param5;   // raw float from COMMAND_LONG
const double lon = cmd.param6;

COMMAND_LONG carries lat/lon as IEEE 754 single-precision floats (~7 significant digits), which is not enough for sub-meter geographic coordinates. COMMAND_INT carries them as scaled int32 (*1e7), giving full precision. Navigator rejects the degraded coordinates at lines 284-411, but the ACK was already sent by Commander:

// navigator_main.cpp:413
// CMD_DO_REPOSITION is acknowledged by commander

A GCS sending COMMAND_LONG DO_REPOSITION gets ACK=ACCEPTED and a mode switch to HOLD/LOITER, but the vehicle never moves.


B3. MAV_CMD_DO_SET_MODE — cannot request modes that PX4 itself advertises

PX4 broadcasts available modes in heartbeat’s custom_mode field, including PX4_CUSTOM_MAIN_MODE_ORBIT, VTOL_TAKEOFF, and PX4_CUSTOM_MAIN_MODE_TERMINATION. Commander.cpp:843-928 decodes DO_SET_MODE with a sparse switch:

// Commander.cpp:843-928
switch (static_cast<int>(cmd.param2)) {
    case PX4_CUSTOM_MAIN_MODE_MANUAL:
    case PX4_CUSTOM_MAIN_MODE_ALTCTL:
    case PX4_CUSTOM_MAIN_MODE_POSCTL:
    case PX4_CUSTOM_MAIN_MODE_AUTO:
        // sub-mode decoding
        break;
    // no case for PX4_CUSTOM_MAIN_MODE_TERMINATION
    // ORBIT is a sub_mode of POSCTL, but sub_mode=POSCTL_ORBIT maps to no nav_state
}

TERMINATION being unreachable via DO_SET_MODE matters for safety systems: an external monitor cannot command flight termination through the standard mode-change command even though heartbeat says the mode exists. For automation that round-trips modes from heartbeat to DO_SET_MODE, the gap produces false mode-mismatch alarms.


B4. MAV_CMD_NAV_WAYPOINT — per-waypoint acceptance radius ignored on fixed-wing

param2 defines a per-waypoint acceptance radius so operators can tune when each waypoint is considered reached. The parser at mavlink_mission.cpp:1442 stores it correctly:

// mavlink_mission.cpp:1442
mission_item->acceptance_radius = mavlink_mission_item->param2;

But mission_block.cpp:313-315 applies it only for rotary-wing:

// mission_block.cpp:313-315
if (_navigator->get_vstatus()->vehicle_type == vehicle_status_s::VEHICLE_TYPE_ROTARY_WING) {
    acceptance_radius = mission_item->acceptance_radius;
} else {
    acceptance_radius = _navigator->get_acceptance_radius();  // global NAV_ACC_RAD
}

Every fixed-wing vehicle ignores per-waypoint radii and uses the global NAV_ACC_RAD. Missions that need tighter tolerances on specific waypoints (narrow corridors, survey patterns) run with the wrong radius.


C. Field read from the wrong MAVLink parameter index

C1. MAV_CMD_NAV_LOITER_TO_ALT — loiter radius taken from param2 instead of param3

Every other loiter command (LOITER_TIME, LOITER_TURNS, LOITER_UNLIM) reads the loiter radius from param3. param2 is “Unused” per MAVLink spec. But at mavlink_mission.cpp:1479, the parser for LOITER_TO_ALT reads it from param2:

// mavlink_mission.cpp:1479
case MAV_CMD_NAV_LOITER_TO_ALT:
    if (mavlink_mission_item->frame == MAV_FRAME_MISSION) {
        mission_item->loiter_radius = mavlink_mission_item->param2;  // wrong field
        mission_item->loiter_direction = (mavlink_mission_item->param3 > 0) ?
            MissionItem::LoiterDirection::CW :
            MissionItem::LoiterDirection::CCW;
    }

For comparison, LOITER_TIME at line 1452:

// mavlink_mission.cpp:1452
case MAV_CMD_NAV_LOITER_TIME:
    mission_item->loiter_radius = mavlink_mission_item->param3;  // correct

And LOITER_TURNS at line 1468:

// mavlink_mission.cpp:1468
case MAV_CMD_NAV_LOITER_TURNS:
    mission_item->loiter_radius = mavlink_mission_item->param3;  // correct

Because param3 is also used for direction, the direction logic shifts too: what the operator set as direction in param4 gets interpreted from param3 instead. A radius uploaded through QGC or Mission Planner (which puts it in param3) is silently ignored in favor of whatever value is in param2.


D. Parser accepts the command, but Navigator has no handler for it

These commands pass upload validation and are stored in the mission. Navigator either has no case for the stored nav_cmd, or marks the item complete without doing the work.

D1. MAV_CMD_CONDITION_DELAY — stored as nav_cmd 112, Navigator only handles 93

MAV_CMD_CONDITION_DELAY (MAVLink command 112) should pause the vehicle for param1 seconds, same as NAV_CMD_DELAY. At upload, mavlink_mission.cpp:1633 stores it as raw nav_cmd:

// mavlink_mission.cpp:1633
case MAV_CMD_CONDITION_DELAY:
    mission_item->nav_cmd = (NAV_CMD)mavlink_mission_item->command;  // = 112
    break;

At execution, mission_block.cpp:136 only handles NAV_CMD_DELAY (93):

// mission_block.cpp:136-146
case NAV_CMD_DELAY:  // = 93, not 112
    // sets up delay with time_inside
    break;

nav_cmd 112 falls through to default at line 182, where it is treated as a 3D waypoint and completes immediately:

// mission_block.cpp:182-203 (default branch)
default:
    // treated as position item, completes when within acceptance radius
    return true;

The mission item that was meant to pause instead advances instantly.


D2. MAV_CMD_CONDITION_DISTANCE — no handler anywhere in Navigator

MAV_CMD_CONDITION_DISTANCE should pause the vehicle until it is within param1 meters of the next waypoint. At upload, mavlink_mission.cpp:1634 accepts it in the same generic passthrough as D1:

// mavlink_mission.cpp:1634
case MAV_CMD_CONDITION_DISTANCE:
    mission_item->nav_cmd = (NAV_CMD)mavlink_mission_item->command;
    break;

I searched src/modules/navigator/ for any handler and found nothing. MissionBlock::is_mission_item_reached_or_completed() does not include it. MissionBlock::item_contains_position() does not classify it as a position item. MissionBlock::item_has_timeout() does not classify it as a timed item. It hits the same default case as D1 and completes immediately. Same result: what should be a distance-gated pause becomes an instant skip.


D3. MAV_CMD_DO_AUTOTUNE_ENABLE — mission item completes, autotune never starts

The direct command path at mavlink_receiver.cpp:648-680 validates autotune state and publishes a vehicle_command that activates it:

// mavlink_receiver.cpp:648-680 (direct command handler)
if (_autotune_module != nullptr && !_autotune_module->isIdle()) {
    // reject, autotune already running
} else if (_status.is_transitioning) {
    // reject, VTOL transitioning
} else {
    _vehicle_command_pub.publish(vcmd);
}

The mission path at mission_block.cpp:86 classifies it as immediate-complete:

// mission_block.cpp:86
case NAV_CMD_DO_AUTOTUNE_ENABLE:
    return true;  // immediately complete, no action

Navigator publishes a vehicle_command at navigator_main.cpp:770-783, but only logs warnings for unsupported params and never calls into the autotune module:

// navigator_main.cpp:770-783
case vehicle_command_s::VEHICLE_CMD_DO_AUTOTUNE_ENABLE: {
    if (/* unsupported params */) {
        mavlink_log_warning(_mavlink_log_pub, "Autotune: unsupported params\t");
    }
    // no autotune activation
    break;
}

A mission with an autotune step uploads fine, the step “completes” in flight, and nothing happens.


Summary

Type What goes wrong Commands Direct path
A Parser drops fields ARM_DISARM, NAV_DELAY, ROI_WPNEXT_OFFSET Works
B Protocol encoding mismatch LOITER_TIME, DO_REPOSITION, DO_SET_MODE, NAV_WAYPOINT Format-dependent
C Field read from wrong param index LOITER_TO_ALT Works
D Parser accepts, Navigator has no consumer CONDITION_DELAY, CONDITION_DISTANCE, AUTOTUNE_ENABLE Works (AUTOTUNE)

All of these are confirmed by source inspection at the cited locations. Most were also tested in SITL where the log confirmed the described behavior: mission advancing without delay, wrong radius values in use, ACK received after a no-op, etc.

cc: @hamishwillee @JulianOes