SerialOutputPass (Ctrl+F > Mavlink): mirrors started from the grid can never be stopped #3768

Issue details

MAVLink mirrors started from the grid in the SerialOutputPass window (Ctrl+F > Mavlink, also
reachable from Config > Planner > Advanced) cannot be stopped. Once started, the output keeps
running for the whole lifetime of the Mission Planner process.

I hit this in normal use: I had set up an output to a particular address and had to close and
reopen Mission Planner to stop it, because nothing in the window stops an output once it is
started. I then reproduced it against SITL and measured it.

Steps to reproduce

1. Connect to a vehicle or SITL (I used Simulation > Multirotor).
2. Start any UDP listener on the target port. I used a small Python script that counts packets per
   second and distinct source ports.
3. Ctrl+F > Mavlink.
4. Add a grid row: Type=UDP, Direction=Outbound, Port=14561, Extra=127.0.0.1,
   Write=unchecked.
5. Click the Go cell. The cell shows "Started" and MAVLink starts flowing to 127.0.0.1:14561.
6. Try to stop it.

Expected: the stream stops.

Actual:

• Clicking Go again does not stop the mirror. It adds another mirror to the same destination,
  so the receiver gets the same packets again. Every further click adds one more.
• Disconnecting the vehicle pauses the traffic (there is no vehicle data to mirror), but the mirrors
  survive: on reconnect the same sockets resume sending.
• There is no other UI affordance that stops it. The legacy Connect/Stop button, together with
  the serial port / baud / write-access controls, has been Visible = False since 3827d9a
  (2024-11-06), so it cannot be reached either.
• The only way to stop the output is to restart Mission Planner.

Measurement

One grid row, target 127.0.0.1:14561, SITL multirotor. Packets per second measured at the target,
alongside the number of distinct UDP source ports (each source port is one socket, i.e. one Mirror
object on the Mission Planner side):

The source ports observed were 53325, 54785, 54786, 54787, 54815 and 53236 - one new socket per
click, and after the disconnect/reconnect cycle all six came back, which is what a fix has to
address: the Mirror objects outlive the link.

Throughput scales linearly with the click count (1x = 64 pps, 5x = 320 pps, 6x = 380 pps), and the
duplicate-payload count is non-zero from the second click onwards, i.e. the target receives each
MAVLink packet once per accumulated mirror.

Analysis

Verified against master at 2b5589f4.

Controls/SerialOutputPass.cs, myDataGridView1_CellContentClick (L197-271) only ever adds:

MainV2.comPort.Mirrors.Add(mirror);                          // L263
myDataGridView1[Go.Index, e.RowIndex].Value = "Started";     // L264
Started.Add(e.RowIndex);                                     // L265

There is no Close(), no Mirrors.Remove(), and no "already started" check anywhere in the method.
The only Close() in the whole file is at L55, inside the hidden legacy BUT_connect_Click.

In ExtLibs/ArduPilot/Mavlink/MAVLinkInterface.cs, Mirrors.Clear() appears exactly once in the
whole file, inside Dispose() (L6848). Dispose() is only ever invoked from MainV2.cs L3038,
which explicitly skips the primary interface:

// skip primary interface
if (port == comPort)
    continue;

Grid mirrors are attached to MainV2.comPort, i.e. the primary interface, so the list is never
cleared while the application runs - which matches the observation that the same six sockets came
back after a disconnect/reconnect cycle. ProcessMirrorStream() (L5463) writes to every entry on
every packet.

Related problems in the same area:

• static private List<int> Started (L190) is only ever appended to, never cleared, and it is keyed
  by row index, so it breaks if rows are added, removed or reordered.
• The TCP Inbound path uses a single static TcpListener listener (L19). Starting a second TCP
  Inbound row overwrites it (L217); the first listener is orphaned and can never be stopped.
  listener.Stop() is never called anywhere in the file.
• Mirrors is a plain List<T>. ProcessMirrorStream enumerates it on the telemetry read thread
  (readPacketAsync, L4667), while the UI thread calls Mirrors.Add. A concurrent modification
  throws InvalidOperationException out of the foreach, which is swallowed by the outer
  catch (Exception ex) { log.Error(ex); } in readPacketAsync (L5452-5455) - so it silently drops
  packet processing rather than crashing. Adding a remove path makes this more likely to be hit, so
  it should be handled at the same time.

Two smaller things I ran into while reproducing, both in the same handler:

• Clicking Go on a row whose Write checkbox has never been toggled fails with
  Error: Object reference not set to an instance of an object. The Write column
  (SerialOutputPass.Designer.cs L132) is a DataGridViewCheckBoxColumn with no TrueValue /
  FalseValue / ThreeState set, so the cell value is null until it is edited, and L208 calls
  .Value.ToString() on it unguarded. The same applies to the other four cells if left empty.
• The Go button renders with no label until a row is started. Go.Text = "Go" is set
  (SerialOutputPass.Designer.cs L138) but UseColumnTextForButtonValue is not, so the button
  shows the cell value, which is null until the code writes "Started" into it.

This looks like an unfinished feature rather than intended behaviour. The
SerialOutputPass: add list (7f3150e) and SerialOutputPass: multi mirrors (3827d9a) commits only
ever added start paths; the latter also hid the legacy Connect/Stop UI without providing a
replacement.

Two further items I deliberately leave out of a fix, noting them here:

• MirrorStream / MirrorStreamWrite (MAVLinkInterface.cs L427-459) insert an empty Mirror
  into the list when it is empty, as a side effect of the getter. SerialOutputPass's constructor
  triggers this (L27, L35), so Mirrors[0] is usually a dummy. Changing it would affect
  Controls/SerialSupportProxy.cs (L22, L53-55, L79-80) and plugins/example12-forwarding.cs
  (L50, L55, L74), which rely on those compatibility properties, so it needs its own change.
• DoAcceptTcpClientCallback (L132) casts ar.AsyncState to
  ValueTuple<TcpListener, MAVLinkInterface.Mirror>, while the legacy BUT_connect_Click (L74)
  passes a bare TcpListener. That path is currently unreachable because the button is hidden, but
  the mismatch is still there.

Related, still-open reports about this same window: #2085, #2416, #2194.

I'm happy to open a PR that turns the Go cell into a proper start/stop toggle, tracks running
mirrors and listeners per row, and takes a lock around the Mirrors iteration.

Version

Mission Planner 1.3.83 (build 1.3.9384.38258), Windows 11 Home 25H2 (build 26200.9168).
Source analysis done against master at 2b5589f4.

Platform

[ ] All
[x] Mission Planner (GCS only, not vehicle specific)

Airframe type

N/A - GCS side issue, reproduced against SITL multirotor.

Hardware type

N/A - reproduced against SITL.

Logs

N/A - no vehicle log needed. Reproducible with any UDP listener on the mirror target; the graph
above was produced from a packet counter binding the target port.