Repository navigation
wp: fix Draw altitude and add remembered frame selection - #1769
Conversation
Draw read the wrong argument index, leaving the requested altitude unused. Reuse the frame/altitude dialog to remember session choices and create waypoints with the selected AboveHome, AGL or AMSL frame.
Previous review (2026-10-04)Automated review note — AI-generated (Claude), validated against the live diff. Please sanity-check before acting. Reviewed at head Thanks, the single-argument altitude fix is real and the new tests pass. One problem remains: the frame handling now overrides the automatic terrain default. The Draw dialog fixes its default frame when the wp module loads, before TERRAIN_FOLLOW is known (https://github.com/ardupilot/mavproxy/pull/1769/files#diff-32e9fbffd1d495e943604af13b0c1ea91d3151abf35c3281abc5fa0278ac0503R39). On a terrain-following vehicle, accepting the defaults now gives AboveHome (frame 3) waypoints instead of terrain-relative ones (frame 10). Separately, a plain "wp draw" stores the automatically chosen frame in draw_frame (https://github.com/ardupilot/mavproxy/pull/1769/files#diff-32e9fbffd1d495e943604af13b0c1ea91d3151abf35c3281abc5fa0278ac0503R322), so later draws skip get_default_frame() (https://github.com/ardupilot/mavproxy/pull/1769/files#diff-32e9fbffd1d495e943604af13b0c1ea91d3151abf35c3281abc5fa0278ac0503R309) even after TERRAIN_FOLLOW or terrainalt changes. Could draw_frame hold only frames the operator chose explicitly, with the default worked out at draw time (for example, a "Default" dropdown entry that leaves the frame argument off)? A minor optional point: frames chosen on the CLI are not reflected in the dialog defaults (https://github.com/ardupilot/mavproxy/pull/1769/files#diff-32e9fbffd1d495e943604af13b0c1ea91d3151abf35c3281abc5fa0278ac0503R36). Checked offline with mocked wx and map; not checked with a live GUI or SITL. |
Capturing the frame at menu creation or caching an automatic choice ignored terrain parameters loaded or changed later. Add a Default option and remember only explicit frame choices, binding the resolved frame to each active draw.
|
Automated review note — AI-generated (Claude+Codex), validated against the live diff. Please sanity-check before acting. Reviewed at head Thanks, the follow-up commit fixes both earlier blocking points. 'Default' is now resolved when each draw starts, and only explicit frames are remembered. Automatic draws track TERRAIN_FOLLOW changes as base did, and the new tests catch the old behaviour. One small, non-blocking suggestion: the frame is now fixed for the vehicle selected when the draw starts (https://github.com/ardupilot/mavproxy/pull/1769/files#diff-32e9fbffd1d495e943604af13b0c1ea91d3151abf35c3281abc5fa0278ac0503R324), but the completed line goes to whichever vehicle is selected when it finishes. Switching vehicles mid-draw can therefore give the second vehicle the first one's default frame. Base already uploads the line to the wrong vehicle in that case, so recording target_system in cmd_draw and dropping or warning on a completion for a different target would fix both. As before, CLI choices are not shown as the dialog defaults (https://github.com/ardupilot/mavproxy/pull/1769/files#diff-32e9fbffd1d495e943604af13b0c1ea91d3151abf35c3281abc5fa0278ac0503R37), which is optional. Checked offline with mocked wx and map; not checked with a live GUI or SITL. |
2 participants
Draw read the wrong argument index, leaving the requested altitude unused. Reuse the frame/altitude dialog to remember session choices and create waypoints with the selected AboveHome, AGL or AMSL frame.