Refuse MAV_CMD_NAV_VTOL_TAKEOFF sent as COMMAND_LONG

[peterbarker]

Summary

Copter now refuses MAV_CMD_NAV_VTOL_TAKEOFF sent via COMMAND_LONG with MAV_RESULT_COMMAND_INT_ONLY. The command is still accepted via COMMAND_INT. Positional commands that have no frame defined for COMMAND_LONG now also get MAV_RESULT_COMMAND_INT_ONLY instead of MAV_RESULT_UNSUPPORTED.

Classification & Testing (check all that apply and add your own)

• Checked by a human programmer
• Non-functional change
• No-binary change
• Infrastructure change (e.g. unit tests, helper scripts)
• Automated test(s) verify changes (e.g. unit test, autotest)
• Tested manually, description below (e.g. SITL)
• Tested on hardware
• Logs attached
• Logs available on request

These autotests pass in SITL:

• test.Copter.MAV_CMD_NAV_TAKEOFF: NAV_VTOL_TAKEOFF via COMMAND_LONG returns MAV_RESULT_COMMAND_INT_ONLY (8).
• test.Copter.MAV_CMD_NAV_TAKEOFF_command_int: NAV_VTOL_TAKEOFF via COMMAND_INT in MAV_FRAME_GLOBAL_RELATIVE_ALT is accepted, and the vehicle climbs to the requested altitude.
• test.QuadPlane.MAV_CMD_NAV_TAKEOFF: NAV_VTOL_TAKEOFF via COMMAND_LONG returns MAV_RESULT_UNSUPPORTED (3), because Plane doesn't handle this command at runtime.
• test.Copter.COMMAND_LONG_positional_command_int_only (new): DO_REPOSITION, EXTERNAL_POSITION_ESTIMATE and DO_SET_GLOBAL_ORIGIN via COMMAND_LONG each return MAV_RESULT_COMMAND_INT_ONLY (8).
• test.Copter.MAV_CMD_DO_SET_GLOBAL_ORIGIN and test.Copter.DO_CHANGE_SPEED_in_guided still pass. Both send these commands via COMMAND_INT.

Description

Copter refuses this command via COMMAND_LONG because the MAVLink spec asks for it. common.xml marks MAV_CMD_NAV_VTOL_TAKEOFF hasLocation="true", and the command protocol encourages flight stacks "to only support positional commands in COMMAND_INT". It also says to reject the other message type with MAV_RESULT_COMMAND_INT_ONLY.

Copter's takeoff handler didn't give wrong results via COMMAND_LONG. It ignores latitude/longitude, and it accepts only MAV_FRAME_GLOBAL_RELATIVE_ALT, which the COMMAND_LONG conversion already supplies by default. This PR refuses the command to follow the spec, not to fix a precision or frame bug.

Copter is the only vehicle that handles this command from a GCS, so the refusal is Copter-specific. Other vehicles still answer MAV_RESULT_UNSUPPORTED.

Who is affected: mavlink/mavlink#1981 found that no GCS's built-in takeoff flow sends this command. QGC's guided takeoff uses MAV_CMD_NAV_TAKEOFF, and MAVSDK doesn't reference the command. Generic pass-through paths can still send it via COMMAND_LONG, for example MAVProxy's long NAV_VTOL_TAKEOFF ... or a user-configured QGC MavlinkAction. Those users now get MAV_RESULT_COMMAND_INT_ONLY, which tells them exactly what to do. MAVProxy can send the command with command_int. We have found no evidence that anyone uses these paths for this command.

Changes:

• Copter, Blimp: remove the mav_frame_for_command_long() overrides, which were never called. The base class only calls that method for commands that store a location, and neither takeoff command does. Those commands get the default MAV_FRAME_GLOBAL_RELATIVE_ALT set in try_command_long_as_command_int(), so their behaviour is unchanged.
• GCS_MAVLink: make mav_frame_for_command_long() non-virtual, since nothing overrides it now.
• GCS_MAVLink: add a virtual command_int_only(MAV_CMD), which returns false by default. If a vehicle returns true for a command, try_command_long_as_command_int() refuses the COMMAND_LONG form with MAV_RESULT_COMMAND_INT_ONLY.
• Copter: return true from command_int_only() for MAV_CMD_NAV_VTOL_TAKEOFF.
• GCS_MAVLink: return MAV_RESULT_COMMAND_INT_ONLY instead of MAV_RESULT_UNSUPPORTED for a positional command with no COMMAND_LONG frame. That covers DO_REPOSITION, EXTERNAL_POSITION_ESTIMATE and DO_SET_GLOBAL_ORIGIN, which could never work via COMMAND_LONG. COMMAND_LONG handling now gives one answer for every command it can't accept. It also matches builds with AP_MAVLINK_COMMAND_LONG_ENABLED disabled, which already answer COMMAND_INT_ONLY.
• autotest: add the checks listed above.

