Plan: add an ALL mission type to edit mission, fence and rally items … by Autonomost · Pull Request #3780 · ArduPilot/MissionPlanner

Autonomost · 2026-09-17T03:04:25Z

…together

The mission type dropdown on the Plan screen gains an ALL entry. In that mode the single command grid holds mission, fence and rally items at once; a row's type follows its command (RALLY_POINT, FENCE_* or anything else), so no extra columns are needed. The map draws all three sets in one overlay with type-aware midline handles, and the fence polygon tools work in this mode.

Read and Write open a small picker (Mission / Fence / Rally, all ticked by default) so one click transfers everything or any subset. Reading a subset keeps the grid rows of the types that were not fetched. Uploads go in mission, fence, rally order; a type the vehicle rejects (for example fence items on old firmware) is reported without aborting the rest.

The MAVFTP download path now updates the cache matching the item type instead of always writing the mission cache.

…together The mission type dropdown on the Plan screen gains an ALL entry. In that mode the single command grid holds mission, fence and rally items at once; a row's type follows its command (RALLY_POINT, FENCE_* or anything else), so no extra columns are needed. The map draws all three sets in one overlay with type-aware midline handles, and the fence polygon tools work in this mode. Read and Write open a small picker (Mission / Fence / Rally, all ticked by default) so one click transfers everything or any subset. Reading a subset keeps the grid rows of the types that were not fetched. Uploads go in mission, fence, rally order; a type the vehicle rejects (for example fence items on old firmware) is reported without aborting the rest. The MAVFTP download path now updates the cache matching the item type instead of always writing the mission cache.

Copilot

🟡 Changes recommended

Unresolved critical and moderate issues affect mixed-item handling and transfers.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Adds an ALL mission type combining mission, fence, and rally items in Flight Planner.

Changes:

Adds unified editing and type-aware map rendering.
Adds selective Mission/Fence/Rally read and write support.
Updates MAVFTP caching by item type.

File summaries

File	Summary
`GCSViews/FlightPlanner.cs`	Implements ALL-mode planning, transfers, rendering, and cache updates.
`Controls/MissionTypePicker.cs`	Adds the Mission/Fence/Rally selection dialog.

Review details

Suppressed comments (3)

GCSViews/FlightPlanner.cs:722

After a subset is selected here, the existing confirmation below still says the read will clear all existing points, while getWPs(sender, types) deliberately preserves rows whose types were not fetched. The prompt is false for the new partial-read flow and can make users cancel a safe refresh; make it conditional on all types being selected or change its wording.

                    types = MissionTypePicker.Show(this, "Read");
                    if (types == null || types.Count == 0)
                        return;

GCSViews/FlightPlanner.cs:8006

This infers the midline type from the physical row immediately before pnt2, but ALL-mode mission midlines are built after fence/rally points are filtered out. A mission edge can therefore span fence rows; dragging it sees a fence vertex at pnt2 - 2 and inserts/recalculates a fence command instead of a mission waypoint. Determine the type from the midline's tagged endpoints or attach the type when creating the marker.

                            if (midtype == MAVLink.MAV_MISSION_TYPE.ALL)
                                midtype = IsFencePolygonVertexRow(pnt2 - 2)
                                    ? MAVLink.MAV_MISSION_TYPE.FENCE
                                    : MAVLink.MAV_MISSION_TYPE.MISSION;

GCSViews/FlightPlanner.cs:850

The picker is shown only after the preflight loop above validates every grid row. In ALL mode, choosing only one type still fails on an invalid number or zero-altitude warning in an unselected fence/rally row, so the new subset write cannot ignore that type. Select the types before validation or limit validation to rows whose command type is selected.

                types = MissionTypePicker.Show(this, "Write", counts);

Files reviewed: 2/2 changed files
Comments generated: 5
Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

- Classify FENCE_HOME_CIRCLE_INCLUSION (legacy home centred fence) as a fence item so it is not uploaded as a mission command. - Reading a subset of types now says it replaces only those types instead of claiming all points will be cleared. - Ask which types to write before checking the grid, and only apply the altitude checks to rows of a type that is being sent. - Distance, gradient and bearing are computed from a mission-only list in ALL mode, so mission legs no longer span fence or rally rows and those rows are left alone. - Dragging a mission midline takes its type from the row it starts at rather than the physical row before its end, which could be a fence vertex in ALL mode. - Insert the home sentinel when the downloaded mission is empty, not only when the mission was not refreshed, so the first fence or rally row is not stripped as home. - When several types are written, a type the vehicle rejects is logged and the remaining types are still sent; the failures are reported together at the end. A single type fails as before. - MissionTypePicker takes its captions from Strings.resx so the dialog is translated like the rest of the planner.

