Skip to content

Preserve relative-alt guided behavior by setting Locationwp.frame at implicit callers - #3728

Open
meee1 with Copilot wants to merge 1 commit into
masterfrom
copilot/fix-set-guided-mode-ripple-effect
Open

Preserve relative-alt guided behavior by setting Locationwp.frame at implicit callers#3728
meee1 with Copilot wants to merge 1 commit into
masterfrom
copilot/fix-set-guided-mode-ripple-effect

Conversation

Copilot AI commented May 29, 2026

Copy link
Copy Markdown
Contributor

A recent guided-mode path change surfaced a behavior shift: callers constructing Locationwp without frame now send default GLOBAL (AMSL) instead of the previously normalized relative-alt behavior. For Copter paths passing relative altitude, this can produce unintended altitude commands.

  • Scope: explicit frame assignment at affected guided callers

    • Added frame = (byte)MAVLink.MAV_FRAME.GLOBAL_RELATIVE_ALT where Locationwp was built with lat/lng/alt only before setGuidedModeWP(...).
    • Updated call sites:
      • Controls/FollowMe.cs
      • Utilities/httpserver.cs (/guide GET + POST handlers)
      • Swarm/FollowPath.cs
      • ExtLibs/ExtGuided/ExtGuidedPlugin.cs
      • Plugins/example14-mass.cs
      • ExtLibs/Xamarin/Xamarin/Linked/httpserver.cs (mirrored HTTP guided handlers)
  • Behavioral intent

    • Restore pre-change semantics for these paths by making altitude frame explicit.
    • Keep existing FlyToHere / GuidedMode.frame-threaded paths untouched.
var gwp = new Locationwp
{
    lat = ...,
    lng = ...,
    alt = ...,
    frame = (byte)MAVLink.MAV_FRAME.GLOBAL_RELATIVE_ALT
};
MainV2.comPort.setGuidedModeWP(gwp);

Copilot AI changed the title [WIP] Fix ripple effect for setGuidedModeWP callers Preserve relative-alt guided behavior by setting Locationwp.frame at implicit callers May 29, 2026
Copilot finished work on behalf of meee1 May 29, 2026 21:01
Copilot AI requested a review from meee1 May 29, 2026 21:01
@meee1
meee1 marked this pull request as ready for review May 29, 2026 21:16
@tridge
tridge force-pushed the copilot/fix-set-guided-mode-ripple-effect branch from 84a9678 to 9edb50b Compare August 29, 2026 23:38
@tridge

tridge commented Aug 30, 2026

Copy link
Copy Markdown
Contributor

Automated review note — AI-generated (Claude), validated against the live diff. Please sanity-check before acting.
Full report: https://uav.tridgell.net/DevCallReviews/2026_08_30/devcall_pr_reviews.html#prMissionPlanner-3728

Reviewed at head 9edb50bffc. Verdict: COMMENT — the diagnosis and the approach are right, and I verified the premise rather than taking it from the description: Locationwp.frame is a plain byte in a struct, so any new Locationwp{…} omitting it gets 0 = MAV_FRAME_GLOBAL (AMSL), and setGuidedModeWP() passes that straight through. Commits fb694923c (Plane, 2024-10-16) and 3d92c21f5 (Copter, 2026-05-04) each replaced a hardcoded relative frame with gotohere.frame, so these frameless callers have silently been commanding AMSL. Per-call-site assignment is the only workable fix, since 0 is both "unset" and a legitimate user choice ("Absolute"), so nothing central can distinguish them. Frame 3 vs the _INT variant is harmless — ArduPilot maps both to AltFrame::ABOVE_HOME. Each patched site does carry a relative altitude (FollowMe's prompt says "relative to home alt"; FollowPath uses relative_alt/1000; example14 hardcodes 50).

Two things to fix or consider before merge:

  1. ISSUE — the fix is incomplete. Two shipped example scripts still build Locationwp with only lat/lng/alt and call setGuidedModeWP, so they keep defaulting to AMSL: Scripts/example3.py:55 (a Plane in Guided at alt = 60 — precisely the "60 m relative read as 60 m AMSL" hazard this PR fixes) and Scripts/example6.py:103. These are the files people copy from. Worth folding into this PR.
  2. ISSUE — pre-existing, on the same path, arguably in scope. Controls/OpenGLtest2.cs:121 calls new Locationwp().Set(lat, lng, MainV2.comPort.MAV.GuidedMode.z, WAYPOINT), and Set() hardcodes frame = 3, discarding MAV.GuidedMode.frame. If the user selected "Absolute" or "Terrain", an AMSL/terrain altitude is sent flagged relative. Fix by assigning gotohere.frame = MainV2.comPort.MAV.GuidedMode.frame after Set(), mirroring FlightData.cs:3113.

Non-blocking notes: external plugins/scripts calling this public API with a zero-initialised Locationwp still send AMSL and can't be fixed from inside the tree — worth a release note; and /guide still has no way to request absolute or terrain altitude, so an optional &frame= parameter would be a sensible follow-up. Separately, Plugins/example14-mass.cs:132-133 divides FDMenuMapPosition.Lat/.Lng by 1e7 although they're already degrees, so that fly-to is broken regardless of the frame fix — not yours to fix, just be aware the added line is cosmetic there.

Census checked: of the 15 setGuidedModeWP call sites outside MAVLinkInterface.cs, 8 are patched here, the 4 in FlightData.cs already thread frame and are correctly untouched, and the rest are the three above. Both httpserver.cs copies were caught; there is no third. The failing submit-nuget check fails identically on master and is unrelated. A cold second review argued this silently switches Plane users from AMSL to relative — I checked fb694923c's diff and that's not so (the Plane branch was hardcoded GLOBAL_RELATIVE_ALT before it), so the PR restores the long-standing contract rather than changing it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Heads-up that this change has a subtle ripple effect for setGuidedModeWP callers that don't explicitly set

3 participants