What MAV_RESULT_COMMAND_INT_ONLY means here: it says which message type ArduPilot accepts for the command. It doesn't promise that the vehicle supports the command via COMMAND_INT.

• Example: AntennaTracker has no DO_REPOSITION handler. It answers COMMAND_INT_ONLY via COMMAND_LONG, then UNSUPPORTED when the GCS retries via COMMAND_INT. The same can happen for EXTERNAL_POSITION_ESTIMATE on boards built without AP_AHRS_POSITION_RESET_ENABLED.
• This is deliberate. Builds with AP_MAVLINK_COMMAND_LONG_ENABLED disabled already answer COMMAND_INT_ONLY to every COMMAND_LONG, whether or not the vehicle supports the command. The devguide likewise describes these results as a way "to indicate the correct message type for a command". The COMMAND_INT reply then says whether the vehicle supports it.
• Why no per-vehicle check: tailoring the COMMAND_LONG answer to each vehicle's COMMAND_INT handling would need a per-vehicle list for every positional command. That isn't worth saving one round trip.
• Why NAV_VTOL_TAKEOFF is different: the three positional commands can't work via COMMAND_LONG on any vehicle, because ArduPilot has no COMMAND_LONG frame for them. So the generic answer is true everywhere. NAV_VTOL_TAKEOFF could still work via COMMAND_LONG, because it gets the default frame. Refusing it is therefore a per-vehicle choice, made through command_int_only().

Frames: ArduPilot will not add support for MAV_FRAME_GLOBAL_RELATIVE_ALT_INT or any other MAV_FRAME_*_INT frame. MAVLink superseded them in 2024-03 in favour of the equivalent non-_INT frames. COMMAND_INT carries scaled-integer coordinates whatever the frame, so the _INT variants add nothing. Copter's takeoff handlers accept MAV_FRAME_GLOBAL_RELATIVE_ALT, and a GCS moving to COMMAND_INT must use that frame. A GCS that sends a superseded frame should be fixed; ArduPilot will not accommodate it.

Unchanged:

• MAV_CMD_NAV_TAKEOFF via COMMAND_LONG.
• Mission items, which never go through this path.
• Lua scripts that take over a command with mavlink:block_command(). Scripting receives the command before these checks run.

Not in this PR:

• No text message with the refusal. MAV_RESULT_COMMAND_INT_ONLY has been in common.xml since 2023-11 (mavlink b5b1429e), and Mission Planner decodes it. A GCS_SEND_TEXT explaining a spec-defined result code would cost flash in every build for little gain.

• RockBLOCK applet. libraries/AP_Scripting/applets/RockBlock.lua does its own COMMAND_LONG to COMMAND_INT conversion and calls gcs:run_command_int() directly, so this PR doesn't affect it. The applet has two existing bugs that stop takeoff working over RockBLOCK on Copter:

  • Line 339 passes the frame as int_frame, but the binding reads frame, so the frame is always 0.
  • The applet's frame mapping (around line 310) falls back to 0 (MAV_FRAME_GLOBAL) for takeoff anyway.

  Both will be fixed together in a separate PR.

[AP-Review]

Deprecated — see below for the updated review.

Previous review (2026-09-16)

Automated review note — AI-generated (Claude), validated against the live diff. Please sanity-check before acting.

Reviewed at head c7c63abd18.

Full report, including everything that was checked and found clean: https://uav.tridgell.net/DevCallReviews/2026_09_17_AIReview/devcall_pr_reviews.html#pr34420

Verdict: REQUEST CHANGES

