MAVLink DO_REPOSITION: MAV_FRAME_GLOBAL is enforced but the altitude is used as relative to home

[SebastianKumor]

Current Behavior

MAV_CMD_DO_REPOSITION over MAVLink accepts only MAV_FRAME_GLOBAL, but then uses the altitude as
if it were relative to home. The frame check and the altitude conversion disagree with each other,
so a GCS that follows the MAVLink specification makes the aircraft climb by the field elevation.

handleIncoming_COMMAND_INT rejects anything that is not MAV_FRAME_GLOBAL
(src/main/telemetry/mavlink.c):

if (!(msg.frame == MAV_FRAME_GLOBAL)) { //|| msg.frame == MAV_FRAME_GLOBAL_RELATIVE_ALT || msg.frame == MAV_FRAME_GLOBAL_TERRAIN_ALT)) {
        mavlink_msg_command_ack_pack(..., MAV_RESULT_UNSUPPORTED, ...);

In MAVLink, MAV_FRAME_GLOBAL means the altitude is AMSL, and MAV_FRAME_GLOBAL_RELATIVE_ALT is
the frame that means "relative to home". The handler then fills wp.alt = msg.z * 100.0f and calls
setWaypoint(255, &wp), which converts with GEO_ALT_RELATIVE
(src/main/navigation/navigation.c):

else if ((wpNumber == 255) && (wpData->action == NAV_WP_ACTION_WAYPOINT) && isGCSValid()) {
    // Convert to local coordinates
    geoConvertGeodeticToLocal(&wpPos.pos, &posControl.gpsOrigin, &wpLLH, GEO_ALT_RELATIVE);

and that branch takes the value verbatim
(src/main/navigation/navigation_geo.c):

// If flag GEO_ALT_RELATIVE, than llh altitude is already relative to origin
if (altConv == GEO_ALT_RELATIVE) {
    pos->z = llh->alt;
} else {
    pos->z = llh->alt - origin->alt;
}

So the accepted frame promises AMSL and the code consumes metres above home. Only one of the two can
be right.

The same applies to the MISSION_ITEM guided path (current == 2), which enforces
MAV_FRAME_GLOBAL in the same way and reaches the same setWaypoint(255, ...).

Steps to Reproduce

1. Set mavlink_autopilot_type = ARDUPILOT (or leave it GENERIC), connect a GCS over MAVLink, arm,
   enter POSHOLD with the GCS NAV box active so isGCSValid() is true.
2. From the GCS, send MAV_CMD_DO_REPOSITION as COMMAND_INT with frame = MAV_FRAME_GLOBAL and
   z set to a spec-correct AMSL altitude, i.e. the desired height above ground plus the field
   elevation. For a 300 m field and an 80 m target that is z = 380.
3. The command is accepted with MAV_RESULT_ACCEPTED.
4. The aircraft climbs toward 380 m above home instead of 80 m.

The size of the error is exactly the field elevation, so it is invisible at sea level and large in
the mountains.

Expected behavior

Either interpretation is fine as long as the frame that is accepted matches the conversion used:

• Keep MAV_FRAME_GLOBAL and convert AMSL to a home-relative altitude for setWaypoint(255, ...),
  i.e. use GEO_ALT_ABSOLUTE so pos->z = llh->alt - origin->alt; or
• Accept MAV_FRAME_GLOBAL_RELATIVE_ALT (the frame whose meaning already matches the code) and keep
  GEO_ALT_RELATIVE.

The second is the smaller change and matches what the handler already does with the number. It would
also line up with the commented-out frames in the current check, which suggests relative altitude
was the intent.

Suggested solution(s)

Smallest fix, accept the frame that means what the code does:

if (!(msg.frame == MAV_FRAME_GLOBAL_RELATIVE_ALT)) {
    // ... MAV_RESULT_UNSUPPORTED
}

Or keep the frame and fix the conversion, replacing GEO_ALT_RELATIVE with GEO_ALT_ABSOLUTE on
the WP #255 path only. Supporting both frames and choosing the conversion per frame would be the
complete answer, and would also address the // TODO: Alt modes note next to wp.p2 in the same
handler.

Happy to open a PR for whichever direction the maintainers prefer.

Additional context

Found while adding INAV support to a ground station (Kite Ground Control) and verified by reading
master rather than in flight, so there is no board or blackbox log to attach: the report is a code
analysis, and the reproduction above is what the code does, not a flight I made. I did not want to
guess at hardware details for the fields below.

The handler arrived in #11061, so this affects 9.0.0 onward; 7.x and 8.x do not handle COMMAND_INT
at all.

Worth noting for anyone testing a GCS against this: a simulator written from the MAVLink
specification will happily agree with a GCS that converts to AMSL, and both will be wrong in the
same direction. That is how this stayed hidden in our own testing until the firmware was read
directly.

---

• FC Board name and vendor: not applicable, source analysis against master
• INAV version string: master (9.1.0), and 9.0.x by inspection