Initial usage of ArduCopter SITL to test the flightcontroller backend

.codespell-exclude-file

@@ -4,3 +4,4 @@
                 "Version": "LAF"
              datas= [],
                ardupilot_methodic_configuratorAny.datas,
+    critical_param_prefixes = ["FRAME", "BATT", "COMPASS", "SERVO", "MOT"]

.github/instructions/SITL_TESTING.md

@@ -0,0 +1,191 @@
+# SITL Testing Setup
+
+This document describes how to set up and run integration tests using ArduPilot SITL (Software In The Loop) for testing the `backend_flightcontroller.py` module.
+
+## Overview
+
+SITL testing provides real MAVLink communication validation instead of mocked tests.
+This ensures the flight controller backend works correctly with actual ArduPilot firmware.
+
+## Architecture
+
+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.parm` with battery monitoring enabled
+
+## Prerequisites
+
+### For CI/CD (GitHub Actions)
+
+- No additional setup required - SITL binaries are downloaded automatically during tests
+
+### For Local Development
+
+#### Download Pre-built SITL (Recommended)
+
+Download the latest pre-built SITL binary directly from the official ArduPilot firmware server:
+
+```bash
+./scripts/run_sitl_tests.sh download
+```
+
+This downloads ArduCopter SITL from `https://firmware.ardupilot.org/Copter/latest/SITL_x86_64_linux_gnu/arducopter`
+
+## Usage
+
+### CI/CD Testing
+
+SITL tests run automatically in GitHub Actions when SITL artifacts are available. The test workflow:
+
+1. Downloads the latest SITL artifact
+2. Extracts and sets up SITL binary
+3. Runs tests marked with `@pytest.mark.sitl`
+4. Falls back to mocked tests if SITL is unavailable
+
+### Local Development
+
+Use the provided script for local SITL testing. You can either download pre-built SITL or use a locally built version:
+
+#### Using Downloaded SITL (Recommended)
+
+```bash
+# Download ArduCopter SITL from official firmware server
+./scripts/run_sitl_tests.sh download
+
+# Download and run tests in one command
+./scripts/run_sitl_tests.sh download-test
+
+# Check if downloaded SITL is available
+./scripts/run_sitl_tests.sh check
+```
+
+#### Using Locally Built SITL
+
+```bash
+# Set up environment for locally built SITL
+export ARDUPILOT_DIR="$HOME/ardupilot-sitl"
+
+# Check if locally built SITL is available
+./scripts/run_sitl_tests.sh check
+
+# Set up SITL for testing
+./scripts/run_sitl_tests.sh setup
+
+# Run SITL integration tests
+./scripts/run_sitl_tests.sh test
+```
+
+#### General Commands
+
+```bash
+# Clean up SITL processes and cache
+./scripts/run_sitl_tests.sh cleanup
+
+# Show help
+./scripts/run_sitl_tests.sh help
+```
+
+### Manual Testing
+
+Run specific SITL tests:
+
+```bash
+# Run all SITL tests
+python -m pytest tests/test_backend_flightcontroller_sitl.py -v
+
+# Run only SITL tests (skip if SITL unavailable)
+python -m pytest -m sitl -v
+
+# Run SITL tests or fallback to mocked tests
+python -m pytest -m "sitl or not sitl" -v
+```
+
+## Test Coverage
+
+SITL tests cover:
+
+- **Real MAVLink Connection**: Validates actual protocol communication on TCP port 5760
+- **Parameter Management**: Download, set, and verify parameters with real firmware
+- **Motor Testing**: Test motor commands against actual ArduPilot firmware
+- **Battery Monitoring**: Test battery status reporting with enabled monitoring
+- **Frame Information**: Validate vehicle configuration queries
+
+## Implementation Details
+
+### SITL Configuration
+
+SITL runs with the following command line parameters:
+
+```bash
+arducopter --model quad --home "40.071374,-105.229930,1440,0" --defaults sitl/copter.parm --sysid 1 --speedup 10
+```
+
+### Connection Details
+
+- **Protocol**: MAVLink over TCP
+- **Port**: 5760
+- **Connection String**: "tcp:127.0.0.1:5760"
+- **Vehicle Type**: ArduCopter (Quadcopter)
+- **System ID**: 1
+
+### Parameter Requirements
+
+Some tests require specific parameters to be set in `sitl/copter.parm`:
+
+- `BATT_MONITOR = 4` (Analog voltage and current)
+- `BATT_VOLT_PIN = 1`
+- `BATT_CURR_PIN = 2`
+- `BATT_VOLT_MULT = 10.0`
+- `BATT_AMP_PERVOLT = 17.0`
+
+## Configuration
+
+### Environment Variables
+
+- `SITL_BINARY`: Path to ArduCopter SITL binary (auto-detected in CI)
+- `ARDUPILOT_DIR`: Path to ArduPilot directory for local development
+
+### Test Markers
+
+- `@pytest.mark.sitl`: Marks tests requiring SITL
+- Tests automatically skip if SITL is unavailable
+
+## Troubleshooting
+
+### SITL Not Found
+
+- **For downloaded SITL**: Run `./scripts/run_sitl_tests.sh download` to download from ArduPilot website
+- **For locally built SITL**: Ensure ArduPilot is built with `./waf configure --board=sitl && ./waf copter`
+- Check `ARDUPILOT_DIR` environment variable for locally built SITL
+- Verify SITL binary exists at expected path
+
+### Connection Failures
+
+- SITL may take time to start - tests include startup delays
+- Check for port conflicts on TCP port 5760
+- Verify MAVLink heartbeat detection
+- Ensure connection string format is "tcp:127.0.0.1:5760"
+
+### Test Timeouts
+
+- SITL tests are slower than mocked tests
+- Increase timeout values if needed
+- Check system performance for SITL simulation
+
+## Benefits
+
+1. **Real Validation**: Tests actual MAVLink protocol implementation
+2. **Regression Detection**: Catches firmware compatibility issues
+3. **CI/CD Integration**: Automated testing with pre-built artifacts
+4. **Development Flexibility**: Local testing with fallback to mocks
+5. **Cost Efficiency**: Monthly builds reduce CI resource usage
+
+## Future Enhancements
+
+- Multiple vehicle types (ArduPlane, Rover, etc.)
+- SITL version pinning for reproducible tests
+- Performance optimization for faster test execution
+- Multi-SITL instance testing for complex scenarios