Draft. The mechanism, the layer and the test are all correct, and no real-world COMMAND_LONG sender was found for this command. The blocking concern is scope: the refusal is added to the vehicle-agnostic base class, but MAV_CMD_NAV_VTOL_TAKEOFF is handled as a runtime command by Copter alone, so Plane/QuadPlane, Rover, Sub, Blimp and Tracker now answer COMMAND_INT_ONLY for a form that COMMAND_INT also rejects. Verified empirically on quadplane SITL built from this head, and the test was proven to cover the change by mutation rather than by reading.

---

ISSUE — libraries/GCS_MAVLink/GCS_Common.cpp:5375

The refusal is global but the command is Copter-only, so other vehicles now return a misleading ACK. On quadplane SITL built from this head: LONG NAV_VTOL_TAKEOFF → 8 (COMMAND_INT_ONLY), INT frame 3 → 3 (UNSUPPORTED), INT frame 6 → 3 (UNSUPPORTED). With the new switch reverted and rebuilt, the same quadplane returns LONG → 3. So for every non-Copter vehicle this replaces an accurate UNSUPPORTED with an ACK telling the GCS to retry in a form that also fails. Copter is the only vehicle with a case MAV_CMD_NAV_VTOL_TAKEOFF in a COMMAND_INT handler (ArduCopter/GCS_MAVLink_Copter.cpp:491); ArduPlane has none and falls through to GCS_MAVLINK::handle_command_int_packet, which returns MAV_RESULT_UNSUPPORTED at GCS_Common.cpp:5921. Suggest making the INT-only list vehicle-aware rather than global.

ISSUE — ArduCopter/GCS_MAVLink_Copter.cpp:582

The only surviving form is COMMAND_INT with frame GLOBAL_RELATIVE_ALT; the _INT frame variant is refused. Verified empirically: LONG → 8, INT frame GLOBAL_RELATIVE_ALT_INT → 2 (DENIED), INT frame GLOBAL_RELATIVE_ALT → 0 (ACCEPTED, vehicle climbed to 5 m). The strictness is pre-existing, but this PR makes COMMAND_INT the only path, so a GCS doing the obvious migration lands on DENIED. Worth accepting MAV_FRAME_GLOBAL_RELATIVE_ALT_INT here.

ISSUE — Tools/autotest/arducopter.py:17877

The new subtest asserts only the refusal; nothing in the tree asserts the COMMAND_INT form still works. A regression breaking COMMAND_INT would pass CI. The full-tree grep for NAV_VTOL_TAKEOFF (34 hits, complete, not head-capped) shows the only arducopter.py occurrence is the new one; all four quadplane.py hits and Tools/renode/tests/test_physics_flight.py:457 are mission items. Adding a run_cmd_int(..., frame=MAV_FRAME_GLOBAL_RELATIVE_ALT, want_result=ACCEPTED) alongside the refusal would close it.

Lower-priority notes (4)

ArduCopter/GCS_MAVLink_Copter.cpp:604 — After this PR the whole Copter override is dead code, not just the line it removes. command_long_stores_location() returns false for MAV_CMD_NAV_TAKEOFF (explicitly commented out at GCS_Common.cpp:5332), and mav_frame_for_command_long() is called only under that guard (GCS_Common.cpp:5383-5387) — the only call site in the tree. Verified by mutation: replacing the entire override body with return false; and rebuilding left COMMAND_LONG MAV_CMD_NAV_TAKEOFF=22 still returning result: 0. Blimp/GCS_MAVLink_Blimp.cpp:255-262 is the identical dead override.

libraries/GCS_MAVLink/GCS_Common.cpp:5386 — The codebase now says the same thing two ways. ArduPilot's existing COMMAND_INT-only commands answer MAV_RESULT_UNSUPPORTED, not COMMAND_INT_ONLY. Verified on Copter SITL at this head: DO_REPOSITION(192) → 3, EXTERNAL_POSITION_ESTIMATE(43003) → 3 via COMMAND_LONG. The new code is the semantically correct one (precedent at GCS_Common.cpp:5476), so converting line 5386 too would make the paths agree.

libraries/AP_Scripting/applets/RockBlock.lua:339 — Pre-existing bug found while checking callers, out of scope here but worth a separate fix. The RockBLOCK applet does its own LONG→INT conversion and calls gcs:run_command_int(), bypassing this refusal entirely. Line 339 passes the key int_frame while lua_GCS_command_int reads frame (libraries/AP_Scripting/lua_bindings.cpp:1160), so the computed frame is silently dropped and pkt.frame stays 0. That is also why the wiki's claim that MAV_CMD_NAV_VTOL_TAKEOFF works over RockBLOCK (common-telemetry-rockblock.rst:42) does not hold on Copter.

