tests(flightcontroller): Update the tests after refactoring

Author: amilcarlucas

.github/instructions/SITL_TESTING.md

@@ -14,7 +14,7 @@ The SITL testing setup consists of:
 1. **Direct Download**: Tests download pre-built ArduCopter SITL binaries directly from the official ArduPilot firmware server (`firmware.ardupilot.org`)
 2. **Pytest Fixtures**: Session-scoped SITLManager class manages SITL process lifecycle
 3. **TCP Connection**: SITL runs on TCP port 5760 with MAVLink protocol
-4. **Parameter Configuration**: SITL uses `sitl/copter.param` with battery monitoring enabled
+4. **Parameter Configuration**: SITL uses `sitl/copter.parm` with battery monitoring enabled
 
 ## Prerequisites
 
@@ -120,7 +120,7 @@ SITL tests cover:
 SITL runs with the following command line parameters:
 
 ```bash
-arducopter --model quad --home "40.071374,-105.229930,1440,0" --defaults ./sitl/copter.param --sysid 1 --speedup 10
+arducopter --model quad --home "40.071374,-105.229930,1440,0" --defaults sitl/copter.parm --sysid 1 --speedup 10
 ```
 
 ### Connection Details
@@ -133,7 +133,7 @@ arducopter --model quad --home "40.071374,-105.229930,1440,0" --defaults ./sitl/
 
 ### Parameter Requirements
 
-Some tests require specific parameters to be set in `sitl/copter.param`:
+Some tests require specific parameters to be set in `sitl/copter.parm`:
 
 - `BATT_MONITOR = 4` (Analog voltage and current)
 - `BATT_VOLT_PIN = 1`

ARCHITECTURE_2_flight_controller_communication.md

@@ -154,10 +154,42 @@ ConfigureVSCode() {
     fi
 }
 
+SetupSITL() {
+    echo ""
+    read -r -p "Do you want to download ArduCopter SITL for integration testing? (y/N) " response
+    response=${response,,}
+
+    if [[ $response =~ ^y(es)?$ ]]; then
+        echo "Downloading ArduCopter SITL from official firmware server..."
+
+        # Use the run_sitl_tests.sh script if available
+        if [ -f "scripts/run_sitl_tests.sh" ]; then
+            bash scripts/run_sitl_tests.sh download
+        else
+            # Fallback to direct download if script not found
+            mkdir -p sitl
+            curl -L -o sitl/arducopter https://firmware.ardupilot.org/Copter/latest/SITL_x86_64_linux_gnu/arducopter
+            curl -L -o sitl/firmware-version.txt https://firmware.ardupilot.org/Copter/latest/SITL_x86_64_linux_gnu/firmware-version.txt
+            curl -L -o sitl/git-version.txt https://firmware.ardupilot.org/Copter/latest/SITL_x86_64_linux_gnu/git-version.txt
+            curl -L -o sitl/copter.parm https://raw.githubusercontent.com/ArduPilot/ardupilot/master/Tools/autotest/default_params/copter.parm
+            chmod +x sitl/arducopter
+            echo "✓ ArduCopter SITL downloaded successfully"
+        fi
+
+        # Set environment variable in shell profile
+        echo ""
+        echo "To enable SITL tests permanently, add this to your ~/.bashrc or ~/.zshrc:"
+        echo "  export SITL_BINARY=$(pwd)/sitl/arducopter"
+    else
+        echo "Skipping SITL download. You can download it later with: ./scripts/run_sitl_tests.sh download"
+    fi
+}
+
 # Call configuration functions
 InstallDependencies
 ConfigureGit
 ConfigureVSCode
+SetupSITL
 
 activate-global-python-argcomplete
 
@@ -176,6 +208,11 @@ echo ""
 echo "source .venv/bin/activate"
 echo "python3 -m ardupilot_methodic_configurator"
 echo ""
+echo "To run SITL integration tests (if SITL was downloaded):"
+echo ""
+echo "export SITL_BINARY=\$(pwd)/sitl/arducopter"
+echo "pytest -m sitl -v"
+echo ""
 echo "For more detailed usage instructions, please refer to the USERMANUAL.md file."
 echo ""
 

SetupDeveloperPC.sh

@@ -14,7 +14,7 @@
 from os import path as os_path
 from pathlib import Path
 from time import sleep as time_sleep
-from typing import Callable, Optional, Union
+from typing import TYPE_CHECKING, Any, Callable, Optional, Union, cast
 
 import serial.tools.list_ports_common
 from pymavlink import mavutil
