Skip to content

Log port list once, then only log changes #1339

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
amilcarlucas merged 1 commit into from
Mar 2, 2026
Merged

Log port list once, then only log changes #1339

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 22 additions & 3 deletions ardupilot_methodic_configurator/backend_flightcontroller_connection.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,7 @@ def __init__( # pylint: disable=too-many-arguments, too-many-positional-argumen
self._baudrate = baudrate
self._network_ports = list(network_ports) if network_ports is not None else self.DEFAULT_NETWORK_PORTS[:]
self._connection_tuples: list[tuple[str, str]] = []
self._logged_connection_tuples: Optional[list[tuple[str, str]]] = None # None = never logged
self._serial_port_discovery: SerialPortDiscovery = serial_port_discovery or SystemSerialPortDiscovery()
self._mavlink_connection_factory: MavlinkConnectionFactory = (
mavlink_connection_factory or SystemMavlinkConnectionFactory()
Expand Down Expand Up @@ -174,12 +175,30 @@ def discover_connections(self, progress_callback: Optional[Callable[[int, int],

# list of tuples with the first element being the port name and the second element being the port description
self._connection_tuples = [(port.device, port.description) for port in comports] + [(port, port) for port in netports]
logging_info(_("Available connection ports are:"))
for port in self._connection_tuples:
logging_info("%s - %s", port[0], port[1])
self._log_connection_changes()
# now that it is logged, add the 'Add another' tuple
self._connection_tuples += [(_("Add another"), _("Add another"))]

def _log_connection_changes(self) -> None:
"""Log port list on first discovery; log only added/removed ports on subsequent discoveries."""
current = self._connection_tuples
previous = self._logged_connection_tuples

if previous is None:
# First discovery - log everything
logging_info(_("Available connection ports are:"))
for port in current:
logging_info("%s - %s", port[0], port[1])
else:
current_set = set(current)
previous_set = set(previous)
for port in sorted(current_set - previous_set):
logging_info(_("Connection port added: %s - %s"), port[0], port[1])
for port in sorted(previous_set - current_set):
logging_info(_("Connection port removed: %s - %s"), port[0], port[1])

self._logged_connection_tuples = list(current)
Comment on lines +182 to +200
Copy link

Copilot AI Mar 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new diff-based logging behavior in _log_connection_changes isn’t covered by tests. Since this repo already uses pytest for FlightControllerConnection, please add a test that captures logs (e.g., via caplog) to assert: (1) first discover_connections() logs the full list once, (2) a second call with the same ports produces no new port-list logs, and (3) adding/removing a port results in exactly the expected "added"/"removed" log lines.

Copilot generated this review using guidance from repository custom instructions.

def disconnect(self) -> None:
"""Close the connection to the flight controller."""
if self.master is not None:
Expand Down
Loading