ArduCopter/GCS_MAVLink_Copter.cpp:601 — Title says MAV_CMD_VTOL_TAKEOFF; the command is MAV_CMD_NAV_VTOL_TAKEOFF. Commit split is one per subsystem (GCS_MAVLink / Copter / autotest), matching house style.

What was checked and found clean

Backward compatibility and test coverage both check out. grep -rnI "VTOL_TAKEOFF" over the whole ardupilot tree plus local MAVProxy, MissionPlanner, pymavlink and MethodicConfigurator clones: MAVProxy hits are mission/map only, MissionPlanner hits are the enum, the Wireshark dissector and waypoint-editor code, pymavlink and MethodicConfigurator have none. AP_DDS calls AP::vehicle()->start_takeoff() directly, so ROS 2 is unaffected. MAV_RESULT_COMMAND_INT_ONLY = 8 is present in modules/mavlink common.xml. The test covers the fix — proven by mutation: deleting the new switch and rebuilding fails it with Expected MAV_RESULT_COMMAND_INT_ONLY got MAV_RESULT_ACCEPTED.

[peterbarker]

The default frame is MAV_FRAME_GLOBAL_RELATIVE_ALT, so MAV_CMD_NAV_TAKEOFF still gets that as the frame to assume.

[AP-Review]

Deprecated — see below for the updated review.

Previous review (head `7aff1cadc4`)

Automated review note — AI-generated (Claude), validated against the live diff. Please sanity-check before acting.

Full report: https://uav.tridgell.net/DevCallReviews/2026_09_17_AIReview/devcall_pr_reviews.html#pr34420

Re-reviewed at head 7aff1cadc4; my earlier comment above is superseded. Still a draft.

The blocking finding from last round is fixed. Verdict moves REQUEST CHANGES → COMMENT.

Previous round

• ISSUE — the refusal was global but the command is Copter-only — RESOLVED. It now sits behind a new virtual bool command_int_only(MAV_CMD) (GCS.h:812) that only Copter overrides. Verified by running both vehicles: QuadPlane answers 3 UNSUPPORTED again, Copter answers 8 COMMAND_INT_ONLY. Mutation-proved — replacing the Copter override's body with return false; turns Copter's answer back into 4.
• ISSUE — nothing asserted the COMMAND_INT form still works — RESOLVED. Tools/autotest/arducopter.py:17956 adds exactly that; I ran it and both MAV_CMD_NAV_TAKEOFF and MAV_CMD_NAV_TAKEOFF_command_int pass, with the logs showing both new subtests actually executing.
• ISSUE — only surviving form is COMMAND_INT with frame GLOBAL_RELATIVE_ALT — you disputed this and you're right. I checked modules/mavlink at your pin 71d925850d: MAV_FRAME_GLOBAL_RELATIVE_ALT_INT carries <superseded since="2024-03">, so calling frame 6 deprecated is accurate. Dropping the "must fix" framing; a reduced note remains below.
• NOTE — the whole Copter/Blimp mav_frame_for_command_long override was dead — RESOLVED, both removed. Your inline reply was correct: GCS_Common.cpp:5378 already initialises frame to MAV_FRAME_GLOBAL_RELATIVE_ALT.

Correction to my own previous comment

I said last round that no GCS sends MAV_CMD_NAV_VTOL_TAKEOFF as a runtime COMMAND_LONG. That's too strong, and I'm correcting it. It holds for every built-in automatic takeoff flow — I re-derived that independently rather than taking it from the PR body — but generic pass-through paths do exist:

• MAVProxy's long command (MAVProxy/modules/mavproxy_cmdlong.py:391) resolves any command by name via getattr(mavutil.mavlink, args[0]) and sends it with command_long_send(), so long NAV_VTOL_TAKEOFF 0 0 0 0 0 0 5 emits exactly the form this PR now refuses. I verified this directly against MAVProxy master.
• QGC's user-configurable MavlinkAction mechanism is the same shape.

