From 4911a7133235924017ea29b5db76ef9ce7284e13 Mon Sep 17 00:00:00 2001 From: "Dr.-Ing. Amilcar do Carmo Lucas" Date: Sun, 30 Nov 2025 23:51:29 +0100 Subject: [PATCH 1/2] fix(debug statement): the debug statement had an incorrect formatter --- .../backend_flightcontroller_connection.py | 2 +- ...est_backend_flightcontroller_connection.py | 33 ++++++++++++++++++- 2 files changed, 33 insertions(+), 2 deletions(-) diff --git a/ardupilot_methodic_configurator/backend_flightcontroller_connection.py b/ardupilot_methodic_configurator/backend_flightcontroller_connection.py index 9ad89556f..81f7ee713 100644 --- a/ardupilot_methodic_configurator/backend_flightcontroller_connection.py +++ b/ardupilot_methodic_configurator/backend_flightcontroller_connection.py @@ -393,7 +393,7 @@ def _select_supported_autopilot(self, detected_vehicles: dict[tuple[int, int], A for (sysid, compid), m in detected_vehicles.items(): self.info.set_system_id_and_component_id(str(sysid), str(compid)) logging_debug( - _("Connection established with systemID %d, componentID %d."), self.info.system_id, self.info.component_id + _("Connection established with systemID %s, componentID %s."), self.info.system_id, self.info.component_id ) self.info.set_autopilot(m.autopilot) if self.info.is_supported: diff --git a/tests/test_backend_flightcontroller_connection.py b/tests/test_backend_flightcontroller_connection.py index c3b4a0983..0bd7d34fa 100755 --- a/tests/test_backend_flightcontroller_connection.py +++ b/tests/test_backend_flightcontroller_connection.py @@ -17,6 +17,7 @@ import pytest import serial.tools.list_ports_common +from pymavlink import mavutil from ardupilot_methodic_configurator.backend_flightcontroller_connection import ( DEFAULT_BAUDRATE, @@ -33,7 +34,7 @@ ) from ardupilot_methodic_configurator.data_model_flightcontroller_info import FlightControllerInfo -# pylint: disable=protected-access +# pylint: disable=protected-access, too-many-lines class TestFlightControllerConnectionServiceInjection: @@ -982,5 +983,35 @@ def test_connection_comport_attribute_lifecycle(self) -> None: assert connection.comport is None +def test_select_supported_autopilot_with_string_ids_does_not_raise() -> None: + """ + Ensure that system/component IDs stored as strings do not cause logging to raise (previously %d was used). + + This is to prevent future regressions + GIVEN: FlightControllerConnection and a detected vehicle + WHEN: _select_supported_autopilot is called and FlightControllerInfo stores + string IDs + THEN: The function returns success and does not raise + """ + connection = FlightControllerConnection(info=FlightControllerInfo()) + + # Minimal dummy heartbeat message - use supported autopilot constant + class DummyHeartbeat: # pylint: disable=too-few-public-methods + """Dummy MAVLink heartbeat used as input to _select_supported_autopilot.""" + + autopilot = mavutil.mavlink.MAV_AUTOPILOT_ARDUPILOTMEGA + type = mavutil.mavlink.MAV_TYPE_GROUND_ROVER + + dummy = DummyHeartbeat() + + # Call with integer keys (function converts to strings internally) and ensure no exception + result = connection._select_supported_autopilot({(1, 1): dummy}) + + # Successful selection returns empty string and system/component IDs are stored as strings + assert result == "" + assert connection.info.system_id == "1" + assert connection.info.component_id == "1" + + if __name__ == "__main__": pytest.main([__file__, "-v"]) From b6968e7c014eafe08ef711be86120fa50db81ff2 Mon Sep 17 00:00:00 2001 From: "Dr.-Ing. Amilcar do Carmo Lucas" Date: Sun, 30 Nov 2025 23:53:06 +0100 Subject: [PATCH 2/2] fix(usage popup): ignore if the parent window is no longer available --- .../frontend_tkinter_usage_popup_window.py | 23 +++++++----- ...est_frontend_tkinter_usage_popup_window.py | 36 ++++++++++++++++++- 2 files changed, 49 insertions(+), 10 deletions(-) diff --git a/ardupilot_methodic_configurator/frontend_tkinter_usage_popup_window.py b/ardupilot_methodic_configurator/frontend_tkinter_usage_popup_window.py index d126e909c..e6a7c29ff 100644 --- a/ardupilot_methodic_configurator/frontend_tkinter_usage_popup_window.py +++ b/ardupilot_methodic_configurator/frontend_tkinter_usage_popup_window.py @@ -21,10 +21,7 @@ from ardupilot_methodic_configurator.backend_filesystem import LocalFilesystem from ardupilot_methodic_configurator.backend_filesystem_program_settings import ProgramSettings from ardupilot_methodic_configurator.frontend_tkinter_base_window import BaseWindow -from ardupilot_methodic_configurator.frontend_tkinter_font import ( - create_scaled_font, - get_safe_font_config, -) +from ardupilot_methodic_configurator.frontend_tkinter_font import create_scaled_font, get_safe_font_config from ardupilot_methodic_configurator.frontend_tkinter_rich_text import RichText @@ -100,11 +97,19 @@ def finalize_setup_popupwindow( BaseWindow.center_window(popup_window.root, parent) # For parent-less, center on screen - popup_window.root.deiconify() # Show the window now that it's positioned - popup_window.root.attributes("-topmost", True) # noqa: FBT003 - popup_window.root.grab_set() # Make the popup modal - - popup_window.root.protocol("WM_DELETE_WINDOW", close_callback) + try: + # Show the window now that it's positioned. Calls may fail if the + # main application has been destroyed (for example during shutdown) + # — guard against tk.TclError so the caller doesn't crash the app. + popup_window.root.deiconify() + popup_window.root.attributes("-topmost", True) # noqa: FBT003 + popup_window.root.grab_set() # Make the popup modal + + popup_window.root.protocol("WM_DELETE_WINDOW", close_callback) + except tk.TclError: + # Application / interpreter has been destroyed or the underlying + # Tk root is no longer available; there's nothing more to do. + pass @staticmethod def close(popup_window: BaseWindow, parent: Optional[tk.Tk]) -> None: diff --git a/tests/test_frontend_tkinter_usage_popup_window.py b/tests/test_frontend_tkinter_usage_popup_window.py index 365f6b34c..d9b8cb0e1 100755 --- a/tests/test_frontend_tkinter_usage_popup_window.py +++ b/tests/test_frontend_tkinter_usage_popup_window.py @@ -14,7 +14,7 @@ import sys import tkinter as tk from tkinter import ttk -from unittest.mock import MagicMock, patch +from unittest.mock import MagicMock, Mock, patch import pytest @@ -581,5 +581,39 @@ def test_confirmation_popup_defaults_to_false_when_closed( assert result is False, "Should default to False when closed without confirmation" +def test_finalize_setup_popupwindow_handles_destroyed_tk() -> None: + """Ensure finalize_setup_popupwindow doesn't raise if Tk has been destroyed and deiconify() raises tk.TclError.""" + + class FakePopup: # pylint: disable=too-few-public-methods + """Fake popup window simulating destroyed Tk.""" + + def __init__(self) -> None: + self.root = Mock() + # Minimal methods used by finalize_setup_popupwindow + self.root.update_idletasks = Mock(return_value=None) + self.root.winfo_reqheight = Mock(return_value=10) + self.root.winfo_reqwidth = Mock(return_value=20) + self.root.geometry = Mock() + + # Simulate a destroyed Tk: deiconify raises TclError + self.root.deiconify = Mock(side_effect=tk.TclError("application has been destroyed")) + # These should not be invoked after deiconify fails + self.root.attributes = Mock() + self.root.grab_set = Mock() + self.root.protocol = Mock() + + fake = FakePopup() + + # Should not raise even though deiconify raises + PopupWindow.finalize_setup_popupwindow(fake, parent=None, close_callback=lambda: None) + + # deiconify was attempted + fake.root.deiconify.assert_called_once() + # attributes/grab_set/protocol should not be called because deiconify failed + assert not fake.root.attributes.called + assert not fake.root.grab_set.called + assert not fake.root.protocol.called + + if __name__ == "__main__": pytest.main([__file__, "-v"])