@@ -39,11 +39,12 @@
 from ardupilot_methodic_configurator.data_model_flightcontroller_info import FlightControllerInfo
 from ardupilot_methodic_configurator.data_model_par_dict import ParDict
 
+if TYPE_CHECKING:
+    from pymavlink.dialects.v20.ardupilotmega import MAVLink_autopilot_version_message
+
 DEFAULT_REBOOT_TIME: int = 7
 
 # Re-export constants for backwards compatibility
-# DEFAULT_BAUDRATE and SUPPORTED_BAUDRATES are from connection module
-# DEFAULT_REBOOT_TIME is defined in this module
 __all__ = [
     "DEFAULT_BAUDRATE",
     "DEFAULT_REBOOT_TIME",
@@ -139,9 +140,13 @@ def __init__(  # pylint: disable=too-many-arguments, too-many-positional-argumen
             fc_parameters=None,  # Let params_manager create its own fc_parameters dict
         )
 
-        self._commands_manager: FlightControllerCommandsProtocol = commands_manager or FlightControllerCommands(
-            params_manager=self._params_manager,
-            connection_manager=self._connection_manager,
+        self._commands_manager: FlightControllerCommandsProtocol = cast(
+            "FlightControllerCommandsProtocol",
+            commands_manager
+            or FlightControllerCommands(
+                params_manager=self._params_manager,
+                connection_manager=self._connection_manager,
+            ),
         )
 
         self._files_manager: FlightControllerFilesProtocol = files_manager or FlightControllerFiles(
@@ -208,6 +213,26 @@ def baudrate(self) -> int:
         """Get the baudrate setting."""
         return self._baudrate
 
+    @property
+    def PARAM_FETCH_POLL_DELAY(self) -> float:  # noqa: N802 # pylint: disable=invalid-name
+        """Get parameter fetch poll delay - delegates to params manager."""
+        return self._params_manager.PARAM_FETCH_POLL_DELAY
+
+    @property
+    def BATTERY_STATUS_CACHE_TIME(self) -> float:  # noqa: N802 # pylint: disable=invalid-name
+        """Get battery status cache time - delegates to commands manager."""
+        return self._commands_manager.BATTERY_STATUS_CACHE_TIME
+
+    @property
+    def BATTERY_STATUS_TIMEOUT(self) -> float:  # noqa: N802 # pylint: disable=invalid-name
+        """Get battery status timeout - delegates to commands manager."""
+        return self._commands_manager.BATTERY_STATUS_TIMEOUT
+
+    @property
+    def COMMAND_ACK_TIMEOUT(self) -> float:  # noqa: N802 # pylint: disable=invalid-name
+        """Get command acknowledgment timeout - delegates to commands manager."""
+        return self._commands_manager.COMMAND_ACK_TIMEOUT
+
     @property
     def fc_parameters(self) -> dict[str, float]:
         """Get flight controller parameters - delegates to params manager."""
@@ -281,6 +306,33 @@ def add_connection(self, connection_string: str) -> bool:
         """Add a new connection to the list of available connections - delegates to connection manager."""
         return self._connection_manager.add_connection(connection_string)
 
+    # Testing-only methods (protected methods exposed for SITL integration tests)
+    def _detect_vehicles_from_heartbeats(self, timeout: int) -> dict[tuple[int, int], Any]:
+        """Detect vehicles from heartbeats - delegates to connection manager (testing only)."""
+        return self._connection_manager._detect_vehicles_from_heartbeats(timeout)  # noqa: SLF001 # pylint: disable=protected-access
+
+    def _extract_firmware_type_from_banner(self, banner_msgs: list[str], os_custom_version_index: Optional[int]) -> str:
+        """Extract firmware type from banner - delegates to connection manager (testing only)."""
+        return self._connection_manager._extract_firmware_type_from_banner(  # noqa: SLF001 # pylint: disable=protected-access
+            banner_msgs, os_custom_version_index
+        )
+
+    def _extract_chibios_version_from_banner(self, banner_msgs: list[str]) -> tuple[str, Optional[int]]:
+        """Extract ChibiOS version from banner - delegates to connection manager (testing only)."""
+        return self._connection_manager._extract_chibios_version_from_banner(banner_msgs)  # noqa: SLF001 # pylint: disable=protected-access
+
+    def _select_supported_autopilot(self, detected_vehicles: dict[tuple[int, int], Any]) -> str:
+        """Select supported autopilot from detected vehicles - delegates to connection manager (testing only)."""
+        return self._connection_manager._select_supported_autopilot(detected_vehicles)  # noqa: SLF001 # pylint: disable=protected-access
+
+    def _populate_flight_controller_info(self, m: "MAVLink_autopilot_version_message") -> None:
+        """Populate flight controller info from autopilot version - delegates to connection manager (testing only)."""
+        self._connection_manager._populate_flight_controller_info(m)  # noqa: SLF001 # pylint: disable=protected-access
+
+    def _retrieve_autopilot_version_and_banner(self, timeout: int) -> str:
+        """Retrieve autopilot version and banner - delegates to connection manager (testing only)."""
+        return self._connection_manager._retrieve_autopilot_version_and_banner(timeout)  # noqa: SLF001 # pylint: disable=protected-access
+
     def register_and_try_connect(
         self,
         comport: Union[mavutil.SerialPort, serial.tools.list_ports_common.ListPortInfo],

ardupilot_methodic_configurator/backend_flightcontroller.py

@@ -451,9 +451,13 @@ def _request_message(self, message_id: int) -> None:
         if self.master is not None:
             # Note: Don't wait for ACK here as this is used internally for autopilot version requests
             # and the response comes as the requested message itself
+            # Convert system_id and component_id from string to int for MAVLink
+            system_id = int(self.info.system_id) if self.info.system_id else 0
+            component_id = int(self.info.component_id) if self.info.component_id else 0
+
             self.master.mav.command_long_send(
-                self.info.system_id,
-                self.info.component_id,
+                system_id,
+                component_id,
                 mavutil.mavlink.MAV_CMD_REQUEST_MESSAGE,
                 0,  # confirmation
                 message_id,

ardupilot_methodic_configurator/backend_flightcontroller_connection.py

@@ -25,7 +25,7 @@
 """
 
 from pathlib import Path
-from typing import TYPE_CHECKING, Callable, Optional, Protocol, Union
+from typing import TYPE_CHECKING, Any, Callable, Optional, Protocol, Union
 
 import serial.tools.list_ports_common
 
@@ -38,6 +38,7 @@
 if TYPE_CHECKING:
     # During type checking, import the actual mavutil module for better type hints
     from pymavlink import mavutil
+    from pymavlink.dialects.v20.ardupilotmega import MAVLink_autopilot_version_message
 
     # Use a union of known connection types for better type safety
     # Note: mavutil.mavlink_connection() returns different types based on the connection string
@@ -140,6 +141,18 @@ def set_master_for_testing(self, master: Optional[MavlinkConnection]) -> None:
 
         """
 
+    def _detect_vehicles_from_heartbeats(self, timeout: int) -> dict[tuple[int, int], Any]: ...
+
+    def _extract_firmware_type_from_banner(self, banner_msgs: list[str], os_custom_version_index: Optional[int]) -> str: ...
+
+    def _extract_chibios_version_from_banner(self, banner_msgs: list[str]) -> tuple[str, Optional[int]]: ...
+
+    def _select_supported_autopilot(self, detected_vehicles: dict[tuple[int, int], Any]) -> str: ...
+
+    def _populate_flight_controller_info(self, m: "MAVLink_autopilot_version_message") -> None: ...
+
+    def _retrieve_autopilot_version_and_banner(self, timeout: int) -> str: ...
+
 
 class FlightControllerParamsProtocol(Protocol):
     """
@@ -152,6 +165,9 @@ class FlightControllerParamsProtocol(Protocol):
             - fc_parameters: dict[str, float] (shared parameter dictionary, created if not provided)
     """
 
+    # Class constant exposed as property for backward compatibility
+    PARAM_FETCH_POLL_DELAY: float
+
     @property
     def fc_parameters(self) -> dict[str, float]:
         """Get the parameter dictionary."""
@@ -189,6 +205,11 @@ class FlightControllerCommandsProtocol(Protocol):
             - connection_manager: FlightControllerConnectionProtocol (to access master)
     """
 
+    # Class constants exposed for testing
+    COMMAND_ACK_TIMEOUT: float
+    BATTERY_STATUS_CACHE_TIME: float
+    BATTERY_STATUS_TIMEOUT: float
+
     def send_command_and_wait_ack(  # pylint: disable=too-many-arguments, too-many-positional-arguments
         self,
         command: int,