So a user with a script, a MAVProxy alias, or a custom QGC action that issues this command will now get 8 instead of a takeoff. That's a small and arguably correct breakage — result 8 tells them precisely what to do, and MAVProxy has command_int available — but it's worth stating in the PR rather than described as affecting nobody. Neither I nor the second reviewer found evidence of anyone actually doing this.

For the record, the built-in flows really are unaffected: QGC's only NAV_VTOL_TAKEOFF reference in APMFirmwarePlugin.cc is inside supportedMissionCommands() (the mission-editor palette), while its runtime guided takeoff is _guidedModeTakeoff() sending MAV_CMD_NAV_TAKEOFF; MAVSDK has zero hits; MAVProxy's and Mission Planner's are waypoint-loader, dissector and display code; pymavlink has nothing outside generated dialects; and in ArduPilot itself every hit is a mission item, with no existing autotest sending this as a runtime COMMAND_LONG.

Still open

ISSUE — libraries/GCS_MAVLink/GCS_Common.cpp:5383 — two mechanisms now give two different answers for the same policy (re-raised, not new). This PR adds an explicit COMMAND_INT-only mechanism returning MAV_RESULT_COMMAND_INT_ONLY, but there's already an implicit one eight lines below: a command in command_long_stores_location() with no entry in mav_frame_for_command_long()'s frame_map falls out at :5383 with MAV_RESULT_UNSUPPORTED. That set is DO_REPOSITION, EXTERNAL_POSITION_ESTIMATE and DO_SET_GLOBAL_ORIGIN — all just as COMMAND_INT-only as NAV_VTOL_TAKEOFF now is. So a GCS sending DO_REPOSITION as COMMAND_LONG gets 3 UNSUPPORTED ("I don't know this command") when the truth is "send it as COMMAND_INT", while the same GCS sending NAV_VTOL_TAKEOFF gets the correct and actionable 8. Verified on both vehicles. Changing :5383 to COMMAND_INT_ONLY makes the two agree and picks up three more commands for free — seems in scope given the stated goal of weaning ArduPilot off the implicit frame, or worth an explicit follow-up.

Smaller points

• ArduCopter/GCS_MAVLink_Copter.cpp:582 — the sole surviving path accepts exactly one frame (INT frame 6 → 2 DENIED, frame 3 → handler). Accepting your point that frame 6 is superseded, so this is a note now. Two things still argue for accepting it: common.xml calls frame 3 a synonymous replacement, and ArduPilot's own mavlink_coordinate_frame_to_location_alt_frame() already maps 3 and 6 to the identical AltFrame::ABOVE_HOME, with a comment about Solo shot manager sending the "wrong" one. A GCS following the obvious "resend as COMMAND_INT" migration and picking the _INT frame gets DENIED with no hint. Pre-existing, but this PR removes the fallback that hid it.
• libraries/GCS_MAVLink/GCS_Common.cpp:5374 — the refusal carries no text. MAV_RESULT_COMMAND_INT_ONLY is recent; Mission Planner does carry it (ExtLibs/Mavlink/Mavlink.cs:3698), but a user on an older GCS sees a bare number. A one-line GCS_SEND_TEXT would make the migration self-explanatory, to be weighed against flash.
• libraries/AP_Scripting/applets/RockBlock.lua:339 — pre-existing and out of scope, but it interacts with this policy: the applet's COMMAND_LONG→INT shim passes int_frame while lua_GCS_command_int reads frame (lua_bindings.cpp:1160), so pkt.frame stays 0. That path calls gcs:run_command_int() directly and so isn't covered by the new refusal, and with frame 0 the Copter handler denies it — meaning the wiki's claim that MAV_CMD_NAV_VTOL_TAKEOFF works over RockBLOCK doesn't hold on Copter today.
• The title says MAV_CMD_VTOL_TAKEOFF; the command is MAV_CMD_NAV_VTOL_TAKEOFF.

MAV_RESULT_COMMAND_INT_ONLY = 8 is the spec-correct code here and strictly better than UNSUPPORTED/DENIED/FAILED. All six commits build individually for copter, blimp and plane, and the tree also builds with AP_MAVLINK_COMMAND_LONG_ENABLED 0, where the #else path already answers COMMAND_INT_ONLY — so the new code agrees with it. Commit split is clean, one subsystem each.