.github/workflows/pytest.yml

@@ -60,6 +60,53 @@ jobs:
         run: |
           uv pip install --editable .[dev,ci_headless_tests]
 
+      - name: Download ArduCopter SITL (if available)
+        run: |
+          # Create cache key based on current quarter (YYYY-Q)
+          CURRENT_YEAR=$(date +%Y)
+          CURRENT_MONTH=$(date +%m)
+          QUARTER=$(( (CURRENT_MONTH-1)/3 + 1 ))
+          CACHE_KEY="${CURRENT_YEAR}-Q${QUARTER}"
+
+          echo "Cache key: ${CACHE_KEY}"
+
+          # Check if we have cached SITL files for this quarter
+          if [ -d "sitl-cache/${CACHE_KEY}" ] && [ -f "sitl-cache/${CACHE_KEY}/arducopter" ]; then
+            echo "Using cached SITL files from ${CACHE_KEY}"
+            mkdir -p sitl/
+            cp sitl-cache/${CACHE_KEY}/* sitl/
+          else
+            echo "Downloading fresh SITL files for ${CACHE_KEY}"
+            mkdir -p sitl/ sitl-cache/${CACHE_KEY}/
+
+            # Download latest ArduCopter SITL from official firmware server
+            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
+
+            # Cache the downloaded files
+            cp sitl/* sitl-cache/${CACHE_KEY}/
+          fi
+
+          # Make executable and verify
+          chmod +x sitl/arducopter
+          ls -la sitl/
+
+          # Set environment variables
+          echo "SITL_BINARY=$(pwd)/sitl/arducopter" >> $GITHUB_ENV
+          echo "SITL_AVAILABLE=true" >> $GITHUB_ENV
+          echo "SITL version: $(cat sitl/git-version.txt)"
+          echo "Firmware version: $(cat sitl/firmware-version.txt)"
+        continue-on-error: true
+
+      - name: Cache SITL files
+        uses: actions/cache@0057852bfaa89a56745cba8c7296529d2fc39830 # v4.3.0
+        with:
+          path: sitl-cache/
+          key: sitl-cache-${{ github.run_id }}
+          restore-keys: |
+            sitl-cache-
+
       - name: Test with pytest
         id: pytest
         continue-on-error: false
@@ -72,7 +119,13 @@ jobs:
           Xvfb :99 -screen 0 1024x768x16 -ac &
           # ensure Xvfb is fully started before running tests
           sleep 2
-          uv run pytest --cov=ardupilot_methodic_configurator --cov-report=xml:tests/coverage.xml --md=tests/results-${{ matrix.python-version }}.md --junit-xml=tests/results-junit.xml
+          if [ "$SITL_AVAILABLE" = "true" ]; then
+            echo "Running tests with SITL support"
+            uv run pytest --cov=ardupilot_methodic_configurator --cov-report=xml:tests/coverage.xml --md=tests/results-${{ matrix.python-version }}.md --junit-xml=tests/results-junit.xml -m "sitl or not sitl"
+          else
+            echo "Running tests without SITL (mocked tests only)"
+            uv run pytest --cov=ardupilot_methodic_configurator --cov-report=xml:tests/coverage.xml --md=tests/results-${{ matrix.python-version }}.md --junit-xml=tests/results-junit.xml -m "not sitl"
+          fi
 
       - name: Fix coverage paths
         run: |

.gitignore

@@ -14,3 +14,7 @@ venv/
 .venv/
 test.txt
 test.xml
+
+sitl/arducopter
+sitl/firmware-version.txt
+sitl/git-version.txt

ARCHITECTURE.md

@@ -76,7 +76,13 @@ Each sub-application has detailed architecture documentation covering requiremen
 2. **[Flight Controller Communication](ARCHITECTURE_2_flight_controller_communication.md)** - Establishes FC connection, downloads parameters and metadata
    - [`frontend_tkinter_connection_selection.py`](ardupilot_methodic_configurator/frontend_tkinter_connection_selection.py)
      - [`frontend_tkinter_flightcontroller_info.py`](ardupilot_methodic_configurator/frontend_tkinter_flightcontroller_info.py)
-     - [`backend_flightcontroller.py`](ardupilot_methodic_configurator/backend_flightcontroller.py)
+     - [`backend_flightcontroller.py`](ardupilot_methodic_configurator/backend_flightcontroller.py) - Main facade class using delegation pattern
+       - [`backend_flightcontroller_connection.py`](ardupilot_methodic_configurator/backend_flightcontroller_connection.py) - Connection management
+       - [`backend_flightcontroller_params.py`](ardupilot_methodic_configurator/backend_flightcontroller_params.py) - Parameter operations
+       - [`backend_flightcontroller_commands.py`](ardupilot_methodic_configurator/backend_flightcontroller_commands.py) - Command execution
+       - [`backend_flightcontroller_files.py`](ardupilot_methodic_configurator/backend_flightcontroller_files.py) - File operations
+       - [`data_model_flightcontroller_info.py`](ardupilot_methodic_configurator/data_model_flightcontroller_info.py) - FC information
+       - [`backend_flightcontroller_protocols.py`](ardupilot_methodic_configurator/backend_flightcontroller_protocols.py) - Protocol definitions
      - [`backend_mavftp.py`](ardupilot_methodic_configurator/backend_mavftp.py)
 
 3. **[Directory and Project Selection](ARCHITECTURE_3_directory_selection.md)** - Creates new projects or opens existing ones
@@ -154,9 +160,63 @@ All applications use one or more of the following shared libraries:
    4. [`backend_filesystem_program_settings.py`](ardupilot_methodic_configurator/backend_filesystem_program_settings.py)
 1. the internet backend communicates with the internet
    1. [`backend_internet.py`](ardupilot_methodic_configurator/backend_internet.py)
-1. the flight controller backend communicates with the flight controller
-   1. [`backend_flightcontroller.py`](ardupilot_methodic_configurator/backend_flightcontroller.py)
-   2. [`backend_mavftp.py`](ardupilot_methodic_configurator/backend_mavftp.py)
+1. the flight controller backend communicates with the flight controller using a **delegation pattern** with specialized managers:
+   1. [`backend_flightcontroller.py`](ardupilot_methodic_configurator/backend_flightcontroller.py) -
+     Main facade class that delegates to specialized managers
+   2. [`backend_flightcontroller_connection.py`](ardupilot_methodic_configurator/backend_flightcontroller_connection.py) -
+     Handles connection establishment, port discovery, and heartbeat detection
+   3. [`backend_flightcontroller_params.py`](ardupilot_methodic_configurator/backend_flightcontroller_params.py) -
+     Manages parameter download, upload, and querying
+   4. [`backend_flightcontroller_commands.py`](ardupilot_methodic_configurator/backend_flightcontroller_commands.py) -
+     Executes MAVLink commands (motor tests, battery status, etc.)
+   5. [`backend_flightcontroller_files.py`](ardupilot_methodic_configurator/backend_flightcontroller_files.py) -
+     Handles file upload/download via MAVFTP
+   6. [`data_model_flightcontroller_info.py`](ardupilot_methodic_configurator/data_model_flightcontroller_info.py) -
+     Stores flight controller metadata and capabilities
+   7. [`backend_flightcontroller_protocols.py`](ardupilot_methodic_configurator/backend_flightcontroller_protocols.py) -
+     Protocol definitions for dependency injection and testing
+   8. [`backend_mavftp.py`](ardupilot_methodic_configurator/backend_mavftp.py) - MAVFTP protocol implementation
+
+   **Flight Controller Backend Architecture:**
+   - The `FlightController` class acts as a facade, delegating operations to specialized managers
+   - Each manager handles a specific concern (connection, parameters, commands, files)
+   - Managers can reference each other (e.g., params manager holds reference to connection manager)
+   - Protocol definitions enable dependency injection for testing
+   - Connection manager is the source of truth for connection state (`master`, `comport`, `info`)
+   - Other managers query connection manager for current state rather than caching it
+
+   **Error Handling Standards:**
+
+   To maintain consistency across the flight controller backend, the following error handling patterns are used:
+
+   1. **Connection and I/O Operations** - Return `str` (empty string on success, error message on failure):
+      - `connect()`, `disconnect()`, `register_and_try_connect()`
+      - Rationale: Allows user-friendly error messages to be displayed directly
+
+   2. **Command Operations** - Return `tuple[bool, str]` (success flag, error message):
+      - `test_motor()`, `test_all_motors()`, `reset_all_parameters_to_default()`
+      - Rationale: Separates success/failure from error details for better control flow
+
+   3. **Query Operations** - Return `Optional[T]` or raise exceptions:
+      - `fetch_param()` - raises `TimeoutError` on timeout
+      - `get_battery_status()` - returns `tuple[Optional[tuple[float, float]], str]`
+      - Rationale: Distinguishes between "not found" (None) and "error" (exception)
+
+   4. **Bulk Operations** - Return data structures or tuples:
+      - `download_params()` - returns `tuple[dict[str, float], ParDict]`
+      - Rationale: Success implied by returned data, errors logged but not returned
+
+   **Testing Hacks and Violations:**
+
+   The following methods exist for testing purposes only and violate architectural principles:
+
+   - `FlightController.set_master_for_testing()` - Directly mutates connection manager's internal state,
+     violating the principle that connection manager should be the sole mutator of connection state.
+     **Never use in production code.**
+
+   - Test parameter loading via `device="test"` in `backend_flightcontroller_params.py` - Bypasses normal
+     parameter download flow to load from local file. **Marked with FIXME for future removal.**
+
 1. the tkinter frontend, which is the GUI the user interacts with
    1. [`frontend_tkinter_autoresize_combobox.py`](ardupilot_methodic_configurator/frontend_tkinter_autoresize_combobox.py)
    1. [`frontend_tkinter_base_window.py`](ardupilot_methodic_configurator/frontend_tkinter_base_window.py)