feat(fc info): More FC info tests

Author: amilcarlucas

ardupilot_methodic_configurator/backend_flightcontroller_info.py

@@ -9,6 +9,7 @@
 """
 
 from collections.abc import Sequence
+from logging import info as logging_info
 from typing import Union
 
 from pymavlink import mavutil
@@ -250,3 +251,21 @@ def __classify_vehicle_type(mav_type_int: int) -> str:
 
         # Return the classified vehicle type based on the MAV_TYPE enum
         return mav_type_to_vehicle_type.get(mav_type_int, "")
+
+    def log_flight_controller_info(self) -> None:
+        """Log flight controller information at INFO level."""
+        logging_info("Firmware Version: %s", self.flight_sw_version_and_type)
+        logging_info("Firmware first 8 hex bytes of the FC git hash: %s", self.flight_custom_version)
+        logging_info("Firmware first 8 hex bytes of the ChibiOS git hash: %s", self.os_custom_version)
+        logging_info("Flight Controller firmware type: %s (%s)", self.firmware_type, self.apj_board_id)
+        logging_info("Flight Controller HW / board version: %s", self.board_version)
+        logging_info("Flight Controller USB vendor ID: %s", self.vendor)
+        logging_info("Flight Controller USB product ID: %s", self.product)
+
+    def format_display_value(self, value: Union[str, dict[str, str], None]) -> str:
+        """Format a value for display in the UI."""
+        if value:
+            if isinstance(value, dict):
+                return ", ".join(value.keys())
+            return str(value)
+        return _("N/A")

ardupilot_methodic_configurator/frontend_tkinter_flightcontroller_info.py

@@ -8,11 +8,10 @@
 SPDX-License-Identifier: GPL-3.0-or-later
 """
 
+import logging
 import tkinter as tk
-
-# from logging import debug as logging_debug
-from logging import info as logging_info
-from tkinter import ttk
+from tkinter import messagebox, ttk
+from typing import Callable, Optional, Union
 
 from ardupilot_methodic_configurator import _, __version__
 from ardupilot_methodic_configurator.annotate_params import Par
@@ -21,64 +20,106 @@
 from ardupilot_methodic_configurator.frontend_tkinter_progress_window import ProgressWindow
 
 
+class FlightControllerInfoPresenter:
+    """
+    Business logic for flight controller information presentation.
+
+    Separated from UI for better testability.
+    """
+
+    def __init__(self, flight_controller: FlightController) -> None:
+        self.flight_controller = flight_controller
+        self.param_default_values: dict[str, Par] = {}
+
+    def get_info_data(self) -> dict[str, Union[str, dict[str, str]]]:
+        """Get formatted flight controller information for display."""
+        return self.flight_controller.info.get_info()
+
+    def log_flight_controller_info(self) -> None:
+        """Log flight controller information."""
+        self.flight_controller.info.log_flight_controller_info()
+
+    def download_parameters(self, progress_callback: Optional[Callable[[int, int], None]] = None) -> dict[str, Par]:
+        """
+        Download flight controller parameters.
+
+        Args:
+            progress_callback: Optional callback function for progress updates
+
+        Returns:
+            Dictionary of parameter default values
+
+        """
+        fc_parameters, param_default_values = self.flight_controller.download_params(progress_callback)
+        self.flight_controller.fc_parameters = fc_parameters
+        self.param_default_values = param_default_values
+        return param_default_values
+
+    def get_param_default_values(self) -> dict[str, Par]:
+        """Get parameter default values."""
+        return self.param_default_values
+
+
 class FlightControllerInfoWindow(BaseWindow):
     """Display flight controller hardware, firmware and parameter information."""
 
     def __init__(self, flight_controller: FlightController) -> None:
         super().__init__()
         self.root.title(_("ArduPilot methodic configurator ") + __version__ + _(" - Flight Controller Info"))
         self.root.geometry("500x350")  # Adjust the window size as needed
-        self.flight_controller = flight_controller
-        self.param_default_values: dict[str, Par] = {}
 
