Skip to content

Commit bd7328e

Browse files
committed
refactor(connection): harden preserved_connections merging and reduce duplication
- Guard against empty strings and duplicate entries in preserved_connections by skipping falsy values and updating auto_discovered after each append, preventing ("", "") tuples and redundant entries in the connection list - Widen preserved_connections type from list[str] to Sequence[str] across protocol, facade and connection manager so callers can pass any iterable without forcing list materialisation - Extract duplicated store_connection + cache-update logic from add_connection() and reconnect() into _persist_and_cache_connection(), centralising edge-case handling in a single place
1 parent d1956d5 commit bd7328e

File tree

4 files changed

+22
-14
lines changed

4 files changed

+22
-14
lines changed

ardupilot_methodic_configurator/backend_flightcontroller.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
"""
1010

1111
from argparse import ArgumentParser
12+
from collections.abc import Sequence
1213
from logging import info as logging_info
1314
from logging import warning as logging_warning
1415
from os import path as os_path
@@ -321,7 +322,7 @@ def reset_and_reconnect(
321322
def discover_connections(
322323
self,
323324
progress_callback: Optional[Callable[[int, int], None]] = None,
324-
preserved_connections: Optional[list[str]] = None,
325+
preserved_connections: Optional[Sequence[str]] = None,
325326
) -> None:
326327
"""Discover available connections - delegates to connection manager."""
327328
self._connection_manager.discover_connections(

ardupilot_methodic_configurator/backend_flightcontroller_connection.py

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
"""
1010

1111
import contextlib
12+
from collections.abc import Sequence
1213
from logging import debug as logging_debug
1314
from logging import error as logging_error
1415
from logging import info as logging_info
@@ -152,7 +153,7 @@ def __init__( # pylint: disable=too-many-arguments, too-many-positional-argumen
152153
def discover_connections(
153154
self,
154155
progress_callback: Optional[Callable[[int, int], None]] = None,
155-
preserved_connections: Optional[list[str]] = None,
156+
preserved_connections: Optional[Sequence[str]] = None,
156157
) -> None:
157158
"""
158159
Discover all available connections (serial and network ports).
@@ -162,7 +163,7 @@ def discover_connections(
162163
163164
Args:
164165
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+
preserved_connections: Optional sequence of connection strings (e.g. from persistent history)
166167
that should always appear in the available connections list even when not auto-discovered.
167168
The caller (frontend) is responsible for supplying these so the backend stays
168169
free of any dependency on the settings/filesystem layer.
@@ -187,8 +188,13 @@ def discover_connections(
187188
if preserved_connections:
188189
auto_discovered = {t[0] for t in self._connection_tuples}
189190
for conn in preserved_connections:
191+
# Skip falsy/empty connection identifiers
192+
if not conn:
193+
continue
190194
if conn not in auto_discovered:
191195
self._connection_tuples.append((conn, conn))
196+
# Track newly added preserved connections to avoid duplicates
197+
auto_discovered.add(conn)
192198
self._log_connection_changes()
193199
# now that it is logged, add the 'Add another' tuple
194200
self._connection_tuples += [(_("Add another"), _("Add another"))]

ardupilot_methodic_configurator/backend_flightcontroller_protocols.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
SPDX-License-Identifier: GPL-3.0-or-later
2525
"""
2626

27+
from collections.abc import Sequence
2728
from pathlib import Path
2829
from typing import TYPE_CHECKING, Any, Callable, Optional, Protocol, Union
2930

@@ -97,7 +98,7 @@ def baudrate(self) -> int:
9798
def discover_connections(
9899
self,
99100
progress_callback: Optional[Callable[[int, int], None]] = None,
100-
preserved_connections: Optional[list[str]] = None,
101+
preserved_connections: Optional[Sequence[str]] = None,
101102
) -> None: ...
102103

103104
def disconnect(self) -> None: ...

ardupilot_methodic_configurator/frontend_tkinter_connection_selection.py

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -122,6 +122,14 @@ def __init__( # pylint: disable=too-many-arguments, too-many-positional-argumen
122122
# Start periodic port refresh
123123
self.start_periodic_refresh()
124124

125+
def _persist_and_cache_connection(self, connection_string: str) -> None:
126+
"""Persist a connection string to settings and update the in-memory history cache."""
127+
ProgramSettings.store_connection(connection_string)
128+
# Update cache in-memory (most-recent-first, deduplicated) to avoid a disk re-read.
129+
self._connection_history_cache = [connection_string] + [
130+
c for c in self._connection_history_cache if c != connection_string
131+
]
132+
125133
def start_periodic_refresh(self) -> None:
126134
"""Start periodic refresh of available ports every 3 seconds."""
127135
if self._refresh_timer_id is None:
@@ -212,11 +220,7 @@ def add_connection(self) -> str:
212220
),
213221
)
214222
if selected_connection:
215-
ProgramSettings.store_connection(selected_connection)
216-
# Update cache in-memory (most-recent-first, deduplicated) to avoid a disk re-read.
217-
self._connection_history_cache = [selected_connection] + [
218-
c for c in self._connection_history_cache if c != selected_connection
219-
]
223+
self._persist_and_cache_connection(selected_connection)
220224
error_msg = _("Will add new connection: {selected_connection} if not duplicated")
221225
logging_debug(error_msg.format(**locals()))
222226
self.flight_controller.add_connection(selected_connection)
@@ -275,11 +279,7 @@ def reconnect(self, selected_connection: str = "") -> bool: # Default is auto-c
275279
self.previous_selection = connection_identifier
276280
# Persist the connection so it is available next time the program opens
277281
# and so it survives periodic auto-discovery refreshes during the current session.
278-
ProgramSettings.store_connection(connection_identifier)
279-
# Update cache in-memory (most-recent-first, deduplicated) to avoid a disk re-read.
280-
self._connection_history_cache = [connection_identifier] + [
281-
c for c in self._connection_history_cache if c != connection_identifier
282-
]
282+
self._persist_and_cache_connection(connection_identifier)
283283
self.flight_controller.add_connection(connection_identifier)
284284
if self.destroy_parent_on_connect:
285285
self.parent.root.destroy()

0 commit comments

Comments
 (0)