Skip to content

misseditor: add survey planning and live draft mission maps - #1770

Merged
tridge merged 3 commits into
ArduPilot:masterfrom
tridge:pr-survey-grid
Oct 8, 2026
Merged

tridge merged 3 commits into
ArduPilot:masterfrom
tridge:pr-survey-grid

Conversation

@tridge

@tridge tridge commented Oct 3, 2026

Copy link
Copy Markdown
Contributor

Generate rectangular lawnmower surveys from a selected waypoint with a live blue preview, camera spacing controls and selectable altitude frames.

Render local mission edits on the maps while keeping them MODIFIED and separate from the vehicle transfer cache; only Write WPs uploads, while Read WPs restores the controller mission without overwriting newer edits.

image image

Generate rectangular lawnmower surveys from a selected waypoint with a live
blue preview, camera spacing controls and selectable altitude frames.

Render local mission edits on the maps while keeping them MODIFIED and
separate from the vehicle transfer cache; only Write WPs uploads, while
Read WPs restores the controller mission without overwriting newer edits.
@tridge tridge added the AIReview label Oct 3, 2026
@AP-Review

AP-Review commented Oct 4, 2026 •

Copy link
Copy Markdown

Deprecated — see below for the updated review.

Previous review (2026-10-04)

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

Reviewed at head 14594f2610.
Full report: https://firmware.ardupilot.org/Tools/APReview/DevCallReviews/PRReviews/ardupilot/mavproxy/1770/1.html#prMAVProxy-1770