+        self.presenter = FlightControllerInfoPresenter(flight_controller)
+
+        self._create_info_display()
+        self.presenter.log_flight_controller_info()
+
+        # Schedule parameter download after UI is ready
+        self.root.after(50, self._download_flight_controller_parameters)
+        self.root.mainloop()
+
+    def _create_info_display(self) -> None:
+        """Create the flight controller information display."""
         # Create a frame to hold all the labels and text fields
         self.info_frame = ttk.Frame(self.main_frame)
         self.info_frame.pack(fill=tk.BOTH, padx=20, pady=20)
 
         # Dynamically create labels and text fields for each attribute
-        for row_nr, (description, attr_value) in enumerate(flight_controller.info.get_info().items()):
-            label = ttk.Label(self.info_frame, text=f"{description}:")
-            label.grid(row=row_nr, column=0, sticky="w")
-
-            text_field = ttk.Entry(self.info_frame)
-            text_field.grid(row=row_nr, column=1, sticky="ew", columnspan=1)
-
-            # Check if the attribute exists and has a non-empty value before inserting
-            if attr_value:
-                if isinstance(attr_value, dict):
-                    text_field.insert(tk.END, (", ").join(attr_value.keys()))
-                else:
-                    text_field.insert(tk.END, attr_value)
-            else:
-                text_field.insert(tk.END, _("N/A"))  # Insert "Not Available" if the attribute is missing or empty
-            text_field.configure(state="readonly")
+        info_data = self.presenter.get_info_data()
+        for row_nr, (description, attr_value) in enumerate(info_data.items()):
+            self._create_info_row(row_nr, description, attr_value)
 
         self.info_frame.columnconfigure(1, weight=1)
 
-        logging_info(_("Firmware Version: %s"), flight_controller.info.flight_sw_version_and_type)
-        logging_info(_("Firmware first 8 hex bytes of the FC git hash: %s"), flight_controller.info.flight_custom_version)
-        logging_info(_("Firmware first 8 hex bytes of the ChibiOS git hash: %s"), flight_controller.info.os_custom_version)
-        logging_info(
-            _("Flight Controller firmware type: %s (%s)"),
-            flight_controller.info.firmware_type,
-            flight_controller.info.apj_board_id,
-        )
-        logging_info(_("Flight Controller HW / board version: %s"), flight_controller.info.board_version)
-        logging_info(_("Flight Controller USB vendor ID: %s"), flight_controller.info.vendor)
-        logging_info(_("Flight Controller USB product ID: %s"), flight_controller.info.product)
+    def _create_info_row(self, row_nr: int, description: str, attr_value: Union[str, dict[str, str]]) -> None:
+        """Create a single row of information display."""
+        label = ttk.Label(self.info_frame, text=f"{description}:")
+        label.grid(row=row_nr, column=0, sticky="w")
 
-        self.root.after(50, self.download_flight_controller_parameters())  # type: ignore[func-returns-value]
-        self.root.mainloop()
+        text_field = ttk.Entry(self.info_frame)
+        text_field.grid(row=row_nr, column=1, sticky="ew", columnspan=1)
+
+        # Format the value for display using the backend logic
+        display_value = self.presenter.flight_controller.info.format_display_value(attr_value)
+        text_field.insert(tk.END, display_value)
+        text_field.configure(state="readonly")
 
-    def download_flight_controller_parameters(self) -> None:
+    def _download_flight_controller_parameters(self) -> None:
+        """Download flight controller parameters with progress feedback."""
         param_download_progress_window = ProgressWindow(
             self.root, _("Downloading FC parameters"), _("Downloaded {} of {} parameters")
         )
-        self.flight_controller.fc_parameters, self.param_default_values = self.flight_controller.download_params(
-            param_download_progress_window.update_progress_bar
-        )
-        param_download_progress_window.destroy()  # for the case that '--device test' and there is no real FC connected
-        self.root.destroy()
+
+        try:
+            self.presenter.download_parameters(param_download_progress_window.update_progress_bar)
+        except Exception as e:  # pylint: disable=broad-exception-caught
+            # Log the error
+            logging.error("Failed to download parameters: %s", e)
+            # Show an error message to the user
+            messagebox.showerror(_("Error"), f"{_('Failed to download parameters')}: {e}")
+        finally:
+            param_download_progress_window.destroy()  # for the case that '--device test' and there is no real FC connected
+            self.root.destroy()
 
     def get_param_default_values(self) -> dict[str, Par]:
-        return self.param_default_values
+        """Get parameter default values from the presenter."""
+        return self.presenter.get_param_default_values()