CI at this head: the one fail is build (sitl) in the macOS workflow, and reading the job log it's Failed to connect to github.com port 443 during submodule checkout — infrastructure, not this PR. Three autotest jobs pending; everything else green.

[AP-Review]

Automated review note — AI-generated (Claude), cross-checked by an independent Codex pass against the live diff. Please sanity-check before acting.

Full report, including everything that was checked and found clean: https://uav.tridgell.net/DevCallReviews/2026_09_17_AIReview/devcall_pr_reviews.html#pr34420

---

COMMENT

Re-reviewed at head 87f546e34c (previously 7aff1cadc4); my earlier comment above is superseded.

The two new commits close the one finding that was still open from last round — the generic path and the new command_int_only() hook now give the same answer. I ran the new autotest and mutation-proved it: reverting only the GCS_Common.cpp:5383 return fails it with Expected MAV_RESULT_COMMAND_INT_ONLY got MAV_RESULT_UNSUPPORTED. No blockers; one thing worth a deliberate decision.

Still open

• libraries/GCS_MAVLink/GCS_Common.cpp:5383 — the new blanket COMMAND_INT_ONLY is vehicle-agnostic.
  command_long_stores_location() lists DO_REPOSITION, EXTERNAL_POSITION_ESTIMATE and DO_SET_GLOBAL_ORIGIN
  unconditionally, but their COMMAND_INT handlers are not unconditional. AntennaTracker has no DO_REPOSITION case, so
  I built antennatracker SITL at this head and probed it: LONG now answers COMMAND_INT_ONLY, but INT frame 3 answers
  UNSUPPORTED — i.e. we tell the GCS to retry in a form that also fails. Mutation-proved: with only :5383
  reverted, the same tracker answers LONG -> UNSUPPORTED. EXTERNAL_POSITION_ESTIMATE is the same shape on 1MB
  boards, where its only INT case sits behind AP_AHRS_POSITION_RESET_ENABLED (source-verified only).
  Much narrower than the round-1 blocker and it costs a wasted round trip rather than a function — but it is the
  same argument that motivated adding command_int_only(), so it seems worth deciding rather than inheriting.

Smaller points

• ArduCopter/GCS_MAVLink_Copter.cpp:582 — the description's technical rationale does not apply to this
  handler. The body cites COMMAND_LONG's missing frame and float lat/lon precision, but
  handle_MAV_CMD_NAV_TAKEOFF() ignores lat/lng outright (its own comments say "not supported") and the only frame it
  accepts is the one try_command_long_as_command_int() already supplies by default. Confirmed by probe before the
  refusal. The spec-alignment argument is the solid one and the body already makes it well.
• libraries/GCS_MAVLink/GCS.h:809 — the parameter is spelled fame. Pre-existing, but this PR edits that
  exact line, so it is free to fix.
• Re-raised (unchanged since my last comment): the refusal sends no GCS_SEND_TEXT, so a GCS that does not carry
  MAV_RESULT_COMMAND_INT_ONLY shows a bare 8. Mission Planner does carry it, and the Codex pass points out the enum
  dates from 2023, so this is smaller than I implied last time. Non-blocking.
• Out of scope, re-raised: RockBlock.lua:339 passes int_frame where the binding reads frame. The Codex pass
  adds that the applet's frame mapping at :310 already selects 0 for takeoff, so fixing the key alone would still
  leave takeoff denied — worth fixing both together, separately from this PR.

Dropped from my previous comment: the suggestion to also accept MAV_FRAME_GLOBAL_RELATIVE_ALT_INT. You were right
and I had already conceded it — that frame carries <superseded since="2024-03"> in the pinned modules/mavlink.

[peterbarker]

@stephendade on the RockBLOCK stuff

[peterbarker]

ArduCopter/GCS_MAVLink_Copter.cpp:582 — the description's technical rationale does not apply to this
handler. The body cites COMMAND_LONG's missing frame and float lat/lon precision, but
handle_MAV_CMD_NAV_TAKEOFF() ignores lat/lng outright (its own comments say "not supported") and the only frame it
accepts is the one try_command_long_as_command_int() already supplies by default. Confirmed by probe before the
refusal. The spec-alignment argument is the solid one and the body already makes it well.

ArduPilot is not the only stack which supports this command, and others do support lat/lng. Us restricting to int-only is an effort to make GCS writer's life easier.