diff --git a/ARCHITECTURE_firmware_upload.md b/ARCHITECTURE_firmware_upload.md new file mode 100644 index 000000000..6e23167e1 --- /dev/null +++ b/ARCHITECTURE_firmware_upload.md @@ -0,0 +1,126 @@ +# Firmware Upload Architecture + +Firmware upload flashes an ArduPilot APJ image through the board bootloader and +reconnects to the flight controller. It is separate from parameter configuration +because entering the bootloader interrupts the active MAVLink connection. + +## Responsibilities + +```mermaid +flowchart TD + CALLER[Upload caller] --> FACADE[backend_flightcontroller.py] + FACADE --> MAV[MAVLink connection] + FACADE --> BOOT[backend_flightcontroller_bootloader.py] + BOOT --> MODEL[data_model_firmware_upload.py] + BOOT --> SERIAL[pyserial] + BOOT --> FC[ArduPilot bootloader] +``` + +- `backend_flightcontroller.py` owns `upload_apj_firmware()`, the active MAVLink + connection, bootloader entry, disconnection, reconnection, and post-flash identity. +- `backend_flightcontroller_bootloader.py` owns APJ file I/O, serial discovery, + bootloader protocol traffic, progress, retries, and cleanup. +- `data_model_firmware_upload.py` contains APJ parsing, image normalization, + compatibility rules, upload stages, and typed errors. It does not access files, + serial ports, MAVLink, or Tkinter. + +The connection manager remains the source of truth for MAVLink state. The bootloader +adapter may close and recreate that connection, but does not keep a second long-lived +connection state. + +## Upload workflow and safety rules + +1. Require a directly addressable serial connection, a valid connected board ID, + stable USB identity, explicit confirmation, and a trusted SHA-256 for the exact + APJ descriptor. +2. Parse and validate the APJ input before rebooting the controller. APJ is the only + supported input format; raw BIN images are rejected. +3. Enter bootloader mode through MAVLink, release the MAVLink connection, and + rediscover the bootloader serial port after re-enumeration. +4. Identify the bootloader and validate protocol revision, board ID, flash capacity, + and image compatibility. +5. After final confirmation, erase, program, and verify the internal and optional + external image regions, then reboot. +6. Reconnect through the normal connection manager and require the returned board + identity before reporting success. + +The following invariants apply throughout the workflow: + +- Never erase or program before APJ validation, board matching, capacity checks, + trusted-digest verification, and final confirmation pass. +- Reconnection is bound to the captured USB serial number and/or physical location. + Interface metadata is only a disambiguation hint and may differ because the + application and bootloader can expose different CDC interfaces; interface suffixes + such as Linux `1-2.3:1.2` are ignored for physical matching. Ambiguous matches fail + closed. +- Network MAVLink connections and force flashing are unsupported. +- Cancellation is checked at safe boundaries and never interrupts a bootloader + packet. Before erase, the held bootloader is rebooted only after an explicit + successful reboot acknowledgement, except for protocol revision 2 which does + not send one; otherwise the error requires a power cycle. +- Bootloader discovery has a 15-second wall-clock budget. Serial response timeouts + and retry delays are capped by the remaining budget, so a port that opens but + does not answer cannot extend discovery indefinitely. +- After erase begins, any erase, programming, verification, or reboot failure + reports that the flash may be incomplete and instructs the user to power-cycle + before reconnecting. The facade does not attempt a normal MAVLink reconnect + unless a safe bootloader abort has confirmed a reboot. +- Serial transports are closed on successful uploads and handled protocol/error + paths, and cleanup cannot mask the original upload failure. The source APJ is not + modified, and a failed verify or reconnect is never reported as success. +- Full-chip erase is refused because the client cannot yet determine target MCU + capability safely; normal erase remains supported. + +## Implemented modules + +### `backend_flightcontroller_bootloader.py` + +`FlightControllerBootloaderBackend` is an internal transport adapter. It loads the +APJ, enters and discovers the held bootloader, and passes the image to +`BootloaderClient`. The public facade performs trusted-digest and stable-USB-identity +policy checks before invoking it. The client handles protocol synchronization, board +and capacity checks, erase/program/verify, reboot, and transport cleanup; the backend +owns serial-open retries and their wall-clock budget. Discovery resolves the captured +USB identity before each open, fails closed on zero or multiple matches, and shares +injected clock and sleep functions with the client for bounded, testable timing. + +### `data_model_firmware_upload.py` + +The model provides `FirmwareImage`, `BootloaderInfo`, `UploadStage`, APJ parsing, +image padding, CRC helpers, compatibility checks, and typed upload errors. Its +functions are usable without serial or GUI dependencies. + +### `backend_flightcontroller.py` + +`FlightController.upload_apj_firmware()` validates the active connection, trusted +digest, and pre-reboot board identity; delegates bootloader entry and post-reboot +reconnection to dedicated collaborators; and verifies the returned +`AUTOPILOT_VERSION` board ID. A successful flash also invalidates cached parameters. + +## Domain and error model + +`data_model_firmware_upload.py` provides the immutable image and bootloader data +objects, APJ validation, compatibility checks, upload-stage state machine, CRC +helpers, and typed errors. Compatibility comes from APJ metadata and bootloader +identification, never from a filename. + +Errors identify the failed stage and are translated by the caller into user-facing +messages. They cover invalid APJ, digest or identity mismatch, unsupported protocol, +serial discovery, erase/program/verify, cancellation/recovery, and reconnect failure. + +## Testing boundary + +Use pure tests for APJ parsing, image normalization, compatibility, CRCs, state +transitions, and error classification. Use fake serial transports and fake MAVLink +connections for protocol framing, retries, erase/program/verify ordering, USB +re-enumeration, cancellation, cleanup, reconnect, and post-flash identity checks. + +Hardware tests should be separately marked and require an explicitly selected serial +device. + +## References + +- [ArduPilot uploader](https://github.com/ArduPilot/ardupilot/blob/master/Tools/scripts/uploader.py) +- [ArduPilot bootloader documentation](https://ardupilot.org/dev/docs/bootloader.html) +- [pymavlink](https://github.com/ArduPilot/pymavlink) +- [`ARCHITECTURE.md`](ARCHITECTURE.md) diff --git a/ardupilot_methodic_configurator/backend_flightcontroller.py b/ardupilot_methodic_configurator/backend_flightcontroller.py index efbd0d069..0167a3a58 100644 --- a/ardupilot_methodic_configurator/backend_flightcontroller.py +++ b/ardupilot_methodic_configurator/backend_flightcontroller.py @@ -14,6 +14,7 @@ from logging import warning as logging_warning from os import path as os_path from pathlib import Path +from time import monotonic as time_monotonic from time import sleep as time_sleep from typing import TYPE_CHECKING, Any, cast @@ -22,6 +23,18 @@ from ardupilot_methodic_configurator import _ from ardupilot_methodic_configurator.argparse_check_range import CheckRange +from ardupilot_methodic_configurator.backend_flightcontroller_bootloader import ( + CancellationRequested, + ConfirmationRequested, + FlightControllerBootloaderBackend, + ProgressCallback, + SerialDeviceIdentity, + SerialFactory, + capture_serial_device_identity, + has_stable_bootloader_device_identity, + report_progress_safely, + resolve_bootloader_device, +) from ardupilot_methodic_configurator.backend_flightcontroller_commands import ( CompassCalibrationUpdate, FlightControllerCommands, @@ -41,6 +54,16 @@ FlightControllerParamsProtocol, MavlinkConnection, ) +from ardupilot_methodic_configurator.data_model_firmware_upload import ( + BootloaderInfo, + FirmwareBootloaderRecoveryError, + FirmwareConfirmationError, + FirmwareConnectionError, + FirmwareReconnectError, + UploadStage, + verify_expected_firmware_digest, + verify_reconnected_firmware, +) from ardupilot_methodic_configurator.data_model_flightcontroller_info import FlightControllerInfo from ardupilot_methodic_configurator.data_model_par_dict import ParDict @@ -48,6 +71,78 @@ from pymavlink.dialects.v20.ardupilotmega import MAVLink_autopilot_version_message DEFAULT_REBOOT_TIME: int = 8 +FIRMWARE_RECONNECT_RESOLVE_TIMEOUT: float = 15.0 + + +class _BootloaderEntryCoordinator: # pylint: disable=too-few-public-methods + """Enter bootloader mode and release the active MAVLink connection.""" + + def __init__( + self, + master_getter: Callable[[], MavlinkConnection | None], + reboot: Callable[[], tuple[bool, str]], + disconnect: Callable[[], None], + sleep: Callable[[float], None], + ) -> None: + self._master_getter = master_getter + self._reboot = reboot + self._disconnect = disconnect + self._sleep = sleep + self.entered = False + + def __call__(self) -> None: + if self._master_getter() is None: + msg = _("flight-controller connection was lost before bootloader entry") + raise FirmwareConnectionError(msg) + accepted, error_message = self._reboot() + if not accepted: + msg = _("flight controller rejected bootloader entry: {error}").format(error=error_message) + raise FirmwareConnectionError(msg) + self.entered = True + self._sleep(0.3) # Allow the MAVLink frame to leave before releasing the port. + self._disconnect() + + +class _BootloaderReconnector: # pylint: disable=too-few-public-methods + """Resolve and reconnect the flight controller after bootloader reboot.""" + + def __init__( # pylint: disable=too-many-arguments,too-many-positional-arguments + self, + device: str, + identity: SerialDeviceIdentity | None, + active_baudrate: Callable[[], int], + resolve_device: Callable[[str, SerialDeviceIdentity | None], str], + connect: Callable[[str, int], str], + clock: Callable[[], float], + sleep: Callable[[float], None], + ) -> None: + self._device = device + self._identity = identity + self._active_baudrate = active_baudrate + self._resolve_device = resolve_device + self._connect = connect + self._clock = clock + self._sleep = sleep + + def __call__(self) -> str: + deadline = self._clock() + FIRMWARE_RECONNECT_RESOLVE_TIMEOUT + last_error = "" + while self._clock() < deadline: + try: + reconnect_device = self._resolve_device(self._device, self._identity) + except OSError as exc: + last_error = str(exc) + self._sleep(0.1) + continue + # ``connect`` reports transient failures as strings rather than + # raising, so keep retrying until the deadline is exhausted. + connect_error = self._connect(reconnect_device, self._active_baudrate()) + if not connect_error: + return "" + last_error = connect_error + self._sleep(0.1) + return _("cannot resolve the flight-controller serial device: {error}").format(error=last_error) + # Re-export constants for backwards compatibility __all__ = [ @@ -61,7 +156,7 @@ ] -class FlightController: # pylint: disable=too-many-public-methods +class FlightController: # pylint: disable=too-many-public-methods,too-many-instance-attributes """ Facade for flight controller operations using delegation pattern. @@ -94,7 +189,7 @@ class FlightController: # pylint: disable=too-many-public-methods """ - def __init__( # pylint: disable=too-many-arguments, too-many-positional-arguments + def __init__( # noqa: PLR0913, PLR0917 # pylint: disable=too-many-arguments, too-many-positional-arguments self, reboot_time: int = DEFAULT_REBOOT_TIME, baudrate: int = DEFAULT_BAUDRATE, @@ -105,6 +200,8 @@ def __init__( # pylint: disable=too-many-arguments, too-many-positional-argumen commands_manager: FlightControllerCommandsProtocol | None = None, files_manager: FlightControllerFilesProtocol | None = None, progress_callback: Callable[[int, int], None] | None = None, + sleep: Callable[[float], None] | None = None, + clock: Callable[[], float] | None = None, ) -> None: """ Initialize the FlightController communication object. @@ -121,6 +218,8 @@ def __init__( # pylint: disable=too-many-arguments, too-many-positional-argumen progress_callback: Optional callback function for displaying initialization progress. If None, no progress updates are shown. Signature: callback(current, total) Used to provide user feedback during component initialization phases + sleep: Optional delay function used by reset and firmware-upload recovery workflows. + clock: Optional monotonic clock used by firmware-upload reconnection retries. Note: If not provided, managers are created in dependency order: @@ -136,6 +235,8 @@ def __init__( # pylint: disable=too-many-arguments, too-many-positional-argumen progress_callback(5, 100) self._reboot_time = reboot_time + self._sleep = time_sleep if sleep is None else sleep + self._clock = time_monotonic if clock is None else clock self._network_ports = network_ports if network_ports is not None else FlightControllerConnection.DEFAULT_NETWORK_PORTS if progress_callback: @@ -295,7 +396,7 @@ def reset_and_reconnect( # Issue a reset self.master.reboot_autopilot() logging_info(_("Reset command sent to ArduPilot.")) - time_sleep(0.3) # Short delay for command to be sent + self._sleep(0.3) # Short delay for command to be sent self.disconnect() @@ -312,7 +413,7 @@ def reset_and_reconnect( reset_progress_callback(current_step, sleep_time) # Wait for sleep_time seconds - time_sleep(1) + self._sleep(1) current_step += 1 # Call the progress callback with the current progress @@ -338,6 +439,119 @@ def disconnect(self) -> None: # Clear parameter cache via params manager self._params_manager.clear_parameters() + def upload_apj_firmware( # pylint: disable=too-many-arguments,too-many-locals + self, + path: Path, + *, + bootloader_baudrate: int = 115200, + bootloader_timeout: float = 2.0, + full_erase: bool = False, + expected_firmware_sha256: str | None = None, + serial_factory: SerialFactory | None = None, + cancellation_requested: CancellationRequested | None = None, + confirmation_requested: ConfirmationRequested | None = None, + progress_callback: ProgressCallback | None = None, + ) -> BootloaderInfo: + """ + Flash an APJ through the active direct serial connection and reconnect. + + This synchronous facade operation is intended for a worker thread. It is + deliberately unavailable without the current MAVLink connection and its + directly-addressable serial device, so a UDP/TCP connection cannot be used + accidentally for bootloader flashing. + """ + network_prefixes = ("udp:", "udpin:", "udpout:", "tcp:", "tcpin:", "tcpout:", "ws:", "wss:") + device = self.comport_device + if self.master is None or self.comport is None or not device or device.lower().startswith(network_prefixes): + msg = _("firmware upload requires an active direct serial flight-controller connection") + raise FirmwareConnectionError(msg) + if confirmation_requested is None: + msg = _("firmware upload requires explicit confirmation before entering the bootloader") + raise FirmwareConfirmationError(msg) + try: + connected_board_id = int(self.info.apj_board_id) + if connected_board_id < 0: + raise ValueError + except (TypeError, ValueError): + msg = _("firmware upload requires the connected flight controller APJ board_id") + raise FirmwareConnectionError(msg) from None + device_identity = capture_serial_device_identity(device) + entry = _BootloaderEntryCoordinator( + lambda: self.master, + self._commands_manager.reboot_to_bootloader, + self.disconnect, + self._sleep, + ) + reconnector = _BootloaderReconnector( + device, + device_identity, + lambda: getattr(self._connection_manager, "active_baudrate", self.baudrate), + resolve_bootloader_device, + lambda reconnect_device, baudrate: self.connect(reconnect_device, log_errors=False, baudrate=baudrate), + self._clock, + self._sleep, + ) + + backend_kwargs: dict[str, Any] = { + "timeout": bootloader_timeout, + "enter_bootloader": entry, + "device_identity": device_identity, + "sleep": self._sleep, + } + if serial_factory is not None: + backend_kwargs["serial_factory"] = serial_factory + backend = FlightControllerBootloaderBackend( + device, + bootloader_baudrate, + **backend_kwargs, + ) + image = backend.inspect_firmware(path) + if expected_firmware_sha256 is None: + msg = _("firmware upload requires a trusted SHA-256 for the selected APJ payload") + raise FirmwareConfirmationError(msg) + verify_expected_firmware_digest(image, expected_firmware_sha256) + if not has_stable_bootloader_device_identity(device, device_identity): + msg = _("firmware upload requires a stable USB serial number or USB location") + raise FirmwareConnectionError(msg) + report_progress_safely(progress_callback, UploadStage.ENTERING_BOOTLOADER, 0, 1) + try: + info = backend.upload( + image, + full_erase=full_erase, + cancellation_requested=cancellation_requested, + confirmation_requested=confirmation_requested, + progress_callback=progress_callback, + connected_board_id=connected_board_id, + ) + except Exception as exc: + if isinstance(exc, FirmwareBootloaderRecoveryError): + # ``hold_in_bootloader`` disables the normal boot timeout. If + # no bootloader transport could be opened, no in-band reboot is + # possible and a MAVLink reconnect cannot succeed. + raise + if entry.entered and getattr(exc, "bootloader_rebooted", False): + recovery_error = reconnector() + if recovery_error: + logging_warning(_("Unable to reconnect after safe firmware-upload abort: %s"), recovery_error) + raise + # A flashed image invalidates every cached parameter. The reconnect is a + # required part of success, rather than an optimistic best-effort step. + self._params_manager.clear_parameters() + report_progress_safely(progress_callback, UploadStage.RECONNECTING, 0, 1) + reconnect_error = reconnector() + if reconnect_error: + msg = _( + "firmware was written and verified, but the flight controller could not be reconnected automatically: " + "{error}. Reconnect it manually before continuing." + ).format(error=reconnect_error) + raise FirmwareReconnectError(msg) + verify_reconnected_firmware( + image, + board_id=self.info.apj_board_id, + ) + report_progress_safely(progress_callback, UploadStage.RECONNECTING, 1, 1) + return info + @property def banner_text_buffer(self) -> list[str]: """Return the most recently received FC banner text.""" diff --git a/ardupilot_methodic_configurator/backend_flightcontroller_bootloader.py b/ardupilot_methodic_configurator/backend_flightcontroller_bootloader.py new file mode 100644 index 000000000..5dbf8483f --- /dev/null +++ b/ardupilot_methodic_configurator/backend_flightcontroller_bootloader.py @@ -0,0 +1,861 @@ +""" +Flight-controller bootloader adapter for ArduPilot APJ firmware uploads. + +All byte-level bootloader protocol and local-file I/O is deliberately kept here; +``data_model_firmware_upload`` contains only parsing and upload policy. + +This file is part of ArduPilot Methodic Configurator. https://github.com/ArduPilot/MethodicConfigurator + +SPDX-FileCopyrightText: 2024-2026 Amilcar do Carmo Lucas + +SPDX-License-Identifier: GPL-3.0-or-later +""" + +from __future__ import annotations + +import struct +from collections.abc import Callable, Iterator +from contextlib import suppress +from dataclasses import dataclass +from pathlib import Path +from time import monotonic +from time import sleep as time_sleep +from typing import Protocol, cast + +import serial +import serial.tools.list_ports + +from ardupilot_methodic_configurator import _ +from ardupilot_methodic_configurator.data_model_firmware_upload import ( + BL_REV_MAX, + BL_REV_MIN, + MAX_ENCODED_BLOB_SIZE, + BootloaderInfo, + BootloaderProtocolError, + FirmwareBootloaderRecoveryError, + FirmwareCompatibilityError, + FirmwareConfirmationError, + FirmwareFileError, + FirmwareImage, + FirmwareUploadCancelledError, + FirmwareUploadError, + UploadStage, + check_bootloader_matches_connected_board, + check_compatibility, + parse_apj, +) + +# ArduPilot/PX4 bootloader protocol bytes, from Tools/scripts/uploader.py. +INSYNC = b"\x12" +EOC = b"\x20" +OK = b"\x10" +FAILED = b"\x11" +INVALID = b"\x13" +BAD_SILICON_REV = b"\x14" + +GET_SYNC = b"\x21" +GET_DEVICE = b"\x22" +CHIP_ERASE = b"\x23" +CHIP_VERIFY = b"\x24" +PROG_MULTI = b"\x27" +READ_MULTI = b"\x28" +GET_CRC = b"\x29" +REBOOT = b"\x30" +EXTF_ERASE = b"\x34" +EXTF_PROG_MULTI = b"\x35" +EXTF_GET_CRC = b"\x37" +CHIP_FULL_ERASE = b"\x40" + +INFO_BL_REV = b"\x01" +INFO_BOARD_ID = b"\x02" +INFO_BOARD_REV = b"\x03" +INFO_FLASH_SIZE = b"\x04" +INFO_EXTF_SIZE = b"\x06" + +PROG_MULTI_MAX = 252 # protocol maximum is 255; writes must be word aligned +READ_MULTI_MAX = 252 +ERASE_TIMEOUT = 20.0 +EXTF_CRC_TIMEOUT = 10.0 +MAX_APJ_DESCRIPTOR_SIZE = (MAX_ENCODED_BLOB_SIZE * 2) + (1024 * 1024) +BOOTLOADER_ENUMERATION_TIMEOUT = 15.0 +BOOTLOADER_RETRY_DELAY = 0.5 +BOOTLOADER_OPEN_RETRIES = int(BOOTLOADER_ENUMERATION_TIMEOUT / BOOTLOADER_RETRY_DELAY) + 1 +# Recovery is best-effort; do not let an unresponsive held bootloader delay the +# caller for longer than a normal bootloader command timeout. +BOOTLOADER_ABORT_TIMEOUT = 2.0 + + +class BootloaderTransport(Protocol): + """Minimal blocking byte stream required by the bootloader protocol.""" + + def write(self, data: bytes) -> int: ... + + def read(self, size: int = 1) -> bytes: ... + + def flush(self) -> None: ... + + def reset_input_buffer(self) -> None: ... + + def close(self) -> None: ... + + +ProgressCallback = Callable[[UploadStage, int, int], None] +ConfirmationRequested = Callable[[FirmwareImage, BootloaderInfo], bool] +CancellationRequested = Callable[[], bool] +SerialFactory = Callable[[str, int, float], BootloaderTransport] +BootloaderEntry = Callable[[], None] + + +def _append_recovery_message(exc: Exception, recovery_message: str) -> None: + """ + Append recovery guidance without breaking exception formatting. + + ``OSError`` keeps its rendered message in ``errno``/``strerror`` rather + than deriving it from ``args``. Updating only ``args`` therefore loses + the guidance for errors such as ``OSError(5, "Input/output error")``. + Third-party exceptions may also have no arguments at all. + """ + if isinstance(exc, OSError) and exc.errno is not None: + strerror = exc.strerror or "" + updated = f"{strerror}; {recovery_message}" if strerror else recovery_message + exc.strerror = updated + args = exc.args + if len(args) >= 2: + exc.args = (args[0], updated, *args[2:]) + elif args: + exc.args = (args[0], updated) + else: + exc.args = (exc.errno, updated) + return + + args = exc.args + if args: + exc.args = (f"{args[0]}; {recovery_message}", *args[1:]) + else: + exc.args = (recovery_message,) + + +def _restore_transport_timeout(transport: BootloaderTransport, timeout: float) -> None: + """Restore the normal command timeout after a discovery attempt succeeds.""" + # pyserial exposes both properties; lightweight test/third-party + # transports are not required to, so leave those transports unchanged. + try: + if hasattr(transport, "timeout"): + transport.timeout = timeout # pyright: ignore[reportAttributeAccessIssue] + except (AttributeError, OSError, ValueError): + pass + try: + if hasattr(transport, "write_timeout"): + transport.write_timeout = timeout # pyright: ignore[reportAttributeAccessIssue] + except (AttributeError, OSError, ValueError): + pass + + +@dataclass(frozen=True) +class SerialDeviceIdentity: + """Stable USB attributes which survive a bootloader serial-port rename.""" + + location: str = "" + serial_number: str = "" + interface: str = "" + + +DeviceResolver = Callable[[str, SerialDeviceIdentity | None], str] + + +def capture_serial_device_identity(device: str) -> SerialDeviceIdentity | None: + """Capture stable USB location/serial metadata for the active serial port.""" + try: + resolved_device = Path(device).resolve(strict=True) + except OSError: + resolved_device = None + for port in serial.tools.list_ports.comports(): + if port.device != device and ( + resolved_device is None or not _serial_port_matches_device(port.device, resolved_device) + ): + continue + identity = SerialDeviceIdentity( + location=str(getattr(port, "location", "") or ""), + serial_number=str(getattr(port, "serial_number", "") or ""), + interface=str(getattr(port, "interface", "") or ""), + ) + return identity if identity.location or identity.serial_number else None + return None + + +def _serial_port_matches_device(port_device: str, resolved_device: Path) -> bool: + """Return whether a listed serial device resolves to the selected device.""" + try: + return Path(port_device).resolve(strict=True) == resolved_device + except OSError: + return False + + +def has_stable_bootloader_device_identity(device: str, identity: SerialDeviceIdentity | None) -> bool: + """Return whether reopening this serial target can be tied to physical hardware.""" + del device # The identity must have been captured before the reboot request. + return identity is not None and bool(identity.location or identity.serial_number) + + +def _physical_usb_location(location: object) -> str: + """Return the USB device location without a platform interface suffix.""" + return str(location or "").split(":", 1)[0] + + +def resolve_bootloader_device(device: str, identity: SerialDeviceIdentity | None = None) -> str: + """ + Resolve by captured USB identity after re-enumeration. + + With a known USB identity this is fail-closed: a stale port is never opened + while the board is re-enumerating or the hardware match is ambiguous. + """ + # A captured USB identity must take precedence over the old device path. + # During re-enumeration a generic path such as /dev/ttyACM0 may already + # belong to another controller, whereas the location or serial number + # identifies the controller that was connected before the reboot. + if identity is not None: + if identity.location or identity.serial_number: + matches = [ + port + for port in serial.tools.list_ports.comports() + if all( + not expected + or ( + _physical_usb_location(getattr(port, attribute, "")) == _physical_usb_location(expected) + if attribute == "location" + else getattr(port, attribute, "") == expected + ) + for attribute, expected in ( + ("location", identity.location), + ("serial_number", identity.serial_number), + ) + ) + ] + # Prefer the captured, interface-qualified location when application + # firmware exposes several CDC ports. The bootloader commonly exposes + # only one port with a different suffix, so retain physical matches when + # there is no exact match. + if len(matches) > 1 and identity.location: + exact = [port for port in matches if str(getattr(port, "location", "") or "") == identity.location] + if len(exact) == 1: + matches = exact + # An application-port interface identifier need not be reproduced by the + # bootloader. It can only disambiguate ports from one identified device; + # it must never eliminate a sole candidate or select between devices. + # Linux and Windows typically leave this field unset for ArduPilot CDC ports. + if len(matches) > 1 and identity.interface: + locations = {_physical_usb_location(getattr(port, "location", "")) for port in matches} + if len(locations) == 1 and locations != {""}: + narrowed = [port for port in matches if str(getattr(port, "interface", "") or "") == identity.interface] + if len(narrowed) == 1: + matches = narrowed + if len(matches) == 1: + return str(matches[0].device) + msg = _("cannot uniquely locate the bootloader USB device") + raise OSError(msg) + msg = _("cannot uniquely locate the bootloader USB device") + raise OSError(msg) + return device + + +def report_progress_safely(progress_callback: ProgressCallback | None, stage: UploadStage, completed: int, total: int) -> None: + """Invoke presentation code without allowing it to interrupt a flash operation.""" + if progress_callback is None: + return + try: + progress_callback(stage, completed, total) + except Exception: # pylint: disable=broad-exception-caught + return + + +def load_apj(path: Path) -> FirmwareImage: + """Read an APJ file, then pass its contents to the pure domain parser.""" + if path.suffix.lower() != ".apj": + msg = _("unsupported firmware format {suffix}, only .apj is supported").format(suffix=path.suffix) + raise FirmwareFileError(msg) + try: + if path.stat().st_size > MAX_APJ_DESCRIPTOR_SIZE: + msg = _("APJ descriptor exceeds {limit} bytes").format(limit=MAX_APJ_DESCRIPTOR_SIZE) + raise FirmwareFileError(msg) + with path.open("rb") as descriptor: + contents = descriptor.read(MAX_APJ_DESCRIPTOR_SIZE + 1) + if len(contents) > MAX_APJ_DESCRIPTOR_SIZE: + msg = _("APJ descriptor exceeds {limit} bytes").format(limit=MAX_APJ_DESCRIPTOR_SIZE) + raise FirmwareFileError(msg) + return parse_apj(contents, path=path) + except OSError as exc: + msg = _("cannot read APJ descriptor: {error}").format(error=exc) + raise FirmwareFileError(msg) from exc + + +def encode_get_sync() -> bytes: + return GET_SYNC + EOC + + +def encode_get_device(param: bytes) -> bytes: + return GET_DEVICE + param + EOC + + +def encode_chip_erase(*, full: bool = False) -> bytes: + return (CHIP_FULL_ERASE if full else CHIP_ERASE) + EOC + + +def encode_chip_verify() -> bytes: + return CHIP_VERIFY + EOC + + +def _encode_multi(command: bytes, chunk: bytes) -> bytes: + if not 0 < len(chunk) <= PROG_MULTI_MAX or len(chunk) % 4: + msg = _("PROG_MULTI chunk must be 4..{limit} bytes and a multiple of 4, got {size}").format( + limit=PROG_MULTI_MAX, size=len(chunk) + ) + raise ValueError(msg) + return command + bytes([len(chunk)]) + chunk + EOC + + +def encode_prog_multi(chunk: bytes) -> bytes: + return _encode_multi(PROG_MULTI, chunk) + + +def encode_extf_prog_multi(chunk: bytes) -> bytes: + return _encode_multi(EXTF_PROG_MULTI, chunk) + + +def encode_read_multi(length: int) -> bytes: + if not 0 < length <= READ_MULTI_MAX: + msg = _("READ_MULTI length must be 1..{limit}, got {length}").format(limit=READ_MULTI_MAX, length=length) + raise ValueError(msg) + return READ_MULTI + bytes([length]) + EOC + + +def _encode_external_size(command: bytes, size: int) -> bytes: + if not 0 < size <= 0xFFFFFFFF: + msg = _("external flash size must be 1..4294967295 bytes, got {size}").format(size=size) + raise ValueError(msg) + return command + struct.pack(" bytes: + return _encode_external_size(EXTF_ERASE, size) + + +def encode_extf_get_crc(size: int) -> bytes: + return _encode_external_size(EXTF_GET_CRC, size) + + +def encode_get_crc() -> bytes: + return GET_CRC + EOC + + +def encode_reboot() -> bytes: + return REBOOT + EOC + + +def decode_sync(reply: bytes) -> None: + """Validate an INSYNC/status reply.""" + if len(reply) != 2: + msg = _("short reply, expected INSYNC and status, got {reply}").format(reply=reply) + raise BootloaderProtocolError(msg) + if reply[0:1] != INSYNC: + msg = _("expected INSYNC, got {byte}").format(byte=reply[0:1]) + raise BootloaderProtocolError(msg) + status = reply[1:2] + if status == OK: + return + messages = { + INVALID: _("bootloader reports INVALID OPERATION"), + FAILED: _("bootloader reports OPERATION FAILED"), + BAD_SILICON_REV: _("programming not supported for this silicon revision"), + } + msg = messages.get(status) or _("unexpected status {status} instead of OK").format(status=status) + raise BootloaderProtocolError(msg) + + +def decode_uint32(reply: bytes) -> int: + if len(reply) != 4: + msg = _("short reply, expected 4 bytes, got {size}").format(size=len(reply)) + raise BootloaderProtocolError(msg) + return int(struct.unpack(" Iterator[bytes]: + """Yield protocol-sized chunks without copying the whole image into a list.""" + for offset in range(0, len(image), PROG_MULTI_MAX): + yield image[offset : offset + PROG_MULTI_MAX] + + +class BootloaderClient: + """ + Synchronous bootloader session over an injected transport. + + The transport timeout controls every read. A short read is never accepted as a + protocol response, preventing a timeout or partial serial transfer from being + mistaken for a successful flash operation. + """ + + def __init__( + self, + transport: BootloaderTransport, + *, + timeout: float = 2.0, + clock: Callable[[], float] = monotonic, + sleep: Callable[[float], None] = time_sleep, + ) -> None: + if timeout <= 0: + msg = _("bootloader response timeout must be positive") + raise ValueError(msg) + self._transport = transport + self._timeout = timeout + self._clock = clock + self._sleep = sleep + + def close(self) -> None: + # Cleanup must never replace the protocol error which caused the upload + # to fail. pyserial reports close failures as OSError/SerialException, + # while test and third-party transports may use another Exception type. + with suppress(Exception): + self._transport.close() + + def abort_before_erase(self, *, deadline: float | None = None, expect_ack: bool = True) -> bool: + """ + Best-effort reboot for a declined or failed upload before anything was erased. + + Protocol revision two reboots without an acknowledgement. Later revisions + must acknowledge the reboot so a failed command is not mistaken for a safe + abort. + """ + try: + if expect_ack: + self._command(encode_reboot(), deadline=deadline) + else: + self._write(encode_reboot()) + except BootloaderProtocolError: + return False + return True + + def _reset_input_buffer(self) -> None: + try: + self._transport.reset_input_buffer() + except (OSError, serial.SerialException) as exc: + msg = _("cannot clear bootloader serial input: {error}").format(error=exc) + raise BootloaderProtocolError(msg) from exc + + @staticmethod + def _report(progress_callback: ProgressCallback | None, stage: UploadStage, completed: int, total: int) -> None: + report_progress_safely(progress_callback, stage, completed, total) + + def _write(self, request: bytes) -> None: + try: + written = self._transport.write(request) + self._transport.flush() + except (OSError, serial.SerialException) as exc: + msg = _("bootloader transport write failed: {error}").format(error=exc) + raise BootloaderProtocolError(msg) from exc + if written != len(request): + msg = _("bootloader transport wrote {written} of {expected} bytes").format(written=written, expected=len(request)) + raise BootloaderProtocolError(msg) + + def _read_exact(self, size: int, *, timeout: float | None = None, deadline: float | None = None) -> bytes: + received = bytearray() + read_deadline = deadline if deadline is not None else self._clock() + (self._timeout if timeout is None else timeout) + try: + while len(received) < size: + if self._clock() >= read_deadline: + msg = _("timeout waiting for {expected} bootloader bytes; received {actual}").format( + expected=size, actual=len(received) + ) + raise BootloaderProtocolError(msg) + chunk = self._transport.read(size - len(received)) + if not chunk: + remaining = read_deadline - self._clock() + if remaining > 0: + # A non-blocking transport can return empty reads in a tight + # loop. Yield briefly so a malformed transport cannot consume + # an entire CPU while the response deadline is pending. + self._sleep(min(0.01, remaining)) + continue + msg = _("timeout waiting for {expected} bootloader bytes; received {actual}").format( + expected=size, actual=len(received) + ) + raise BootloaderProtocolError(msg) + received.extend(chunk) + except (OSError, serial.SerialException) as exc: + msg = _("bootloader transport read failed: {error}").format(error=exc) + raise BootloaderProtocolError(msg) from exc + return bytes(received) + + def _sync(self, *, timeout: float | None = None, deadline: float | None = None) -> None: + decode_sync(self._read_exact(2, timeout=timeout, deadline=deadline)) + + def _command(self, request: bytes, reply_size: int = 0, *, deadline: float | None = None) -> bytes: + self._write(request) + reply = self._read_exact(reply_size, deadline=deadline) if reply_size else b"" + self._sync(deadline=deadline) + return reply + + def identify(self, *, deadline: float | None = None) -> BootloaderInfo: + self._reset_input_buffer() + self._command(encode_get_sync(), deadline=deadline) + revision = decode_uint32(self._command(encode_get_device(INFO_BL_REV), 4, deadline=deadline)) + if not BL_REV_MIN <= revision <= BL_REV_MAX: + msg = _("unsupported bootloader protocol revision {revision}").format(revision=revision) + raise BootloaderProtocolError(msg) + + # Old bootloaders can reject this newer optional query. Resynchronize and + # conservatively report no external flash in that case. + try: + extf_size = decode_uint32(self._command(encode_get_device(INFO_EXTF_SIZE), 4, deadline=deadline)) + except BootloaderProtocolError: + # An old bootloader answers this newer optional query with INSYNC/INVALID + # (two bytes where four payload bytes are expected), so the read above + # spins the shared deadline to exhaustion before raising. The recovery + # resync and the remaining identity queries must run on a fresh bounded + # deadline; reusing the now-expired one would reject the bootloader's + # valid INSYNC/OK before it is ever read. A None deadline already gets + # a fresh budget per read, so leave it untouched. + extf_size = 0 + self._reset_input_buffer() + deadline = None if deadline is None else self._clock() + self._timeout + self._command(encode_get_sync(), deadline=deadline) + return BootloaderInfo( + protocol_revision=revision, + board_id=decode_uint32(self._command(encode_get_device(INFO_BOARD_ID), 4, deadline=deadline)), + board_revision=decode_uint32(self._command(encode_get_device(INFO_BOARD_REV), 4, deadline=deadline)), + flash_size=decode_uint32(self._command(encode_get_device(INFO_FLASH_SIZE), 4, deadline=deadline)), + extf_size=extf_size, + ) + + def _erase_external(self, size: int) -> None: + self._write(encode_extf_erase(size)) + self._sync() + # The bootloader emits percentage bytes until it is almost done, then the + # final INSYNC/OK acknowledgement. Values below 90 are progress reports. + deadline = self._clock() + ERASE_TIMEOUT + last_pct = 0 + while True: + first = self._read_exact(1, deadline=deadline) + if last_pct >= 90 and first == INSYNC: + decode_sync(first + self._read_exact(1, deadline=deadline)) + return + progress = first[0] + if progress > 100: + msg = _("invalid external-flash erase progress {progress}").format(progress=progress) + raise BootloaderProtocolError(msg) + if progress < last_pct: + msg = _("external-flash erase progress regressed from {previous} to {progress}").format( + previous=last_pct, progress=progress + ) + raise BootloaderProtocolError(msg) + if progress > last_pct: + last_pct = progress + deadline = self._clock() + ERASE_TIMEOUT + + def _verify_v2(self, image: FirmwareImage) -> None: + self._command(encode_chip_verify()) + for offset in range(0, len(image.image), READ_MULTI_MAX): + expected = image.image[offset : offset + READ_MULTI_MAX] + programmed = self._command(encode_read_multi(len(expected)), len(expected)) + if programmed != expected: + msg = _("firmware read-back verification failed") + raise BootloaderProtocolError(msg) + + def _verify_v3(self, image: FirmwareImage, flash_size: int) -> None: + actual = decode_uint32(self._command(encode_get_crc(), 4)) + if actual != image.crc(flash_size): + msg = _("firmware CRC verification failed") + raise BootloaderProtocolError(msg) + + def _verify_external(self, image: FirmwareImage) -> None: + self._write(encode_extf_get_crc(image.metadata.extf_image_size)) + actual = decode_uint32(self._read_exact(4, timeout=EXTF_CRC_TIMEOUT)) + self._sync(timeout=EXTF_CRC_TIMEOUT) + if actual != image.extf_crc(): + msg = _("external firmware CRC verification failed") + raise BootloaderProtocolError(msg) + + def upload( # noqa: PLR0915 # pylint: disable=too-many-arguments,too-many-branches,too-many-statements,too-many-locals + self, + image: FirmwareImage, + *, + full_erase: bool = False, + cancellation_requested: CancellationRequested | None = None, + confirmation_requested: ConfirmationRequested | None = None, + progress_callback: ProgressCallback | None = None, + bootloader: BootloaderInfo | None = None, + connected_board_id: int | None = None, + ) -> BootloaderInfo: + """ + Identify, validate, program all image regions, verify, and reboot. + + The transport is closed on success and on every error path. Revision two + uses CHIP_VERIFY plus READ_MULTI and intentionally receives no reboot ACK; + revision three and later use CRC and require that ACK. + """ + stage = UploadStage.AWAITING_CONFIRMATION + info = bootloader + try: + if confirmation_requested is None: + msg = _("firmware upload requires explicit confirmation") + raise FirmwareConfirmationError(msg) + if cancellation_requested is not None and cancellation_requested(): + msg = _("firmware upload cancelled before entering the bootloader") + raise FirmwareUploadCancelledError(msg, stage=stage.value) + stage = UploadStage.IDENTIFYING + self._report(progress_callback, stage, 0, 1) + info = info or self.identify() + self._report(progress_callback, stage, 1, 1) + check_bootloader_matches_connected_board(info, connected_board_id) + check_compatibility(image, info) + if full_erase: + # AP_Bootloader only implements CHIP_FULL_ERASE on STM32F7/H7. + # This client does not yet use the bootloader's MCU-identification + # responses to gate that capability, so refuse rather than send a + # command that an unsupported target may ignore until timeout. + msg = _("full firmware erase is unavailable because support cannot yet be determined safely") + raise FirmwareCompatibilityError(msg) + stage = UploadStage.AWAITING_CONFIRMATION + self._report(progress_callback, stage, 0, 1) + if not confirmation_requested(image, info): + msg = _("firmware upload was not confirmed") + raise FirmwareUploadCancelledError(msg, stage=stage.value) + self._report(progress_callback, stage, 1, 1) + if cancellation_requested is not None and cancellation_requested(): + msg = _("firmware upload cancelled before erase") + raise FirmwareUploadCancelledError(msg, stage=stage.value) + if image.metadata.extf_image_size: + stage = UploadStage.ERASING + self._report(progress_callback, stage, 0, 1) + self._erase_external(image.metadata.extf_image_size) + self._report(progress_callback, stage, 1, 1) + stage = UploadStage.PROGRAMMING + chunk_count = (len(image.extf_image) + PROG_MULTI_MAX - 1) // PROG_MULTI_MAX + for index, chunk in enumerate(program_chunks(image.extf_image), start=1): + self._command(encode_extf_prog_multi(chunk)) + self._report(progress_callback, stage, index, chunk_count) + stage = UploadStage.VERIFYING + self._report(progress_callback, stage, 0, 1) + self._verify_external(image) + self._report(progress_callback, stage, 1, 1) + if image.metadata.image_size: + stage = UploadStage.ERASING + self._report(progress_callback, stage, 0, 1) + self._write(encode_chip_erase(full=full_erase)) + self._sync(timeout=ERASE_TIMEOUT) + self._report(progress_callback, stage, 1, 1) + stage = UploadStage.PROGRAMMING + chunk_count = (len(image.image) + PROG_MULTI_MAX - 1) // PROG_MULTI_MAX + for index, chunk in enumerate(program_chunks(image.image), start=1): + self._command(encode_prog_multi(chunk)) + self._report(progress_callback, stage, index, chunk_count) + stage = UploadStage.VERIFYING + self._report(progress_callback, stage, 0, 1) + if info.protocol_revision == 2: + self._verify_v2(image) + else: + self._verify_v3(image, info.flash_size) + self._report(progress_callback, stage, 1, 1) + stage = UploadStage.REBOOTING + self._report(progress_callback, stage, 0, 1) + if info.protocol_revision == 2: + self._write(encode_reboot()) + else: + self._command(encode_reboot()) + self._report(progress_callback, stage, 1, 1) + return info + except FirmwareUploadError as exc: + if stage in {UploadStage.IDENTIFYING, UploadStage.AWAITING_CONFIRMATION} and not self.abort_before_erase( + expect_ack=info is None or info.protocol_revision != 2 + ): + msg = _("cannot reboot the held bootloader; power-cycle the flight controller before reconnecting") + raise FirmwareBootloaderRecoveryError(msg) from exc + if stage in {UploadStage.IDENTIFYING, UploadStage.AWAITING_CONFIRMATION}: + exc.bootloader_rebooted = True # type: ignore[attr-defined] + if isinstance(exc, BootloaderProtocolError): + exc.stage = stage.value + if stage in {UploadStage.ERASING, UploadStage.PROGRAMMING, UploadStage.VERIFYING, UploadStage.REBOOTING}: + recovery_message = _( + "firmware upload failed during {stage}; the firmware flash may be incomplete. " + "power-cycle the flight controller before reconnecting" + ).format(stage=stage.value) + _append_recovery_message(exc, recovery_message) + raise + except Exception as exc: + if stage in {UploadStage.IDENTIFYING, UploadStage.AWAITING_CONFIRMATION} and not self.abort_before_erase( + expect_ack=info is None or info.protocol_revision != 2 + ): + msg = _("cannot reboot the held bootloader; power-cycle the flight controller before reconnecting") + raise FirmwareBootloaderRecoveryError(msg) from exc + if stage in {UploadStage.IDENTIFYING, UploadStage.AWAITING_CONFIRMATION}: + exc.bootloader_rebooted = True # type: ignore[attr-defined] + if stage in {UploadStage.ERASING, UploadStage.PROGRAMMING, UploadStage.VERIFYING, UploadStage.REBOOTING}: + recovery_message = _( + "firmware upload failed during {stage}; the firmware flash may be incomplete. " + "power-cycle the flight controller before reconnecting" + ).format(stage=stage.value) + _append_recovery_message(exc, recovery_message) + raise + finally: + self.close() + + +def open_serial_transport(device: str, baudrate: int, timeout: float) -> BootloaderTransport: + """Open the real serial transport with explicit read/write timeouts.""" + return cast("BootloaderTransport", serial.Serial(device, baudrate, timeout=timeout, write_timeout=timeout, exclusive=True)) + + +class FlightControllerBootloaderBackend: # pylint:disable=too-many-instance-attributes + """ + Internal bootloader transport adapter with injectable entry and serial transport. + + The public upload workflow is ``FlightController.upload_apj_firmware``. It + performs the trusted-digest and stable-USB-identity checks before invoking + this lower-level adapter; direct callers are responsible for those policy + preconditions. + """ + + def __init__( # noqa: PLR0913 # pylint: disable=too-many-arguments + self, + device: str, + baudrate: int, + *, + timeout: float = 2.0, + enter_bootloader: BootloaderEntry | None = None, + serial_factory: SerialFactory = open_serial_transport, + device_resolver: DeviceResolver = resolve_bootloader_device, + device_identity: SerialDeviceIdentity | None = None, + open_retries: int = BOOTLOADER_OPEN_RETRIES, + retry_delay: float = BOOTLOADER_RETRY_DELAY, + sleep: Callable[[float], None] = time_sleep, + clock: Callable[[], float] = monotonic, + ) -> None: + if timeout <= 0: + msg = _("bootloader response timeout must be positive") + raise ValueError(msg) + self._device = device + self._device_resolver = device_resolver + self._device_identity = device_identity + self._baudrate = baudrate + self._timeout = timeout + self._enter_bootloader = enter_bootloader + self._serial_factory = serial_factory + self._open_retries = open_retries + self._retry_delay = retry_delay + self._sleep = sleep + self._clock = clock + + def inspect_firmware(self, path: Path) -> FirmwareImage: + return load_apj(path) + + def upload( # pylint: disable=too-many-arguments + self, + image: FirmwareImage, + *, + full_erase: bool = False, + cancellation_requested: CancellationRequested | None = None, + confirmation_requested: ConfirmationRequested | None = None, + progress_callback: ProgressCallback | None = None, + connected_board_id: int | None = None, + ) -> BootloaderInfo: + if confirmation_requested is None: + msg = _("firmware upload requires explicit confirmation") + raise FirmwareConfirmationError(msg) + if cancellation_requested is not None and cancellation_requested(): + msg = _("firmware upload cancelled before entering the bootloader") + raise FirmwareUploadCancelledError(msg, stage=UploadStage.ENTERING_BOOTLOADER.value) + if full_erase: + # CHIP_FULL_ERASE support cannot yet be determined safely, so refuse + # before rebooting into the bootloader. Rejecting later (after + # _enter_bootloader has sent MAV_CMD_PREFLIGHT_REBOOT_SHUTDOWN and + # dropped the MAVLink link) would disrupt the board for nothing. + msg = _("full firmware erase is unavailable because support cannot yet be determined safely") + raise FirmwareCompatibilityError(msg) + if self._enter_bootloader is not None: + self._enter_bootloader() + client, info = self._wait_for_bootloader(cancellation_requested) + # pylint: disable=duplicate-code + return client.upload( + image, + full_erase=full_erase, + cancellation_requested=cancellation_requested, + confirmation_requested=confirmation_requested, + progress_callback=progress_callback, + bootloader=info, + connected_board_id=connected_board_id, + ) + # pylint: enable=duplicate-code + + def _wait_for_bootloader( # pylint: disable=too-many-locals + self, cancellation_requested: CancellationRequested | None = None + ) -> tuple[BootloaderClient, BootloaderInfo]: + if self._open_retries < 1: + msg = _("bootloader open retries must be at least one") + raise ValueError(msg) + last_error: FirmwareUploadError | OSError | serial.SerialException | None = None + deadline = self._clock() + BOOTLOADER_ENUMERATION_TIMEOUT + for attempt in range(self._open_retries): + if cancellation_requested is not None and cancellation_requested(): + msg = _( + "firmware upload was cancelled while waiting for the bootloader; " + "power-cycle the flight controller before reconnecting" + ) + raise FirmwareBootloaderRecoveryError(msg) from last_error + remaining = deadline - self._clock() + if remaining <= 0: + break + attempt_timeout = min(self._timeout, remaining) + transport, open_error = self._try_open_transport(attempt_timeout) + if transport is not None: + client = BootloaderClient( + transport, + # Identification belongs to this retry attempt and must not + # consume the remaining global discovery budget through a + # stale transport timeout. + timeout=self._timeout, + clock=self._clock, + sleep=self._sleep, + ) + try: + # The global deadline controls how long discovery retries; + # each identification attempt gets its own bounded deadline + # so a slow re-enumeration can be retried. + attempt_deadline = min(deadline, self._clock() + attempt_timeout) + info = client.identify(deadline=attempt_deadline) + _restore_transport_timeout(transport, self._timeout) + return client, info + except BootloaderProtocolError as exc: + last_error = exc + retry_would_exhaust_budget = self._clock() + self._retry_delay >= deadline + if attempt + 1 == self._open_retries or retry_would_exhaust_budget: + abort_deadline = self._clock() + min(self._timeout, BOOTLOADER_ABORT_TIMEOUT) + if not client.abort_before_erase(deadline=abort_deadline, expect_ack=True): + client.close() + msg = _( + "could not confirm reboot of the held bootloader; " + "power-cycle the flight controller before reconnecting" + ) + raise FirmwareBootloaderRecoveryError(msg) from exc + exc.bootloader_rebooted = True # type: ignore[attr-defined] + exc.stage = UploadStage.IDENTIFYING.value + client.close() + else: + last_error = open_error + if attempt + 1 < self._open_retries: + remaining = deadline - self._clock() + if remaining > 0: + self._sleep(min(self._retry_delay, remaining)) + if isinstance(last_error, BootloaderProtocolError): + raise last_error + msg = _( + "cannot open the held bootloader on serial port {device}: {error}; " + "power-cycle the flight controller before reconnecting" + ).format(device=self._device, error=last_error) + raise FirmwareBootloaderRecoveryError(msg) from last_error + + def _try_open_transport( + self, timeout: float | None = None + ) -> tuple[BootloaderTransport | None, OSError | serial.SerialException | None]: + try: + device = self._device_resolver(self._device, self._device_identity) + return self._serial_factory(device, self._baudrate, self._timeout if timeout is None else timeout), None + except (OSError, serial.SerialException) as exc: + return None, exc diff --git a/ardupilot_methodic_configurator/backend_flightcontroller_commands.py b/ardupilot_methodic_configurator/backend_flightcontroller_commands.py index dddaad954..b960c1477 100644 --- a/ardupilot_methodic_configurator/backend_flightcontroller_commands.py +++ b/ardupilot_methodic_configurator/backend_flightcontroller_commands.py @@ -200,6 +200,14 @@ def send_command_and_wait_ack( # pylint: disable=too-many-arguments,too-many-po logging_error(error_msg) return False, error_msg + def reboot_to_bootloader(self) -> tuple[bool, str]: + """Request reboot into the bootloader and wait for its command acknowledgment.""" + return self.send_command_and_wait_ack( + mavutil.mavlink.MAV_CMD_PREFLIGHT_REBOOT_SHUTDOWN, + param1=3, + timeout=self.COMMAND_ACK_TIMEOUT, + ) + def reset_all_parameters_to_default(self) -> tuple[bool, str]: """ Reset all parameters to their factory default values. diff --git a/ardupilot_methodic_configurator/backend_flightcontroller_connection.py b/ardupilot_methodic_configurator/backend_flightcontroller_connection.py index 0f497195a..88246d9f0 100644 --- a/ardupilot_methodic_configurator/backend_flightcontroller_connection.py +++ b/ardupilot_methodic_configurator/backend_flightcontroller_connection.py @@ -143,6 +143,7 @@ def __init__( # pylint: disable=too-many-arguments, too-many-positional-argumen self.master: MavlinkConnection | None = None self.comport: mavutil.SerialPort | serial.tools.list_ports_common.ListPortInfo | None = None self._baudrate = baudrate + self._active_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: list[tuple[str, str]] | None = None # None = never logged @@ -327,6 +328,7 @@ def connect( """ connection_baudrate = baudrate if baudrate is not None else self._baudrate + self._active_baudrate = connection_baudrate # Always clear cached metadata before attempting a new connection so UI # components never display stale data while we probe ports. @@ -870,6 +872,7 @@ def create_connection_with_retry( # pylint: disable=too-many-arguments, too-man indicating a successful connection. """ + self._active_baudrate = baudrate if self.comport is None or self.comport.device == DEVICE_FC_PARAM_FROM_FILE: # will read parameters from a params.param file instead of a from a flight controller return "" @@ -939,6 +942,11 @@ def baudrate(self) -> int: """Get the default baud rate for serial connections.""" return self._baudrate + @property + def active_baudrate(self) -> int: + """Get the baud rate used for the current or most recent serial session.""" + return self._active_baudrate + def set_master_for_testing( self, master: Optional["MavlinkConnection"], diff --git a/ardupilot_methodic_configurator/backend_flightcontroller_protocols.py b/ardupilot_methodic_configurator/backend_flightcontroller_protocols.py index 159860d32..a6ae2ad36 100644 --- a/ardupilot_methodic_configurator/backend_flightcontroller_protocols.py +++ b/ardupilot_methodic_configurator/backend_flightcontroller_protocols.py @@ -98,6 +98,11 @@ def baudrate(self) -> int: """Get the default baud rate for serial connections.""" ... # pylint: disable=unnecessary-ellipsis + @property + def active_baudrate(self) -> int: + """Get the baud rate used for the current or most recent serial session.""" + ... # pylint: disable=unnecessary-ellipsis + def discover_connections( self, progress_callback: Callable[[int, int], None] | None = None, @@ -233,6 +238,8 @@ def send_command_and_wait_ack( # pylint: disable=too-many-arguments, too-many-p timeout: float, ) -> tuple[bool, str]: ... + def reboot_to_bootloader(self) -> tuple[bool, str]: ... + def reset_all_parameters_to_default(self) -> tuple[bool, str]: ... def test_motor( # pylint: disable=too-many-arguments, too-many-positional-arguments diff --git a/ardupilot_methodic_configurator/data_model_firmware_upload.py b/ardupilot_methodic_configurator/data_model_firmware_upload.py new file mode 100755 index 000000000..fa29a0072 --- /dev/null +++ b/ardupilot_methodic_configurator/data_model_firmware_upload.py @@ -0,0 +1,407 @@ +#!/usr/bin/env python3 + +""" +Firmware upload domain model: APJ image decoding, bootloader protocol codec and validation rules. + +No serial, MAVLink, Tkinter or dialogs live here. The backend adapter performs the I/O and +feeds bytes in and out of the pure functions defined in this module. + +The protocol constants and the image normalisation mirror ArduPilot Tools/scripts/uploader.py so the +backend can follow the same erase/program/verify sequence as the reference uploader. + +This file is part of ArduPilot Methodic Configurator. https://github.com/ArduPilot/MethodicConfigurator + +SPDX-FileCopyrightText: 2024-2026 Amilcar do Carmo Lucas + +SPDX-License-Identifier: GPL-3.0-or-later +""" + +import base64 +import hashlib +import hmac +import json +import re +import zlib +from binascii import Error as BinasciiError +from dataclasses import dataclass +from enum import Enum +from pathlib import Path +from typing import Any + +from ardupilot_methodic_configurator import _ + +BL_REV_MIN = 2 +BL_REV_MAX = 5 + +# Sanity bound on each decompressed image, well above any current flight controller flash. +MAX_IMAGE_SIZE = 64 * 1024 * 1024 +# A base64 APJ blob can be roughly 4/3 the compressed input. Permit a small +# compression-stream overhead while rejecting a descriptor before decoding it. +MAX_ENCODED_BLOB_SIZE = ((MAX_IMAGE_SIZE + 65536) * 4 // 3) + 4 + + +class FirmwareUploadError(Exception): + """Base class for typed firmware upload errors, `stage` names where it happened.""" + + stage = "unknown" + + def __init__(self, message: str, *, stage: str | None = None) -> None: + super().__init__(message) + if stage is not None: + self.stage = stage + + +class FirmwareFileError(FirmwareUploadError): + """The firmware file is unreadable, malformed or not a supported format.""" + + stage = "inspecting" + + +class FirmwareCompatibilityError(FirmwareUploadError): + """The image does not match the connected board or does not fit in flash.""" + + stage = "identifying" + + +class BootloaderProtocolError(FirmwareUploadError): + """The bootloader answered something unexpected, or a protocol revision is unsupported.""" + + stage = "identifying" + + +class FirmwareReconnectError(FirmwareUploadError): + """Firmware was flashed but the normal flight-controller connection did not return.""" + + stage = "reconnecting" + + +class FirmwareConnectionError(FirmwareUploadError): + """The active flight-controller connection cannot be used for firmware upload.""" + + stage = "entering_bootloader" + + +class FirmwareBootloaderRecoveryError(FirmwareConnectionError): + """The held bootloader could not be reached to reboot into the installed firmware.""" + + +class FirmwareTargetMismatchError(FirmwareUploadError): + """The bootloader belongs to a different board than the connected flight controller.""" + + stage = "identifying" + + +class FirmwareIdentityError(FirmwareUploadError): + """The firmware reported after reboot does not match the selected APJ image.""" + + stage = "reconnecting" + + +class FirmwareUploadCancelledError(FirmwareUploadError): + """The user cancelled before firmware erase began.""" + + stage = "identifying" + + +class FirmwareConfirmationError(FirmwareUploadError): + """An irreversible upload was requested without explicit confirmation.""" + + stage = "awaiting_confirmation" + + +class FirmwareIntegrityError(FirmwareUploadError): + """The decoded APJ bytes do not match the trusted digest supplied by the caller.""" + + stage = "inspecting" + + +@dataclass(frozen=True) +class FirmwareImageMetadata: # pylint: disable=too-many-instance-attributes + """What the user sees before confirming an upload, read from the APJ descriptor and never from the filename.""" + + path: Path + board_id: int + image_size: int + extf_image_size: int + firmware_version: str + git_identity: str + apj_sha256: str + board_revision: int | None = None + + +@dataclass(frozen=True) +class FirmwareImage: + """Decoded, 4-byte padded image ready for the bootloader.""" + + metadata: FirmwareImageMetadata + image: bytes + extf_image: bytes = b"" + + def crc(self, flash_size: int) -> int: + """CRC32 of the image padded with 0xFF up to flash_size, as computed by the bootloader GET_CRC.""" + if flash_size < 0 or flash_size % 4: + msg = _("bootloader flash size must be a non-negative multiple of four") + raise ValueError(msg) + state = bootloader_crc32(self.image) + remaining = max(0, flash_size - len(self.image)) + while remaining: + padding_size = min(remaining, 4) + state = bootloader_crc32(b"\xff" * padding_size, state) + remaining -= padding_size + return state + + def extf_crc(self) -> int: + """CRC32 of the unpadded external-flash payload.""" + return bootloader_crc32(self.extf_image[: self.metadata.extf_image_size]) + + +@dataclass(frozen=True) +class BootloaderInfo: + """Answers to GET_DEVICE queries during identification.""" + + protocol_revision: int + board_id: int + board_revision: int + flash_size: int + extf_size: int = 0 + + +class UploadStage(Enum): + """Workflow state, transitions are validated by `next_stage`.""" + + IDLE = "idle" + INSPECTING = "inspecting" + AWAITING_CONFIRMATION = "awaiting_confirmation" + ENTERING_BOOTLOADER = "entering_bootloader" + IDENTIFYING = "identifying" + ERASING = "erasing" + PROGRAMMING = "programming" + VERIFYING = "verifying" + REBOOTING = "rebooting" + RECONNECTING = "reconnecting" + COMPLETED = "completed" + FAILED = "failed" + CANCELLED = "cancelled" + + +_HAPPY_PATH = [ + UploadStage.IDLE, + UploadStage.INSPECTING, + UploadStage.ENTERING_BOOTLOADER, + UploadStage.IDENTIFYING, + UploadStage.AWAITING_CONFIRMATION, + UploadStage.ERASING, + UploadStage.PROGRAMMING, + UploadStage.VERIFYING, + UploadStage.REBOOTING, + UploadStage.RECONNECTING, + UploadStage.COMPLETED, +] + +# Cancellation is only allowed before the flash is touched. Once erase started, the only +# ways out are completed or failed, so a half written flash is never reported as cancelled. +_CANCELLABLE = frozenset(_HAPPY_PATH[: _HAPPY_PATH.index(UploadStage.ERASING)]) +_TERMINAL = frozenset({UploadStage.COMPLETED, UploadStage.FAILED, UploadStage.CANCELLED}) + + +def next_stage(current: UploadStage, target: UploadStage) -> UploadStage: + """Return target if the transition is legal, raise ValueError otherwise.""" + if current in _TERMINAL: + msg = _("{stage} is terminal").format(stage=current.value) + raise ValueError(msg) + if target == UploadStage.FAILED: + return target + if target == UploadStage.CANCELLED: + if current in _CANCELLABLE: + return target + msg = _("cannot cancel during {stage}").format(stage=current.value) + raise ValueError(msg) + if current == UploadStage.VERIFYING and target == UploadStage.ERASING: + return target + if _HAPPY_PATH.index(target) == _HAPPY_PATH.index(current) + 1: + return target + msg = _("illegal transition {current} -> {target}").format(current=current.value, target=target.value) + raise ValueError(msg) + + +def parse_apj(contents: str | bytes, *, path: Path = Path()) -> FirmwareImage: # pylint: disable=too-many-locals + """Purely parse and validate APJ contents; callers own file-system access.""" + try: + source_bytes = contents.encode("utf-8") if isinstance(contents, str) else contents + desc: Any = json.loads(contents) + except (RecursionError, UnicodeError, ValueError, TypeError) as exc: + msg = _("cannot parse APJ descriptor: {error}").format(error=exc) + raise FirmwareFileError(msg) from exc + if not isinstance(desc, dict): + msg = _("APJ descriptor is not a JSON object") + raise FirmwareFileError(msg) + + raw_image = _decode_blob(desc, "image") + raw_extf_image = _decode_blob(desc, "extf_image") if "extf_image" in desc else b"" + + try: + board_id = _parse_integer_metadata(desc["board_id"]) + image_size = _parse_integer_metadata(desc["image_size"]) + extf_image_size = _parse_integer_metadata(desc.get("extf_image_size", 0)) + except (KeyError, TypeError, ValueError) as exc: + msg = _("APJ descriptor is missing or has invalid metadata: {error}").format(error=exc) + raise FirmwareFileError(msg) from exc + if board_id < 0 or image_size < 0 or extf_image_size < 0 or (image_size == 0 and extf_image_size == 0): + msg = _("APJ metadata values are out of range") + raise FirmwareFileError(msg) + if image_size != len(raw_image) or extf_image_size != len(raw_extf_image): + msg = _("APJ image_size does not match the decoded image") + raise FirmwareFileError(msg) + + firmware_version = desc.get("version", "") + if not isinstance(firmware_version, str): + msg = _("APJ version metadata must be text") + raise FirmwareFileError(msg) + git_identity = desc.get("git_identity", "") + if not isinstance(git_identity, str): + msg = _("APJ git_identity metadata must be text") + raise FirmwareFileError(msg) + + board_rev = desc.get("board_revision") + try: + parsed_board_rev = _parse_integer_metadata(board_rev) if board_rev is not None else None + except (TypeError, ValueError) as exc: + msg = _("APJ descriptor is missing or has invalid metadata: {error}").format(error=exc) + raise FirmwareFileError(msg) from exc + metadata = FirmwareImageMetadata( + path=path, + board_id=board_id, + image_size=image_size, + extf_image_size=extf_image_size, + firmware_version=firmware_version, + git_identity=git_identity, + apj_sha256=hashlib.sha256(source_bytes).hexdigest(), + board_revision=parsed_board_rev, + ) + return FirmwareImage(metadata=metadata, image=_pad4(raw_image), extf_image=_pad4(raw_extf_image)) + + +def _parse_integer_metadata(value: object) -> int: + """Parse APJ integer metadata without silently accepting booleans or floats.""" + if isinstance(value, (bool, float)): + msg = _("APJ metadata integer must not be boolean or floating point") + raise ValueError(msg) + if not isinstance(value, (int, str)): + msg = _("APJ metadata integer must be an integer or integer string") + raise TypeError(msg) + return int(value) + + +def _decode_blob(desc: dict, key: str) -> bytes: + try: + encoded = desc[key] + if not isinstance(encoded, (str, bytes)): + msg = _("APJ {key} is not valid base64+zlib data: expected text").format(key=key) + raise FirmwareFileError(msg) + if len(encoded) > MAX_ENCODED_BLOB_SIZE: + msg = _("APJ {key} exceeds {limit} encoded bytes").format(key=key, limit=MAX_ENCODED_BLOB_SIZE) + raise FirmwareFileError(msg) + compressed = base64.b64decode(encoded, validate=True) + decompressor = zlib.decompressobj() + raw = decompressor.decompress(compressed, MAX_IMAGE_SIZE + 1) + if len(raw) > MAX_IMAGE_SIZE or decompressor.unconsumed_tail: + msg = _("APJ {key} exceeds {limit} bytes").format(key=key, limit=MAX_IMAGE_SIZE) + raise FirmwareFileError(msg) + # ``unconsumed_tail`` above proves the bounded decompress call consumed + # the entire stream. ``flush`` merely releases the decoder's small + # pending buffer; its argument is not an output limit. + raw += decompressor.flush() + if len(raw) > MAX_IMAGE_SIZE: + msg = _("APJ {key} exceeds {limit} bytes").format(key=key, limit=MAX_IMAGE_SIZE) + raise FirmwareFileError(msg) + if not decompressor.eof: + msg = _("APJ {key} is not valid base64+zlib data: truncated stream").format(key=key) + raise FirmwareFileError(msg) + if decompressor.unused_data: + msg = _("APJ {key} is not valid base64+zlib data: trailing bytes").format(key=key) + raise FirmwareFileError(msg) + except (BinasciiError, KeyError, TypeError, ValueError, zlib.error) as exc: + msg = _("APJ {key} is not valid base64+zlib data: {error}").format(key=key, error=exc) + raise FirmwareFileError(msg) from exc + return raw + + +def _pad4(data: bytes) -> bytes: + return data + b"\xff" * (-len(data) % 4) + + +def bootloader_crc32(data: bytes, state: int = 0) -> int: + """ + Return the raw CRC-32 state used by ArduPilot's bootloader protocol. + + ArduPilot's ``crc32_small`` and ``Tools/scripts/uploader.py`` expose the + raw running state beginning at zero. ``zlib.crc32`` uses the same update + table but applies conventional initial/final XOR values, so translate the + state at the boundary instead of processing one bit at a time in Python. + """ + return zlib.crc32(data, state ^ 0xFFFFFFFF) ^ 0xFFFFFFFF + + +def verify_expected_firmware_digest(image: FirmwareImage, expected_sha256: str) -> None: + """Require a release SHA-256 for the exact APJ descriptor bytes selected for upload.""" + if not re.fullmatch(r"[0-9a-fA-F]{64}", expected_sha256): + msg = _("trusted firmware SHA-256 must be 64 hexadecimal characters") + raise FirmwareIntegrityError(msg) + actual = image.metadata.apj_sha256 + if not hmac.compare_digest(actual, expected_sha256.lower()): + msg = _("selected firmware SHA-256 does not match the trusted digest") + raise FirmwareIntegrityError(msg) + + +def _board_ids_are_compatible(bootloader_board_id: int, image_board_id: int) -> bool: + """Return whether the bootloader's board ID accepts the firmware board ID.""" + compatible_board_ids = {33: 9} + return bootloader_board_id == image_board_id or compatible_board_ids.get(bootloader_board_id) == image_board_id + + +def check_bootloader_matches_connected_board(bootloader: BootloaderInfo, connected_board_id: int | None) -> None: + """Refuse a bootloader that differs from the board identified over MAVLink before reboot.""" + if connected_board_id is None or _board_ids_are_compatible(bootloader.board_id, connected_board_id): + return + msg = _("bootloader board_id {bootloader_id} differs from connected board_id {connected_id}").format( + bootloader_id=bootloader.board_id, connected_id=connected_board_id + ) + raise FirmwareTargetMismatchError(msg) + + +def check_compatibility(image: FirmwareImage, bootloader: BootloaderInfo) -> None: + """Raise FirmwareCompatibilityError unless the image can safely be flashed on this bootloader.""" + if not BL_REV_MIN <= bootloader.protocol_revision <= BL_REV_MAX: + msg = _("unsupported bootloader protocol revision {revision}").format(revision=bootloader.protocol_revision) + raise BootloaderProtocolError(msg) + meta = image.metadata + if not _board_ids_are_compatible(bootloader.board_id, meta.board_id): + msg = _("firmware is for board_id {image_id}, board reports {board_id}").format( + image_id=meta.board_id, board_id=bootloader.board_id + ) + raise FirmwareCompatibilityError(msg) + if bootloader.flash_size < 0 or bootloader.flash_size % 4: + msg = _("bootloader reports an invalid flash size of {flash} bytes").format(flash=bootloader.flash_size) + raise FirmwareCompatibilityError(msg) + if len(image.image) > bootloader.flash_size: + msg = _("image of {size} bytes exceeds flash of {flash} bytes").format( + size=len(image.image), flash=bootloader.flash_size + ) + raise FirmwareCompatibilityError(msg) + if len(image.extf_image) > bootloader.extf_size: + msg = _("external image of {size} bytes exceeds external flash of {extf} bytes").format( + size=len(image.extf_image), extf=bootloader.extf_size + ) + raise FirmwareCompatibilityError(msg) + + +def verify_reconnected_firmware(image: FirmwareImage, *, board_id: str) -> None: + """Require the MAVLink APJ board identity after reboot to match the selected image.""" + if not board_id: + msg = _("reconnected flight controller did not report an APJ board_id") + raise FirmwareIdentityError(msg) + if board_id != str(image.metadata.board_id): + msg = _("reconnected board_id {actual_id} does not match firmware board_id {expected_id}").format( + actual_id=board_id, expected_id=image.metadata.board_id + ) + raise FirmwareIdentityError(msg) diff --git a/pyproject.toml b/pyproject.toml index f9c375e41..45e7387a3 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -76,6 +76,7 @@ dev = [ "pytest-cov==7.1.0", "pytest-md==0.2.0", "pytest-mock==3.15.1", + "pytest-timeout==2.4.0", "python-gettext==5.0", "ruff==0.16.3", "types-requests==2.33.0.20260712", diff --git a/pytest.ini b/pytest.ini index f5ff274a7..10279c498 100644 --- a/pytest.ini +++ b/pytest.ini @@ -14,6 +14,7 @@ python_files = tests/plugins/unit_*.py pythonpath = ardupilot_methodic_configurator tests .github/skills/update-gui-translations/scripts addopts = -v --strict-config --continue-on-collection-errors +timeout = 30 filterwarnings = ignore::DeprecationWarning:matplotlib.*: markers = diff --git a/tests/test_architecture_firmware_upload.py b/tests/test_architecture_firmware_upload.py new file mode 100644 index 000000000..a9c2d055b --- /dev/null +++ b/tests/test_architecture_firmware_upload.py @@ -0,0 +1,56 @@ +""" +Documentation contracts for the firmware upload architecture. + +This file is part of ArduPilot Methodic Configurator. https://github.com/ArduPilot/MethodicConfigurator + +SPDX-FileCopyrightText: 2026 Amilcar do Carmo Lucas + +SPDX-License-Identifier: GPL-3.0-or-later +""" + +# ruff: noqa: INP001 + +from pathlib import Path + +from ardupilot_methodic_configurator import backend_flightcontroller_bootloader as bootloader + + +def _documents_firmware_upload_contract(document: str) -> bool: + """Return whether the architecture identifies the live upload backend contract.""" + required_details = ( + "APJ input", + "`backend_flightcontroller_bootloader.py`", + "`FlightControllerBootloaderBackend`", + "`BootloaderClient`", + "`data_model_firmware_upload.py`", + "`FlightController.upload_apj_firmware()`", + f"{bootloader.BOOTLOADER_ENUMERATION_TIMEOUT:g}-second wall-clock budget", + ) + return all(detail in document for detail in required_details) and "APJ/BIN" not in document + + +def test_firmware_upload_architecture_documents_apj_only_input() -> None: + """ + The firmware-upload architecture documents the format accepted by the model. + + GIVEN: The firmware-upload architecture document describes file selection + WHEN: The documented input format is inspected + THEN: It identifies APJ input and does not advertise unsupported BIN input + """ + architecture = Path(__file__).parents[1] / "ARCHITECTURE_firmware_upload.md" + document = architecture.read_text(encoding="utf-8") + + assert _documents_firmware_upload_contract(document) + + +def test_firmware_upload_architecture_contract_rejects_stale_backend_details() -> None: + """The contract must reject renamed backend details and an incorrect timeout.""" + architecture = Path(__file__).parents[1] / "ARCHITECTURE_firmware_upload.md" + document = architecture.read_text(encoding="utf-8") + stale_document = ( + document.replace("`backend_flightcontroller_bootloader.py`", "`obsolete_bootloader_backend.py`") + .replace("`FlightControllerBootloaderBackend`", "`ObsoleteBootloaderBackend`") + .replace("15-second wall-clock budget", "999-second wall-clock budget") + ) + + assert not _documents_firmware_upload_contract(stale_document) diff --git a/tests/test_backend_flightcontroller.py b/tests/test_backend_flightcontroller.py index 1415fc188..12f079aaa 100755 --- a/tests/test_backend_flightcontroller.py +++ b/tests/test_backend_flightcontroller.py @@ -15,6 +15,7 @@ import tempfile from argparse import ArgumentParser +from collections.abc import Callable from typing import Any, cast # pylint: disable=unused-import from unittest.mock import MagicMock, patch @@ -42,6 +43,7 @@ def _build_flight_controller_with_mocks( reboot_time: int = 2, + sleep: Callable[[float], None] | None = None, ) -> tuple[FlightController, MagicMock, MagicMock, MagicMock, MagicMock, MagicMock]: """Helper returning a facade wired with MagicMock managers for delegation tests.""" mock_master = MagicMock() @@ -101,6 +103,7 @@ def _build_flight_controller_with_mocks( params_manager=mock_params_mgr, commands_manager=mock_commands_mgr, files_manager=mock_files_mgr, + sleep=sleep, ) return fc, mock_conn_mgr, mock_params_mgr, mock_commands_mgr, mock_files_mgr, mock_master @@ -701,17 +704,17 @@ def test_user_can_reset_and_reconnect_after_configuration(self) -> None: THEN: The autopilot should reboot and reconnect using retries if needed AND: Progress callbacks should reflect each wait step """ - fc, mock_conn_mgr, *_others, mock_master = _build_flight_controller_with_mocks(reboot_time=2) + sleeps: list[float] = [] + fc, mock_conn_mgr, *_others, mock_master = _build_flight_controller_with_mocks(reboot_time=2, sleep=sleeps.append) mock_conn_mgr.create_connection_with_retry.return_value = "RECONNECTED" progress_updates: list[tuple[int, int]] = [] connection_progress = MagicMock() - with patch("ardupilot_methodic_configurator.backend_flightcontroller.time_sleep", return_value=None): - result = fc.reset_and_reconnect( - reset_progress_callback=lambda current, total: progress_updates.append((current, total)), - connection_progress_callback=connection_progress, - extra_sleep_time=1, - ) + result = fc.reset_and_reconnect( + reset_progress_callback=lambda current, total: progress_updates.append((current, total)), + connection_progress_callback=connection_progress, + extra_sleep_time=1, + ) mock_master.reboot_autopilot.assert_called_once() mock_conn_mgr.disconnect.assert_called_once() @@ -724,6 +727,7 @@ def test_user_can_reset_and_reconnect_after_configuration(self) -> None: ) assert progress_updates[0] == (0, 3) assert progress_updates[-1] == (3, 3) + assert sleeps == [0.3, 1, 1, 1] assert result == "RECONNECTED" def test_reset_and_reconnect_returns_immediately_when_disconnected(self) -> None: diff --git a/tests/test_backend_flightcontroller_bootloader.py b/tests/test_backend_flightcontroller_bootloader.py new file mode 100755 index 000000000..425e9e2be --- /dev/null +++ b/tests/test_backend_flightcontroller_bootloader.py @@ -0,0 +1,2376 @@ +#!/usr/bin/env python3 + +""" +Tests for the injected bootloader transport adapter. + +This file is part of ArduPilot Methodic Configurator. https://github.com/ArduPilot/MethodicConfigurator + +SPDX-FileCopyrightText: 2024-2026 Amilcar do Carmo Lucas + +SPDX-License-Identifier: GPL-3.0-or-later +""" + +import base64 +import hashlib +import json +import struct +import sys +import zlib +from collections.abc import Callable +from pathlib import Path + +import pytest +import serial +from serial.tools.list_ports_common import ListPortInfo + +from ardupilot_methodic_configurator import backend_flightcontroller_bootloader as bl +from ardupilot_methodic_configurator import data_model_firmware_upload as fw +from ardupilot_methodic_configurator.backend_flightcontroller import FlightController +from ardupilot_methodic_configurator.data_model_flightcontroller_info import FlightControllerInfo + +# pylint: disable=too-many-instance-attributes,too-few-public-methods,protected-access,too-many-lines + + +def apj(image: bytes, *, extf_image: bytes = b"", **overrides: object) -> bytes: + desc: dict[str, object] = { + "board_id": 9, + "image_size": len(image), + "image": base64.b64encode(zlib.compress(image)).decode(), + } + if extf_image: + desc.update( + extf_image_size=len(extf_image), + extf_image=base64.b64encode(zlib.compress(extf_image)).decode(), + ) + desc.update(overrides) + return json.dumps(desc).encode() + + +def trusted_digest(image: bytes, extf_image: bytes = b"") -> str: + """Return the independently supplied release digest expected by the facade.""" + return hashlib.sha256(apj(image, extf_image=extf_image)).hexdigest() + + +@pytest.fixture(autouse=True) +def stable_test_device_identity(monkeypatch: pytest.MonkeyPatch) -> None: + """Keep facade tests independent of the host's actual USB inventory.""" + monkeypatch.setattr( + "ardupilot_methodic_configurator.backend_flightcontroller.capture_serial_device_identity", + lambda _device: None, + ) + monkeypatch.setattr( + "ardupilot_methodic_configurator.backend_flightcontroller.has_stable_bootloader_device_identity", + lambda *_args: True, + ) + + +class FakeBootloaderTransport: + """Byte-oriented fake with short reads, rev-2 support, and external flash.""" + + def __init__(self, *, revision: int = 5, board_id: int = 9, flash_size: int = 2048, extf_size: int = 1024) -> None: + self.revision = revision + self.board_id = board_id + self.flash_size = flash_size + self.extf_size = extf_size + self.flash = b"" + self.extf = b"" + self.extf_erase_size: int | None = None + self.closed = False + self.rebooted = False + self.commands: list[bytes] = [] + self.requests: list[bytes] = [] + self._reply = bytearray() + self._read_offset = 0 + + def write(self, data: bytes) -> int: + assert data.endswith(bl.EOC) + command, body = data[:1], data[1:-1] + self.commands.append(command) + self.requests.append(data) + sync = bl.INSYNC + bl.OK + if command == bl.GET_SYNC: + self._reply.extend(sync) + elif command == bl.GET_DEVICE: + values = { + bl.INFO_BL_REV: self.revision, + bl.INFO_BOARD_ID: self.board_id, + bl.INFO_BOARD_REV: 0, + bl.INFO_FLASH_SIZE: self.flash_size, + bl.INFO_EXTF_SIZE: self.extf_size, + } + self._reply.extend(struct.pack("= 3: + self._reply.extend(sync) + return len(data) + + def read(self, size: int = 1) -> bytes: + # Deliberately emulate short serial reads. + count = min(size, 2, len(self._reply)) + data = bytes(self._reply[:count]) + del self._reply[:count] + return data + + def flush(self) -> None: + pass + + def reset_input_buffer(self) -> None: + self._reply.clear() + + def close(self) -> None: + self.closed = True + + +@pytest.mark.parametrize("revision", [2, 5]) +def test_upload_verifies_revision_specific_protocol_and_closes(revision: int) -> None: + image = fw.parse_apj(apj(b"abc")) + transport = FakeBootloaderTransport(revision=revision) + + info = bl.BootloaderClient(transport).upload(image, confirmation_requested=lambda *_args: True) + + assert info.protocol_revision == revision + assert transport.flash == b"abc\xff" + assert transport.closed + if revision == 2: + assert bl.CHIP_VERIFY in transport.commands + assert bl.GET_CRC not in transport.commands + else: + assert bl.GET_CRC in transport.commands + assert bl.CHIP_VERIFY not in transport.commands + + +def test_protocol_byte_constants_match_the_bootloader_wire_specification() -> None: + """Keep the fake transport from masking a protocol-byte renumbering.""" + assert { + "INSYNC": bl.INSYNC, + "EOC": bl.EOC, + "OK": bl.OK, + "FAILED": bl.FAILED, + "INVALID": bl.INVALID, + "BAD_SILICON_REV": bl.BAD_SILICON_REV, + "GET_SYNC": bl.GET_SYNC, + "GET_DEVICE": bl.GET_DEVICE, + "CHIP_ERASE": bl.CHIP_ERASE, + "CHIP_VERIFY": bl.CHIP_VERIFY, + "PROG_MULTI": bl.PROG_MULTI, + "READ_MULTI": bl.READ_MULTI, + "GET_CRC": bl.GET_CRC, + "REBOOT": bl.REBOOT, + "EXTF_ERASE": bl.EXTF_ERASE, + "EXTF_PROG_MULTI": bl.EXTF_PROG_MULTI, + "EXTF_GET_CRC": bl.EXTF_GET_CRC, + "CHIP_FULL_ERASE": bl.CHIP_FULL_ERASE, + "INFO_BL_REV": bl.INFO_BL_REV, + "INFO_BOARD_ID": bl.INFO_BOARD_ID, + "INFO_BOARD_REV": bl.INFO_BOARD_REV, + "INFO_FLASH_SIZE": bl.INFO_FLASH_SIZE, + "INFO_EXTF_SIZE": bl.INFO_EXTF_SIZE, + } == { + "INSYNC": b"\x12", + "EOC": b"\x20", + "OK": b"\x10", + "FAILED": b"\x11", + "INVALID": b"\x13", + "BAD_SILICON_REV": b"\x14", + "GET_SYNC": b"\x21", + "GET_DEVICE": b"\x22", + "CHIP_ERASE": b"\x23", + "CHIP_VERIFY": b"\x24", + "PROG_MULTI": b"\x27", + "READ_MULTI": b"\x28", + "GET_CRC": b"\x29", + "REBOOT": b"\x30", + "EXTF_ERASE": b"\x34", + "EXTF_PROG_MULTI": b"\x35", + "EXTF_GET_CRC": b"\x37", + "CHIP_FULL_ERASE": b"\x40", + "INFO_BL_REV": b"\x01", + "INFO_BOARD_ID": b"\x02", + "INFO_BOARD_REV": b"\x03", + "INFO_FLASH_SIZE": b"\x04", + "INFO_EXTF_SIZE": b"\x06", + } + + +def test_encode_chip_erase_preserves_the_requested_erase_mode() -> None: + assert bl.encode_chip_erase() == b"\x23\x20" + assert bl.encode_chip_erase(full=True) == b"\x40\x20" + + +@pytest.mark.parametrize( + ("encode", "expected"), + [ + (bl.encode_get_sync, b"\x21\x20"), + (lambda: bl.encode_get_device(bl.INFO_BL_REV), b"\x22\x01\x20"), + (bl.encode_chip_verify, b"\x24\x20"), + (lambda: bl.encode_prog_multi(b"\x01\x02\x03\x04"), b"\x27\x04\x01\x02\x03\x04\x20"), + (lambda: bl.encode_extf_prog_multi(b"\x01\x02\x03\x04"), b"\x35\x04\x01\x02\x03\x04\x20"), + (lambda: bl.encode_read_multi(4), b"\x28\x04\x20"), + (lambda: bl.encode_extf_erase(0x01020304), b"\x34\x04\x03\x02\x01\x20"), + (lambda: bl.encode_extf_get_crc(0x01020304), b"\x37\x04\x03\x02\x01\x20"), + (bl.encode_get_crc, b"\x29\x20"), + (bl.encode_reboot, b"\x30\x20"), + ], +) +def test_encoder_emits_the_bootloader_wire_format(encode: Callable[[], bytes], expected: bytes) -> None: + """ + Each command uses the byte layout expected by AP_Bootloader. + + GIVEN: A command encoder and a representative request + WHEN: The request is encoded + THEN: Its bytes match the independent bootloader wire specification + """ + assert encode() == expected + + +def test_v2_verification_uses_the_read_chunk_limit(monkeypatch: pytest.MonkeyPatch) -> None: + """ + Legacy read-back verification uses the bootloader's read limit independently. + + GIVEN: Program and read commands have different protocol chunk limits + WHEN: A revision-two image is verified after programming + THEN: Read-back requests stay within the read-command limit + """ + monkeypatch.setattr(bl, "PROG_MULTI_MAX", 252) + monkeypatch.setattr(bl, "READ_MULTI_MAX", 64) + image = fw.parse_apj(apj(bytes(range(100)), board_id=9)) + + transport = FakeBootloaderTransport(revision=2) + info = bl.BootloaderClient(transport).upload(image, confirmation_requested=lambda *_args: True) + + assert info.protocol_revision == 2 + read_requests = [request for request in transport.requests if request[:1] == bl.READ_MULTI] + assert [request[1] for request in read_requests] == [64, 36] + assert all(request[2:] == bl.EOC for request in read_requests) + + +def test_identify_resets_stale_bytes_before_old_bootloader_fallback() -> None: + """ + Old bootloader fallback synchronization starts with a clean input buffer. + + GIVEN: The optional external-flash query leaves a stale response on the serial input + WHEN: The client falls back after the query fails + THEN: The stale bytes are cleared before fallback synchronization and identification succeeds + """ + + class OldBootloaderTransport(FakeBootloaderTransport): + """Leave stale bytes after rejecting the external-flash query.""" + + def __init__(self) -> None: + super().__init__(revision=2) + self.reset_count = 0 + + def write(self, data: bytes) -> int: + if data[:1] == bl.GET_DEVICE and data[1:-1] == bl.INFO_EXTF_SIZE: + self._reply.extend(bl.INVALID + b"old") + return len(data) + return super().write(data) + + def read(self, size: int = 1) -> bytes: + if not self._reply: + msg = "old bootloader rejected external-flash query" + raise OSError(msg) + return super().read(size) + + def reset_input_buffer(self) -> None: + self.reset_count += 1 + super().reset_input_buffer() + + transport = OldBootloaderTransport() + + info = bl.BootloaderClient(transport).identify() + + assert info == bl.BootloaderInfo(protocol_revision=2, board_id=9, board_revision=0, flash_size=2048, extf_size=0) + assert transport.reset_count == 2 + + +def test_discovery_recovers_from_old_bootloader_rejecting_extf_query_on_a_used_deadline() -> None: + """ + Old-bootloader external-flash rejection does not poison the discovery deadline. + + GIVEN: A pre-2021 bootloader rejects the optional external-flash query with a + short INSYNC/INVALID reply, spinning the shared identification deadline to + exhaustion before the query read times out + WHEN: Firmware is uploaded through the backend discovery path with a bounded clock + THEN: The fallback resync and the remaining identity queries run on a fresh + deadline, so identification succeeds and the board reports no external flash + """ + + class OldBootloaderDiscoveryTransport(FakeBootloaderTransport): + """Reject the external-flash query with a short reply, then behave normally.""" + + def __init__(self) -> None: + super().__init__(revision=5, extf_size=0) + self.extf_query_rejected = False + + def write(self, data: bytes) -> int: + command, body = data[:1], data[1:-1] + if command == bl.GET_DEVICE and body == bl.INFO_EXTF_SIZE: + # Two bytes where four payload bytes are expected: _read_exact(4) + # then spins on empty reads until the deadline, mimicking a board + # whose bootloader predates PROTO_DEVICE_EXTF_SIZE. + self.commands.append(command) + self.requests.append(data) + self._reply.extend(bl.INSYNC + bl.INVALID) + self.extf_query_rejected = True + return len(data) + return super().write(data) + + transport = OldBootloaderDiscoveryTransport() + now = [0.0] + backend = bl.FlightControllerBootloaderBackend( + "COM7", + 115200, + timeout=0.1, + serial_factory=lambda *_args: transport, + clock=lambda: now[0], + sleep=lambda dt: now.__setitem__(0, round(now[0] + dt, 6)), + ) + + info = backend.upload(fw.parse_apj(apj(b"abcd", board_id=9)), confirmation_requested=lambda *_args: True) + + assert transport.extf_query_rejected + assert info.protocol_revision == 5 + assert info.extf_size == 0 + + +def test_uploads_and_verifies_external_flash() -> None: + """ + External-flash erase accepts progress bytes that overlap protocol markers. + + GIVEN: The bootloader reports a monotonic erase-progress ramp containing 18% + WHEN: An APJ with an external image is uploaded + THEN: The client consumes progress and waits for the final INSYNC/OK response + """ + image = fw.parse_apj(apj(b"abcd", extf_image=b"ext")) + transport = FakeBootloaderTransport() + + bl.BootloaderClient(transport).upload(image, confirmation_requested=lambda *_args: True) + + assert transport.extf == b"ext\xff" + assert transport.extf_erase_size == image.metadata.extf_image_size + assert transport.closed + assert transport.requests == [ + bl.encode_get_sync(), + bl.encode_get_device(bl.INFO_BL_REV), + bl.encode_get_device(bl.INFO_EXTF_SIZE), + bl.encode_get_device(bl.INFO_BOARD_ID), + bl.encode_get_device(bl.INFO_BOARD_REV), + bl.encode_get_device(bl.INFO_FLASH_SIZE), + bl.encode_extf_erase(image.metadata.extf_image_size), + bl.encode_extf_prog_multi(image.extf_image[0 : bl.PROG_MULTI_MAX]), + bl.encode_extf_get_crc(image.metadata.extf_image_size), + bl.encode_chip_erase(), + bl.encode_prog_multi(image.image), + bl.encode_get_crc(), + bl.encode_reboot(), + ] + for request in transport.requests: + if request[:1] in {bl.PROG_MULTI, bl.EXTF_PROG_MULTI}: + assert request[1] == len(request[2:-1]) + + +def test_upload_splits_internal_and_external_payloads_at_the_protocol_boundary() -> None: + """ + Both flash regions use complete, word-aligned protocol chunks. + + GIVEN: Each APJ payload is one byte larger than the maximum bootloader chunk + WHEN: The client uploads and verifies both regions + THEN: Each region is sent as a maximum chunk followed by a padded four-byte chunk + """ + internal = bytes(range(253)) + external = bytes(reversed(range(253))) + image = fw.parse_apj(apj(internal, extf_image=external)) + transport = FakeBootloaderTransport() + + bl.BootloaderClient(transport).upload(image, confirmation_requested=lambda *_args: True) + + internal_requests = [request for request in transport.requests if request[:1] == bl.PROG_MULTI] + external_requests = [request for request in transport.requests if request[:1] == bl.EXTF_PROG_MULTI] + assert [request[1] for request in internal_requests] == [252, 4] + assert [request[1] for request in external_requests] == [252, 4] + assert [request[2:-1] for request in internal_requests] == [internal[:252], internal[252:] + b"\xff\xff\xff"] + assert [request[2:-1] for request in external_requests] == [external[:252], external[252:] + b"\xff\xff\xff"] + assert transport.flash == internal + b"\xff\xff\xff" + assert transport.extf == external + b"\xff\xff\xff" + + +def test_external_crc_mismatch_rejects_the_upload() -> None: + """A bad external-flash CRC must not be accepted after programming.""" + + class BadExternalCrcTransport(FakeBootloaderTransport): + """Return an incorrect CRC after external-flash programming.""" + + def write(self, data: bytes) -> int: + if data[:1] == bl.EXTF_GET_CRC: + self._reply.extend(struct.pack(" None: + class RegressingEraseTransport(FakeBootloaderTransport): + """Reports external erase progress that moves backwards.""" + + def write(self, data: bytes) -> int: + if data[:1] == bl.EXTF_ERASE: + self._reply.extend(bl.INSYNC + bl.OK + bytes((10, 20, 19))) + return len(data) + return super().write(data) + + client = bl.BootloaderClient(RegressingEraseTransport()) + + with pytest.raises(fw.BootloaderProtocolError, match="regressed"): + client._erase_external(3) + + +def test_external_erase_rejects_progress_above_one_hundred() -> None: + """An impossible erase percentage is a protocol error, not completion.""" + + class InvalidProgressTransport(FakeBootloaderTransport): + """Reports an external erase percentage outside the protocol range.""" + + def write(self, data: bytes) -> int: + if data[:1] == bl.EXTF_ERASE: + self._reply.extend(bl.INSYNC + bl.OK + bytes((101,))) + return len(data) + return super().write(data) + + with pytest.raises(fw.BootloaderProtocolError, match="invalid external-flash erase progress"): + bl.BootloaderClient(InvalidProgressTransport())._erase_external(3) + + +def test_external_erase_does_not_accept_a_premature_final_status() -> None: + """The final INSYNC marker is valid only after erase progress reaches the completion threshold.""" + + class PrematureCompletionTransport(FakeBootloaderTransport): + """Places a final status marker before the erase is nearly complete.""" + + def write(self, data: bytes) -> int: + if data[:1] == bl.EXTF_ERASE: + self._reply.extend(bl.INSYNC + bl.OK + bytes((10,)) + bl.INSYNC + bl.OK) + return len(data) + return super().write(data) + + with pytest.raises(fw.BootloaderProtocolError, match="regressed"): + bl.BootloaderClient(PrematureCompletionTransport())._erase_external(3) + + +def test_external_erase_reports_a_failed_final_status() -> None: + """A final external erase failure is not mistaken for successful completion.""" + + class FailedEraseTransport(FakeBootloaderTransport): + """Reports a failed final erase acknowledgement after reaching 100%.""" + + def write(self, data: bytes) -> int: + if data[:1] == bl.EXTF_ERASE: + self._reply.extend(bl.INSYNC + bl.OK + bytes((100,)) + bl.INSYNC + bl.FAILED) + return len(data) + return super().write(data) + + with pytest.raises(fw.BootloaderProtocolError, match="OPERATION FAILED"): + bl.BootloaderClient(FailedEraseTransport())._erase_external(3) + + +def test_stalled_external_erase_times_out_without_rebooting_after_erase() -> None: + class StalledEraseTransport(FakeBootloaderTransport): + """Stops reporting progress after starting an external erase.""" + + def write(self, data: bytes) -> int: + if data[:1] == bl.EXTF_ERASE: + self._reply.extend(bl.INSYNC + bl.OK + b"\x00") + return len(data) + return super().write(data) + + now = 0.0 + + def clock() -> float: + nonlocal now + now += 0.25 + return now + + image = fw.parse_apj(apj(b"abcd", extf_image=b"ext")) + transport = StalledEraseTransport() + + with pytest.raises(fw.BootloaderProtocolError, match="timeout waiting"): + bl.BootloaderClient(transport, clock=clock).upload(image, confirmation_requested=lambda *_args: True) + + assert not transport.rebooted + + +@pytest.mark.timeout(5) +def test_external_erase_times_out_when_progress_never_advances() -> None: + """Repeated progress bytes must not keep a stalled erase alive indefinitely.""" + + class RepeatingProgressTransport(FakeBootloaderTransport): + """Reports 1% forever after acknowledging the erase command.""" + + def __init__(self) -> None: + super().__init__() + self.erasing = False + + def write(self, data: bytes) -> int: + if data[:1] == bl.EXTF_ERASE: + self.erasing = True + self._reply.extend(bl.INSYNC + bl.OK) + return len(data) + return super().write(data) + + def read(self, size: int = 1) -> bytes: + reply = super().read(size) + return reply or (b"\x01" if self.erasing else b"") + + now = 0.0 + + def clock() -> float: + nonlocal now + now += 0.25 + return now + + with pytest.raises(fw.BootloaderProtocolError, match="timeout waiting"): + bl.BootloaderClient(RepeatingProgressTransport(), clock=clock)._erase_external(3) + + +def test_external_erase_refreshes_deadline_only_when_progress_advances() -> None: + """A progressing erase may exceed 20 seconds, but each advance refreshes inactivity timeout.""" + + class SlowProgressTransport: + """Deliver one erase-progress byte every two simulated seconds.""" + + def __init__(self) -> None: + self.now = 0.0 + self.reply = bytearray() + + def write(self, data: bytes) -> int: + assert data[:1] == bl.EXTF_ERASE + self.reply.extend(bl.INSYNC + bl.OK + bytes(range(0, 101, 10)) + bl.INSYNC + bl.OK) + return len(data) + + def flush(self) -> None: + pass + + def read(self, size: int = 1) -> bytes: + self.now += 2.0 + count = min(1, size, len(self.reply)) + data = bytes(self.reply[:count]) + del self.reply[:count] + return data + + transport = SlowProgressTransport() + bl.BootloaderClient(transport, timeout=10.0, clock=lambda: transport.now)._erase_external(3) + + assert transport.now > bl.ERASE_TIMEOUT + + +def test_external_only_upload_does_not_touch_internal_flash() -> None: + """External-only firmware skips internal erase, programming, and verification.""" + image = fw.parse_apj(apj(b"", extf_image=b"external")) + transport = FakeBootloaderTransport() + + bl.BootloaderClient(transport).upload(image, confirmation_requested=lambda *_args: True) + + assert transport.flash == b"" + assert transport.extf == b"external" + assert transport.closed + assert not any( + command in {bl.CHIP_ERASE, bl.CHIP_FULL_ERASE, bl.PROG_MULTI, bl.CHIP_VERIFY, bl.READ_MULTI, bl.GET_CRC} + for command in transport.commands + ) + + +def test_upload_reports_each_external_and_internal_region_in_order() -> None: + """ + Progress distinguishes the two flash regions and their verification phases. + + GIVEN: An APJ contains both external and internal firmware regions + WHEN: The bootloader uploads and verifies the APJ + THEN: Progress reports each region in protocol order without merging stages + """ + events: list[tuple[bl.UploadStage, int, int]] = [] + transport = FakeBootloaderTransport() + + bl.BootloaderClient(transport).upload( + fw.parse_apj(apj(b"abcd", extf_image=b"ext")), + confirmation_requested=lambda *_args: True, + progress_callback=lambda stage, done, total: events.append((stage, done, total)), + ) + + assert events == [ + (fw.UploadStage.IDENTIFYING, 0, 1), + (fw.UploadStage.IDENTIFYING, 1, 1), + (fw.UploadStage.AWAITING_CONFIRMATION, 0, 1), + (fw.UploadStage.AWAITING_CONFIRMATION, 1, 1), + (fw.UploadStage.ERASING, 0, 1), + (fw.UploadStage.ERASING, 1, 1), + (fw.UploadStage.PROGRAMMING, 1, 1), + (fw.UploadStage.VERIFYING, 0, 1), + (fw.UploadStage.VERIFYING, 1, 1), + (fw.UploadStage.ERASING, 0, 1), + (fw.UploadStage.ERASING, 1, 1), + (fw.UploadStage.PROGRAMMING, 1, 1), + (fw.UploadStage.VERIFYING, 0, 1), + (fw.UploadStage.VERIFYING, 1, 1), + (fw.UploadStage.REBOOTING, 0, 1), + (fw.UploadStage.REBOOTING, 1, 1), + ] + + +def test_upload_survives_progress_callback_failures() -> None: + """A presentation callback failure cannot interrupt a verified flash.""" + transport = FakeBootloaderTransport() + + def broken_progress(*_args: object) -> None: + msg = "progress UI failed" + raise RuntimeError(msg) + + bl.BootloaderClient(transport).upload( + fw.parse_apj(apj(b"abcd")), + confirmation_requested=lambda *_args: True, + progress_callback=broken_progress, + ) + + assert transport.flash == b"abcd" + assert transport.rebooted + assert transport.closed + + +def test_failed_upload_still_closes_transport() -> None: + image = fw.parse_apj(apj(b"abcd")) + transport = FakeBootloaderTransport(flash_size=0) + + with pytest.raises(fw.FirmwareCompatibilityError, match="exceeds flash"): + bl.BootloaderClient(transport).upload(image, confirmation_requested=lambda *_args: True) + assert transport.closed + + +def test_read_timeout_is_bounded_even_when_serial_keeps_returning_empty_reads() -> None: + class EmptyReadTransport(FakeBootloaderTransport): + """Transport that simulates a serial port timing out without a response.""" + + reads = 0 + + def read(self, size: int = 1) -> bytes: + self.reads += 1 + return b"" + + transport = EmptyReadTransport() + clock_values = iter([0.0, 0.5, 1.0]) + client = bl.BootloaderClient(transport, timeout=1.0, clock=lambda: next(clock_values)) + + with pytest.raises(fw.BootloaderProtocolError, match="timeout waiting"): + client._read_exact(1) # Exercise the transport deadline directly. + + assert transport.reads == 1 + + +def test_client_uses_injected_sleep_for_empty_nonblocking_reads() -> None: + """The read-loop yield is deterministic and does not need a module monkeypatch.""" + + class EmptyReadTransport(FakeBootloaderTransport): + """Return empty reads to exercise the injected yield function.""" + + def read(self, size: int = 1) -> bytes: + return b"" + + sleeps: list[float] = [] + clock_values = iter([0.0, 0.0, 0.5, 1.0]) + client = bl.BootloaderClient(EmptyReadTransport(), timeout=1.0, clock=lambda: next(clock_values), sleep=sleeps.append) + + with pytest.raises(fw.BootloaderProtocolError, match="timeout waiting"): + client._read_exact(1) + + assert sleeps == [0.01] + + +def test_backend_passes_its_injected_sleep_to_the_bootloader_client() -> None: + """ + Bootloader discovery keeps the client's nonblocking-read yield injectable. + + GIVEN: Opening the bootloader succeeds but its first read is empty + WHEN: The backend uploads firmware with an injected sleep function + THEN: The bootloader client uses that function rather than module-global sleep + """ + + class EmptyOnceTransport(FakeBootloaderTransport): + """Return one empty read before providing the bootloader response.""" + + empty_reads = 1 + + def read(self, size: int = 1) -> bytes: + if self.empty_reads: + self.empty_reads -= 1 + return b"" + return super().read(size) + + sleeps: list[float] = [] + backend = bl.FlightControllerBootloaderBackend( + "COM7", + 115200, + serial_factory=lambda *_args: EmptyOnceTransport(), + sleep=sleeps.append, + ) + + backend.upload(fw.parse_apj(apj(b"abcd")), confirmation_requested=lambda *_args: True) + + assert sleeps == [0.01] + + +@pytest.mark.parametrize( + ("operation", "error_type", "message"), + [ + ("write", OSError, "transport write failed"), + ("read", serial.SerialException, "transport read failed"), + ("read", OSError, "transport read failed"), + ("flush", serial.SerialException, "transport write failed"), + ("reset", OSError, "cannot clear bootloader serial input"), + ("reset", serial.SerialException, "cannot clear bootloader serial input"), + ], +) +def test_transport_errors_are_converted_to_typed_protocol_errors( + operation: str, error_type: type[Exception], message: str +) -> None: + """Serial-layer failures must not escape as untyped transport exceptions.""" + + class FailingTransport: + """Raise the selected serial failure from one transport operation.""" + + def write(self, _data: bytes) -> int: + if operation == "write": + msg = "transport failed" + raise error_type(msg) + return 0 + + def read(self, _size: int = 1) -> bytes: + if operation == "read": + msg = "transport failed" + raise error_type(msg) + return b"" + + def flush(self) -> None: + if operation == "flush": + msg = "transport failed" + raise error_type(msg) + + def reset_input_buffer(self) -> None: + if operation == "reset": + msg = "transport failed" + raise error_type(msg) + + def close(self) -> None: + pass + + client = bl.BootloaderClient(FailingTransport(), sleep=lambda _delay: None) + action = { + "write": lambda: client._write(bl.encode_get_sync()), + "read": lambda: client._read_exact(1), + "flush": lambda: client._write(bl.encode_get_sync()), + "reset": client._reset_input_buffer, + }[operation] + + with pytest.raises(fw.BootloaderProtocolError, match=message): + action() + + +@pytest.mark.parametrize( + ("operation", "reply_size"), + [ + ("get_sync", 0), + ("get_device", 4), + ("chip_erase", 0), + ("full_erase", 0), + ("chip_verify", 0), + ("prog_multi", 0), + ("extf_prog_multi", 0), + ("extf_erase", 0), + ("read_multi", 4), + ("get_crc", 4), + ("extf_get_crc", 4), + ("reboot", 0), + ], +) +def test_each_bootloader_command_rejects_a_failed_status(operation: str, reply_size: int) -> None: + """Every command path propagates a failed bootloader status as a typed error.""" + + class FailedStatusTransport: + """Return a failed status for any command while preserving expected reply lengths.""" + + def __init__(self) -> None: + self.reply = bytearray() + + def write(self, data: bytes) -> int: + command = data[:1] + if command in {bl.GET_DEVICE, bl.READ_MULTI, bl.GET_CRC, bl.EXTF_GET_CRC}: + self.reply.extend(b"\x00" * 4) + self.reply.extend(bl.INSYNC + bl.FAILED) + return len(data) + + def read(self, size: int = 1) -> bytes: + data = bytes(self.reply[:size]) + del self.reply[:size] + return data + + def flush(self) -> None: + pass + + def reset_input_buffer(self) -> None: + self.reply.clear() + + def close(self) -> None: + pass + + requests = { + "get_sync": bl.encode_get_sync(), + "get_device": bl.encode_get_device(bl.INFO_BL_REV), + "chip_erase": bl.encode_chip_erase(), + "full_erase": bl.encode_chip_erase(full=True), + "chip_verify": bl.encode_chip_verify(), + "prog_multi": bl.encode_prog_multi(b"\x00\x00\x00\x00"), + "extf_prog_multi": bl.encode_extf_prog_multi(b"\x00\x00\x00\x00"), + "extf_erase": bl.encode_extf_erase(4), + "read_multi": bl.encode_read_multi(4), + "get_crc": bl.encode_get_crc(), + "extf_get_crc": bl.encode_extf_get_crc(4), + "reboot": bl.encode_reboot(), + } + client = bl.BootloaderClient(FailedStatusTransport()) + + with pytest.raises(fw.BootloaderProtocolError, match="OPERATION FAILED"): + client._command(requests[operation], reply_size) + + +def test_partial_transport_write_is_rejected() -> None: + class PartialWriteTransport: + """Accept only part of each request.""" + + def write(self, data: bytes) -> int: + return len(data) - 1 + + def flush(self) -> None: + pass + + client = bl.BootloaderClient(PartialWriteTransport()) + + with pytest.raises(fw.BootloaderProtocolError, match="wrote 1 of 2 bytes"): + client._write(bl.encode_get_sync()) + + +def test_client_requires_confirmation_and_reboots_before_closing() -> None: + transport = FakeBootloaderTransport() + + with pytest.raises(fw.FirmwareConfirmationError, match="explicit confirmation"): + bl.BootloaderClient(transport).upload(fw.parse_apj(apj(b"abcd"))) + + assert transport.rebooted + assert transport.closed + + +def test_declined_confirmation_reboots_before_closing() -> None: + transport = FakeBootloaderTransport() + + with pytest.raises(fw.FirmwareUploadCancelledError, match="not confirmed"): + bl.BootloaderClient(transport).upload(fw.parse_apj(apj(b"abcd")), confirmation_requested=lambda *_args: False) + + assert transport.rebooted + assert transport.closed + + +def test_parse_apj_is_pure_and_requires_declared_size_to_match() -> None: + contents = apj(b"four", image_size=1) + + with pytest.raises(fw.FirmwareFileError, match="image_size"): + fw.parse_apj(contents, path=Path("firmware.apj")) + + +def test_parse_apj_wraps_invalid_unicode_text_as_a_file_error() -> None: + """The public text parser must not leak UnicodeEncodeError.""" + with pytest.raises(fw.FirmwareFileError, match="cannot parse APJ descriptor"): + fw.parse_apj("\ud800") + + +def test_parse_apj_wraps_excessively_nested_json_as_a_file_error() -> None: + """Deep but size-bounded APJ JSON must not leak RecursionError.""" + contents = b'{"x":' * 50_000 + b"0" + b"}" * 50_000 + assert len(contents) < bl.MAX_APJ_DESCRIPTOR_SIZE + + with pytest.raises(fw.FirmwareFileError): + fw.parse_apj(contents) + + +def test_invalid_board_revision_is_a_typed_file_error() -> None: + with pytest.raises(fw.FirmwareFileError, match="metadata"): + fw.parse_apj(apj(b"four", board_revision="not-a-number")) + + +@pytest.mark.skipif(not hasattr(sys, "get_int_max_str_digits"), reason="CPython integer digit limit was added in Python 3.11") +def test_parse_apj_wraps_json_integer_digit_limit_as_file_error() -> None: + """A JSON number beyond CPython's digit limit must not escape untyped.""" + digit_limit = sys.get_int_max_str_digits() + if digit_limit == 0: + pytest.skip("CPython integer digit limit is disabled") + + contents = b'{"board_id":' + (b"9" * (digit_limit + 1)) + b"}" + + with pytest.raises(fw.FirmwareFileError, match="cannot parse APJ descriptor"): + fw.parse_apj(contents) + + +def test_padded_payload_capacity_and_compatible_board_mapping_are_checked() -> None: + image = fw.parse_apj(apj(b"abc")) + with pytest.raises(fw.FirmwareCompatibilityError, match="exceeds flash"): + fw.check_compatibility(image, fw.BootloaderInfo(5, 9, 0, 0)) + + fw.check_compatibility(image, fw.BootloaderInfo(5, 33, 0, 4)) + + +def test_bounded_decompression_rejects_before_full_output_is_allocated(monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setattr(fw, "MAX_IMAGE_SIZE", 32) + + limits: list[int] = [] + + class BoundedDecompressor: + """Expose the decompressor output limit supplied by the parser.""" + + unconsumed_tail = b"still compressed" + unused_data = b"" + eof = True + + def decompress(self, _compressed: bytes, max_length: int) -> bytes: + limits.append(max_length) + return b"x" * max_length + + def flush(self) -> bytes: + return b"" + + monkeypatch.setattr(fw.zlib, "decompressobj", BoundedDecompressor) + + with pytest.raises(fw.FirmwareFileError, match="exceeds"): + fw.parse_apj(apj(b"x" * 64)) + assert limits == [33] + + +def test_parser_rejects_trailing_compressed_data_and_non_text_version() -> None: + encoded = base64.b64encode(zlib.compress(b"abcd") + b"unexpected").decode() + trailing_data = json.dumps({"board_id": 9, "image_size": 4, "image": encoded}) + + with pytest.raises(fw.FirmwareFileError, match="trailing bytes"): + fw.parse_apj(trailing_data) + with pytest.raises(fw.FirmwareFileError, match="version metadata"): + fw.parse_apj(apj(b"abcd", version=46)) + with pytest.raises(fw.FirmwareFileError, match="git_identity metadata"): + fw.parse_apj(apj(b"abcd", git_identity=42)) + + +def test_parser_rejects_oversized_encoded_blob_before_base64_decode(monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setattr(fw, "MAX_ENCODED_BLOB_SIZE", 8) + + with pytest.raises(fw.FirmwareFileError, match="encoded"): + fw.parse_apj(apj(b"abcd")) + + +def test_reader_rejects_oversized_apj_descriptor_before_reading(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: + path = tmp_path / "firmware.apj" + path.write_bytes(apj(b"abcd")) + monkeypatch.setattr(bl, "MAX_APJ_DESCRIPTOR_SIZE", 8) + + with pytest.raises(fw.FirmwareFileError, match="descriptor exceeds"): + bl.load_apj(path) + + +def test_load_apj_rejects_non_apj_suffix(tmp_path: Path) -> None: + path = tmp_path / "firmware.bin" + path.write_bytes(b"whatever") + + with pytest.raises(fw.FirmwareFileError, match="unsupported firmware format"): + bl.load_apj(path) + + +def test_encode_extf_erase_rejects_out_of_range_size() -> None: + with pytest.raises(ValueError, match="external flash size"): + bl.encode_extf_erase(0) + with pytest.raises(ValueError, match="external flash size"): + bl.encode_extf_erase(0x1_0000_0000) + + +def test_bootloader_client_rejects_non_positive_timeout() -> None: + with pytest.raises(ValueError, match="timeout must be positive"): + bl.BootloaderClient(FakeBootloaderTransport(), timeout=0) + + +def test_identify_rejects_unsupported_protocol_revision() -> None: + transport = FakeBootloaderTransport(revision=1) + with pytest.raises(fw.BootloaderProtocolError, match="unsupported bootloader protocol revision"): + bl.BootloaderClient(transport).identify() + + +def test_bootloader_port_is_re_resolved_by_usb_identity_and_ambiguity_is_refused( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """A re-enumerated bootloader must not reuse an arbitrary stale serial port.""" + reenumerated = ListPortInfo("COM11") + reenumerated.location = "1-2.3" + reenumerated.serial_number = "FC-123" + monkeypatch.setattr(bl.serial.tools.list_ports, "comports", lambda: [reenumerated]) + + identity = bl.SerialDeviceIdentity(location="1-2.3", serial_number="FC-123") + assert bl.resolve_bootloader_device("COM7", identity) == "COM11" + + duplicate = ListPortInfo("COM12") + duplicate.location = "1-2.3" + duplicate.serial_number = "FC-123" + monkeypatch.setattr(bl.serial.tools.list_ports, "comports", lambda: [reenumerated, duplicate]) + with pytest.raises(OSError, match="cannot uniquely"): + bl.resolve_bootloader_device("COM7", identity) + + +def test_usb_identity_is_used_for_reenumerated_linux_devices(monkeypatch: pytest.MonkeyPatch) -> None: + """A matching USB identity must win over the old Linux device path.""" + reenumerated = ListPortInfo("/dev/ttyACM1") + reenumerated.location = "1-2.3" + reenumerated.serial_number = "FC-123" + monkeypatch.setattr(bl.serial.tools.list_ports, "comports", lambda: [reenumerated]) + + identity = bl.SerialDeviceIdentity(location="1-2.3", serial_number="FC-123") + + assert bl.resolve_bootloader_device("/dev/ttyACM0", identity) == "/dev/ttyACM1" + + +def test_bootloader_port_prefers_usb_identity_over_reused_linux_device_path(monkeypatch: pytest.MonkeyPatch) -> None: + """A newly assigned tty path must not override the captured controller identity.""" + reenumerated = ListPortInfo("/dev/ttyACM1") + reenumerated.location = "1-2.3" + reenumerated.serial_number = "FC-123" + monkeypatch.setattr(bl.serial.tools.list_ports, "comports", lambda: [reenumerated]) + + assert ( + bl.resolve_bootloader_device("/dev/ttyACM0", bl.SerialDeviceIdentity(location="1-2.3", serial_number="FC-123")) + == "/dev/ttyACM1" + ) + + +def test_bootloader_upload_requires_usb_metadata(monkeypatch: pytest.MonkeyPatch) -> None: + """The upload gate rejects a connection with no stable USB metadata.""" + monkeypatch.setattr(bl.serial.tools.list_ports, "comports", list) + + identity = bl.capture_serial_device_identity("/dev/ttyACM0") + + assert identity is None + assert not bl.has_stable_bootloader_device_identity("/dev/ttyACM0", identity) + + +def test_bootloader_port_refuses_a_different_usb_serial(monkeypatch: pytest.MonkeyPatch) -> None: + """A different USB serial number cannot satisfy the captured identity.""" + replacement = ListPortInfo("/dev/ttyACM1") + replacement.location = "1-2.3" + replacement.serial_number = "OTHER" + monkeypatch.setattr(bl.serial.tools.list_ports, "comports", lambda: [replacement]) + identity = bl.SerialDeviceIdentity( + location="1-2.3", + serial_number="ORIGINAL", + ) + + with pytest.raises(OSError, match="cannot uniquely"): + bl.resolve_bootloader_device("/dev/ttyACM0", identity) + + +def test_client_reports_recovery_error_when_pre_erase_reboot_fails() -> None: + class RebootFailureTransport(FakeBootloaderTransport): + """Fail reboot writes to exercise the recovery error path.""" + + def write(self, data: bytes) -> int: + if data[:1] == bl.REBOOT: + msg = "reboot write failed" + raise OSError(msg) + return super().write(data) + + transport = RebootFailureTransport(board_id=9) + image = fw.parse_apj(apj(b"abcd", board_id=42)) + + with pytest.raises(fw.FirmwareBootloaderRecoveryError, match="power-cycle"): + bl.BootloaderClient(transport).upload(image, confirmation_requested=lambda *_args: True) + + assert transport.closed + + +def test_abort_before_erase_requires_a_successful_reboot_ack() -> None: + class FailedRebootTransport(FakeBootloaderTransport): + """Reject the best-effort reboot command instead of merely accepting its write.""" + + def write(self, data: bytes) -> int: + if data[:1] == bl.REBOOT: + self._reply.extend(bl.INSYNC + bl.FAILED) + return len(data) + return super().write(data) + + assert not bl.BootloaderClient(FailedRebootTransport()).abort_before_erase() + + +def test_client_accepts_no_ack_reboot_when_pre_erase_abort_uses_protocol_v2() -> None: + class V2NoAckRebootTransport(FakeBootloaderTransport): + """Protocol revision two reboots immediately and does not acknowledge it.""" + + def write(self, data: bytes) -> int: + if data[:1] == bl.REBOOT: + self.rebooted = True + self.commands.append(bl.REBOOT) + self.requests.append(data) + return len(data) + return super().write(data) + + transport = V2NoAckRebootTransport(revision=2) + + with pytest.raises(fw.FirmwareUploadCancelledError, match="not confirmed"): + bl.BootloaderClient(transport).upload(fw.parse_apj(apj(b"abcd")), confirmation_requested=lambda *_args: False) + + assert transport.rebooted + assert transport.closed + + +def test_close_failure_does_not_mask_upload_failure() -> None: + class CloseFailureTransport(FakeBootloaderTransport): + """Raise while closing so cleanup cannot hide the protocol failure.""" + + def close(self) -> None: + msg = "close failed" + raise RuntimeError(msg) + + def write(self, data: bytes) -> int: + if data[:1] == bl.PROG_MULTI: + msg = "programming write failed" + raise OSError(msg) + return super().write(data) + + with pytest.raises(fw.BootloaderProtocolError, match="programming write failed"): + bl.BootloaderClient(CloseFailureTransport()).upload( + fw.parse_apj(apj(b"abcd")), confirmation_requested=lambda *_args: True + ) + + +def test_full_erase_is_refused_before_any_flash_command() -> None: + """The client refuses full erase until it can determine the board capability safely.""" + transport = FakeBootloaderTransport() + + with pytest.raises(fw.FirmwareCompatibilityError, match="full firmware erase"): + bl.BootloaderClient(transport).upload( + fw.parse_apj(apj(b"abcd")), full_erase=True, confirmation_requested=lambda *_args: True + ) + + assert bl.CHIP_FULL_ERASE not in transport.commands + assert transport.flash == b"" + + +def test_backend_refuses_full_erase_before_rebooting_into_the_bootloader() -> None: + """ + Full erase is rejected before the board is rebooted into the bootloader. + + GIVEN: A caller requests full_erase, which cannot be supported safely yet + WHEN: The upload runs through the backend + THEN: It refuses before _enter_bootloader fires, so the flight controller is + never rebooted and its MAVLink link is never dropped for nothing + """ + entered = False + + def enter() -> None: + nonlocal entered + entered = True + + def factory(*_args: object) -> bl.BootloaderTransport: + msg = "serial transport must not be opened when full erase is refused" + raise AssertionError(msg) + + backend = bl.FlightControllerBootloaderBackend("COM7", 115200, enter_bootloader=enter, serial_factory=factory) + + with pytest.raises(fw.FirmwareCompatibilityError, match="full firmware erase"): + backend.upload(fw.parse_apj(apj(b"abcd")), full_erase=True, confirmation_requested=lambda *_args: True) + + assert not entered + + +def test_backend_rejects_invalid_timeout_before_entering_bootloader() -> None: + entered = False + + def enter() -> None: + nonlocal entered + entered = True + + with pytest.raises(ValueError, match="timeout"): + bl.FlightControllerBootloaderBackend("COM7", 115200, timeout=0, enter_bootloader=enter) + + assert not entered + + +class _Master: + def __init__(self) -> None: + self.held_in_bootloader = False + + def reboot_autopilot(self, *, hold_in_bootloader: bool) -> None: + self.held_in_bootloader = hold_in_bootloader + + +class _Connection: + def __init__(self, device: str = "COM7") -> None: + self.master = _Master() + self.comport = object() + self.comport_device = device + self.baudrate = 115200 + self.active_baudrate = 921600 + self.info = FlightControllerInfo() + self.info.apj_board_id = "9" + self.disconnected = False + self.reconnected = False + self.reconnect_baudrate: int | None = None + self.reconnect_device: str | None = None + + def discover_connections(self, **_kwargs: object) -> None: + pass + + def disconnect(self) -> None: + self.disconnected = True + self.master = None # type: ignore[assignment] + + def create_connection_with_retry(self, *_args: object, **kwargs: object) -> str: + self.reconnected = True + self.reconnect_baudrate = kwargs.get("baudrate") if isinstance(kwargs.get("baudrate"), int) else None + return "" + + def connect(self, device: str, **kwargs: object) -> str: + self.reconnect_device = device + self.comport_device = device + self.master = _Master() + return self.create_connection_with_retry(**kwargs) + + +class _Params: + def __init__(self) -> None: + self.cleared = 0 + + def clear_parameters(self) -> None: + self.cleared += 1 + + +class _Commands: + COMMAND_ACK_TIMEOUT = 5.0 + + def __init__(self, result: tuple[bool, str] = (True, "")) -> None: + self.result = result + self.calls: list[dict[str, object]] = [] + + def send_command_and_wait_ack(self, **kwargs: object) -> tuple[bool, str]: + self.calls.append(kwargs) + return self.result + + def reboot_to_bootloader(self) -> tuple[bool, str]: + return self.send_command_and_wait_ack(param1=3) + + +def test_facade_reconnects_using_the_reenumerated_serial_device(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: + """ + Reconnection follows the controller's stable identity after re-enumeration. + + GIVEN: The controller was connected on COM7 and is captured with a stable USB identity + WHEN: Flashing makes the controller reappear on COM13 + THEN: The facade reconnects through COM13 and updates the active connection + """ + connection = _Connection() + params = _Params() + commands = _Commands() + transport = FakeBootloaderTransport() + controller = FlightController( + connection_manager=connection, # type: ignore[arg-type] + params_manager=params, # type: ignore[arg-type] + commands_manager=commands, # type: ignore[arg-type] + files_manager=object(), # type: ignore[arg-type] + ) + path = tmp_path / "firmware.apj" + path.write_bytes(apj(b"abcd")) + monkeypatch.setattr( + "ardupilot_methodic_configurator.backend_flightcontroller.resolve_bootloader_device", + lambda _device, _identity: "COM13", + ) + + controller.upload_apj_firmware( + path, + expected_firmware_sha256=trusted_digest(b"abcd"), + serial_factory=lambda *_args: transport, + confirmation_requested=lambda *_args: True, + ) + + assert connection.disconnected + assert connection.reconnected + assert connection.reconnect_device == "COM13" + assert connection.reconnect_baudrate == 921600 + assert commands.calls[0]["param1"] == 3 + assert transport.closed + assert params.cleared == 2 + + +def test_facade_routes_bootloader_and_entry_delays_to_injected_sleep(tmp_path: Path) -> None: + sleeps: list[float] = [] + + class EmptyOnceTransport(FakeBootloaderTransport): + """Return one empty read before providing the bootloader response.""" + + empty_reads = 1 + + def read(self, size: int = 1) -> bytes: + if self.empty_reads: + self.empty_reads -= 1 + return b"" + return super().read(size) + + connection = _Connection() + controller = FlightController( + connection_manager=connection, # type: ignore[arg-type] + params_manager=_Params(), # type: ignore[arg-type] + commands_manager=_Commands(), # type: ignore[arg-type] + files_manager=object(), # type: ignore[arg-type] + sleep=sleeps.append, + ) + path = tmp_path / "firmware.apj" + path.write_bytes(apj(b"abcd")) + + controller.upload_apj_firmware( + path, + expected_firmware_sha256=trusted_digest(b"abcd"), + serial_factory=lambda *_args: EmptyOnceTransport(), + confirmation_requested=lambda *_args: True, + ) + + assert sleeps == [0.3, 0.01] + + +def test_facade_verifies_board_identity_after_reconnect(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: + """ + A successful serial reconnect still requires the expected APJ board identity. + + GIVEN: The bootloader flashes successfully but the reconnected MAVLink board is different + WHEN: The facade completes the reconnect step + THEN: It rejects the result with a firmware identity error + """ + + class WrongBoardAfterReconnect(_Connection): + """Change the reported board identity when the new MAVLink connection opens.""" + + def create_connection_with_retry(self, *_args: object, **kwargs: object) -> str: + result = super().create_connection_with_retry(*_args, **kwargs) + self.info.apj_board_id = "42" + return result + + connection = WrongBoardAfterReconnect() + controller = FlightController( + connection_manager=connection, # type: ignore[arg-type] + params_manager=_Params(), # type: ignore[arg-type] + commands_manager=_Commands(), # type: ignore[arg-type] + files_manager=object(), # type: ignore[arg-type] + ) + path = tmp_path / "firmware.apj" + path.write_bytes(apj(b"abcd")) + monkeypatch.setattr( + "ardupilot_methodic_configurator.backend_flightcontroller.resolve_bootloader_device", + lambda _device, _identity: "COM13", + ) + + with pytest.raises(fw.FirmwareIdentityError, match="does not match firmware"): + controller.upload_apj_firmware( + path, + expected_firmware_sha256=trusted_digest(b"abcd"), + serial_factory=lambda *_args: FakeBootloaderTransport(), + confirmation_requested=lambda *_args: True, + ) + + assert connection.reconnected + assert connection.reconnect_device == "COM13" + + +def test_facade_waits_for_serial_identity_to_reappear_before_reconnecting( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """ + Reconnection waits while the board is temporarily absent during re-enumeration. + + GIVEN: The stable USB identity is unavailable during the first resolver calls + WHEN: The firmware upload completes and starts reconnection + THEN: The facade retries identity resolution and connects after the board reappears + """ + connection = _Connection() + commands = _Commands() + controller = FlightController( + connection_manager=connection, # type: ignore[arg-type] + params_manager=_Params(), # type: ignore[arg-type] + commands_manager=commands, # type: ignore[arg-type] + files_manager=object(), # type: ignore[arg-type] + ) + path = tmp_path / "firmware.apj" + path.write_bytes(apj(b"abcd")) + resolver_calls = 0 + + def resolve(_device: str, _identity: object) -> str: + nonlocal resolver_calls + resolver_calls += 1 + if resolver_calls < 3: + msg = "device is still re-enumerating" + raise OSError(msg) + return "COM13" + + monkeypatch.setattr( + "ardupilot_methodic_configurator.backend_flightcontroller.resolve_bootloader_device", + resolve, + ) + + controller.upload_apj_firmware( + path, + expected_firmware_sha256=trusted_digest(b"abcd"), + serial_factory=lambda *_args: FakeBootloaderTransport(), + confirmation_requested=lambda *_args: True, + ) + + assert resolver_calls == 3 + assert connection.reconnect_device == "COM13" + + +def test_facade_retries_reconnect_when_connect_returns_an_error(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: + """ + Reconnection keeps trying while connect() reports a transient failure. + + GIVEN: resolve_bootloader_device succeeds at once but the board is not open yet + WHEN: connect() returns an error string on the first attempts then an empty string + THEN: The facade spends the retry budget and reports success once connect() succeeds + """ + connect_results = iter(["port not ready", "port not ready", ""]) + + class _FlakyConnection(_Connection): + def connect(self, device: str, **kwargs: object) -> str: + self.reconnect_device = device + self.comport_device = device + self.master = _Master() + error = next(connect_results) + if not error: + self.reconnected = True + return error + + connection = _FlakyConnection() + controller = FlightController( + connection_manager=connection, # type: ignore[arg-type] + params_manager=_Params(), # type: ignore[arg-type] + commands_manager=_Commands(), # type: ignore[arg-type] + files_manager=object(), # type: ignore[arg-type] + ) + path = tmp_path / "firmware.apj" + path.write_bytes(apj(b"abcd")) + monkeypatch.setattr( + "ardupilot_methodic_configurator.backend_flightcontroller.resolve_bootloader_device", + lambda _device, _identity: "COM13", + ) + + controller.upload_apj_firmware( + path, + expected_firmware_sha256=trusted_digest(b"abcd"), + serial_factory=lambda *_args: FakeBootloaderTransport(), + confirmation_requested=lambda *_args: True, + ) + + assert connection.reconnected + assert connection.reconnect_device == "COM13" + + +def test_facade_reports_reconnect_retry_exhaustion_after_a_verified_flash( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """ + Reconnect failures consume the bounded retry window and preserve the recovery message. + + GIVEN: The bootloader flash succeeds but every MAVLink reconnect attempt fails + WHEN: The facade exhausts its reconnect deadline + THEN: It reports that the verified controller must be reconnected manually + """ + now = 0.0 + connect_calls = 0 + + def clock() -> float: + return now + + def sleep(delay: float) -> None: + nonlocal now + now += delay + + class NeverReadyConnection(_Connection): + """Keep reporting a transient connection error until the retry deadline.""" + + def connect(self, device: str, **kwargs: object) -> str: + nonlocal connect_calls + connect_calls += 1 + self.reconnect_device = device + self.comport_device = device + self.master = _Master() + return "port not ready" + + monkeypatch.setattr("ardupilot_methodic_configurator.backend_flightcontroller.FIRMWARE_RECONNECT_RESOLVE_TIMEOUT", 0.25) + monkeypatch.setattr( + "ardupilot_methodic_configurator.backend_flightcontroller.resolve_bootloader_device", + lambda _device, _identity: "COM13", + ) + connection = NeverReadyConnection() + controller = FlightController( + connection_manager=connection, # type: ignore[arg-type] + params_manager=_Params(), # type: ignore[arg-type] + commands_manager=_Commands(), # type: ignore[arg-type] + files_manager=object(), # type: ignore[arg-type] + sleep=sleep, + clock=clock, + ) + path = tmp_path / "firmware.apj" + path.write_bytes(apj(b"abcd")) + + with pytest.raises(fw.FirmwareReconnectError, match=r"written and verified.*Reconnect it manually"): + controller.upload_apj_firmware( + path, + expected_firmware_sha256=trusted_digest(b"abcd"), + serial_factory=lambda *_args: FakeBootloaderTransport(), + confirmation_requested=lambda *_args: True, + ) + + assert connect_calls == 3 + assert now == pytest.approx(0.6) + + +def test_facade_reports_rejected_bootloader_entry_without_releasing_serial(tmp_path: Path) -> None: + """ + A rejected bootloader command stops the upload before disconnecting MAVLink. + + GIVEN: The connected vehicle rejects the reboot-and-hold command + WHEN: A firmware upload requests bootloader entry + THEN: The user receives the rejection reason and the active connection remains available + """ + connection = _Connection() + commands = _Commands((False, "Command denied")) + controller = FlightController( + connection_manager=connection, # type: ignore[arg-type] + params_manager=_Params(), # type: ignore[arg-type] + commands_manager=commands, # type: ignore[arg-type] + files_manager=object(), # type: ignore[arg-type] + ) + path = tmp_path / "firmware.apj" + path.write_bytes(apj(b"abcd")) + + with pytest.raises(fw.FirmwareConnectionError, match="Command denied"): + controller.upload_apj_firmware( + path, + expected_firmware_sha256=trusted_digest(b"abcd"), + confirmation_requested=lambda *_args: True, + ) + + assert not connection.disconnected + assert connection.master is not None + + +def test_facade_requires_trusted_sha_before_flashing(tmp_path: Path) -> None: + connection = _Connection() + controller = FlightController( + connection_manager=connection, # type: ignore[arg-type] + params_manager=_Params(), # type: ignore[arg-type] + commands_manager=_Commands(), # type: ignore[arg-type] + files_manager=object(), # type: ignore[arg-type] + ) + path = tmp_path / "firmware.apj" + path.write_bytes(apj(b"abcd")) + + with pytest.raises(fw.FirmwareConfirmationError, match="trusted SHA-256"): + controller.upload_apj_firmware( + path, + expected_firmware_sha256=None, + serial_factory=lambda *_args: FakeBootloaderTransport(), + confirmation_requested=lambda *_args: True, + ) + assert not connection.disconnected + + +def test_facade_validates_the_trusted_digest_before_bootloader_entry(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: + """The facade must not allow a selected APJ to bypass its release digest.""" + calls: list[str] = [] + connection = _Connection() + controller = FlightController( + connection_manager=connection, # type: ignore[arg-type] + params_manager=_Params(), # type: ignore[arg-type] + commands_manager=_Commands(), # type: ignore[arg-type] + files_manager=object(), # type: ignore[arg-type] + ) + path = tmp_path / "firmware.apj" + path.write_bytes(apj(b"abcd")) + monkeypatch.setattr( + "ardupilot_methodic_configurator.backend_flightcontroller.verify_expected_firmware_digest", + lambda _image, digest: calls.append(digest), + ) + + controller.upload_apj_firmware( + path, + expected_firmware_sha256="0" * 64, + serial_factory=lambda *_args: FakeBootloaderTransport(), + confirmation_requested=lambda *_args: True, + ) + + assert calls == ["0" * 64] + + +def test_facade_reports_a_verified_flash_when_automatic_reconnect_fails( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """A post-flash port ambiguity must tell the user to reconnect manually.""" + connection = _Connection() + controller = FlightController( + connection_manager=connection, # type: ignore[arg-type] + params_manager=_Params(), # type: ignore[arg-type] + commands_manager=_Commands(), # type: ignore[arg-type] + files_manager=object(), # type: ignore[arg-type] + ) + path = tmp_path / "firmware.apj" + path.write_bytes(apj(b"abcd")) + monkeypatch.setattr("ardupilot_methodic_configurator.backend_flightcontroller.FIRMWARE_RECONNECT_RESOLVE_TIMEOUT", 0) + + with pytest.raises(fw.FirmwareReconnectError, match=r"written and verified.*Reconnect it manually"): + controller.upload_apj_firmware( + path, + expected_firmware_sha256=trusted_digest(b"abcd"), + serial_factory=lambda *_args: FakeBootloaderTransport(), + confirmation_requested=lambda *_args: True, + ) + + +def test_facade_refuses_network_mavlink_connection() -> None: + connection = _Connection("udp:127.0.0.1:14550") + controller = FlightController( + connection_manager=connection, # type: ignore[arg-type] + params_manager=_Params(), # type: ignore[arg-type] + commands_manager=object(), # type: ignore[arg-type] + files_manager=object(), # type: ignore[arg-type] + ) + + with pytest.raises(fw.FirmwareConnectionError, match="direct serial"): + controller.upload_apj_firmware(Path("ignored.apj")) + + +def test_facade_requires_pre_reboot_board_identity(tmp_path: Path) -> None: + connection = _Connection() + connection.info.apj_board_id = "" + controller = FlightController( + connection_manager=connection, # type: ignore[arg-type] + params_manager=_Params(), # type: ignore[arg-type] + commands_manager=object(), # type: ignore[arg-type] + files_manager=object(), # type: ignore[arg-type] + ) + path = tmp_path / "firmware.apj" + path.write_bytes(apj(b"abcd")) + + with pytest.raises(fw.FirmwareConnectionError, match="APJ board_id"): + controller.upload_apj_firmware( + path, + expected_firmware_sha256=trusted_digest(b"abcd"), + confirmation_requested=lambda *_args: True, + ) + + assert not connection.master.held_in_bootloader + + +def test_facade_requires_stable_usb_identity_before_bootloader_entry(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: + """An unidentifiable serial connection must not be rebooted into the bootloader.""" + connection = _Connection() + controller = FlightController( + connection_manager=connection, # type: ignore[arg-type] + params_manager=_Params(), # type: ignore[arg-type] + commands_manager=_Commands(), # type: ignore[arg-type] + files_manager=object(), # type: ignore[arg-type] + ) + path = tmp_path / "firmware.apj" + path.write_bytes(apj(b"abcd")) + monkeypatch.setattr( + "ardupilot_methodic_configurator.backend_flightcontroller.has_stable_bootloader_device_identity", + bl.has_stable_bootloader_device_identity, + ) + + with pytest.raises(fw.FirmwareConnectionError, match="stable USB"): + controller.upload_apj_firmware( + path, + expected_firmware_sha256=trusted_digest(b"abcd"), + confirmation_requested=lambda *_args: True, + ) + + assert not connection.master.held_in_bootloader + + +def test_backend_retries_serial_open_after_bootloader_entry() -> None: + image = fw.parse_apj(apj(b"abcd")) + attempts = 0 + delays: list[float] = [] + entered = False + transport = FakeBootloaderTransport() + + def enter() -> None: + nonlocal entered + entered = True + + def open_transport(*_args: object) -> bl.BootloaderTransport: + nonlocal attempts + attempts += 1 + if attempts == 1: + msg = "bootloader port has not appeared yet" + raise OSError(msg) + return transport + + backend = bl.FlightControllerBootloaderBackend( + "COM7", + 115200, + enter_bootloader=enter, + serial_factory=open_transport, + open_retries=2, + retry_delay=0.25, + sleep=delays.append, + ) + + backend.upload(image, confirmation_requested=lambda *_args: True) + + assert entered + assert attempts == 2 + assert delays == [0.25] + + +def test_backend_requires_manual_recovery_after_all_serial_open_retries_fail() -> None: + """ + Exhausting the bootloader port-open budget is reported as a recovery action. + + GIVEN: The controller entered bootloader mode but its serial port never opens + WHEN: Every configured serial-open attempt fails + THEN: The backend reports all attempts and asks the user to power-cycle the controller + """ + attempts = 0 + delays: list[float] = [] + + def enter() -> None: + pass + + def open_transport(*_args: object) -> bl.BootloaderTransport: + nonlocal attempts + attempts += 1 + msg = "bootloader port is still absent" + raise OSError(msg) + + backend = bl.FlightControllerBootloaderBackend( + "COM7", + 115200, + enter_bootloader=enter, + serial_factory=open_transport, + open_retries=3, + retry_delay=0.25, + sleep=delays.append, + ) + + with pytest.raises(fw.FirmwareBootloaderRecoveryError, match="power-cycle") as error: + backend.upload(fw.parse_apj(apj(b"abcd")), confirmation_requested=lambda *_args: True) + + assert attempts == 3 + assert delays == [0.25, 0.25] + assert "bootloader port is still absent" in str(error.value) + + +def test_backend_clamps_final_retry_sleep_to_remaining_budget() -> None: + delays: list[float] = [] + now = 0.0 + + def clock() -> float: + return now + + def sleep(delay: float) -> None: + nonlocal now + delays.append(delay) + now += delay + + def open_transport(*_args: object) -> bl.BootloaderTransport: + msg = "bootloader port is still absent" + raise OSError(msg) + + backend = bl.FlightControllerBootloaderBackend( + "COM7", + 115200, + serial_factory=open_transport, + open_retries=10, + retry_delay=10.0, + clock=clock, + sleep=sleep, + ) + + with pytest.raises(fw.FirmwareBootloaderRecoveryError, match="power-cycle"): + backend.upload(fw.parse_apj(apj(b"abcd")), confirmation_requested=lambda *_args: True) + + assert delays == [10.0, 5.0] + assert now == bl.BOOTLOADER_ENUMERATION_TIMEOUT + + +def test_backend_honours_cancellation_during_bootloader_discovery() -> None: + attempts = 0 + + def enter() -> None: + pass + + def open_transport(*_args: object) -> bl.BootloaderTransport: + nonlocal attempts + attempts += 1 + msg = "bootloader port is still absent" + raise OSError(msg) + + backend = bl.FlightControllerBootloaderBackend( + "COM7", + 115200, + enter_bootloader=enter, + serial_factory=open_transport, + open_retries=3, + retry_delay=0.25, + sleep=lambda _delay: None, + ) + + checks = iter([False, False, True]) + with pytest.raises(fw.FirmwareBootloaderRecoveryError, match=r"cancelled.*power-cycle"): + backend.upload( + fw.parse_apj(apj(b"abcd")), + cancellation_requested=lambda: next(checks), + confirmation_requested=lambda *_args: True, + ) + + assert attempts == 1 + + +def test_backend_default_open_retry_budget_covers_bootloader_enumeration() -> None: + backend = bl.FlightControllerBootloaderBackend("COM7", 115200) + + assert backend._open_retries == bl.BOOTLOADER_OPEN_RETRIES # pylint: disable=protected-access + assert bl.BOOTLOADER_ENUMERATION_TIMEOUT >= 10.0 + assert backend._open_retries * bl.BOOTLOADER_RETRY_DELAY >= bl.BOOTLOADER_ENUMERATION_TIMEOUT # pylint: disable=protected-access + + +def test_backend_bootloader_discovery_uses_a_wall_clock_budget() -> None: + now = 0.0 + attempts = 0 + timeouts: list[float] = [] + + class EmptyTransport(FakeBootloaderTransport): + """Never answer identification, as with a serial port in the wrong mode.""" + + def read(self, size: int = 1) -> bytes: + del size + return b"" + + def clock() -> float: + return now + + def sleep(delay: float) -> None: + nonlocal now + now += delay + + def open_transport(_device: str, _baudrate: int, timeout: float) -> bl.BootloaderTransport: + nonlocal attempts + attempts += 1 + timeouts.append(timeout) + return EmptyTransport() + + backend = bl.FlightControllerBootloaderBackend( + "COM7", + 115200, + serial_factory=open_transport, + open_retries=100, + timeout=2.0, + retry_delay=0.7, + clock=clock, + sleep=sleep, + ) + + with pytest.raises(fw.FirmwareBootloaderRecoveryError, match=r"could not confirm reboot.*power-cycle"): + backend.upload(fw.parse_apj(apj(b"abcd")), confirmation_requested=lambda *_args: True) + + # The final identification attempt starts with only 1.5 seconds remaining, + # and the best-effort reboot gets its own bounded recovery budget. + assert now == pytest.approx(bl.BOOTLOADER_ENUMERATION_TIMEOUT + bl.BOOTLOADER_ABORT_TIMEOUT) + assert attempts == 6 + assert timeouts[-1] == pytest.approx(1.5) + + +def test_backend_retries_after_a_silent_port_until_the_bootloader_appears() -> None: + """A silent serial port must not consume the entire discovery deadline.""" + now = 0.0 + attempts = 0 + + class SilentTransport(FakeBootloaderTransport): + """Accept opens but never answer bootloader identification.""" + + def read(self, size: int = 1) -> bytes: + del size + return b"" + + def clock() -> float: + return now + + def sleep(delay: float) -> None: + nonlocal now + now += delay + + def open_transport(_device: str, _baudrate: int, _timeout: float) -> bl.BootloaderTransport: + nonlocal attempts + attempts += 1 + return SilentTransport() if now < 3.0 else FakeBootloaderTransport() + + backend = bl.FlightControllerBootloaderBackend( + "COM7", + 115200, + serial_factory=open_transport, + open_retries=10, + timeout=2.0, + retry_delay=0.5, + clock=clock, + sleep=sleep, + ) + + info = backend.upload(fw.parse_apj(apj(b"abcd")), confirmation_requested=lambda *_args: True) + + assert info.board_id == 9 + assert attempts == 3 + assert now == pytest.approx(5.0) + + +def test_backend_keeps_command_timeout_when_final_open_attempt_is_clamped(monkeypatch: pytest.MonkeyPatch) -> None: + """A short final discovery attempt must not shorten later bootloader commands.""" + now = 0.0 + attempts = 0 + open_timeouts: list[float] = [] + client_timeouts: list[float] = [] + info = bl.BootloaderInfo(5, 9, 0, 2048, 1024) + + def clock() -> float: + return now + + def sleep(delay: float) -> None: + nonlocal now + now += delay + + def open_transport(_device: str, _baudrate: int, timeout: float) -> object: + nonlocal attempts + attempts += 1 + open_timeouts.append(timeout) + if attempts == 1: + msg = "bootloader port has not appeared yet" + raise OSError(msg) + return object() + + class StubBootloaderClient: + """Record discovery construction without requiring protocol traffic.""" + + def __init__(self, _transport: object, *, timeout: float, **_kwargs: object) -> None: + client_timeouts.append(timeout) + + def identify(self, *, deadline: float | None = None) -> bl.BootloaderInfo: + del deadline + return info + + def upload(self, *_args: object, **_kwargs: object) -> bl.BootloaderInfo: + return info + + monkeypatch.setattr(bl, "BootloaderClient", StubBootloaderClient) + backend = bl.FlightControllerBootloaderBackend( + "COM7", + 115200, + serial_factory=open_transport, + open_retries=2, + timeout=2.0, + retry_delay=14.0, + clock=clock, + sleep=sleep, + ) + + result = backend.upload(fw.parse_apj(apj(b"abcd")), confirmation_requested=lambda *_args: True) + + assert result is info + assert open_timeouts == [2.0, 1.0] + assert client_timeouts == [2.0] + + +def test_backend_requires_manual_recovery_when_the_held_bootloader_never_opens() -> None: + entered = False + + def enter() -> None: + nonlocal entered + entered = True + + def open_transport(*_args: object) -> bl.BootloaderTransport: + msg = "bootloader port has not appeared" + raise OSError(msg) + + backend = bl.FlightControllerBootloaderBackend( + "COM7", + 115200, + enter_bootloader=enter, + serial_factory=open_transport, + open_retries=1, + ) + + with pytest.raises(fw.FirmwareBootloaderRecoveryError, match="power-cycle"): + backend.upload(fw.parse_apj(apj(b"abcd")), confirmation_requested=lambda *_args: True) + + assert entered + + +def test_backend_retries_bootloader_identification_after_stale_serial_data() -> None: + """ + A failed identification retry reuses the held bootloader without re-entry. + + GIVEN: The first bootloader connection returns stale synchronization data + WHEN: The backend closes that connection and retries identification + THEN: It opens the next transport without requesting bootloader entry again + """ + + class StaleTransport(FakeBootloaderTransport): + """Replies with stale bytes to the initial synchronization request.""" + + def write(self, data: bytes) -> int: + if data[:1] == bl.GET_SYNC: + self._reply.extend(b"\x00\x10") + return len(data) + return super().write(data) + + stale = StaleTransport() + working = FakeBootloaderTransport() + transports = iter([stale, working]) + entries = 0 + + def enter() -> None: + nonlocal entries + entries += 1 + + backend = bl.FlightControllerBootloaderBackend( + "COM7", + 115200, + enter_bootloader=enter, + serial_factory=lambda *_args: next(transports), + open_retries=2, + retry_delay=0, + sleep=lambda _delay: None, + ) + + backend.upload(fw.parse_apj(apj(b"abcd")), confirmation_requested=lambda *_args: True) + + assert stale.closed + assert working.closed + assert entries == 1 + assert not stale.rebooted + + +def test_backend_restores_command_timeout_after_discovery() -> None: + class TimeoutAwareTransport(FakeBootloaderTransport): + """Expose pyserial-like timeout attributes for the restoration check.""" + + def __init__(self) -> None: + super().__init__() + self.timeout = 0.1 + self.write_timeout = 0.1 + + transport = TimeoutAwareTransport() + + backend = bl.FlightControllerBootloaderBackend( + "COM7", + 115200, + timeout=3.0, + serial_factory=lambda *_args: transport, + open_retries=1, + ) + + backend.upload(fw.parse_apj(apj(b"abcd")), confirmation_requested=lambda *_args: True) + + assert transport.timeout == 3.0 + assert transport.write_timeout == 3.0 + + +def test_upload_reports_protocol_stages_and_confirmation_boundary() -> None: + events: list[tuple[bl.UploadStage, int, int]] = [] + confirmed: list[fw.BootloaderInfo] = [] + transport = FakeBootloaderTransport() + + bl.BootloaderClient(transport).upload( + fw.parse_apj(apj(b"abcd")), + confirmation_requested=lambda _image, info: confirmed.append(info) is None, + progress_callback=lambda stage, done, total: events.append((stage, done, total)), + ) + + assert confirmed == [fw.BootloaderInfo(5, 9, 0, 2048, 1024)] + assert events == [ + (fw.UploadStage.IDENTIFYING, 0, 1), + (fw.UploadStage.IDENTIFYING, 1, 1), + (fw.UploadStage.AWAITING_CONFIRMATION, 0, 1), + (fw.UploadStage.AWAITING_CONFIRMATION, 1, 1), + (fw.UploadStage.ERASING, 0, 1), + (fw.UploadStage.ERASING, 1, 1), + (fw.UploadStage.PROGRAMMING, 1, 1), + (fw.UploadStage.VERIFYING, 0, 1), + (fw.UploadStage.VERIFYING, 1, 1), + (fw.UploadStage.REBOOTING, 0, 1), + (fw.UploadStage.REBOOTING, 1, 1), + ] + + +def test_verify_failure_reports_the_verifying_stage() -> None: + class BadCrcTransport(FakeBootloaderTransport): + """Reports a CRC which cannot match the flashed image.""" + + def write(self, data: bytes) -> int: + if data[:1] == bl.GET_CRC: + self._reply.extend(struct.pack(" None: + transport = FakeBootloaderTransport() + + with pytest.raises(fw.FirmwareUploadCancelledError, match="before entering"): + bl.BootloaderClient(transport).upload( + fw.parse_apj(apj(b"abcd")), + cancellation_requested=lambda: True, + confirmation_requested=lambda *_args: True, + ) + + assert transport.flash == b"" + assert transport.extf == b"" + assert transport.closed + + +def test_facade_requires_explicit_confirmation(tmp_path: Path) -> None: + controller = FlightController( + connection_manager=_Connection(), # type: ignore[arg-type] + params_manager=_Params(), # type: ignore[arg-type] + commands_manager=object(), # type: ignore[arg-type] + files_manager=object(), # type: ignore[arg-type] + ) + path = tmp_path / "firmware.apj" + path.write_bytes(apj(b"abcd")) + + with pytest.raises(fw.FirmwareConfirmationError, match="explicit confirmation"): + controller.upload_apj_firmware(path) + + +def test_facade_recovers_connection_after_declined_confirmation(tmp_path: Path) -> None: + connection = _Connection() + transport = FakeBootloaderTransport() + controller = FlightController( + connection_manager=connection, # type: ignore[arg-type] + params_manager=_Params(), # type: ignore[arg-type] + commands_manager=_Commands(), # type: ignore[arg-type] + files_manager=object(), # type: ignore[arg-type] + ) + path = tmp_path / "firmware.apj" + path.write_bytes(apj(b"abcd")) + + with pytest.raises(fw.FirmwareUploadCancelledError, match="not confirmed"): + controller.upload_apj_firmware( + path, + expected_firmware_sha256=trusted_digest(b"abcd"), + serial_factory=lambda *_args: transport, + confirmation_requested=lambda *_args: False, + ) + + assert transport.rebooted + assert connection.reconnected + + +def test_facade_refuses_a_different_bootloader_and_recovers(tmp_path: Path) -> None: + connection = _Connection() + connection.info.apj_board_id = "42" + transport = FakeBootloaderTransport(board_id=9) + controller = FlightController( + connection_manager=connection, # type: ignore[arg-type] + params_manager=_Params(), # type: ignore[arg-type] + commands_manager=_Commands(), # type: ignore[arg-type] + files_manager=object(), # type: ignore[arg-type] + ) + path = tmp_path / "firmware.apj" + path.write_bytes(apj(b"abcd")) + + with pytest.raises(fw.FirmwareTargetMismatchError, match="differs from connected"): + controller.upload_apj_firmware( + path, + expected_firmware_sha256=trusted_digest(b"abcd"), + serial_factory=lambda *_args: transport, + confirmation_requested=lambda *_args: True, + ) + + assert transport.rebooted + assert connection.reconnected + + +def test_facade_recovers_after_confirmation_callback_error(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: + """A GUI callback failure before erase must not strand the board in bootloader mode.""" + connection = _Connection() + transport = FakeBootloaderTransport() + controller = FlightController( + connection_manager=connection, # type: ignore[arg-type] + params_manager=_Params(), # type: ignore[arg-type] + commands_manager=_Commands(), # type: ignore[arg-type] + files_manager=object(), # type: ignore[arg-type] + ) + path = tmp_path / "firmware.apj" + path.write_bytes(apj(b"abcd")) + monkeypatch.setattr( + "ardupilot_methodic_configurator.backend_flightcontroller.capture_serial_device_identity", + lambda _device: None, + ) + monkeypatch.setattr( + "ardupilot_methodic_configurator.backend_flightcontroller.has_stable_bootloader_device_identity", + lambda *_args: True, + ) + + def raise_from_confirmation(*_args: object) -> bool: + msg = "confirmation UI failed" + raise RuntimeError(msg) + + with pytest.raises(RuntimeError, match="confirmation UI failed"): + controller.upload_apj_firmware( + path, + expected_firmware_sha256=trusted_digest(b"abcd"), + serial_factory=lambda *_args: transport, + confirmation_requested=raise_from_confirmation, + ) + + assert transport.rebooted + assert connection.reconnected + + +def test_facade_does_not_attempt_mavlink_reconnect_without_a_bootloader_reboot( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + connection = _Connection() + controller = FlightController( + connection_manager=connection, # type: ignore[arg-type] + params_manager=_Params(), # type: ignore[arg-type] + commands_manager=_Commands(), # type: ignore[arg-type] + files_manager=object(), # type: ignore[arg-type] + ) + path = tmp_path / "firmware.apj" + path.write_bytes(apj(b"abcd")) + + def unavailable_upload( + backend: bl.FlightControllerBootloaderBackend, *_args: object, **_kwargs: object + ) -> fw.BootloaderInfo: + assert backend._enter_bootloader is not None + backend._enter_bootloader() + msg = "power-cycle the flight controller before reconnecting" + raise fw.FirmwareBootloaderRecoveryError(msg) + + monkeypatch.setattr(bl.FlightControllerBootloaderBackend, "upload", unavailable_upload) + + with pytest.raises(fw.FirmwareBootloaderRecoveryError, match="power-cycle"): + controller.upload_apj_firmware( + path, + expected_firmware_sha256=trusted_digest(b"abcd"), + confirmation_requested=lambda *_args: True, + ) + + assert connection.disconnected + assert not connection.reconnected + + +def test_facade_does_not_reboot_or_reconnect_after_programming_failure( + tmp_path: Path, +) -> None: + """A programming failure must leave recovery to the bootloader safety check.""" + + class ProgrammingFailureTransport(FakeBootloaderTransport): + """Fail the first internal-flash programming command.""" + + def write(self, data: bytes) -> int: + if data[:1] == bl.PROG_MULTI: + msg = "programming write failed" + raise OSError(msg) + return super().write(data) + + connection = _Connection() + transport = ProgrammingFailureTransport() + controller = FlightController( + connection_manager=connection, # type: ignore[arg-type] + params_manager=_Params(), # type: ignore[arg-type] + commands_manager=_Commands(), # type: ignore[arg-type] + files_manager=object(), # type: ignore[arg-type] + ) + path = tmp_path / "firmware.apj" + path.write_bytes(apj(b"abcd")) + + with pytest.raises(fw.BootloaderProtocolError, match=r"programming write failed.*incomplete.*power-cycle"): + controller.upload_apj_firmware( + path, + expected_firmware_sha256=trusted_digest(b"abcd"), + serial_factory=lambda *_args: transport, + confirmation_requested=lambda *_args: True, + ) + + assert not transport.rebooted + assert not connection.reconnected + + +def test_client_preserves_recovery_guidance_for_empty_argument_exceptions() -> None: + class EmptyMessageFailureTransport(FakeBootloaderTransport): + """Raise an exception with no message during programming.""" + + def write(self, data: bytes) -> int: + if data[:1] == bl.PROG_MULTI: + raise RuntimeError + return super().write(data) + + with pytest.raises(RuntimeError, match="power-cycle"): + bl.BootloaderClient(EmptyMessageFailureTransport()).upload( + fw.parse_apj(apj(b"abcd")), confirmation_requested=lambda *_args: True + ) + + +def test_recovery_guidance_preserves_oserror_text_and_errno() -> None: + error = OSError(5, "Input/output error") + + bl._append_recovery_message(error, "power-cycle the flight controller") # pylint: disable=protected-access + + assert error.errno == 5 + assert error.strerror == "Input/output error; power-cycle the flight controller" + assert str(error) == "[Errno 5] Input/output error; power-cycle the flight controller" diff --git a/tests/test_backend_flightcontroller_commands.py b/tests/test_backend_flightcontroller_commands.py index 17cdf00ea..89b1c54bc 100755 --- a/tests/test_backend_flightcontroller_commands.py +++ b/tests/test_backend_flightcontroller_commands.py @@ -24,6 +24,26 @@ # pylint: disable=too-many-lines +def test_reboot_to_bootloader_holds_bootloader_without_force_flags() -> None: + """The reboot command must hold in bootloader mode and preserve param6=0.""" + master = MagicMock() + master.target_system = 1 + master.target_component = 1 + acknowledgement = MagicMock() + acknowledgement.command = mavutil.mavlink.MAV_CMD_PREFLIGHT_REBOOT_SHUTDOWN + acknowledgement.result = mavutil.mavlink.MAV_RESULT_ACCEPTED + master.recv_match.return_value = acknowledgement + + connection = Mock() + connection.master = master + commands = FlightControllerCommands(params_manager=Mock(), connection_manager=connection) + + assert commands.reboot_to_bootloader() == (True, "") + + sent_parameters = master.mav.command_long_send.call_args.args[4:11] + assert sent_parameters == (3, 0, 0, 0, 0, 0, 0) + + class TestFlightControllerCommandsInitialization: """Test command manager initialization and setup.""" diff --git a/tests/test_bootloader_identity.py b/tests/test_bootloader_identity.py new file mode 100644 index 000000000..531c44067 --- /dev/null +++ b/tests/test_bootloader_identity.py @@ -0,0 +1,182 @@ +""" +Regression tests for bootloader device identity resolution. + +This file is part of ArduPilot Methodic Configurator. https://github.com/ArduPilot/MethodicConfigurator + +SPDX-FileCopyrightText: 2026 Amilcar do Carmo Lucas + +SPDX-License-Identifier: GPL-3.0-or-later +""" + +# ruff: noqa: INP001 + +from pathlib import Path + +import pytest +from serial.tools.list_ports_common import ListPortInfo + +from ardupilot_methodic_configurator import backend_flightcontroller_bootloader as bl + + +def test_bootloader_port_refuses_a_partial_usb_identity_match(monkeypatch: pytest.MonkeyPatch) -> None: + """ + A bootloader port must match every captured USB identity attribute. + + GIVEN: A controller was captured with both a USB location and serial number + WHEN: Only one of those attributes matches a re-enumerated port + THEN: The stale or different controller is rejected + """ + port = ListPortInfo("COM11") + port.location = "1-2.3" + port.serial_number = "FC-other" + monkeypatch.setattr(bl.serial.tools.list_ports, "comports", lambda: [port]) + + identity = bl.SerialDeviceIdentity(location="1-2.3", serial_number="FC-123") + + with pytest.raises(OSError, match="cannot uniquely"): + bl.resolve_bootloader_device("COM7", identity) + + +def test_bootloader_port_refuses_an_ambiguous_macos_dual_cdc_match(monkeypatch: pytest.MonkeyPatch) -> None: + """An application-port interface identifier cannot safely identify a bootloader port.""" + mavlink_port = ListPortInfo("/dev/cu.usbmodem14101") + mavlink_port.location = "1-2.3" + mavlink_port.serial_number = "FC-123" + mavlink_port.interface = "0" + console_port = ListPortInfo("/dev/cu.usbmodem14102") + console_port.location = "1-2.3" + console_port.serial_number = "FC-123" + console_port.interface = "2" + monkeypatch.setattr(bl.serial.tools.list_ports, "comports", lambda: [mavlink_port, console_port]) + + identity = bl.SerialDeviceIdentity(location="1-2.3", serial_number="FC-123") + + with pytest.raises(OSError, match="cannot uniquely"): + bl.resolve_bootloader_device("/dev/cu.usbmodem14101", identity) + + +def test_bootloader_port_matches_linux_interface_qualified_location(monkeypatch: pytest.MonkeyPatch) -> None: + """A second Linux CDC interface still maps to the same bootloader device.""" + bootloader_port = ListPortInfo("/dev/ttyACM0") + bootloader_port.location = "1-2.3:1.0" + bootloader_port.serial_number = "FC-123" + bootloader_port.interface = None + monkeypatch.setattr(bl.serial.tools.list_ports, "comports", lambda: [bootloader_port]) + + identity = bl.SerialDeviceIdentity(location="1-2.3:1.2", serial_number="FC-123") + + assert bl.resolve_bootloader_device("/dev/ttyACM1", identity) == "/dev/ttyACM0" + + +def test_application_port_prefers_an_exact_interface_qualified_location(monkeypatch: pytest.MonkeyPatch) -> None: + """A dual-CDC application reconnects to the originally selected port.""" + mavlink_port = ListPortInfo("/dev/ttyACM0") + mavlink_port.location = "1-2.3:1.0" + mavlink_port.serial_number = "FC-123" + mavlink_port.interface = None + console_port = ListPortInfo("/dev/ttyACM1") + console_port.location = "1-2.3:1.2" + console_port.serial_number = "FC-123" + console_port.interface = None + monkeypatch.setattr(bl.serial.tools.list_ports, "comports", lambda: [mavlink_port, console_port]) + + identity = bl.SerialDeviceIdentity(location="1-2.3:1.0", serial_number="FC-123") + + assert bl.resolve_bootloader_device("/dev/ttyACM0", identity) == "/dev/ttyACM0" + + +def test_bootloader_port_refuses_identical_macos_dual_cdc_interfaces(monkeypatch: pytest.MonkeyPatch) -> None: + """Identical CDC metadata must remain fail-closed after reboot.""" + first_port = ListPortInfo("/dev/cu.usbmodem14101") + first_port.location = "1-2.3" + first_port.serial_number = "FC-123" + first_port.interface = "0" + second_port = ListPortInfo("/dev/cu.usbmodem14102") + second_port.location = "1-2.3" + second_port.serial_number = "FC-123" + second_port.interface = "0" + monkeypatch.setattr(bl.serial.tools.list_ports, "comports", lambda: [first_port, second_port]) + + identity = bl.SerialDeviceIdentity(location="1-2.3", serial_number="FC-123", interface="0") + + with pytest.raises(OSError, match="cannot uniquely"): + bl.resolve_bootloader_device("/dev/cu.usbmodem14101", identity) + + +def test_bootloader_port_selects_the_captured_macos_dual_cdc_interface(monkeypatch: pytest.MonkeyPatch) -> None: + """A captured CDC interface disambiguates the two ports exposed by macOS.""" + mavlink_port = ListPortInfo("/dev/cu.usbmodem14101") + mavlink_port.location = "1-2.3" + mavlink_port.serial_number = "FC-123" + mavlink_port.interface = "0" + console_port = ListPortInfo("/dev/cu.usbmodem14102") + console_port.location = "1-2.3" + console_port.serial_number = "FC-123" + console_port.interface = "2" + monkeypatch.setattr(bl.serial.tools.list_ports, "comports", lambda: [mavlink_port, console_port]) + + identity = bl.SerialDeviceIdentity(location="1-2.3", serial_number="FC-123", interface="0") + + assert bl.resolve_bootloader_device("/dev/cu.usbmodem14101", identity) == "/dev/cu.usbmodem14101" + + +def test_bootloader_port_accepts_a_sole_match_when_its_interface_changes(monkeypatch: pytest.MonkeyPatch) -> None: + """The application interface must not exclude the sole bootloader candidate.""" + port = ListPortInfo("/dev/cu.usbmodem14101") + port.location = "1-2.3" + port.serial_number = "FC-123" + port.interface = "Bootloader" + monkeypatch.setattr(bl.serial.tools.list_ports, "comports", lambda: [port]) + + identity = bl.SerialDeviceIdentity(location="1-2.3", serial_number="FC-123", interface="ArduPilot") + + assert bl.resolve_bootloader_device("/dev/cu.usbmodem14101", identity) == "/dev/cu.usbmodem14101" + + +def test_bootloader_port_refuses_duplicate_serials_on_different_devices_despite_interface_match( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """An interface name must not select one of several physical USB devices.""" + bootloader_port = ListPortInfo("/dev/ttyACM0") + bootloader_port.serial_number = "DUP" + bootloader_port.location = "1-2.3" + bootloader_port.interface = "Bootloader" + other_port = ListPortInfo("/dev/ttyACM1") + other_port.serial_number = "DUP" + other_port.location = "1-2.4" + other_port.interface = "ArduPilot" + monkeypatch.setattr(bl.serial.tools.list_ports, "comports", lambda: [bootloader_port, other_port]) + + identity = bl.SerialDeviceIdentity(serial_number="DUP", interface="ArduPilot") + + with pytest.raises(OSError, match="cannot uniquely"): + bl.resolve_bootloader_device("/dev/ttyACM0", identity) + + +def test_capture_serial_identity_follows_a_symlinked_device_path(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: + """USB metadata is retained when the selected port is a symlink such as by-id.""" + device = tmp_path / "ttyACM0" + device.touch() + device_link = tmp_path / "usb-ArduPilot" + device_link.symlink_to(device) + port = ListPortInfo(str(device)) + port.location = "1-2.3" + port.serial_number = "FC-123" + monkeypatch.setattr(bl.serial.tools.list_ports, "comports", lambda: [port]) + + assert bl.capture_serial_device_identity(str(device_link)) == bl.SerialDeviceIdentity( + location="1-2.3", serial_number="FC-123" + ) + + +def test_capture_serial_identity_preserves_macos_cdc_interface(monkeypatch: pytest.MonkeyPatch) -> None: + """The selected macOS CDC interface is retained for post-reboot resolution.""" + port = ListPortInfo("/dev/cu.usbmodem14101") + port.location = "1-2.3" + port.serial_number = "FC-123" + port.interface = "0" + monkeypatch.setattr(bl.serial.tools.list_ports, "comports", lambda: [port]) + + assert bl.capture_serial_device_identity(port.device) == bl.SerialDeviceIdentity( + location="1-2.3", serial_number="FC-123", interface="0" + ) diff --git a/tests/test_bootloader_partial_read.py b/tests/test_bootloader_partial_read.py new file mode 100644 index 000000000..c498b4f83 --- /dev/null +++ b/tests/test_bootloader_partial_read.py @@ -0,0 +1,87 @@ +""" +Regression tests for bounded bootloader reads. + +This file is part of ArduPilot Methodic Configurator. https://github.com/ArduPilot/MethodicConfigurator + +SPDX-FileCopyrightText: 2026 Amilcar do Carmo Lucas + +SPDX-License-Identifier: GPL-3.0-or-later +""" + +# ruff: noqa: INP001 + +import pytest + +from ardupilot_methodic_configurator import backend_flightcontroller_bootloader as bl + + +def test_read_timeout_is_bounded_when_partial_reads_arrive_after_deadline() -> None: + """ + A partial serial response cannot extend the bootloader read deadline. + + GIVEN: A bootloader read has received only part of its expected response + WHEN: The deadline expires before the remaining bytes arrive + THEN: The read fails with a bounded protocol timeout + """ + + class PartialReadTransport: # pylint: disable=too-few-public-methods + """Return one byte per read to exercise the response deadline.""" + + reads = 0 + + def read(self, _size: int = 1) -> bytes: + self.reads += 1 + return b"x" + + transport = PartialReadTransport() + clock_values = iter([0.0, 0.5, 1.0]) + client = bl.BootloaderClient(transport, timeout=1.0, clock=lambda: next(clock_values)) + + with pytest.raises(bl.BootloaderProtocolError, match="timeout waiting"): + client._read_exact(2) # pylint: disable=protected-access + assert transport.reads == 1 + + +@pytest.mark.parametrize("arrival_time", [2.0, 2.05]) +def test_read_accepts_a_complete_reply_at_or_after_deadline(arrival_time: float) -> None: + """A complete serial reply is valid even when the clock reaches its deadline.""" + now = 0.0 + + class CompleteReadTransport: # pylint: disable=too-few-public-methods + """Return the complete response in one serial read.""" + + def read(self, _size: int = 1) -> bytes: + nonlocal now + now = arrival_time + return b"\x12\x10" + + client = bl.BootloaderClient(CompleteReadTransport(), timeout=2.0, clock=lambda: now) + + assert client._read_exact(2) == b"\x12\x10" # pylint: disable=protected-access + + +def test_empty_reads_yield_until_the_deadline() -> None: + """A non-blocking empty transport must not busy-spin while awaiting bytes.""" + now = 0.0 + sleeps: list[float] = [] + + class EmptyReadTransport: # pylint: disable=too-few-public-methods + """Always report no data without blocking.""" + + def read(self, _size: int = 1) -> bytes: + return b"" + + def clock() -> float: + return now + + def sleep(delay: float) -> None: + nonlocal now + sleeps.append(delay) + now += delay + + client = bl.BootloaderClient(EmptyReadTransport(), timeout=0.025, clock=clock, sleep=sleep) + + with pytest.raises(bl.BootloaderProtocolError, match="timeout waiting"): + client._read_exact(1) # pylint: disable=protected-access + + assert sleeps == pytest.approx([0.01, 0.01, 0.005]) diff --git a/tests/test_data_model_firmware_upload.py b/tests/test_data_model_firmware_upload.py new file mode 100755 index 000000000..ace10c18a --- /dev/null +++ b/tests/test_data_model_firmware_upload.py @@ -0,0 +1,489 @@ +#!/usr/bin/env python3 + +""" +Unit tests for the firmware upload domain model. + +This file is part of ArduPilot Methodic Configurator. https://github.com/ArduPilot/MethodicConfigurator + +SPDX-FileCopyrightText: 2024-2026 Amilcar do Carmo Lucas + +SPDX-License-Identifier: GPL-3.0-or-later +""" + +import base64 +import hashlib +import json +import struct +import zlib +from pathlib import Path + +import pytest + +from ardupilot_methodic_configurator import backend_flightcontroller_bootloader as bootloader +from ardupilot_methodic_configurator import data_model_firmware_upload as fw + +# pylint: disable=redefined-outer-name, too-few-public-methods, too-many-return-statements + + +def write_apj(tmp_path: Path, image: bytes, **overrides: object) -> Path: + desc: dict[str, object] = { + "board_id": 9, + "image_size": len(image), + "image": base64.b64encode(zlib.compress(image)).decode(), + "version": "4.6.0", + "git_identity": "abc1234", + } + desc.update(overrides) + path = tmp_path / "arducopter.apj" + path.write_text(json.dumps(desc), encoding="utf-8") + return path + + +@pytest.fixture +def small_image() -> bytes: + return bytes(range(256)) * 3 + b"\x01\x02\x03" # 771 bytes, not a multiple of 4 + + +class TestApjLoading: + """User selects a firmware file and the model tells them what it is before touching the board.""" + + def test_user_sees_metadata_from_a_valid_apj_file(self, tmp_path: Path, small_image: bytes) -> None: + """ + Test user sees metadata from a valid apj file. + + GIVEN: a well formed APJ file + WHEN: it is loaded + THEN: metadata comes from the descriptor and the image is padded to 4 bytes with 0xFF + """ + image = bootloader.load_apj(write_apj(tmp_path, small_image)) + + assert image.metadata.board_id == 9 + assert image.metadata.image_size == len(small_image) + assert image.metadata.firmware_version == "4.6.0" + assert image.metadata.board_revision is None + assert image.metadata.apj_sha256 == hashlib.sha256((tmp_path / "arducopter.apj").read_bytes()).hexdigest() + assert image.image == small_image + b"\xff" + assert len(image.image) % 4 == 0 + assert image.extf_image == b"" + + def test_user_can_load_an_external_only_apj_file(self, tmp_path: Path) -> None: + """ + An APJ may contain only an external-flash image. + + GIVEN: An APJ descriptor with an empty internal image and external payload + WHEN: It is loaded for firmware upload + THEN: The empty internal region is accepted and the external payload is preserved + """ + external_image = b"ext" + image = bootloader.load_apj( + write_apj( + tmp_path, + b"", + extf_image_size=len(external_image), + extf_image=base64.b64encode(zlib.compress(external_image)).decode(), + ) + ) + + assert image.metadata.image_size == 0 + assert image.metadata.extf_image_size == len(external_image) + assert image.image == b"" + assert image.extf_image == external_image + b"\xff" + + def test_user_gets_a_clear_error_for_a_non_apj_file(self, tmp_path: Path) -> None: + """ + Test user gets a clear error for a non apj file. + + GIVEN: a .bin file + WHEN: the user tries to load it + THEN: a typed file error explains only .apj is supported + """ + path = tmp_path / "arducopter.bin" + path.write_bytes(b"\x00" * 16) + + with pytest.raises(fw.FirmwareFileError, match=r"only \.apj"): + bootloader.load_apj(path) + + @pytest.mark.parametrize( + ("content", "reason"), + [ + ("not json", "descriptor"), + ("[1, 2]", "not a JSON object"), + (json.dumps({"board_id": 9, "image_size": 4, "image": "!!notbase64!!"}), "base64"), + (json.dumps({"board_id": 9, "image_size": 4, "image": base64.b64encode(b"notzlib").decode()}), "zlib"), + (json.dumps({"image_size": 4, "image": base64.b64encode(zlib.compress(b"abcd")).decode()}), "metadata"), + ( + json.dumps({"board_id": 9, "image_size": 99, "image": base64.b64encode(zlib.compress(b"abcd")).decode()}), + "image_size", + ), + ], + ) + def test_malformed_apj_is_rejected_before_any_serial_access(self, tmp_path: Path, content: str, reason: str) -> None: + """ + Test malformed apj is rejected before any serial access. + + GIVEN: a corrupt or inconsistent APJ file + WHEN: it is loaded + THEN: a FirmwareFileError names the problem + """ + path = tmp_path / "bad.apj" + path.write_text(content, encoding="utf-8") + + with pytest.raises(fw.FirmwareFileError, match=reason): + bootloader.load_apj(path) + + def test_oversized_image_is_rejected(self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: + """ + Test oversized image is rejected. + + GIVEN: an APJ whose image decompresses beyond the sanity bound + WHEN: it is loaded + THEN: it is refused instead of allocating the whole thing + """ + monkeypatch.setattr(fw, "MAX_IMAGE_SIZE", 1024) + big = b"\x00" * 4096 + path = write_apj(tmp_path, big) + + with pytest.raises(fw.FirmwareFileError, match="exceeds"): + bootloader.load_apj(path) + + def test_crc_matches_the_reference_uploader_computation(self, tmp_path: Path, small_image: bytes) -> None: + """ + Test crc matches the reference uploader computation. + + GIVEN: a loaded image and a flash size larger than the image + WHEN: the CRC is computed + THEN: it equals ArduPilot's raw CRC-32 over the image padded with 0xFF up to flash size + """ + image = bootloader.load_apj(write_apj(tmp_path, small_image)) + flash_size = 2048 + expected = fw.bootloader_crc32(image.image + b"\xff" * (flash_size - len(image.image))) + + assert image.crc(flash_size) == expected + + def test_crc_rejects_an_unaligned_flash_size(self, tmp_path: Path) -> None: + """CRC cannot safely represent an unaligned bootloader flash region.""" + image = bootloader.load_apj(write_apj(tmp_path, b"abcd")) + flash_size = 7 + + with pytest.raises(ValueError, match="multiple of four"): + image.crc(flash_size) + + def test_crc_matches_ardupilot_uploader_known_vector(self) -> None: + """The uploader's raw CRC state is not Python's finalized binascii CRC-32.""" + assert fw.bootloader_crc32(b"abc") == 0xCA6598D0 + assert fw.bootloader_crc32(b"bc", fw.bootloader_crc32(b"a")) == 0xCA6598D0 + + def test_external_crc_uses_only_the_declared_unpadded_payload(self, tmp_path: Path) -> None: + external_image = b"ext" + image = bootloader.load_apj( + write_apj( + tmp_path, + b"abcd", + extf_image_size=len(external_image), + extf_image=base64.b64encode(zlib.compress(external_image)).decode(), + ) + ) + + assert image.extf_image == external_image + b"\xff" + assert image.extf_crc() == fw.bootloader_crc32(external_image) + + def test_valid_board_revision_is_preserved(self, tmp_path: Path) -> None: + image = bootloader.load_apj(write_apj(tmp_path, b"abcd", board_revision=7)) + + assert image.metadata.board_revision == 7 + + +class TestCompatibility: + """The model refuses to flash anything that does not match the detected board.""" + + @pytest.fixture + def image(self, tmp_path: Path, small_image: bytes) -> fw.FirmwareImage: + return bootloader.load_apj(write_apj(tmp_path, small_image)) + + def test_matching_board_and_enough_flash_is_accepted(self, image: fw.FirmwareImage) -> None: + fw.check_compatibility(image, fw.BootloaderInfo(5, 9, 0, 2048)) + + def test_board_id_mismatch_is_refused_by_default(self, image: fw.FirmwareImage) -> None: + with pytest.raises(fw.FirmwareCompatibilityError, match="board_id 9, board reports 42"): + fw.check_compatibility(image, fw.BootloaderInfo(5, 42, 0, 2048)) + + def test_board_id_mismatch_cannot_be_overridden(self, image: fw.FirmwareImage) -> None: + with pytest.raises(fw.FirmwareCompatibilityError, match="board_id 9, board reports 42"): + fw.check_compatibility(image, fw.BootloaderInfo(5, 42, 0, 2048)) + + def test_bootloader_must_match_the_connected_board_when_known(self, image: fw.FirmwareImage) -> None: # pylint: disable=unused-argument + with pytest.raises(fw.FirmwareTargetMismatchError, match="differs from connected"): + fw.check_bootloader_matches_connected_board(fw.BootloaderInfo(5, 9, 0, 2048), 42) + + def test_missing_connected_board_identity_does_not_block_upload(self, image: fw.FirmwareImage) -> None: # pylint: disable=unused-argument + fw.check_bootloader_matches_connected_board(fw.BootloaderInfo(5, 9, 0, 2048), None) + + def test_image_larger_than_flash_is_refused(self, image: fw.FirmwareImage) -> None: + with pytest.raises(fw.FirmwareCompatibilityError, match="exceeds flash"): + fw.check_compatibility(image, fw.BootloaderInfo(5, 9, 0, 512)) + + def test_unaligned_bootloader_flash_size_is_refused(self, image: fw.FirmwareImage) -> None: + with pytest.raises(fw.FirmwareCompatibilityError, match="invalid flash size"): + fw.check_compatibility(image, fw.BootloaderInfo(5, 9, 0, 513)) + + def test_reconnected_firmware_board_identity_is_required(self, image: fw.FirmwareImage) -> None: + with pytest.raises(fw.FirmwareIdentityError, match="did not report an APJ board_id"): + fw.verify_reconnected_firmware(image, board_id="") + + with pytest.raises(fw.FirmwareIdentityError, match="board_id 42"): + fw.verify_reconnected_firmware(image, board_id="42") + + fw.verify_reconnected_firmware(image, board_id="9") + + def test_trusted_digest_must_match_the_complete_apj_descriptor(self, image: fw.FirmwareImage) -> None: + fw.verify_expected_firmware_digest(image, image.metadata.apj_sha256.upper()) + + with pytest.raises(fw.FirmwareIntegrityError, match="does not match"): + fw.verify_expected_firmware_digest(image, hashlib.sha256(b"unrelated").hexdigest()) + + def test_trusted_digest_rejects_board_metadata_tampering(self, tmp_path: Path) -> None: + """ + A trusted digest covers APJ metadata as well as the firmware payload. + + GIVEN: A trusted digest was recorded for an APJ descriptor + WHEN: The same payload is rewritten with a different board ID + THEN: The descriptor is rejected before upload + """ + original_path = write_apj(tmp_path, b"abcd", board_id=9) + trusted_digest = hashlib.sha256(original_path.read_bytes()).hexdigest() + tampered_path = tmp_path / "tampered.apj" + tampered_path.write_bytes(write_apj(tmp_path, b"abcd", board_id=42).read_bytes()) + + with pytest.raises(fw.FirmwareIntegrityError, match="does not match"): + fw.verify_expected_firmware_digest(bootloader.load_apj(tampered_path), trusted_digest) + + @pytest.mark.parametrize( + ("field", "value"), + [ + ("board_id", True), + ("board_id", 9.9), + ("image_size", False), + ("image_size", 4.9), + ("extf_image_size", 0.1), + ("board_revision", True), + ], + ) + def test_boolean_and_float_metadata_are_rejected(self, tmp_path: Path, field: str, value: object) -> None: + """ + APJ integer metadata rejects values that would be silently coerced. + + GIVEN: An APJ descriptor contains boolean or floating-point metadata + WHEN: The descriptor is loaded for firmware upload + THEN: Loading fails with a metadata validation error + """ + overrides = {field: value} + with pytest.raises(fw.FirmwareFileError, match="metadata"): + bootloader.load_apj(write_apj(tmp_path, b"abcd", **overrides)) + + @pytest.mark.parametrize("revision", [1, 6]) + def test_unsupported_bootloader_revision_is_refused(self, image: fw.FirmwareImage, revision: int) -> None: + with pytest.raises(fw.BootloaderProtocolError, match="revision"): + fw.check_compatibility(image, fw.BootloaderInfo(revision, 9, 0, 2048)) + + +class TestStateMachine: + """Cancellation is only offered while it is still safe.""" + + def test_happy_path_walks_every_stage_in_order(self) -> None: + """ + The happy path confirms the image after bootloader identification. + + GIVEN: A firmware upload starts in the idle state + WHEN: The state machine advances through the successful workflow + THEN: Bootloader identification precedes confirmation and erase + """ + stage = fw.UploadStage.IDLE + for target in ( + fw.UploadStage.INSPECTING, + fw.UploadStage.ENTERING_BOOTLOADER, + fw.UploadStage.IDENTIFYING, + fw.UploadStage.AWAITING_CONFIRMATION, + fw.UploadStage.ERASING, + fw.UploadStage.PROGRAMMING, + fw.UploadStage.VERIFYING, + fw.UploadStage.REBOOTING, + fw.UploadStage.RECONNECTING, + fw.UploadStage.COMPLETED, + ): + stage = fw.next_stage(stage, target) + assert stage is fw.UploadStage.COMPLETED + + def test_user_can_cancel_before_the_flash_is_erased(self) -> None: + assert fw.next_stage(fw.UploadStage.AWAITING_CONFIRMATION, fw.UploadStage.CANCELLED) is fw.UploadStage.CANCELLED + assert fw.next_stage(fw.UploadStage.IDENTIFYING, fw.UploadStage.CANCELLED) is fw.UploadStage.CANCELLED + + def test_dual_region_upload_can_return_to_erasing_for_the_internal_image(self) -> None: + """The state machine permits external verification before internal flashing.""" + stage = fw.UploadStage.IDLE + for target in ( + fw.UploadStage.INSPECTING, + fw.UploadStage.ENTERING_BOOTLOADER, + fw.UploadStage.IDENTIFYING, + fw.UploadStage.AWAITING_CONFIRMATION, + fw.UploadStage.ERASING, + fw.UploadStage.PROGRAMMING, + fw.UploadStage.VERIFYING, + fw.UploadStage.ERASING, + fw.UploadStage.PROGRAMMING, + fw.UploadStage.VERIFYING, + fw.UploadStage.REBOOTING, + fw.UploadStage.RECONNECTING, + fw.UploadStage.COMPLETED, + ): + stage = fw.next_stage(stage, target) + + assert stage is fw.UploadStage.COMPLETED + + @pytest.mark.parametrize("stage", [fw.UploadStage.ERASING, fw.UploadStage.PROGRAMMING, fw.UploadStage.VERIFYING]) + def test_user_cannot_cancel_once_the_flash_is_being_written(self, stage: fw.UploadStage) -> None: + with pytest.raises(ValueError, match="cannot cancel"): + fw.next_stage(stage, fw.UploadStage.CANCELLED) + + def test_any_stage_can_fail(self) -> None: + assert fw.next_stage(fw.UploadStage.PROGRAMMING, fw.UploadStage.FAILED) is fw.UploadStage.FAILED + + def test_skipping_a_stage_is_rejected(self) -> None: + with pytest.raises(ValueError, match="illegal transition"): + fw.next_stage(fw.UploadStage.INSPECTING, fw.UploadStage.ERASING) + + def test_terminal_stages_do_not_move(self) -> None: + with pytest.raises(ValueError, match="terminal"): + fw.next_stage(fw.UploadStage.COMPLETED, fw.UploadStage.IDLE) + + +class FakeBootloader: + """ + Minimal rev 5 bootloader behind a byte pipe. + + It consumes exactly what the codec encodes and answers like the real protocol, so a + mismatch between encoder and decoder fails here instead of on hardware. + """ + + def __init__(self, board_id: int = 9, flash_size: int = 2048) -> None: + self.board_id = board_id + self.flash_size = flash_size + self.flash = b"" + self.erased = False + self.rebooted = False + + def exchange(self, request: bytes) -> bytes: # noqa: PLR0911 + assert request.endswith(bootloader.EOC) + cmd, body = request[0:1], request[1:-1] + if cmd == bootloader.GET_SYNC: + return bootloader.INSYNC + bootloader.OK + if cmd == bootloader.GET_DEVICE: + values = { + bootloader.INFO_BL_REV: 5, + bootloader.INFO_BOARD_ID: self.board_id, + bootloader.INFO_BOARD_REV: 0, + bootloader.INFO_FLASH_SIZE: self.flash_size, + bootloader.INFO_EXTF_SIZE: 0, + } + return struct.pack(" fw.BootloaderInfo: + bootloader.decode_sync(bl.exchange(bootloader.encode_get_sync())) + + def info(param: bytes) -> int: + reply = bl.exchange(bootloader.encode_get_device(param)) + bootloader.decode_sync(reply[4:]) + return bootloader.decode_uint32(reply[:4]) + + return fw.BootloaderInfo( + info(bootloader.INFO_BL_REV), + info(bootloader.INFO_BOARD_ID), + info(bootloader.INFO_BOARD_REV), + info(bootloader.INFO_FLASH_SIZE), + info(bootloader.INFO_EXTF_SIZE), + ) + + +class TestBootloaderCodec: + """Encoders and decoders agree with a fake bootloader on the full erase, program, verify sequence.""" + + def test_full_upload_sequence_verifies_against_fake_bootloader(self, tmp_path: Path, small_image: bytes) -> None: + """ + Test full upload sequence verifies against fake bootloader. + + GIVEN: a loaded image and a fake bootloader with a matching board id + WHEN: identify, erase, program every chunk, then GET_CRC + THEN: the bootloader CRC equals the image CRC and the reboot is acknowledged + """ + image = bootloader.load_apj(write_apj(tmp_path, small_image)) + bl = FakeBootloader() + + info = identify(bl) + fw.check_compatibility(image, info) + bootloader.decode_sync(bl.exchange(bootloader.encode_chip_erase())) + chunk_count = 0 + for chunk in bootloader.program_chunks(image.image): + bootloader.decode_sync(bl.exchange(bootloader.encode_prog_multi(chunk))) + chunk_count += 1 + crc_reply = bl.exchange(bootloader.encode_get_crc()) + bootloader.decode_sync(crc_reply[4:]) + bootloader.decode_sync(bl.exchange(bootloader.encode_reboot())) + + assert info == fw.BootloaderInfo(5, 9, 0, 2048, 0) + assert chunk_count == -(-len(image.image) // bootloader.PROG_MULTI_MAX) + assert bl.flash == image.image + assert bootloader.decode_uint32(crc_reply[:4]) == image.crc(info.flash_size) + assert bl.rebooted + + def test_programming_before_erase_surfaces_as_a_protocol_error(self, tmp_path: Path, small_image: bytes) -> None: + image = bootloader.load_apj(write_apj(tmp_path, small_image)) + bl = FakeBootloader() + first_chunk = next(bootloader.program_chunks(image.image)) + + with pytest.raises(fw.BootloaderProtocolError, match="OPERATION FAILED"): + bootloader.decode_sync(bl.exchange(bootloader.encode_prog_multi(first_chunk))) + + @pytest.mark.parametrize( + ("reply", "reason"), + [ + (b"", "short reply"), + (b"\x00\x10", "expected INSYNC"), + (bootloader.INSYNC + bootloader.INVALID, "INVALID"), + (bootloader.INSYNC + bootloader.BAD_SILICON_REV, "silicon"), + (bootloader.INSYNC + b"\x7f", "unexpected status"), + ], + ) + def test_bad_sync_replies_are_typed_errors(self, reply: bytes, reason: str) -> None: + with pytest.raises(fw.BootloaderProtocolError, match=reason): + bootloader.decode_sync(reply) + + @pytest.mark.parametrize("length", [0, 3, bootloader.PROG_MULTI_MAX + 4]) + def test_prog_multi_refuses_chunks_the_bootloader_would_reject(self, length: int) -> None: + with pytest.raises(ValueError, match="PROG_MULTI"): + bootloader.encode_prog_multi(b"\x00" * length) + + def test_read_multi_encodes_length_byte(self) -> None: + assert bootloader.encode_read_multi(252) == bootloader.READ_MULTI + b"\xfc" + bootloader.EOC + with pytest.raises(ValueError, match="READ_MULTI"): + bootloader.encode_read_multi(253) + + def test_short_uint32_reply_is_a_protocol_error(self) -> None: + with pytest.raises(fw.BootloaderProtocolError, match="4 bytes"): + bootloader.decode_uint32(b"\x01\x02") diff --git a/tests/test_pytest_configuration.py b/tests/test_pytest_configuration.py new file mode 100755 index 000000000..f21a02d67 --- /dev/null +++ b/tests/test_pytest_configuration.py @@ -0,0 +1,22 @@ +#!/usr/bin/env python3 + +""" +Regression checks for repository-wide pytest safeguards. + +This file is part of ArduPilot Methodic Configurator. https://github.com/ArduPilot/MethodicConfigurator + +SPDX-FileCopyrightText: 2026 Amilcar do Carmo Lucas + +SPDX-License-Identifier: GPL-3.0-or-later +""" + +from configparser import ConfigParser +from pathlib import Path + + +def test_pytest_enforces_a_default_test_timeout() -> None: + """A hung test must fail without each test opting into a timeout marker.""" + config = ConfigParser() + config.read(Path(__file__).parents[1] / "pytest.ini", encoding="utf-8") + + assert config.getint("pytest", "timeout") == 30