Thanks for this, survey planning and live draft maps are a nice addition. One thing needs fixing before merge. If the editor has unsaved edits and you use Load File, the file is still uploaded to the vehicle, but the editor never refreshes. The new draft check in idle_task (https://github.com/ardupilot/mavproxy/pull/1770/files#diff-0ab3018f1d2d9bd9bfddf3d8f1b1eb69430582a485e5e70a4c2abce261ae73e7R360) skips the refresh, and the MEGE_LOAD_MISSION guard (https://github.com/ardupilot/mavproxy/pull/1770/files#diff-3ccdbffa105123db9eb6299fa2be25cc401eca58bd520167b46e445b02a3f483R515) would reject it anyway. The grid and maps keep showing the old draft while the vehicle has the file, and a later Write WPs would quietly undo the load. Before this PR the editor refreshed here. An explicit file load should probably be treated as a user replacement, separate from unsolicited controller refreshes. Two smaller points. First, after an editor MAVLink write num_wps_expected stays at the count, so a later console 'wp list' goes through the old incremental path (https://github.com/ardupilot/mavproxy/pull/1770/files#diff-0ab3018f1d2d9bd9bfddf3d8f1b1eb69430582a485e5e70a4c2abce261ae73e7R413). That overwrites edits without the new guard and leaves the map draft stale. The overwrite predates this PR, but now that reads are atomic it may be simplest to remove that branch or reset the counter after the write. Second, the 2D map only notices a new draft when MAVLink packets arrive, so planning without telemetry doesn't update the 2D map. That is the existing trigger for all 2D waypoint changes, so this is just a suggestion. Not covered: native wx GUI behaviour, live vehicle or SITL transfers, and real map rendering. CI was still pending at this head.

File loads were uploaded while the editor and maps retained an older draft.
Import them as modified local missions, and accept mission packets only for
explicit editor reads so later console reads cannot overwrite local edits.
Refresh map missions during idle as well, allowing planning without telemetry.
@AP-Review

AP-Review commented Oct 4, 2026 •

Copy link
Copy Markdown

Deprecated — see below for the updated review.

Previous review (2026-10-04)

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

Reviewed at head f62339e041.
Full report: https://firmware.ardupilot.org/Tools/APReview/DevCallReviews/PRReviews/ardupilot/mavproxy/1770/2.html#prMAVProxy-1770

Thanks for the quick follow-up. f62339e fixes all three points from the last review. Load File is now a local draft replacement, console reads can no longer overwrite editor edits, and the 2D map redraws without telemetry. One problem comes with the local Load. Save WP File was not changed, so it still runs 'wp save', which downloads and saves the vehicle's mission instead of the grid (MAVProxy/modules/mavproxy_misseditor/missionEditorFrame.py:986 and mission_editor.py:193). Load now also sets last_mission_file_path (https://github.com/ardupilot/mavproxy/pull/1770/files#diff-3ccdbffa105123db9eb6299fa2be25cc401eca58bd520167b46e445b02a3f483R830), so the Save dialog suggests the file you just loaded. Load a file, edit it and save it back, and the file is overwritten with whatever is on the vehicle. In a headless reproduction a file at alt 321, edited to 444, was saved with alt 120. With no vehicle connected nothing is written and the editor shows no error. Before this commit Load uploaded the file, so Load then Save kept the contents. Writing the grid locally would fix it: build a MAVWPLoader as update_map_mission does and call save(path), as the read_only branch already does. A smaller point, not blocking: reading_mission and num_wps_expected are set under event_queue_lock but cleared on completion under gui_event_queue_lock (https://github.com/ardupilot/mavproxy/pull/1770/files#diff-0ab3018f1d2d9bd9bfddf3d8f1b1eb69430582a485e5e70a4c2abce261ae73e7R409). A second MAVLink Read pressed just as the first finishes can be cancelled by it. The first result is still displayed, so this is minor, but a per-read generation number would close the gap. Not covered: native wx dialogs, live vehicle or SITL transfers, and real map rendering.

Save the validated grid directly so saving a loaded mission cannot overwrite it with a controller download. Preserve unnamed frames and default empty optional parameters during local saves. Serialize read state and tag replies by request so an older completion cannot cancel a newer read or replace its draft.
@AP-Review

Copy link
Copy Markdown

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

Reviewed at head d1ca86cbb3.
Full report: https://firmware.ardupilot.org/Tools/APReview/DevCallReviews/PRReviews/ardupilot/mavproxy/1770/3.html#prMAVProxy-1770

Thanks for d1ca86c. It fixes both points from the last review: Save WP File now writes the grid locally, and overlapping MAVLink reads are tagged and checked by id. Load, edit and Save keeps the edited values, and the new tests fail at f62339e and pass at head. Nothing blocking remains, but there are a few small points. Write WPs still uses plain cmd_reverse_lookup (MAVProxy/modules/mavproxy_misseditor/missionEditorFrame.py:796), which returns 0 for a numeric command that isn't in the installed dialect. Load used to go through 'wp load', which kept the number. Now a loaded file with, say, command 65500 shows 65500 in the grid and saves as 65500, but uploads as command 0. Using the same int() fallback as mission_wploader would fix it (https://github.com/ardupilot/mavproxy/pull/1770/files#diff-3ccdbffa105123db9eb6299fa2be25cc401eca58bd520167b46e445b02a3f483R726). Local Save writes autocontinue=1 on every row (https://github.com/ardupilot/mavproxy/pull/1770/files#diff-3ccdbffa105123db9eb6299fa2be25cc401eca58bd520167b46e445b02a3f483R735), so a file with autocontinue=0 changes after an unedited load and save. Uploads have always sent 1, so this only affects the file. MAVLink packets are queued without a read id. If read 1's replies are still in mavlink_message_queue when Read is pressed again, they complete read 2 with read 1's data, and read 2's own replies are dropped (https://github.com/ardupilot/mavproxy/pull/1770/files#diff-0ab3018f1d2d9bd9bfddf3d8f1b1eb69430582a485e5e70a4c2abce261ae73e7R392). It is about a 0.1 s window and normally the same mission, but clearing the queue in start_read would close it. Map drafts are labelled with whichever vehicle is selected when the backend handles the event (https://github.com/ardupilot/mavproxy/pull/1770/files#diff-0ab3018f1d2d9bd9bfddf3d8f1b1eb69430582a485e5e70a4c2abce261ae73e7R106). An edit made just before a vehicle switch shows up as the new vehicle's draft. It is display-only, and the next edit would be attributed the same way anyway. Not covered: live vehicle or SITL transfers, real map rendering, and native wx dialogs.

@tridge
tridge merged commit c521f16 into ArduPilot:master Oct 8, 2026
4 checks passed
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.

2 participants