Skip to content

Commit 6b29326

Browse files
committed
fix(connection): preserve history connections across periodic port rediscovery
The 3-second periodic port refresh rebuilt _connection_tuples entirely from auto-discovered ports, silently dropping any connections that were added from persistent history or via the UI but were not physically present on the bus at that moment. - Add `preserved_connections` parameter to `discover_connections()` so the caller (frontend) can supply history entries to re-merge after each auto-discovery scan, keeping the backend free of settings/filesystem dependencies - Pass `ProgramSettings.get_connection_history()` as `preserved_connections` on every periodic refresh in `ConnectionSelectionWidgets._refresh_ports()` - Store successfully connected device to `ProgramSettings` and register it via `add_connection()` in `reconnect()`, not only when the connection was typed via the "Add another" dialog but also when an existing combobox entry is selected - Update protocol, facade, and tests accordingly
1 parent 59eb235 commit 6b29326

7 files changed

+132
-8
lines changed

ardupilot_methodic_configurator/backend_flightcontroller.py

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -318,9 +318,15 @@ def reset_and_reconnect(
318318
# Reconnect to the flight controller
319319
return self.create_connection_with_retry(connection_progress_callback, baudrate=self.baudrate)
320320

321-
def discover_connections(self, progress_callback: Optional[Callable[[int, int], None]] = None) -> None:
321+
def discover_connections(
322+
self,
323+
progress_callback: Optional[Callable[[int, int], None]] = None,
324+
preserved_connections: Optional[list[str]] = None,
325+
) -> None:
322326
"""Discover available connections - delegates to connection manager."""
323-
self._connection_manager.discover_connections(progress_callback=progress_callback)
327+
self._connection_manager.discover_connections(
328+
progress_callback=progress_callback, preserved_connections=preserved_connections
329+
)
324330

325331
def disconnect(self) -> None:
326332
"""Close the connection to the flight controller - delegates to connection manager."""

ardupilot_methodic_configurator/backend_flightcontroller_connection.py

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -149,7 +149,11 @@ def __init__( # pylint: disable=too-many-arguments, too-many-positional-argumen
149149
mavlink_connection_factory or SystemMavlinkConnectionFactory()
150150
)
151151

152-
def discover_connections(self, progress_callback: Optional[Callable[[int, int], None]] = None) -> None:
152+
def discover_connections(
153+
self,
154+
progress_callback: Optional[Callable[[int, int], None]] = None,
155+
preserved_connections: Optional[list[str]] = None,
156+
) -> None:
153157
"""
154158
Discover all available connections (serial and network ports).
155159
@@ -158,6 +162,10 @@ def discover_connections(self, progress_callback: Optional[Callable[[int, int],
158162
159163
Args:
160164
progress_callback: Optional callback for progress updates. Signature: callback(current, total)
165+
preserved_connections: Optional list of connection strings (e.g. from persistent history)
166+
that should always appear in the combobox even when not auto-discovered.
167+
The caller (frontend) is responsible for supplying these so the backend stays
168+
free of any dependency on the settings/filesystem layer.
161169
162170
"""
163171
if progress_callback:
@@ -175,6 +183,12 @@ def discover_connections(self, progress_callback: Optional[Callable[[int, int],
175183

176184
# list of tuples with the first element being the port name and the second element being the port description
177185
self._connection_tuples = [(port.device, port.description) for port in comports] + [(port, port) for port in netports]
186+
# Merge caller-supplied preserved connections (e.g. persistent history) that are not auto-discovered
187+
if preserved_connections:
188+
auto_discovered = {t[0] for t in self._connection_tuples}
189+
for conn in preserved_connections:
190+
if conn not in auto_discovered:
191+
self._connection_tuples.append((conn, conn))
178192
self._log_connection_changes()
179193
# now that it is logged, add the 'Add another' tuple
180194
self._connection_tuples += [(_("Add another"), _("Add another"))]

ardupilot_methodic_configurator/backend_flightcontroller_protocols.py

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,11 @@ def baudrate(self) -> int:
9494
"""Get the default baud rate for serial connections."""
9595
... # pylint: disable=unnecessary-ellipsis
9696

97-
def discover_connections(self, progress_callback: Optional[Callable[[int, int], None]] = None) -> None: ...
97+
def discover_connections(
98+
self,
99+
progress_callback: Optional[Callable[[int, int], None]] = None,
100+
preserved_connections: Optional[list[str]] = None,
101+
) -> None: ...
98102

99103
def disconnect(self) -> None: ...
100104

ardupilot_methodic_configurator/frontend_tkinter_connection_selection.py

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -134,7 +134,10 @@ def _refresh_ports(self) -> None:
134134
try:
135135
# Scan for new ports and get current data
136136
current_selection = self.conn_selection_combobox.get_selected_key()
137-
self.flight_controller.discover_connections(progress_callback=None)
137+
self.flight_controller.discover_connections(
138+
progress_callback=None,
139+
preserved_connections=ProgramSettings.get_connection_history(),
140+
)
138141
new_connection_tuples = self.flight_controller.get_connection_tuples()
139142

140143
# update the UI
@@ -257,7 +260,12 @@ def reconnect(self, selected_connection: str = "") -> bool: # Default is auto-c
257260
self.stop_periodic_refresh()
258261
# Store the current connection as the previous selection
259262
if self.flight_controller.comport and hasattr(self.flight_controller.comport, "device"):
260-
self.previous_selection = self.flight_controller.comport.device
263+
device: str = self.flight_controller.comport.device
264+
self.previous_selection = device
265+
# Persist the connection so it appears at the top of the list next time the program opens,
266+
# and so it survives periodic auto-discovery refreshes during the current session.
267+
ProgramSettings.store_connection(device)
268+
self.flight_controller.add_connection(device)
261269
if self.destroy_parent_on_connect:
262270
self.parent.root.destroy()
263271
if self.download_params_on_connect and hasattr(self.parent, "download_flight_controller_parameters"):

tests/test_backend_flightcontroller.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -966,4 +966,6 @@ def track_progress(current: int, total: int) -> None: # pylint: disable=unused-
966966
fc.discover_connections(progress_callback=track_progress)
967967

968968
# Assert: Connection manager received progress callback
969-
mock_conn_mgr.discover_connections.assert_called_once_with(progress_callback=track_progress)
969+
mock_conn_mgr.discover_connections.assert_called_once_with(
970+
progress_callback=track_progress, preserved_connections=None
971+
)

tests/test_backend_flightcontroller_connection.py

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -470,6 +470,90 @@ def test_custom_connection_string_deduplication(self) -> None:
470470
count = sum(1 for t in tuples if t[0] == "/dev/ttyUSB0")
471471
assert count == 1
472472

473+
def test_preserved_connection_survives_port_rediscovery(self) -> None:
474+
"""
475+
Caller-supplied preserved connection persists across auto-discovery refreshes.
476+
477+
GIVEN: User has previously used a custom connection (e.g. a TCP address stored in history)
478+
WHEN: discover_connections() is called again (periodic 3s refresh) and returns different ports
479+
AND the caller supplies the history list as preserved_connections
480+
THEN: The preserved connection should still be present in the combobox
481+
AND: Auto-discovered ports that disappeared should be gone
482+
AND: Newly auto-discovered ports should appear
483+
"""
484+
# Given: Connection manager with a mocked serial port discovery
485+
mock_discovery = Mock()
486+
mock_port = Mock()
487+
mock_port.device = "COM3"
488+
mock_port.description = "USB Serial"
489+
490+
# First discovery: COM3 is present
491+
mock_discovery.get_available_ports.return_value = [mock_port]
492+
connection = FlightControllerConnection(
493+
info=FlightControllerInfo(),
494+
serial_port_discovery=mock_discovery,
495+
network_ports=[],
496+
)
497+
connection.discover_connections()
498+
499+
# When: Port list changes (COM3 disappears, COM4 appears) but caller preserves the TCP address
500+
mock_port2 = Mock()
501+
mock_port2.device = "COM4"
502+
mock_port2.description = "Another USB Serial"
503+
mock_discovery.get_available_ports.return_value = [mock_port2]
504+
connection.discover_connections(preserved_connections=["tcp:127.0.0.1:5760"])
505+
506+
# Then: Preserved TCP connection is present
507+
tuples = connection.get_connection_tuples()
508+
assert any(t[0] == "tcp:127.0.0.1:5760" for t in tuples)
509+
# COM3 is gone (no longer auto-discovered, not in preserved list)
510+
assert not any(t[0] == "COM3" for t in tuples)
511+
# COM4 is present (auto-discovered)
512+
assert any(t[0] == "COM4" for t in tuples)
513+
514+
def test_without_preserved_connections_non_discovered_ports_disappear(self) -> None:
515+
"""
516+
Without preserved_connections, ports that are no longer auto-discovered are removed.
517+
518+
GIVEN: Many USB devices are connected and auto-discovered
519+
WHEN: discover_connections() is called again without preserved_connections
520+
AND one port is no longer detected
521+
THEN: The missing port is removed from the combobox
522+
AND: The backend has no hidden state keeping old connections alive
523+
"""
524+
# Given: Connection manager with multiple auto-discovered ports
525+
mock_discovery = Mock()
526+
ports = []
527+
for i in range(5):
528+
p = Mock()
529+
p.device = f"COM{i + 1}"
530+
p.description = f"USB Device {i + 1}"
531+
ports.append(p)
532+
mock_discovery.get_available_ports.return_value = ports
533+
534+
connection = FlightControllerConnection(
535+
info=FlightControllerInfo(),
536+
serial_port_discovery=mock_discovery,
537+
network_ports=[],
538+
)
539+
540+
# When: Initial discovery
541+
connection.discover_connections()
542+
tuples = connection.get_connection_tuples()
543+
for i in range(5):
544+
assert any(t[0] == f"COM{i + 1}" for t in tuples)
545+
546+
# When: Re-discovery with fewer ports and no preserved list
547+
mock_discovery.get_available_ports.return_value = ports[:2] # Only COM1, COM2 remain
548+
connection.discover_connections()
549+
550+
# Then: Only auto-discovered ports are present - COM3..COM5 are gone
551+
tuples = connection.get_connection_tuples()
552+
assert any(t[0] == "COM1" for t in tuples)
553+
assert any(t[0] == "COM2" for t in tuples)
554+
for i in range(2, 5):
555+
assert not any(t[0] == f"COM{i + 1}" for t in tuples)
556+
473557

474558
class TestFlightControllerConnectionInfo:
475559
"""Test flight controller information gathering."""

tests/test_frontend_tkinter_connection_selection.py

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -934,6 +934,10 @@ def setUp(self) -> None:
934934
patch("tkinter.ttk.Combobox"),
935935
patch("tkinter.StringVar", return_value=mock_baudrate_var),
936936
patch("ardupilot_methodic_configurator.frontend_tkinter_connection_selection.show_tooltip"),
937+
patch(
938+
"ardupilot_methodic_configurator.frontend_tkinter_connection_selection.ProgramSettings.get_connection_history",
939+
return_value=[],
940+
),
937941
):
938942
self.widget = ConnectionSelectionWidgets(
939943
self.mock_parent,
@@ -957,7 +961,9 @@ def test_periodic_refresh_starts_on_init(self) -> None:
957961
"""
958962
# The widget is already initialized in setUp
959963
# Verify observable behavior: port scan actually ran
960-
self.mock_flight_controller.discover_connections.assert_called_once_with(progress_callback=None)
964+
self.mock_flight_controller.discover_connections.assert_called_once_with(
965+
progress_callback=None, preserved_connections=[]
966+
)
961967
# Verify the recurring timer was scheduled via the tkinter event loop
962968
self.mock_parent.root.after.assert_called_with(3000, self.widget._refresh_ports)
963969
assert self.widget._is_refreshing is False

0 commit comments

Comments
 (0)