Autonomost

Made the changes as requested.

Autonomost · 2026-09-29T03:39:01Z

@meee1 this should be good to go now

Copilot

Warning

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

Copilot review overview

Review effort: Lite
Findings: 6 · 1

Open (7)

SelectedMissionType only checks SelectedValue, but ComboBox data-binding can yield enums via… · New
These new/modified user-facing strings are hardcoded (prompt) and the partial read/write failures… · New
In ALL mode, UNKNOWN rows are treated as sending unconditionally, which can cause validation… · New
In ALL mode, UNKNOWN rows are treated as sending unconditionally, which can cause validation… · New
These new/modified user-facing strings are hardcoded (prompt) and the partial read/write failures… · New
These new/modified user-facing strings are hardcoded (prompt) and the partial read/write failures… · New
using System.Drawing; is unused in this file. Removing it avoids unnecessary dependencies and… · New

Resolved since last review (5)

ALL-mode processToScreen always removes the first item as the mission home. If a successful…
Every UploadMissionItems exception escapes the outer try, so a rejected or unsupported type…
In ALL mode wpOverlay.pointlist contains mission, fence, and rally points, but…
FENCE_HOME_CIRCLE_INCLUSION (MAV_CMD 5005) is also a fence mission item, but this classifier…
This new WinForms dialog hardcodes all user-visible text (Read, Write, Cancel, Mission,…

Files not reviewed (1)

ExtLibs/Strings/Strings.Designer.cs: Generated file

Brings the 32-bit system id work (e6454cc, 0077a89, 1517bce) so the branch merges cleanly again. Stops short of ae8c290, whose regenerated Mavlink.cs does not compile. Conflict: upstream dropped the (byte) cast on sysidcurrent in the item upload code, which this branch had moved into UploadMissionItems; the cast is dropped there instead.

- The confirmation prompt, the ALL mode hint and the two "some items could not be read / sent" messages come from Strings.resx like the picker's text, and the per-type failures use the picker's localized type names. - Before a write, a row's item type comes from MissionTypeOfRow, which reads an UNKNOWN row's command id from the Command cell's Tag the way the upload does. An UNKNOWN row of a type the user did not select is no longer validated (it used to count as being sent, so its zero altitude check could block a fence-only or rally-only write). - MissionTypePicker drops an unused using.

Found by a review pass over the whole PR before pushing: - Switching the dropdown to ALL with no mission cached deleted the first fence or rally row, because processToScreen strips row 0 as home. GetAllCachedItems now puts a placeholder in front in that case, the same one getWPs uses; it sits at 0, 0 so the "Reset Home" prompt cannot fire on it. - Dropping the first mission midline (home to waypoint 1) inserted a fence vertex whenever the row above the first waypoint was a fence vertex. The midline marker now carries the type of leg it was drawn on (writeKML: mission, addFenceMidlines: fence) and the drop uses that; the row inference and IsFencePolygonVertexRow are gone. - The Write picker ticks only the types that have rows, so a mission-only plan no longer sends an empty fence and rally, which cleared them on the vehicle. - Automatic reads (after a guided upload, on connect, from a plugin) fetch the mission only, as before ALL existed: no picker, and no fence or rally download that fails on older firmware. A full ALL read no longer parses the grid first, so a half edited cell cannot stop it; a partial read still keeps the rows of the types it does not fetch. - Home is read back once after a write instead of once per type, and the terrain prefetch covers home and the mission legs that were sent instead of every grid row. A MAVFTP upload now updates the vehicle side cache like the item by item path does, so switching the dropdown afterwards shows what was sent. - The pointlist the KML server and the 3D view read holds the mission legs only. - "Some items could not be read / sent" is shown as the message itself rather than as an unexpected error with a stack trace, and saveWPs releases giveComport in a finally. - Progress text uses the localized type names on read and write, MAVFTP's -1 progress passes through unscaled, the write validation reuses the parsed grid instead of re-reading the Command cells, the remaining raw dropdown casts use SelectedMissionType, ReCalcFence uses IsFencePolygonVertex, and the Strings.Designer comment for AllModeHint has the full text.

Autonomost · 2026-10-04T02:03:10Z

Three commits since the last review:

18bb4bc merges master up to 1517bce so the branch is mergeable again. It stops short of ae8c290, whose regenerated Mavlink.cs does not compile (every workflow on master and on every PR has been red since 2026-09-30 for that reason). The one conflict was upstream dropping the (byte) cast on sysidcurrent in the item upload code, which this branch had moved into UploadMissionItems; the cast is dropped there.
cfa1c2c answers the review: the new user-facing strings live in Strings.resx, an UNKNOWN row no longer counts as sent whatever its type (before a write it is classified by the command id in its Tag, the way the upload treats it), and the unused using is gone. The SelectedValue finding is answered on its thread: the combo is data-bound with no ValueMember, so SelectedValue is the enum item itself.
39ee73f is a second look at the whole feature before pushing, listed in its commit message. The two user-visible bugs: switching the dropdown to ALL with no mission cached deleted the first fence or rally row (processToScreen strips row 0 as home), and dropping the first mission midline after fence rows inserted a fence vertex instead of a waypoint. Also from that pass: the Write picker now ticks only the types that have rows, so a mission-only plan no longer clears the fence and rally on the vehicle; automatic reads (guided upload, connect, plugins) fetch the mission only and show no picker; home is read back once per write instead of once per type; and partial results are shown as a message rather than as an unexpected error. It also simplifies the UNKNOWN fix from cfa1c2c: the write checks take the type of each row from the parsed grid that the upload itself uses.

Built on Windows (MissionPlanner.csproj, Release). The MissionTypeOf classification is covered by unit tests in our fork, which pass.

@mission

A read or write of several item types carried on after any failure. That suits a vehicle that refuses a type, such as fence items it does not support, but not one that stopped answering: every remaining type waited out the same timeouts, and a write could start the next transfer while the vehicle was still inside the one that timed out. The mission protocol throws TimeoutException when its retries run out and a plain exception for a refusal, so a timeout now ends the read or write, and the message names the types that were skipped. A refusal still moves on to the next type. With MAVFTP on, a write went through MAVFtp.UploadFile, which ignores the results of its create and write steps. A vehicle that did not answer therefore looked like a finished upload: the write reported success and never fell back to sending the items one by one. The upload now runs those steps itself and falls back when they get no reply, so a silent vehicle ends in a timeout and one without MAVFTP still receives the items. A session reset with no answer already means no MAVFTP, so the file steps, which wait about 30 s each, are skipped then. A read already fell back when the open got no reply, by way of a null reference; that case is now handled explicitly. The three @mission paths are in one helper used by both directions, and the partial result message is built in one place.

Autonomost · 2026-10-04T02:50:15Z

One more commit, 05dc47f, from a last look at how ALL mode behaves when the vehicle stops answering:

A read or write of several types now stops at the first timeout instead of going on to the next type. The mission protocol throws TimeoutException when its retries run out and a plain exception when the vehicle refuses, so a refusal (fence items the vehicle does not support, for example) still moves on to the next type. The message lists the types that were skipped.
With MAVFTP enabled, MAVFtp.UploadFile ignores the results of its steps, so a vehicle that did not answer looked like a finished upload: the write reported success and never fell back to the item by item upload. That was already the case on master before this PR, but a write here can cover three types. The upload now runs those steps itself and falls back when they get no reply. A session reset with no answer already counts as no MAVFTP, which skips the two file steps that wait about 30 s each.

Built on Windows (MissionPlanner.csproj, Release). In our fork a test runs the upload against a simulated vehicle that keeps sending heartbeats but answers nothing: it now ends in a timeout after about 8 s instead of reporting success. Not yet tried on a real vehicle.

Switching the mission type dropdown loaded the new type from the vehicle's copy and threw the grid away, so rally points dropped in RALLY mode were gone after a look at the mission, and rows edited in ALL mode were lost on the way to any single type. The grid's rows are now kept per type when the dropdown leaves a type (all three types apart when it leaves ALL), and a type that has been shown comes back as it was, sent to the vehicle or not. A type never shown still comes from the vehicle's copy, and a read or a file load replaces what is shown as before. ALL joins the kept or cached parts in its usual order, with the home placeholder in front when there is no mission.

The dropdown's first event fires while the page is being built, and it kept the empty grid as an empty mission list, which came back as a lone home placeholder. processToScreen with a non-empty list calls Invoke, which throws before the page has a handle; the ComboBox data binding swallowed the exception, so the placeholder row stayed in the grid and quickadd was left set. Kept rows that are empty now stay empty (nothing to strip, so no placeholder), and ALL with nothing to show is empty too, which is the path the page took before rows were kept.

Autonomost · 2026-10-05T01:36:05Z

Two more commits: switching the dropdown no longer throws the grid away.

2775233 Rally points dropped in RALLY mode were gone after a look at the mission, and rows edited in ALL mode were lost on the way to a single type, because each switch reloaded the new type from the vehicle's copy. The grid's rows are now kept per type when the dropdown leaves a type (the three types apart when it leaves ALL) and come back as they were when the type is chosen again, sent to the vehicle or not. A type never shown still comes from the vehicle's copy, and a read or a file load replaces what is shown as before. ALL joins the kept or cached parts in its usual order, with the home placeholder in front when there is no mission.
cc75ba2 An empty kept list shows an empty grid. The first commit let the dropdown's constructor-time event bring an empty mission back as a lone placeholder row, and processToScreen with a non-empty list calls Invoke before the page has a handle; the ComboBox data binding swallowed the exception, which left the placeholder row in the grid and quickadd set. The fork's unit tests caught it (a FlightPlanner built in a test had an extra row); the empty list now takes the path it always took.

The keep/show helpers behind the dropdown handler are covered by unit tests in my fork: rows kept per type, the home placeholder in front of kept mission rows and none when they are empty, the ALL split and join, a type never shown coming from the vehicle's copy, and a new page starting with an empty grid.

@meee1 could you take another look?

Autonomost · 2026-10-18T20:30:33Z

@meee1 any chance you can merge this in?

meee1 · 2026-10-22T06:57:17Z

Review notes. The per-type split upload is right (separate uploads, home only on the mission, fence frame GLOBAL, mission -> fence -> rally order), as are partial reads and the narrowed validation. But:

High: DO_JUMP and WP numbers diverge from the vehicle's sequence numbers in ALL mode. A map click appends the new WAYPOINT after existing fence/rally rows. The grid "#", the marker labels and DO_JUMP p1 use grid row numbers, but saveWPs renumbers mission items among the mission rows only. With mission rows 1-3 and fence rows 4-7, a click adds WP "8", which the vehicle stores as seq 4, so a DO_JUMP to 8 jumps to a missing or wrong item. Suggest inserting new mission items after the last mission row (keeping mission/fence/rally ordered), and rejecting interleaved rows or remapping row references before upload.
Medium: keptRows goes stale across reconnects and vehicles; it's never cleared on connect or on a sysid change. Switching to FENCE/RALLY now shows rows from an earlier session or vehicle instead of MAV.fencepoints/rallypoints, and ALL Write pre-ticks every type that has rows, so one click can upload vehicle A's fence and rally to vehicle B. Clear it on connect/MAV change and after a successful read/write, or key it by sysid/compid.
Low: on FormatException, KeepGridRows returns without saving, so switching type with one invalid cell loses that type's edits. Block the switch and tell the user instead.
Low: an ALL-mode file loaded in MISSION mode mixes RALLY_POINT/FENCE_* items into the mission, and the upload is then rejected. Warn or filter on load.
Expect a trivial Strings.resx conflict with Plan/Data/SITL: enter and show home, POIs and SITL start in GEO, UTM … #3781 / master.

Generated by Claude Code

The PR had become conflicting: upstream's firmware selection dialog (977551b, b43f01f) appended its strings to Strings.resx and Strings.Designer.cs at the same place as the ALL mode strings. Both sets are kept; nothing else conflicted.

Row numbers: a map click (or any of the add menus) in ALL mode appended the new mission item after the fence and rally rows. The grid's row numbers, the map labels and DO_JUMP's target use the row number, but saveWPs sends each type apart and the vehicle numbers the mission items among themselves, so with mission rows 1-3 and fence rows 4-7 a new waypoint showed as 8 and was stored as 4. New rows now go after the last row of their own type (AddRowFor: mission, fence, rally order), and Write refuses to send the mission while a mission row comes after a fence or rally row (Strings.AllModeMissionFirst), for rows moved there by hand. Kept rows: the rows kept per type when the dropdown switches were never cleared, so after a reconnect or with another vehicle FENCE and RALLY showed the rows of the earlier one, and an ALL Write ticked them for the new vehicle. They are now kept per MAVState in a ConditionalWeakTable: MAVLinkInterface.Open clears the MAV list, so a new connection or another sysid starts with nothing kept and shows that vehicle's copy. A cell that is not a number made KeepGridRows give up, and the switch then lost that type's edits. The dropdown now goes back to the type shown and says which row is wrong. Loading a file saved in ALL mode while MISSION, FENCE or RALLY is shown put every item into the grid, and a mission upload was then refused for its fence and rally items. Outside ALL only the dropdown's type is loaded (home stays), and a message says how many items were left out (Strings.FileItemsOfOtherTypes).

Autonomost · 2026-10-25T23:35:32Z

Thanks for the review. Upstream master (989bda3) is merged in as e8792f0 (the Strings.resx/Designer conflict with the firmware selection strings; both kept), and 13954f4 answers the points:

Taken. In ALL mode a new row goes after the last row of its own type, in mission, fence, rally order (AddRowFor, used by the map click and every add menu), so a new waypoint lands above the fence and rally rows and keeps the number the vehicle will give it. Write also refuses to send the mission while a mission row comes after a fence or rally row (moved there by hand), with a message that says why, rather than remapping DO_JUMP targets.
Taken. The kept rows are now held per MAVState, in a ConditionalWeakTable. MAVLinkInterface.Open clears the MAV list, so a reconnect or another sysid starts with nothing kept and FENCE/RALLY show that vehicle's copy; rows kept for one vehicle can no longer be shown for, or written to, another. A read or write doesn't need to clear them: they only stand in for a type while another one is shown, and switching away from a type keeps what the grid holds at that moment.
Taken. A cell that is not a number now blocks the switch: the dropdown goes back to the type shown and the message names the row.
Taken. Outside ALL, loading a file keeps only the dropdown's type (and home) and says how many items were left out, and that ALL loads them all.
Resolved by the merge.

Built on Windows (MissionPlanner.csproj, Release). The row ordering and the check before Write are covered by tests kept in my fork.

@meee1 could you take another look?

Conflict in GCSViews/FlightPlanner.cs: upstream's new fence item FENCE_HOME_CIRCLE_INCLUSION (f3e3a45) added its column names where the fence command list used to be built inline, which this branch moved into addFenceCMDParams so that ALL mode gets the fence columns too. The home circle's names and multipliers go into addFenceCMDParams instead (lat/lng unlabelled, radius in param 1). Adaptation: the new "fence circle around home" map menu item (1719a29) shows when editing the fence or everything at once, like the fence conversion items, instead of in FENCE mode alone; AddCommand puts it at the end of the fence rows in ALL mode as for any fence item.

Autonomost · 2026-11-05T15:25:53Z

Merged upstream master as 4340ee7 to clear the conflict. The home circle fence item (f3e3a45) added its column names where the fence command list used to be built inline, which this branch moved into addFenceCMDParams so that ALL mode gets the fence columns too; the home circle's names and multipliers went there. The new "fence circle around home" menu item shows in FENCE and ALL mode, like the fence conversion items, and AddCommand puts it at the end of the fence rows in ALL mode. Only the PR's four files differ from master.

meee1 · 2026-11-10T00:58:49Z

The earlier review points are all addressed, thanks.

One bug remains: saved rows are now tied to the vehicle (MAVState) object, which is cleared on every connect. So unsent edits for any mission type not currently shown are silently lost on connect or reconnect.

Repro:

Plan a fence offline in ALL mode.
Switch to MISSION.
Connect.
Switch back to ALL. The fence is gone.

A telemetry drop followed by a reconnect does the same. Please keep unsent edits independent of the connection.

Generated by Claude Code

The rows kept for the item types not on screen were held per MAVState, and a connect makes a new MAVState. So a fence drawn offline in ALL mode was gone after switching to MISSION, connecting and switching back, and a telemetry drop with a reconnect lost unsent edits the same way. The kept rows now belong to the page, like the rows on screen, and a type's rows are kept only while they differ from the vehicle's copy, that is, while something in them is unsent. Rows that only repeat the vehicle's copy are not kept and the type is shown from the vehicle's copy again, so one vehicle's items are still not shown for, or offered to Write to, the next vehicle (the reason they were tied to the MAVState before). The comparison is as the vehicle stores an item: command, frame, position to 1e-7 degrees, and the parameters and altitude as floats, since a row read from the vehicle has been through the grid's cells as text. A fence row's frame is left out: fence items are always sent as GLOBAL. Read and Write both leave the vehicle's copy equal to the rows, so after either nothing is kept.

Second look at the commit before. Locationwp.Convert cuts a position down to the vehicle's 1e-7 degree step with an (int) cast instead of rounding it, so after a Write a row and the item sent from it can be one step apart (about a centimetre). Compared to half a step, such a row counted as unsent, was kept, and would have been shown for the next vehicle. Positions are now the same within one step.

Autonomost · 2026-11-14T02:27:42Z

You're right, and it came from my fix for your 10-22 point 2: tying the kept rows to the MAVState is what lost them on a connect. fcf45be and 5642635 change the rule instead of the owner.

The kept rows belong to the page again, like the rows on screen, so a plan drawn offline or an unsent edit is still there after a connect or a reconnect. Your repro (fence offline in ALL, switch to MISSION, connect, back to ALL) keeps the fence.
A type's rows are kept only while they differ from the vehicle's copy, that is, while something in them is unsent. Rows that only repeat the vehicle's copy are not kept, and the type is shown from the vehicle's copy, whichever vehicle that is by then. So vehicle A's fence as read is still not shown for vehicle B and Write does not pre-tick it for B (the 10-22 case). A's unsent edits do stay, as the rows on screen always have.
The comparison is as the vehicle stores an item: command, frame, the parameters and altitude as floats, and the position within one 1e-7 degree step. The step is there because Locationwp.Convert cuts a position down with an (int) cast rather than rounding it, so a row and the item sent from it can be one step apart. A fence row's frame is left out, since fence items are always sent as GLOBAL. Read and Write both leave the vehicle's copy equal to the rows, so after either nothing is kept.

Built on Windows (MissionPlanner.csproj, Release). Covered by tests kept in my fork: your repro, an unsent edit across a reconnect, an unedited fence not shown for the next vehicle, a mission that repeats the vehicle's copy, and the comparison itself.

@meee1 could you take another look?

meee1

fcf45be and 5642635 fix the last point (unsent rows of a hidden type lost on connect), and I found no new defects. CI passes and the branch merges cleanly.

What I checked:

The kept rows are a field of the page again, so a connect or reconnect no longer clears them. The 11-10 repro (fence offline in ALL, switch to MISSION, connect, back to ALL) keeps the fence.
A type's rows are kept only while they differ from the vehicle's copy, so a fence read from vehicle A is not shown for, or pre-ticked to Write to, vehicle B: the 10-22 case stays fixed.
SameItem holds up: position within one 1e-7 degree step (Locationwp.Convert truncates), altitude and params as floats with a relative tolerance. That is safe because the grid cells hold the numbers themselves, not formatted text, so an altitude shown in feet comes back as the same float and a read-only mission is not kept as unsent. Home is stripped from the vehicle's mission before comparing, and a fence row's frame is ignored since fence items are sent as GLOBAL.

Notes:

The tests described in the PR conversation (the reconnect repro, the A to B fence case, the comparison itself) live in your fork. MissionPlannerTests/GCSViews/FlightPlannerTests.cs exists upstream; this logic has had three rounds of fixes, so it would be worth bringing them in. SameItem is internal, so it needs InternalsVisibleTo or to be made public to be tested directly.
A consequence of keeping unsent rows with the page, as asked for: they also follow a switch to another vehicle on the same link with the vehicle selector. An unsent fence edited for vehicle A, then hidden, comes back in ALL mode while vehicle B is selected, and the Write picker pre-ticks it with its item count. It is visible, but easy to send to the wrong vehicle. Marking kept types as unsent in the picker would help; that can be a follow-up.
The branch has 15 commits including five merges from master, so a squash merge would keep the history readable.

Review by Claude Code (Claude Opus 5.5, model claude-opus-5-5, medium effort) on behalf of @meee1, from the commits since the last review (fcf45be, 5642635) at 5642635, and the FlightPlanner code they touch. Not built or run.

🤖 Generated with Claude Code

KeptRowsTests drives the helpers behind the item type dropdown (KeepGridRows, RowsToShow, AllRowsToShow) on a Plan page that is never shown: rows come back after another type was shown, kept mission rows get a home placeholder, ALL mode splits and joins the three types, a fence drawn offline is still there after a connect, an unsent edit survives a reconnect, rows that only repeat the vehicle's copy are not shown for the next vehicle, new rows go to the end of their type's rows, and SameItem compares as the vehicle stores an item. MissionTypeOfTests covers which commands are fence, rally and mission items. Every position in them is made up. SameItem, MissionRowsFirst and MissionTypeOf are internal, so the test project is named in InternalsVisibleTo.

Autonomost · 2026-11-14T14:50:29Z

Thanks. ec08f3c brings the tests over (your note 1): KeptRowsTests (13) and MissionTypeOfTests (3) under MissionPlannerTests/GCSViews. They cover the reconnect repro, an unsent edit across a reconnect, the A to B fence case, a mission that repeats the vehicle's copy, where new rows go in ALL mode, and SameItem. SameItem, MissionRowsFirst and MissionTypeOf stay internal; the test project is named in InternalsVisibleTo in Properties/AssemblyInfo.cs. All 16 pass on this branch on Windows. CI builds them; they run there only if the workflow's filter is widened.

On note 2, unsent rows following a switch to another vehicle on the same link: agreed, it is the other side of keeping them with the page. Marking the kept types as unsent in the Write picker is a good follow-up, and I have left it out of this PR.

meee1 requested a lite review from Copilot September 17, 2026 03:35

Copilot started reviewing on behalf of meee1 September 17, 2026 03:35

View session

Copilot AI reviewed Sep 17, 2026

View reviewed changes

Comment thread GCSViews/FlightPlanner.cs Resolved

Comment thread GCSViews/FlightPlanner.cs Resolved

Comment thread GCSViews/FlightPlanner.cs Resolved

Comment thread GCSViews/FlightPlanner.cs Resolved

Comment thread Controls/MissionTypePicker.cs Resolved

Autonomost commented Sep 17, 2026

View reviewed changes

meee1 requested a lite review from Copilot September 30, 2026 21:16

Copilot AI reviewed Sep 30, 2026

View reviewed changes

Copilot started reviewing on behalf of meee1 September 30, 2026 21:39

View session

Autonomost added 3 commits October 3, 2026 21:00

This was referenced Oct 4, 2026

Plan/Data/SITL: enter and show home, POIs and SITL start in GEO, UTM … #3781

Open

Plan: elevation graph over the bottom of the map #3813

Open

Autonomost added 2 commits October 4, 2026 21:18

Autonomost added 2 commits October 4, 2026 21:42

Merge branch 'master' into feature/plan-all-mission-type

8a39adb

Merge branch 'master' into feature/plan-all-mission-type

2115410

Autonomost added 2 commits October 24, 2026 07:34

Autonomost added 2 commits November 13, 2026 21:05

meee1 reviewed Nov 14, 2026

View reviewed changes

Conversation

Autonomost commented Sep 17, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Suppressed comments (3)

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Autonomost left a comment

Choose a reason for hiding this comment

Uh oh!

Autonomost commented Sep 29, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Autonomost commented Oct 4, 2026

Uh oh!

Autonomost commented Oct 4, 2026

Uh oh!

Autonomost commented Oct 5, 2026

Uh oh!

Autonomost commented Oct 18, 2026

Uh oh!

meee1 commented Oct 22, 2026

Uh oh!

Autonomost commented Oct 25, 2026

Uh oh!

Autonomost commented Nov 5, 2026

Uh oh!

meee1 commented Nov 10, 2026

Uh oh!

Autonomost commented Nov 14, 2026

Uh oh!

meee1 left a comment

Choose a reason for hiding this comment

Uh oh!

Autonomost commented Nov 14, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants