diff --git a/BlocksScreen/BlocksScreen.py b/BlocksScreen/BlocksScreen.py index 35e6ca87..9cc399e7 100644 --- a/BlocksScreen/BlocksScreen.py +++ b/BlocksScreen/BlocksScreen.py @@ -14,7 +14,7 @@ from lib.panels.mainWindow import MainWindow # noqa: E402 from logger import CrashHandler, LogManager, install_crash_handler, setup_logging from PyQt6 import QtCore, QtGui, QtWidgets # noqa: E402 -from tools.configuration_manager import ConfigManager +from tools.configuration_manager import DeviceConfigManager, DeviceConfigWatcher install_crash_handler() @@ -166,12 +166,17 @@ def on_quit() -> None: LogManager.shutdown() +conf_man: DeviceConfigManager | None = None +device_watcher: DeviceConfigWatcher | None = None + + def initialize_conf_manager() -> None: global conf_man try: - conf_man = ConfigManager(get_configparser()) + logging.info("[DeviceConfigManager] Initializing...") + conf_man = DeviceConfigManager(get_configparser()) except Exception as e: - _logger.error( + logging.error( "Caught Exception on configuration_manager tool: %s" % e, exc_info=True ) @@ -294,14 +299,15 @@ def eventFilter(self, a0: QtCore.QObject | None, a1: QtCore.QEvent | None) -> bo if __name__ == "__main__": setup_logging( filename="logs/BlocksScreen.log", - level=logging.DEBUG, + level=logging.INFO, console_output=True, - console_level=logging.DEBUG, + console_level=logging.INFO, capture_stderr=True, capture_stdout=False, ) _logger = logging.getLogger(__name__) _logger.info("============ BlocksScreen Initializing ============") + initialize_conf_manager() BlocksScreen = BlocksScreenApp([]) BlocksScreen.setApplicationName("BlocksScreen") @@ -309,9 +315,33 @@ def eventFilter(self, a0: QtCore.QObject | None, a1: QtCore.QEvent | None) -> bo BlocksScreen.setDesktopFileName("BlocksScreen") _splash = show_splash() main_window = MainWindow() + + if conf_man is not None: + if conf_man.boot_changed: + # Klipper may have started before the boot sync rewrote + # printer.cfg; restarted once Moonraker is up and no job runs. + main_window.request_klipper_restart() + # Live side of the device templates: printer.cfg follows devices + # plugged in while running (after the user's OK for templates that + # ask for it), and Klipper is restarted to load it. + # Optional: without it the screen works as before, just no hotplug. + try: + _logger.info("[DEVICE WATCHER] Initializing...") + device_watcher = DeviceConfigWatcher(conf_man, parent=main_window) + device_watcher.config_changed.connect(main_window.on_device_config_changed) + device_watcher.confirmation_required.connect( + main_window.on_device_confirmation_required + ) + main_window.device_config_accepted.connect(device_watcher.accept) + main_window.device_config_declined.connect(device_watcher.decline) + device_watcher.start() + BlocksScreen.aboutToQuit.connect(device_watcher.stop) + except Exception as e: # noqa: BLE001 + _logger.error("Device hotplug disabled: %s", e, exc_info=True) BlocksScreen.processEvents() BlocksScreen.aboutToQuit.connect(on_quit) _setup_sigterm(BlocksScreen) + # Event-driven repaint: fire the moment the window first paints (parented to # main_window so it lives as long as the window). _FirstPaintRefresh(main_window) diff --git a/BlocksScreen/devices/discovery/__init__.py b/BlocksScreen/devices/discovery/__init__.py new file mode 100644 index 00000000..b3606a76 --- /dev/null +++ b/BlocksScreen/devices/discovery/__init__.py @@ -0,0 +1,13 @@ +"""Client side of the device discovery service. + +The daemon itself lives in the separate DeviceDiscovery repo and runs as +device-discoveryd.service; this package only talks to its Unix socket. + +- `DeviceDiscoveryClient`: long-lived Qt client (snapshot + hotplug signals) +- `fetch_snapshot`: one-shot blocking query, no Qt event loop needed +- `serial_devices.SerialScanner`: daemon scan with a /dev/serial/by-id fallback +""" + +from .client import DeviceDiscoveryClient, default_socket_path, fetch_snapshot + +__all__ = ["DeviceDiscoveryClient", "default_socket_path", "fetch_snapshot"] diff --git a/BlocksScreen/devices/discovery/client.py b/BlocksScreen/devices/discovery/client.py new file mode 100644 index 00000000..ac7ec3aa --- /dev/null +++ b/BlocksScreen/devices/discovery/client.py @@ -0,0 +1,311 @@ +"""Qt client for the device_discoveryd daemon. + +The daemon lives in its own repo (BlocksTechnology/DeviceDiscovery) and runs +as device-discoveryd.service; its docs/PROTOCOL.md is the contract this client +implements. + +Owns a dedicated background thread that connects to the daemon's Unix +socket and blocks reading newline-delimited JSON messages. Results are +bridged back to Qt via pyqtSignals, mirroring the shape of +lib/updater_worker.py's D-Bus client - a background thread doing blocking +I/O, reconnecting with backoff, never touching Qt objects except through +signal emission (which is thread-safe: PyQt auto-queues the connection +when emitter and receiver live on different threads). + +Unlike updater_worker.py this doesn't need asyncio: the protocol here is +one blocking recv() loop, so an event loop would add ceremony without +buying anything. +""" + +from __future__ import annotations + +import json +import logging +import os +import socket +import threading +import time +from typing import Any + +from PyQt6 import QtCore + +_log = logging.getLogger(__name__) + +_RECONNECT_DELAYS = (2.0, 5.0, 10.0, 30.0) +_RECV_CHUNK = 65536 +# A line longer than this is not a real message (the daemon caps what it +# queues per client at the same size): drop the connection instead of +# buffering without bound. +_MAX_LINE = 1 << 20 + +SUPPORTED_PROTOCOL = 1 +"""Protocol version (the daemon's `hello.proto`) this client understands.""" + +SYSTEMD_SOCKET_PATH = "/run/device-discovery/device_discovery.sock" +"""Where device-discoveryd.service puts the socket (RuntimeDirectory=).""" + + +def default_socket_path() -> str: + """Resolve the daemon socket path. + + `$DEVICE_DISCOVERY_SOCKET` wins, then the systemd service's path if it + exists, then the daemon's fallback for runs outside systemd + (`$XDG_RUNTIME_DIR`). The daemon's last-resort /tmp path is deliberately + not followed: any local user could create it and feed the UI fake + devices. With no match, the systemd path is returned and the client + keeps retrying until the service comes up. + """ + env_path = os.environ.get("DEVICE_DISCOVERY_SOCKET") + if env_path: + return env_path + if os.path.exists(SYSTEMD_SOCKET_PATH): + return SYSTEMD_SOCKET_PATH + runtime_dir = os.environ.get("XDG_RUNTIME_DIR") + if runtime_dir: + return os.path.join(runtime_dir, "blockscreen", "device_discovery.sock") + return SYSTEMD_SOCKET_PATH + + +def _describe(device: dict[str, Any]) -> str: + """One-line summary of a daemon device object for the log.""" + # vid_hex is only meaningful on USB entries (the daemon sends 0 for Serial). + usb_id = ( + f"{device.get('vid_hex')}:{device.get('pid_hex')}" + if device.get("connection") == "USB" + else "" + ) + return "{} [{}] {} {}".format( + device.get("symlink_name") or device.get("name") or "?", + device.get("connection", "?"), + device.get("device_path") or "", + usb_id, + ).strip() + + +def _parse_message(line: bytes) -> dict[str, Any] | None: + """Decode and validate one protocol line. + + Returns None for anything that isn't a well-formed message, so a broken + or misbehaving daemon can never raise into the caller: unhandled + exceptions (in any thread) make CrashHandler exit BlocksScreen. + """ + try: + msg = json.loads(line) + except (ValueError, RecursionError): # JSONDecodeError, bad UTF-8, deep nesting + _log.warning("Malformed message from device_discoveryd: %r", line[:200]) + return None + if not isinstance(msg, dict) or not isinstance(msg.get("type"), str): + _log.warning("Invalid message from device_discoveryd: %r", line[:200]) + return None + msg_type = msg["type"] + if msg_type in ("added", "removed") and not isinstance(msg.get("device"), dict): + _log.warning("device_discoveryd %s without a device object", msg_type) + return None + if msg_type == "snapshot": + devices = msg.get("devices") + if not isinstance(devices, list): + _log.warning("device_discoveryd snapshot without a device list") + return None + msg["devices"] = [d for d in devices if isinstance(d, dict)] + return msg + + +def _split_lines(buf: bytes) -> tuple[list[bytes], bytes] | None: + """Complete non-empty lines in buf and the unterminated rest, None if oversized""" + *lines, rest = buf.split(b"\n") + if len(rest) > _MAX_LINE: + _log.warning("device_discoveryd sent an oversized line") + return None + return [line for line in lines if line], rest + + +def _proto_supported(msg: dict[str, Any]) -> bool: + """False, with a warning, for a hello in a protocol this client doesn't speak""" + if msg["type"] != "hello" or msg.get("proto") == SUPPORTED_PROTOCOL: + return True + _log.warning( + "device_discoveryd speaks protocol %s, client supports %s", + msg.get("proto"), + SUPPORTED_PROTOCOL, + ) + return False + + +def fetch_snapshot( + socket_path: str | None = None, timeout: float = 2.0 +) -> list[dict[str, Any]] | None: + """One-shot blocking query: connect, read hello + snapshot, disconnect. + + For code that needs the current device list synchronously and has no Qt + event loop, such as the configuration manager at startup. Returns the + snapshot's device dicts, or None when the daemon is unreachable, speaks + an unsupported protocol, or sends no snapshot within `timeout` seconds - + callers treat None as "fall back to another source". + """ + path = socket_path or default_socket_path() + deadline = time.monotonic() + timeout + try: + with socket.socket(socket.AF_UNIX, socket.SOCK_STREAM) as sock: + sock.settimeout(timeout) + sock.connect(path) + buf = b"" + while True: + remaining = deadline - time.monotonic() + if remaining <= 0: + _log.debug("device_discoveryd: no snapshot within %.1fs", timeout) + return None + sock.settimeout(remaining) + chunk = sock.recv(_RECV_CHUNK) + if not chunk: + return None + framed = _split_lines(buf + chunk) + if framed is None: + return None + lines, buf = framed + for line in lines: + msg = _parse_message(line) + if msg is None: + continue + if not _proto_supported(msg): + return None + if msg["type"] == "snapshot": + return msg["devices"] + except OSError as exc: + _log.debug("device_discoveryd snapshot unavailable at %s: %s", path, exc) + return None + + +class DeviceDiscoveryClient(QtCore.QObject): + """Background client for the device-discovery daemon's report socket. + + Connect to the signals first, then call start(): the daemon sends its + snapshot immediately on connect, so starting earlier can drop it. + """ + + device_added = QtCore.pyqtSignal(dict, name="dd-device-added") + device_removed = QtCore.pyqtSignal(dict, name="dd-device-removed") + snapshot_ready = QtCore.pyqtSignal(list, name="dd-snapshot-ready") + daemon_connected = QtCore.pyqtSignal(name="dd-daemon-conn") + daemon_disconnected = QtCore.pyqtSignal(name="dd-daemon-disc") + + def __init__(self, socket_path: str | None = None) -> None: + super().__init__() + # None = resolve on every connection attempt: the systemd socket may + # not exist yet when BlocksScreen starts (the daemon is Type=simple, + # so it can still be binding), and a path fixed at construction + # would then point at the fallback forever. + self._explicit_path = socket_path + self._stop_event = threading.Event() + self._sock: socket.socket | None = None + self._thread = threading.Thread( + target=self._run, daemon=True, name="DeviceDiscoveryClient" + ) + + def start(self) -> None: + """Starts the reader thread. Call once, after connecting signals.""" + self._thread.start() + + def stop(self) -> None: + """Stops the client and joins its thread. Safe to call once, from any thread.""" + self._stop_event.set() + sock = self._sock + if sock is not None: + try: + sock.shutdown(socket.SHUT_RDWR) + except OSError: + pass + if self._thread.is_alive(): + self._thread.join(timeout=5.0) + + def _socket_path(self) -> str: + return self._explicit_path or default_socket_path() + + def _run(self) -> None: + attempt = 0 + while not self._stop_event.is_set(): + path = self._socket_path() + try: + self._connect_and_read(path) + if not self._stop_event.is_set(): + _log.info("device_discoveryd closed the connection, reconnecting") + attempt = 0 # clean disconnect after a working connection + except OSError as exc: + # First failure at INFO so a missing/unreachable daemon shows + # up in the journal; the retries after it stay at DEBUG. + log = _log.info if attempt == 0 else _log.debug + log("device_discoveryd not reachable at %s: %s", path, exc) + except Exception: # noqa: BLE001 # pylint: disable=broad-except + # A bug here must not end the thread: CrashHandler would + # exit the whole UI. Log it and reconnect with backoff. + _log.exception("device_discoveryd client error, reconnecting") + + if self._stop_event.is_set(): + return + + self.daemon_disconnected.emit() + delay = _RECONNECT_DELAYS[min(attempt, len(_RECONNECT_DELAYS) - 1)] + attempt += 1 + self._stop_event.wait(delay) + + def _connect_and_read(self, path: str) -> None: + sock = socket.socket(socket.AF_UNIX, socket.SOCK_STREAM) + sock.settimeout(2.0) # periodic wake-up so stop_event is noticed promptly + try: + sock.connect(path) + except OSError: + sock.close() + raise + self._sock = sock + self.daemon_connected.emit() + _log.info("Connected to device_discoveryd at %s", path) + + buf = b"" + try: + while not self._stop_event.is_set(): + try: + chunk = sock.recv(_RECV_CHUNK) + except TimeoutError: + continue + if not chunk: + return # daemon closed the connection + framed = _split_lines(buf + chunk) + if framed is None: + return # reconnect + lines, buf = framed + for line in lines: + self._dispatch(line) + finally: + self._sock = None + sock.close() + + def _dispatch(self, line: bytes) -> None: + msg = _parse_message(line) + if msg is None: + return + msg_type = msg["type"] + if msg_type == "snapshot": + _log.info( + "device_discoveryd snapshot: %d devices, serial: %s", + len(msg["devices"]), + [ + d.get("symlink_name") + for d in msg["devices"] + if d.get("connection") == "Serial" + ], + ) + self.snapshot_ready.emit(msg["devices"]) + elif msg_type == "added": + _log.info("device_discoveryd added: %s", _describe(msg["device"])) + self.device_added.emit(msg["device"]) + elif msg_type == "removed": + _log.info("device_discoveryd removed: %s", _describe(msg["device"])) + self.device_removed.emit(msg["device"]) + elif msg_type == "hello": + _proto_supported(msg) + _log.info( + "device_discoveryd hello: proto=%s pid=%s", + msg.get("proto"), + msg.get("pid"), + ) + else: + _log.debug("Unknown message type from device_discoveryd: %s", msg_type) diff --git a/BlocksScreen/devices/discovery/serial_devices.py b/BlocksScreen/devices/discovery/serial_devices.py new file mode 100644 index 00000000..6129fe0a --- /dev/null +++ b/BlocksScreen/devices/discovery/serial_devices.py @@ -0,0 +1,271 @@ +"""Serial-device scanning backed by device_discoveryd. + +Devices come from the daemon's snapshot (only `connection == "Serial"` +entries - the daemon also reports plain USB devices). When the daemon can't +be used - not installed on this machine, not running yet, or speaking an +unsupported protocol - scanning falls back to listing /dev/serial/by-id. + +USB vendor/product IDs: the daemon only reports them for its `USB` entries +(0 for `Serial` ones), and the by-id name never carries them. So whenever a +serial device arrives without an ID, it is read here from sysfs - the +idVendor / idProduct of the USB device behind the tty - which is what lets +device profiles match on the IDs from USB_IDS.md instead of the by-id name. +""" + +from __future__ import annotations + +import logging +import os +from collections.abc import Iterable +from dataclasses import dataclass +from enum import Enum +from typing import Any + +from .client import fetch_snapshot + +_logger = logging.getLogger(__name__) + +SERIAL_BY_ID_PATH = "/dev/serial/by-id/" +SYSFS_TTY_PATH = "/sys/class/tty" +# tty -> USB interface -> USB device is two levels; the extra headroom covers +# drivers that add a level (usb-serial adapters: ttyUSB0/.../port/interface). +_SYSFS_MAX_DEPTH = 4 + + +class FirmwareState(Enum): + """Firmware a serial device appears to run, guessed from its by-id name.""" + + # pylint: disable=invalid-name + Klipper = "Klipper" + Katapult = "Katapult" + Unflashed = "Unflashed" + Unknown = "Unknown" + + +@dataclass +class Device: # pylint: disable=too-many-instance-attributes + """A serial device under /dev/serial/by-id""" + + name: str = "" + manufacturer: str = "" + product: str = "" + serial_number: str = "" + mcu_type: str = "" + firmware: FirmwareState = FirmwareState.Unknown + + symlink_name: str = "" + device_path: str = "" + interface: str = "" + + # USB vendor/product ID of the device's parent USB interface, as set by + # the MCU firmware's Kconfig USB_VENDOR_ID/USB_DEVICE_ID at build time. + # 0 means "not reported" (by-id scan, or a daemon/protocol version + # that doesn't send it yet) - never treat 0 as a real ID. + vendor_id: int = 0 + product_id: int = 0 + + @property + def symlink(self) -> str: + """Full /dev/serial/by-id path""" + return f"{SERIAL_BY_ID_PATH}{self.symlink_name}" + + @property + def vid_hex(self) -> str: + """Vendor ID as '0x1d50', or '' if not reported""" + return f"0x{self.vendor_id:04x}" if self.vendor_id else "" + + @property + def pid_hex(self) -> str: + """Product ID as '0x614e', or '' if not reported""" + return f"0x{self.product_id:04x}" if self.product_id else "" + + @property + def is_klipper(self) -> bool: + """Whether the device runs Klipper firmware""" + return self.firmware == FirmwareState.Klipper + + @property + def is_katapult(self) -> bool: + """Whether the device is in the Katapult (CanBoot) bootloader""" + return self.firmware == FirmwareState.Katapult + + @property + def is_unflashed(self) -> bool: + """Whether the device is a bare USB-serial adapter with no firmware""" + return self.firmware == FirmwareState.Unflashed + + def __repr__(self) -> str: + return ( + f" {self.device_path}> " + f"{self.symlink_name} " + ) + + +_DEVICE_FIELDS = ( + "name", + "manufacturer", + "product", + "serial_number", + "mcu_type", + "symlink_name", + "device_path", + "interface", +) + + +def _firmware(value: str) -> FirmwareState: + try: + return FirmwareState(value) + except ValueError: + return FirmwareState.Unknown + + +def _usb_id(value: Any) -> int: + """Parse a vendor_id/product_id value from the daemon (int, or absent/0).""" + try: + return int(value) + except (TypeError, ValueError): + return 0 + + +def _read_hex(path: str) -> int: + try: + with open(path, encoding="ascii") as f: + return int(f.read().strip(), 16) + except (OSError, ValueError): + return 0 + + +def usb_ids_from_sysfs(dev: Device) -> tuple[int, int]: + """(vendor_id, product_id) of the USB device behind dev's tty, (0, 0) if unknown. + + /sys/class/tty//device is the USB interface the tty belongs to; the + idVendor/idProduct files live on its parent USB device. + """ + tty_path = dev.device_path + if not tty_path and dev.symlink_name: + tty_path = os.path.realpath(dev.symlink) + tty = os.path.basename(tty_path) + if not tty.startswith("tty"): + return 0, 0 + node = os.path.realpath(os.path.join(SYSFS_TTY_PATH, tty, "device")) + for _ in range(_SYSFS_MAX_DEPTH): + vendor_id = _read_hex(os.path.join(node, "idVendor")) + if vendor_id: + return vendor_id, _read_hex(os.path.join(node, "idProduct")) + parent = os.path.dirname(node) + if parent == node: + break + node = parent + return 0, 0 + + +def fill_usb_ids(dev: Device) -> Device: + """Fill dev's vendor/product ID from sysfs when nothing reported one.""" + if not dev.vendor_id: + dev.vendor_id, dev.product_id = usb_ids_from_sysfs(dev) + return dev + + +def device_from_json(data: dict[str, Any]) -> Device: + """Build a Device from one device object of the daemon's protocol.""" + dev = Device( + firmware=_firmware(str(data.get("firmware") or "")), + vendor_id=_usb_id(data.get("vendor_id")), + product_id=_usb_id(data.get("product_id")), + ) + for f in _DEVICE_FIELDS: + setattr(dev, f, str(data.get(f) or "")) + return fill_usb_ids(dev) + + +def serial_from_json(devices: Iterable[Any]) -> list[Device]: + """The daemon's Serial devices (it also reports plain USB ones)""" + return [ + device_from_json(d) + for d in devices + if isinstance(d, dict) and d.get("connection") == "Serial" + ] + + +def _detect_firmware(symlink_name: str) -> FirmwareState: + if "Klipper" in symlink_name: + return FirmwareState.Klipper + if "Katapult" in symlink_name or "CanBoot" in symlink_name: + return FirmwareState.Katapult + if any(m in symlink_name for m in ("1a86", "FTDI", "Silicon_Labs", "Prolific")): + return FirmwareState.Unflashed + return FirmwareState.Unknown + + +def device_from_symlink(symlink_name: str, device_path: str) -> Device: + """Parse a by-id name like usb-Klipper_stm32h723xx_-if00 into a Device""" + dev = Device( + symlink_name=symlink_name, + device_path=device_path, + firmware=_detect_firmware(symlink_name), + ) + rest = symlink_name.removeprefix("usb-") + head, sep, tail = rest.rpartition("-if") + if sep: + rest, dev.interface = head, tail.split("-")[0] + parts = rest.split("_") + if len(parts) >= 3: + dev.manufacturer, dev.serial_number = parts[0], parts[-1] + dev.product = "_".join(parts[1:-1]) + elif len(parts) == 2: + dev.manufacturer, dev.product = parts + else: + dev.manufacturer = rest + if dev.is_klipper or dev.is_katapult: + dev.mcu_type = dev.product.split("_")[0] + dev.name = f"{dev.manufacturer} {dev.mcu_type or dev.product}".strip() + return fill_usb_ids(dev) + + +class SerialScanner: + """Lists /dev/serial/by-id devices via device_discoveryd. + + Falls back to reading /dev/serial/by-id when the daemon is unavailable. + """ + + def __init__(self, socket_path: str | None = None, timeout: float = 2.0) -> None: + self._socket_path = socket_path + self._timeout = timeout + + def daemon_scan(self) -> list[Device] | None: + """Serial devices from device_discoveryd, None when it can't be used""" + devices = fetch_snapshot(self._socket_path, self._timeout) + return None if devices is None else serial_from_json(devices) + + def scan(self) -> list[Device]: + """All serial devices - equivalent to ls /dev/serial/by-id/*""" + devices = self.daemon_scan() + if devices is None: + _logger.info("device_discoveryd unavailable, scanning /dev/serial/by-id") + return self.legacy_scan() + return devices + + def scan_klipper(self) -> list[Device]: + """Devices running Klipper""" + return [d for d in self.scan() if d.is_klipper] + + def scan_katapult(self) -> list[Device]: + """Devices in the Katapult bootloader""" + return [d for d in self.scan() if d.is_katapult] + + def scan_unflashed(self) -> list[Device]: + """Bare USB-serial adapters (CH340, FTDI, CP210x, PL2303)""" + return [d for d in self.scan() if d.is_unflashed] + + @staticmethod + def legacy_scan() -> list[Device]: + """Plain /dev/serial/by-id listing, without the daemon""" + try: + with os.scandir(SERIAL_BY_ID_PATH) as entries: + return [ + device_from_symlink(e.name, os.path.realpath(e.path)) + for e in entries + ] + except OSError: # udev removes by-id when no serial device is plugged in + return [] diff --git a/BlocksScreen/lib/panels/mainWindow.py b/BlocksScreen/lib/panels/mainWindow.py index f96f11ac..802c9dc9 100644 --- a/BlocksScreen/lib/panels/mainWindow.py +++ b/BlocksScreen/lib/panels/mainWindow.py @@ -118,6 +118,12 @@ class MainWindow(QtWidgets.QMainWindow): in_case_error = QtCore.pyqtSignal(name="in-case-error") call_load_panel = QtCore.pyqtSignal(bool, str, bool, name="call-load-panel") + device_config_accepted: typing.ClassVar[QtCore.pyqtSignal] = QtCore.pyqtSignal( + list, name="device-config-accepted" + ) + device_config_declined: typing.ClassVar[QtCore.pyqtSignal] = QtCore.pyqtSignal( + list, name="device-config-declined" + ) def __init__(self): """Set up UI, instantiate subsystems, and wire all inter-component signals.""" @@ -134,6 +140,7 @@ def __init__(self): self._reconnect_timer.setInterval(15_000) self._reconnect_timer.timeout.connect(self._on_reconnect_timer) self._klippy_ready: bool = False + self._klippy_state: str | None = None self._klipper_auto_restart_pending: bool = False self._klipper_restart_timeout = QtCore.QTimer(self) self._klipper_restart_timeout.setSingleShot(True) @@ -197,6 +204,7 @@ def __init__(self): self.conn_window.on_websocket_connection_achieved ) self.ws.connected_signal.connect(self._arm_health_bless) + self.ws.connected_signal.connect(self._try_device_config_restart) self.ws.connection_lost.connect(self.conn_window.on_websocket_connection_lost) self.ws.klippy_state_signal.connect(self._on_klippy_state) self.ws.klippy_state_signal.connect(self.conn_window.on_klippy_state) @@ -356,9 +364,18 @@ def __init__(self): # Source of truth for job activity; keeps print tab off the main page mid-job. self._print_state = "standby" + self._print_state_known = False self.printer.print_stats_update[str, str].connect(self._track_print_state) self.print_status = "idle" + # Plugged-in devices whose printer.cfg template waits for the user's + # OK (the question waits for the end of a running print), and the + # ones the open popup is asking about. + self._pending_device_confirm: list[str] = [] + self._device_confirm_names: list[str] = [] + self._device_config_popup: BasePopup | None = None + # printer.cfg changed for a device; Klipper must restart to load it. + self._device_restart_pending = False self.chamber_temp_display.hide() if self.config.has_section("server"): @@ -455,6 +472,10 @@ def show_update_page(self, fullscreen: bool): @QtCore.pyqtSlot(str, name="on-klippy-state") def _on_klippy_state(self, state: str) -> None: self._klippy_ready = state == "ready" + self._klippy_state = state + if state != "ready": + # print_stats is re-reported once Klipper is ready again. + self._print_state_known = False if state == "disconnected": self._klipper_disconnect_grace.start() else: @@ -469,6 +490,8 @@ def _on_klippy_state(self, state: str) -> None: self._klipper_restart_timeout.stop() if not self._post_update_reconnect: self.loadscreen.hide() + self._try_device_config_restart() + self._prompt_device_confirmation() @QtCore.pyqtSlot(name="auto-restart-klipper") def _auto_restart_klipper(self) -> None: @@ -481,11 +504,15 @@ def _auto_restart_klipper(self) -> None: ): return _logger.info("Klipper disconnected — auto-restarting service") + self._begin_klipper_restart() + self.ws.api.restart_service("klipper") + + def _begin_klipper_restart(self) -> None: + """Mark a Klipper restart in flight so "disconnected" won't restart it again""" self._klipper_auto_restart_pending = True self.loadwidget.set_status_message("Restarting Klipper...") self.loadscreen.show() self._klipper_restart_timeout.start() - self.ws.api.restart_service("klipper") @QtCore.pyqtSlot(name="arm-health-bless") def _arm_health_bless(self) -> None: @@ -676,10 +703,131 @@ def disable_tab_bar(self) -> bool: ] ) + @QtCore.pyqtSlot(list, name="on-device-config-confirmation-required") + def on_device_confirmation_required(self, profiles: list) -> None: + """Ask whether printer.cfg should be configured for newly plugged-in devices. + + Nothing is written until the user confirms (device_config_accepted). + The question waits while a job is running or popups are disabled. + """ + _logger.info("Device configuration prompt requested for %s", profiles) + for name in profiles: + if ( + name not in self._pending_device_confirm + and name not in self._device_confirm_names + ): + self._pending_device_confirm.append(name) + self._prompt_device_confirmation() + + def _device_prompt_blocked(self) -> str: + """Why the device prompt has to wait ('' when it can be shown).""" + if self._is_job_active() or self.print_status == "printing": + return "a job is running" + if self._popup_toggle: + return "popups are disabled" + if self._update_in_progress: + return "an update is running" + return "" + + def _prompt_device_confirmation(self) -> None: + if not self._pending_device_confirm: + return + reason = self._device_prompt_blocked() + if reason: + _logger.info( + "Device configuration prompt for %s deferred: %s", + self._pending_device_confirm, + reason, + ) + return + try: + self._show_device_confirmation() + except Exception: # noqa: BLE001 # pylint: disable=broad-except + # Optional prompt: never let it take the screen down. + _logger.exception("Could not show the device configuration prompt") + + def _show_device_confirmation(self) -> None: + self._device_confirm_names.extend(self._pending_device_confirm) + self._pending_device_confirm = [] + if self._device_config_popup is None: + self._device_config_popup = BasePopup(self, floating=True) + self._device_config_popup.accepted.connect(self._on_device_config_accept) + self._device_config_popup.rejected.connect(self._on_device_config_decline) + devices = ", ".join(n.upper() for n in self._device_confirm_names) + self._device_config_popup.set_message( + f"New device detected: {devices}\n" + "Update the printer configuration for it\n" + "and restart Klipper?" + ) + self._device_config_popup.confirm_button_text("Configure") + self._device_config_popup.cancel_button_text("Not now") + if not self._device_config_popup.isVisible(): + self._device_config_popup.open() + _logger.info("Device configuration prompt shown for %s", devices) + + @QtCore.pyqtSlot(name="on-device-config-accept") + def _on_device_config_accept(self) -> None: + names, self._device_confirm_names = self._device_confirm_names, [] + if names: + self.device_config_accepted.emit(names) + + @QtCore.pyqtSlot(name="on-device-config-decline") + def _on_device_config_decline(self) -> None: + names, self._device_confirm_names = self._device_confirm_names, [] + if names: + self.device_config_declined.emit(names) + + @QtCore.pyqtSlot(list, name="on-device-config-changed") + def on_device_config_changed(self, profiles: list) -> None: + """printer.cfg was updated for plugged-in devices: restart Klipper.""" + _logger.info("printer.cfg updated for %s", ", ".join(profiles)) + self.request_klipper_restart() + + def request_klipper_restart(self) -> None: + """Restart Klipper as soon as it is safe (never during a job).""" + self._device_restart_pending = True + self._try_device_config_restart() + + def _device_restart_blocked(self) -> str: + """Why the Klipper restart has to wait ('' when it can go ahead).""" + if self._klippy_state is None: + return "Klipper state not known yet" + if self._klippy_state == "ready" and not self._print_state_known: + return "print state not known yet" + if self._is_job_active() or self.print_status == "printing": + return "a job is running" + if self._update_in_progress: + return "an update is running" + if self._klipper_auto_restart_pending: + return "a Klipper restart is already in progress" + return "" + + @QtCore.pyqtSlot(name="try-device-config-restart") + def _try_device_config_restart(self) -> None: + if not self._device_restart_pending: + return + reason = self._device_restart_blocked() + if reason: + _logger.info("Klipper restart for new device config deferred: %s", reason) + return + # A service restart (not FIRMWARE_RESTART) works whatever state + # Klippy is in, including when the old config kept it from starting. + if not self.ws.api.restart_service("klipper"): + _logger.warning( + "Klipper restart for new device config deferred: " + "Moonraker not connected" + ) + return + _logger.info("Restarting Klipper to load the new device config") + self._device_restart_pending = False + self._begin_klipper_restart() + @QtCore.pyqtSlot(bool, name="toggle-popups") def popup_toggle(self, toggle: bool) -> None: """Toggles app popups""" self._popup_toggle = toggle + if not toggle: + self._prompt_device_confirmation() @QtCore.pyqtSlot(bool, name="set-ui-lock") def set_ui_lock(self, locked: bool) -> None: @@ -703,6 +851,10 @@ def _track_print_state(self, field: str, value: str) -> None: """Track ``print_stats.state`` as the source of truth for job activity.""" if field == "state": self._print_state = value + self._print_state_known = True + if not self._is_job_active(): + self._try_device_config_restart() + self._prompt_device_confirmation() def _is_job_active(self) -> bool: """True while a job occupies the printer; paused included so runout pauses hold the page.""" @@ -1270,6 +1422,8 @@ def event(self, event: QtCore.QEvent) -> bool: events.PrintCancelled.type(), ): self.print_status = "idle" + QtCore.QTimer.singleShot(0, self._prompt_device_confirmation) + QtCore.QTimer.singleShot(0, self._try_device_config_restart) if event.type() == events.PrintCancelled.type(): self.handle_cancel_print() self.enable_tab_bar() diff --git a/BlocksScreen/tools/configuration_manager/USB_IDS.md b/BlocksScreen/tools/configuration_manager/USB_IDS.md new file mode 100644 index 00000000..95d3a512 --- /dev/null +++ b/BlocksScreen/tools/configuration_manager/USB_IDS.md @@ -0,0 +1,80 @@ +# BlocksTechnology USB vendor/product IDs + +Klipper (and Katapult, its bootloader) let you set the USB `idVendor`/ +`idProduct` a board's firmware reports, per build, via `make menuconfig` → +enable "Enable extra low-level configuration options" (`LOW_LEVEL_OPTIONS`) +→ "USB ids" → `USB_VENDOR_ID` / `USB_DEVICE_ID` (`src/Kconfig` in both +projects). Stock builds all report the same pair (Klipper `1d50:614e`, +Katapult `1d50:6177`), which is why device identification in this repo has +so far relied on the `/dev/serial/by-id` symlink name instead (see +`device_profiles.yaml`) - every stock-ID board looks identical on the bus. + +This table is the standard set of IDs for BlocksTechnology-specific boards +(starting with AMU), so a board can be identified by `vendor_id`/ +`product_id` alone - reliable even if someone changes the USB descriptor +strings that the symlink name (and therefore the regex `match` fallback) +is built from. + +## Vendor ID + +Keep `0x1d50` (OpenMoko) - the same VID stock Klipper/Katapult already use, +so a Blocks board still reads as "part of the Klipper family". **This VID +is informally, not centrally, shared**: Klipper's own docs credit OpenMoko +for `0x614e`, but there is no single authority enforcing sub-allocations +under `0x1d50` (the pid.codes registry's shared VID is actually a +*different* one, `0x1209` - see below). Collision risk against `0x1d50` is +therefore non-zero, if low. For a permanent, collision-free allocation, +register a real range through (their shared VID +`0x1209`, free for open-source projects) and migrate this table to it - do +that before this scheme is relied on outside our own fleet. + +## Product ID range + +**`0xB000`-`0xB0FF`** reserved for BlocksTechnology boards. Checked against +the community `usb.ids` database (`linux-usb.org`, mirrored at +`github.com/gentoo/hwids`) as of 2026-09-23: nothing under `0x1d50` is +registered between `0x6123` and `0xFFFF` except `0x8085` and `0xcc15`, so +this block is clear of every *documented* device - it is not a guarantee +against an undocumented/unregistered use of the same numbers elsewhere. + +Layout: firmware PID in `0xB0`**`0`**`0`-`0xB0`**`7`**`F`, the matching +Katapult-bootloader PID for the same board at `+0x80` (`0xB0`**`8`**`0`- +`0xB0FF`) - so `board_pid | 0x80` always finds its own bootloader without a +second lookup table. + +| Board | Klipper firmware | Katapult bootloader | +|---|---|---| +| AMU V1| `0xB001` | `0xB081` | +| *(next board)* | `0xB002` | `0xB082` | + +To add a board: take the next unused low byte, keep the `+0x80` bootloader +pairing, add a row here, and add/update its entry in `device_profiles.yaml` +(`vendor_id: '0x1d50'`, `product_id: '0xb0xx'`). + +## Applying to a board's firmware build + +In that board's Klipper `.config` (and its Katapult `.config`, using the +bootloader PID): + +``` +CONFIG_LOW_LEVEL_OPTIONS=y +CONFIG_USB_VENDOR_ID=0x1d50 +CONFIG_USB_DEVICE_ID=0xb001 # this board's PID from the table above +``` + +## How matching uses the IDs + +Each serial device's USB vendor/product ID comes from device_discoveryd when +it reports one; otherwise BlocksScreen reads it from sysfs itself - the +`idVendor`/`idProduct` of the USB device behind the tty +(`devices/discovery/serial_devices.py`, `usb_ids_from_sysfs`). The daemon +currently sends `0` for `Serial` entries, so in practice the sysfs lookup is +what supplies the IDs, for daemon reports and the legacy by-id scan alike. + +`DeviceProfile.matches()` treats a known ID as final for profiles that set +`vendor_id`: a board still on Klipper's stock `1d50:614e` does **not** match +the AMU profile, even if its USB serial string contains "AMU". Reflash every +AMU with its assigned PID before relying on detection. + +The profile's `match` regex is only used when no ID can be found at all +(device not backed by a USB tty in sysfs). diff --git a/BlocksScreen/tools/configuration_manager/__init__.py b/BlocksScreen/tools/configuration_manager/__init__.py index 9bfb8511..a400d89c 100644 --- a/BlocksScreen/tools/configuration_manager/__init__.py +++ b/BlocksScreen/tools/configuration_manager/__init__.py @@ -1,6 +1,8 @@ """Configuration manager package contains tools that automatically configure a Blocks machine.""" from .configurator import ConfigManager +from .device_configurator import DeviceConfigManager +from .device_watcher import DeviceConfigWatcher __version__ = "0.0.1" -__all__ = ["ConfigManager"] +__all__ = ["ConfigManager", "DeviceConfigManager", "DeviceConfigWatcher"] diff --git a/BlocksScreen/tools/configuration_manager/configurator.py b/BlocksScreen/tools/configuration_manager/configurator.py index 9f809ca5..cd619be3 100644 --- a/BlocksScreen/tools/configuration_manager/configurator.py +++ b/BlocksScreen/tools/configuration_manager/configurator.py @@ -11,7 +11,7 @@ from datetime import datetime from typing import Literal -from tools.serial_scanner import SerialScanner +from devices.discovery.serial_devices import SerialScanner HOME = pathlib.Path.home() CONFIG_REPO: pathlib.Path = HOME / "RF50-Klipper" @@ -109,11 +109,7 @@ def check_broken_symlinks(dir: pathlib.Path) -> list[pathlib.Path]: def resolve_symlink(file: pathlib.Path, resolved: set[str]) -> bool: - """Check if `file` has a valid symlink on `target` dir - - Returns: - bool: whether or not the provided directory has a working symlink pointing to the src file - """ + """Whether `file` resolves to one of the `resolved` symlink targets""" return file.resolve().as_posix() in resolved @@ -330,7 +326,7 @@ def _get_missing_symlinks( # ignore .git dir files if ".git" in d.parts: continue - if d.resolve().as_posix() not in _resolved: + if not resolve_symlink(d, _resolved): _missing.append(d) return _missing @@ -523,9 +519,7 @@ def merge_cfg( ) for k, v in src_keys.items(): if k not in tgt_keys: - merged_save_lines.append( - f"#*# \t{k} = {v}\n" - ) + merged_save_lines.append(f"#*# {k} = {v}\n") # Then: append any sections that exist only in source for name, block in src_sc_sections: @@ -733,23 +727,14 @@ def _fix_beacon_serial(self, config_text: str) -> str: return config_text def sync(self) -> None: - """Synchronizes configuration repo with the - machines configuration""" + """Synchronizes configuration repo with the machines configuration""" + # Every step only writes on a difference, so none is gated. try: - _missing = self._get_missing_symlinks(self.config_dir, self.repo) - _logger.info(_missing) - if _missing or any(self.cmp_cpy_files()): - if self.check_nested(): - self._cleanup_nested() - - self.cleanup_broken_symlinks(self.config_dir) - self._symlink_config(_missing) - self._cpy_cfg_files() - else: - _logger.info("Configuration correct, proceding") - except NotADirectoryError as e: - _logger.error("%s" % e) - except FileNotFoundError as e: - _logger.error("%s" % e) + missing = self._get_missing_symlinks(self.config_dir, self.repo) + if self.check_nested(): + self._cleanup_nested() + self.cleanup_broken_symlinks(self.config_dir) + self._symlink_config(missing) + self._cpy_cfg_files() except Exception as e: - _logger.error("Caught exception: %s" % e) + _logger.error("Caught exception during sync: %s", e) diff --git a/BlocksScreen/tools/configuration_manager/device_configurator.py b/BlocksScreen/tools/configuration_manager/device_configurator.py new file mode 100644 index 00000000..a7cfbc10 --- /dev/null +++ b/BlocksScreen/tools/configuration_manager/device_configurator.py @@ -0,0 +1,279 @@ +"""Device-aware configuration manager. + +`DeviceConfigManager` is `ConfigManager` (configurator.py, left unchanged) +plus device templates (device_profiles.py): + +- At startup it syncs ~/printer_data/config from the config repo as before, + then applies the templates of the devices plugged in right now. So + printer.cfg always equals the repo's version plus what the connected + devices require - an AMU unplugged since the last boot is reverted too. +- While running, `apply_devices()` applies templates for devices reported by + the discovery daemon (see device_watcher.py). + +Profiles marked `confirm: true` (the AMU) are only applied once the user has +accepted them (`accept()`, persisted in `DeviceConsent`). Until then boot +leaves printer.cfg alone for them and `pending_confirmation()` reports them, +so the UI can ask. +""" + +from __future__ import annotations + +import json +import logging +import os +import pathlib +import shutil +import tempfile +from collections.abc import Iterable, Sequence + +from devices.discovery.serial_devices import Device, SerialScanner + +from .configurator import ConfigManager +from .device_profiles import ( + DeviceProfile, + apply_device_profiles, + load_profiles, + match_devices, +) + +_logger = logging.getLogger(__name__) + +PRINTER_CFG = "printer.cfg" +# The boot scan runs before the UI exists. A healthy daemon answers in +# milliseconds; this bounds how long a hung one can delay startup before the +# legacy /dev/serial/by-id scan is used instead. +BOOT_SCAN_TIMEOUT = 1.0 + +CONSENT_PATH = pathlib.Path.home() / ".config" / "blockscreen" / "device_consent.json" + + +def atomic_write_text(path: pathlib.Path, text: str) -> None: + """Replace path's content in one step: readers see the old or the new file. + + Klipper may read printer.cfg at any moment (its own start, a restart), and + a power cut mid-write must not leave a truncated file - the next boot's + merge would then rebuild it from the repo and lose the SAVE_CONFIG block. + Symlinks are followed so the link itself is kept. + """ + target = path.resolve() + fd, tmp = tempfile.mkstemp(dir=target.parent, prefix=f".{target.name}.") + try: + with os.fdopen(fd, "w", encoding="utf-8") as f: + f.write(text) + f.flush() + os.fsync(f.fileno()) + if target.exists(): + shutil.copymode(target, tmp) + os.replace(tmp, target) + except BaseException: + try: + os.unlink(tmp) + except OSError: + pass + raise + + +class DeviceConsent: + """Profile names the user accepted, kept across reboots.""" + + def __init__(self, path: pathlib.Path | None = CONSENT_PATH) -> None: + self._path = path + self._accepted: set[str] = set() + if path is None: + return + try: + data = json.loads(path.read_text(encoding="utf-8")) + self._accepted = {str(n) for n in data.get("accepted", [])} + except FileNotFoundError: + pass + except (OSError, ValueError, AttributeError, TypeError) as e: + _logger.error("Ignoring unreadable device consent file %s: %s", path, e) + + def is_accepted(self, name: str) -> bool: + """Whether the user accepted the profile called name""" + return name in self._accepted + + def accept(self, names: Iterable[str]) -> None: + """Remember names as accepted (saved immediately)""" + new = set(names) - self._accepted + if not new: + return + self._accepted |= new + if self._path is None: + return + try: + self._path.parent.mkdir(parents=True, exist_ok=True) + atomic_write_text( + self._path, json.dumps({"accepted": sorted(self._accepted)}) + "\n" + ) + except OSError as e: + # Still accepted for this session; boot will just ask again. + _logger.error("Cannot save device consent to %s: %s", self._path, e) + + +class DeviceConfigManager(ConfigManager): + """ConfigManager that edits printer.cfg for the devices that are plugged in.""" + + def __init__( + self, + config, + profiles: Sequence[DeviceProfile] | None = None, + consent: DeviceConsent | None = None, + ) -> None: + # ConfigManager.__init__ ends by calling sync(), which needs these. + self._profiles: list[DeviceProfile] = list( + load_profiles() if profiles is None else profiles + ) + self._consent = consent if consent is not None else DeviceConsent() + self._devices: list[Device] = [] + self._merging: str | None = None + # Set by sync(): printer.cfg differs from what it was before this + # boot's sync, so a Klipper that already loaded it must restart. + self.boot_changed = False + _logger.info( + "Device profiles: %s", ", ".join(p.name for p in self._profiles) or "none" + ) + super().__init__(config) + + @property + def printer_cfg(self) -> pathlib.Path: + """The machine's printer.cfg""" + return self.config_dir / PRINTER_CFG + + def _allowed_profiles(self) -> list[DeviceProfile]: + return [ + p + for p in self._profiles + if not p.confirm or self._consent.is_accepted(p.name) + ] + + @staticmethod + def scan_devices() -> list[Device]: + """Serial devices plugged in now: the daemon's view plus /dev/serial/by-id. + + Both are consulted because the daemon can answer before it has + enumerated a board udev already linked (early boot), and a device + missing here gets its config stripped from printer.cfg. + """ + devices: list[Device] = [] + try: + devices = SerialScanner(timeout=BOOT_SCAN_TIMEOUT).daemon_scan() or [] + except Exception as e: # noqa: BLE001 # pylint: disable=broad-except + # Never let a scan problem stop the config sync at boot. + _logger.error("Device scan failed: %s", e) + try: + seen = {d.symlink_name for d in devices} + devices += [ + d for d in SerialScanner.legacy_scan() if d.symlink_name not in seen + ] + except Exception as e: # noqa: BLE001 # pylint: disable=broad-except + _logger.error("/dev/serial/by-id scan failed: %s", e) + return devices + + def _read_printer_cfg(self) -> str | None: + try: + return self.printer_cfg.read_text(encoding="utf-8") + except OSError: + return None + + def sync(self) -> None: + """Sync the machine configuration with the repo, then apply device templates""" + before = self._read_printer_cfg() + self._devices = self.scan_devices() + _logger.info( + "Devices at sync: %s", + ", ".join(d.symlink_name for d in self._devices) or "none", + ) + super().sync() + # printer.cfg is skipped above when already equal to the repo's copy. + try: + self.apply_devices(self._devices) + except Exception as e: # noqa: BLE001 # pylint: disable=broad-except + _logger.error("Applying device templates failed: %s", e) + self.boot_changed = self._read_printer_cfg() != before + if self.boot_changed: + _logger.info("printer.cfg changed during boot sync") + + def merge_cfg(self, src_file, target_file, marker="") -> bool: + """ConfigManager.merge_cfg, remembering which file is being merged""" + self._merging = pathlib.Path(target_file).name + try: + return super().merge_cfg(src_file, target_file, marker) + finally: + self._merging = None + + def _fix_beacon_serial(self, config_text: str) -> str: + # Hook ConfigManager.merge_cfg calls on every merged file before + # writing it. Applying the templates here (printer.cfg only) means + # a boot with an AMU plugged in writes printer.cfg once, instead of + # writing the repo layout and then the AMU layout. Beacon is one of + # the templates, so the old beacon lookup isn't needed. + if self._merging != PRINTER_CFG: + return config_text + text, _ = apply_device_profiles( + config_text, self._allowed_profiles(), self._devices + ) + return text + + def matching_profiles(self, device: Device) -> list[str]: + """Names of the profiles that recognise device (for logging)""" + return [p.name for p in self._profiles if p.matches(device)] + + def pending_confirmation(self, devices: Sequence[Device]) -> list[str]: + """Matched profiles waiting for the user's OK that would change printer.cfg""" + waiting = [ + p + for p in self._profiles + if p.confirm and not self._consent.is_accepted(p.name) + ] + if not devices or not waiting: + return [] + text = self._read_printer_cfg() + if text is None: + _logger.warning( + "Cannot read %s to check device templates", self.printer_cfg + ) + return [] + names = [] + for profile, device in match_devices(waiting, devices): + _, changed = apply_device_profiles(text, [profile], [device]) + if changed: + names.append(profile.name) + else: + _logger.info( + "%s matches %s but printer.cfg already has its layout", + device.symlink_name, + profile.name, + ) + return names + + def accept(self, names: Iterable[str]) -> None: + """Record that the user accepted these profiles (then call apply_devices).""" + self._consent.accept(names) + + def apply_devices(self, devices: Sequence[Device]) -> list[str]: + """Apply the allowed templates matching devices to printer.cfg. + + Returns the names of the profiles that changed the file (empty when + nothing had to change or printer.cfg couldn't be read/written). + """ + profiles = self._allowed_profiles() + if not devices or not profiles: + return [] + path = self.printer_cfg + with self.mergeLock: + try: + text = path.read_text(encoding="utf-8") + except OSError as e: + _logger.error("Cannot read %s: %s", path, e) + return [] + new_text, changed = apply_device_profiles(text, profiles, devices) + if not changed: + return [] + try: + atomic_write_text(path, new_text) + except OSError as e: + _logger.error("Cannot write %s: %s", path, e) + return [] + _logger.info("printer.cfg updated for: %s", ", ".join(changed)) + return changed diff --git a/BlocksScreen/tools/configuration_manager/device_profiles.py b/BlocksScreen/tools/configuration_manager/device_profiles.py new file mode 100644 index 00000000..91732728 --- /dev/null +++ b/BlocksScreen/tools/configuration_manager/device_profiles.py @@ -0,0 +1,416 @@ +"""Device templates: which printer.cfg changes a plugged-in device requires. + +Profiles come from device_profiles.yaml (bundled next to this module), merged +by name with an optional per-machine override in ~/printer_data/config. Each +profile says how to recognise a device from its /dev/serial/by-id name and +what to do to printer.cfg when it's present: create or update one section +from templated options, and comment/uncomment [include ...] lines. + +Everything here is a pure text transform (only load_profiles touches the +filesystem), and every transform is idempotent: applying a profile to text +it was already applied to returns the text unchanged, so callers can apply +on every boot and every hotplug event and only write when something changed. +""" + +from __future__ import annotations + +import logging +import pathlib +import re +import string +from collections.abc import Iterable, Sequence +from dataclasses import dataclass +from typing import Any + +import yaml +from devices.discovery.serial_devices import Device + +from .configurator import SV_CONFIG_MARKER + +_logger = logging.getLogger(__name__) + +BUNDLED_PATH = pathlib.Path(__file__).with_name("device_profiles.yaml") +OVERRIDE_PATH = ( + pathlib.Path.home() / "printer_data" / "config" / "blockscreen_devices.yaml" +) + + +_CONTROL_CHARS = re.compile(r"[\x00-\x08\x0a-\x1f\x7f]") + + +def _sanitize(value: str) -> str: + """Strip control characters (newlines included) from device-derived text. + + printer.cfg is plain text where a bare newline starts a new directive: + a device string (name/serial/path) that reaches a config line unfiltered + could inject extra options or even a whole [section]. Values here can + originate from the device's own USB descriptor strings, which a + corrupted or malicious device fully controls - so this runs on every + value before it's substituted into a template, not just on ones today's + bundled profiles happen to use. + """ + return _CONTROL_CHARS.sub("", value) + + +@dataclass(frozen=True) +class DeviceProfile: # pylint: disable=too-many-instance-attributes + """One device template (see device_profiles.yaml for the format).""" + + name: str + match: re.Pattern[str] + section: str + options: tuple[tuple[str, str], ...] = () + enable_includes: tuple[str, ...] = () + disable_includes: tuple[str, ...] = () + # USB vendor/product ID (from Klipper's Kconfig USB_VENDOR_ID/USB_DEVICE_ID), + # when the profile wants ID-based matching instead of the symlink regex. + vendor_id: int | None = None + product_id: int | None = None + # Ask the user before applying (and before re-applying at boot until they + # have accepted once) - for devices whose config swaps a whole printer + # variant, like the AMU. + confirm: bool = False + + def matches(self, device: Device) -> bool: + """Whether this profile applies to the given serial device. + + Prefers the USB vendor/product ID when both this profile and the + device carry one - falls back to the symlink-name regex otherwise + (device.vendor_id is 0 until both the daemon and the device's + firmware report real IDs, so behaviour is unchanged until then). + """ + if self.vendor_id is not None and device.vendor_id: + if device.vendor_id != self.vendor_id: + return False + return self.product_id is None or device.product_id == self.product_id + return re.search(self.match, device.symlink_name) is not None + + def render_options(self, device: Device) -> list[tuple[str, str]]: + """Options with ${serial}/${device_path}/${name} filled in for device. + + The substituted values ultimately come from the device's own USB + descriptor strings (name is built from manufacturer/product) - a + corrupted or malicious device can put anything there, including + control characters. _sanitize strips those so a plugged-in device + can never inject an extra config line/section via a template value. + """ + values = { + "serial": _sanitize(device.symlink), + "device_path": _sanitize(device.device_path), + "name": _sanitize(device.name), + } + return [ + (key, string.Template(value).safe_substitute(values)) + for key, value in self.options + ] + + +# -------------------------------------------------------------------------- +# Loading +# -------------------------------------------------------------------------- + + +def _read_entries(path: pathlib.Path) -> list[dict[str, Any]]: + with open(path, encoding="utf-8") as f: + data = yaml.safe_load(f) or {} + entries = data.get("profiles", []) if isinstance(data, dict) else [] + return [e for e in entries if isinstance(e, dict)] + + +def _merge(base: list[dict], override: list[dict]) -> list[dict]: + """Merge override entries into base by name (as updater/components.py does).""" + merged = {e["name"]: dict(e) for e in base if e.get("name")} + for entry in override: + name = entry.get("name") + if not name: + continue + if name in merged: + merged[name].update({k: v for k, v in entry.items() if v is not None}) + else: + merged[name] = dict(entry) + return list(merged.values()) + + +def _str_list(value: Any) -> tuple[str, ...]: + if value is None: + return () + if isinstance(value, str): + return (value,) + return tuple(str(v) for v in value) + + +def _usb_id(value: Any, key: str) -> int | None: + """Parse an optional vendor_id/product_id: int, or hex/decimal string.""" + if value is None: + return None + try: + return int(value) if isinstance(value, int) else int(str(value), 0) + except ValueError as e: + raise ValueError(f"'{key}' must be an integer or hex string") from e + + +def _bool(value: Any, key: str) -> bool: + if not isinstance(value, bool): + raise TypeError(f"'{key}' must be true or false") + return value + + +def _parse(entry: dict[str, Any]) -> DeviceProfile | None: + name = entry.get("name") + try: + if not entry.get("match") or not entry.get("section"): + raise ValueError("'match' and 'section' are required") + options = entry.get("options") or {} + if not isinstance(options, dict): + raise TypeError("'options' must be a mapping") + product_id = _usb_id(entry.get("product_id"), "product_id") + if product_id is not None and entry.get("vendor_id") is None: + raise ValueError("'product_id' requires 'vendor_id'") + return DeviceProfile( + name=str(name), + match=re.compile(str(entry["match"]), re.IGNORECASE), + section=" ".join(str(entry["section"]).split()), + options=tuple((str(k), str(v)) for k, v in options.items()), + enable_includes=_str_list(entry.get("enable_includes")), + disable_includes=_str_list(entry.get("disable_includes")), + vendor_id=_usb_id(entry.get("vendor_id"), "vendor_id"), + product_id=product_id, + confirm=_bool(entry.get("confirm", False), "confirm"), + ) + except (ValueError, TypeError, re.error) as e: + _logger.error("Skipping device profile %r: %s", name, e) + return None + + +def _override_is_safe(path: pathlib.Path) -> bool: + # The override can inject arbitrary Klipper config: refuse one that + # other users could have written, like the updater does for its own. + try: + mode = path.stat().st_mode + except OSError: + _logger.warning("Cannot stat device profile override %s, skipping", path) + return False + if mode & 0o022: + _logger.warning( + "Skipping device profile override %s: writable by group/others (mode %o)", + path, + mode & 0o777, + ) + return False + return True + + +def load_profiles( + bundled: pathlib.Path = BUNDLED_PATH, + override: pathlib.Path | None = OVERRIDE_PATH, +) -> list[DeviceProfile]: + """Bundled profiles merged with the per-machine override, enabled ones only""" + try: + entries = _read_entries(bundled) + except (OSError, yaml.YAMLError) as e: + _logger.error("Cannot read bundled device profiles %s: %s", bundled, e) + entries = [] + + if override is not None and override.exists() and _override_is_safe(override): + try: + entries = _merge(entries, _read_entries(override)) + _logger.info("Device profile override applied from %s", override) + except (OSError, yaml.YAMLError) as e: + _logger.error("Cannot read device profile override %s: %s", override, e) + + profiles = [] + for entry in entries: + if entry.get("enabled", True) is False: + continue + profile = _parse(entry) + if profile is not None: + profiles.append(profile) + _warn_duplicate_ids(profiles) + return profiles + + +def _warn_duplicate_ids(profiles: Sequence[DeviceProfile]) -> None: + """Flag profiles that would both claim the same USB vendor_id/product_id. + + A copy-paste mistake in the override (or an unlucky new allocation) would + otherwise silently apply both profiles to the same physical device - see + USB_IDS.md for how product_id is meant to be assigned. + """ + seen: dict[tuple[int, int | None], str] = {} + for p in profiles: + if p.vendor_id is None: + continue + key = (p.vendor_id, p.product_id) + if key in seen: + _logger.error( + "Device profiles %r and %r both match vendor_id=0x%04x product_id=%s " + "- both will apply to the same device", + seen[key], + p.name, + p.vendor_id, + f"0x{p.product_id:04x}" if p.product_id is not None else "any", + ) + else: + seen[key] = p.name + + +# -------------------------------------------------------------------------- +# Text transforms +# -------------------------------------------------------------------------- + +_ANY_SECTION = re.compile(r"^\[[^\]]+\]") + + +def _section_header_re(section: str) -> re.Pattern[str]: + words = r"\s+".join(re.escape(w) for w in section.split()) + return re.compile(rf"^\[\s*{words}\s*\]", re.IGNORECASE) + + +def _split_marker(text: str) -> tuple[str, str]: + """(editable header, SAVE_CONFIG tail) - profiles never touch the tail""" + pos = text.find(SV_CONFIG_MARKER) + if pos < 0: + return text, "" + # Keep the marker's own line (and anything after) in the tail. + line_start = text.rfind("\n", 0, pos) + 1 + return text[:line_start], text[line_start:] + + +def _section_span(lines: list[str], start: int) -> int: + """Index one past the last line of the section whose header is at start""" + end = start + 1 + while end < len(lines) and not _ANY_SECTION.match(lines[end]): + end += 1 + return end + + +_MCU_SECTION = re.compile(r"^\[\s*mcu(\s[^\]]*)?\]", re.IGNORECASE) + + +def _insert_section( + lines: list[str], section: str, options: Sequence[tuple[str, str]] +) -> None: + # Just before the last [mcu ...] header, so device MCUs sit with the + # others. Always at a section boundary, never inside a block: + # ConfigManager.merge_cfg treats every line up to the next header as part + # of the section above (banners included), so splitting a block would + # make its trailing banner belong to the new section and vanish with it + # when the device's section is dropped at the next boot. + mcu_idxs = [i for i, line in enumerate(lines) if _MCU_SECTION.match(line)] + if mcu_idxs: + block = [f"[{section}]\n"] + [f"{k}: {v}\n" for k, v in options] + ["\n"] + insert_at = mcu_idxs[-1] + else: + block = ["\n", f"[{section}]\n"] + [f"{k}: {v}\n" for k, v in options] + insert_at = len(lines) + lines[insert_at:insert_at] = block + + +def _update_options( + lines: list[str], idx: int, options: Sequence[tuple[str, str]] +) -> None: + end = _section_span(lines, idx) + for key, value in options: + opt_re = re.compile(rf"^(\s*){re.escape(key)}\s*[:=]\s*(.*?)\s*$") + for i in range(idx + 1, end): + m = opt_re.match(lines[i].rstrip("\n")) + if m: + if m.group(2).split("#", 1)[0].strip() != value: + lines[i] = f"{m.group(1)}{key}: {value}\n" + break + else: + lines.insert(idx + 1, f"{key}: {value}\n") + end += 1 + + +def set_section_options( + text: str, section: str, options: Sequence[tuple[str, str]] +) -> str: + """Create [section] or update its options so each has the given value.""" + header, tail = _split_marker(text) + lines = header.splitlines(keepends=True) + if lines and not lines[-1].endswith("\n"): + lines[-1] += "\n" + header_re = _section_header_re(section) + idx = next((i for i, line in enumerate(lines) if header_re.match(line)), None) + if idx is None: + _insert_section(lines, section, options) + else: + _update_options(lines, idx, options) + return "".join(lines) + tail + + +def set_include(text: str, path: str, enabled: bool) -> str: + """Comment (enabled=False) or uncomment every [include path] line.""" + header, tail = _split_marker(text) + # [ \t] rather than \s: \s also matches newlines, which would let `#*` + # swallow a "#####" banner line above the include. + include_re = re.compile( + rf"^([ \t]*)(#*)[ \t]*(\[[ \t]*include[ \t]+{re.escape(path)}[ \t]*\].*)$", + re.MULTILINE, + ) + + def _fix(m: re.Match[str]) -> str: + indent, hashes, rest = m.group(1), m.group(2), m.group(3) + if enabled: + return f"{indent}{rest}" + return m.group(0) if hashes else f"{indent}#{rest}" + + return include_re.sub(_fix, header) + tail + + +def apply_profile(text: str, profile: DeviceProfile, device: Device) -> str: + """printer.cfg text with profile applied for device (idempotent).""" + if profile.options: + text = set_section_options( + text, profile.section, profile.render_options(device) + ) + for path in profile.enable_includes: + text = set_include(text, path, True) + for path in profile.disable_includes: + text = set_include(text, path, False) + return text + + +def match_devices( + profiles: Iterable[DeviceProfile], devices: Sequence[Device] +) -> list[tuple[DeviceProfile, Device]]: + """(profile, device) for each profile with a matching device; first match wins. + + A profile has one printer.cfg section to give, so if more than one + connected device matches it, only the first (in `devices` order) is + used - logged rather than silently dropping the rest, since it usually + means two boards of the same type are plugged in at once. + """ + pairs = [] + for profile in profiles: + matches = [d for d in devices if profile.matches(d)] + if not matches: + continue + if len(matches) > 1: + _logger.warning( + "Profile %r matches %d connected devices (%s); using %s, " + "ignoring the rest", + profile.name, + len(matches), + ", ".join(d.symlink_name or d.name or "?" for d in matches), + matches[0].symlink_name or matches[0].name or "?", + ) + pairs.append((profile, matches[0])) + return pairs + + +def apply_device_profiles( + text: str, profiles: Iterable[DeviceProfile], devices: Sequence[Device] +) -> tuple[str, list[str]]: + """Apply every profile that has a matching device. + + Returns the new text and the names of the profiles that changed it. + """ + changed = [] + for profile, device in match_devices(profiles, devices): + new_text = apply_profile(text, profile, device) + if new_text != text: + changed.append(profile.name) + text = new_text + return text, changed diff --git a/BlocksScreen/tools/configuration_manager/device_profiles.yaml b/BlocksScreen/tools/configuration_manager/device_profiles.yaml new file mode 100644 index 00000000..050f972a --- /dev/null +++ b/BlocksScreen/tools/configuration_manager/device_profiles.yaml @@ -0,0 +1,64 @@ +# Device templates: how printer.cfg changes when a known device is plugged in. +# +# Override or extend per machine in ~/printer_data/config/blockscreen_devices.yaml +# (same format). Entries merge by `name`: listed keys replace the bundled +# ones, unknown names are added, `enabled: false` switches a profile off. +# The override is ignored if it's writable by group/others. +# +# Keys: +# match case-insensitive regex searched in the device's +# /dev/serial/by-id name (e.g. usb-Beacon_Beacon_RevH_...-if00) +# section printer.cfg section to create or update (without brackets) +# options option -> value; placeholders: ${serial} (full by-id path), +# ${device_path} (/dev/ttyACM*), ${name} +# enable_includes [include ...] paths to uncomment +# disable_includes [include ...] paths to comment out +# vendor_id optional USB vendor ID (int or hex string, e.g. '0x1d50'). +# When set and the device reports a nonzero vendor_id +# (device_discoveryd reads it from sysfs), matching uses +# the IDs instead of `match`, and a mismatch is final. +# `match` stays required as the fallback when no ID is +# reported (legacy scan without the daemon, older daemon). +# product_id optional USB product ID (int or hex string), only +# meaningful together with vendor_id. Klipper sets both +# at build time via Kconfig's "USB ids" menu +# (USB_VENDOR_ID/USB_DEVICE_ID, under LOW_LEVEL_OPTIONS). +# See USB_IDS.md (next to this file) for the standard +# BlocksTechnology vendor_id/product_id allocation table. +# confirm true: ask the user before applying (Klipper is restarted +# after they accept). Once accepted, the profile is also +# re-applied at every boot without asking; until then boot +# leaves printer.cfg alone and the prompt comes up with the +# UI. Default false (applied silently, like beacon). +# +# Profiles are applied at every boot for the devices plugged in at that +# moment (on top of the RF50-Klipper printer.cfg), and live when the +# device_discoveryd daemon reports a device being plugged in. + +profiles: + - name: beacon + match: 'beacon' + section: beacon + options: + serial: ${serial} + + - name: amu + # vendor_id/product_id: the standard AMU board ID (USB_IDS.md); an AMU + # on Klipper's stock ID is not recognised. match: only used when no ID + # is reported (legacy scan, older daemon) - must match the USB serial + # string the AMU firmware is flashed with. + match: 'AMU' + vendor_id: '0x1d50' + product_id: '0xb001' + confirm: true + # Happy Hare's mmu.cfg (config/variant_mmu/base, included below) declares + # [mcu mmu] with a placeholder serial and all MMU pins use the `mmu:` + # prefix, so the real serial must go into that same section. printer.cfg + # places it after the include, so this serial overrides the placeholder. + section: mcu mmu + options: + serial: ${serial} + enable_includes: + - config/variant_mmu/base/*.cfg + disable_includes: + - config/variant_sync_single/*.cfg diff --git a/BlocksScreen/tools/configuration_manager/device_watcher.py b/BlocksScreen/tools/configuration_manager/device_watcher.py new file mode 100644 index 00000000..14ba4241 --- /dev/null +++ b/BlocksScreen/tools/configuration_manager/device_watcher.py @@ -0,0 +1,252 @@ +"""Hotplug side of the device templates. + +`DeviceConfigWatcher` listens to the discovery daemon through +`DeviceDiscoveryClient` and, when a serial device matching a template shows +up, has `DeviceConfigManager` update printer.cfg. It then emits +`config_changed` with the profile names so the UI can restart Klipper - the +new config only takes effect after one. + +Templates that need the user's OK (`confirm: true`, the AMU) are not +applied straight away: `confirmation_required` asks the UI first, and the +change is only written when the UI calls `accept()`. `decline()` leaves +printer.cfg alone until that device is plugged in again. + +When the daemon isn't reachable (not installed, not started yet, crashed), +/dev/serial/by-id is polled instead so hotplug still works. + +Unplugging is only logged: the next boot's sync reverts what the device +added, which avoids rewriting printer.cfg over a USB glitch mid-print. +""" + +from __future__ import annotations + +import logging +import typing + +from devices.discovery.client import DeviceDiscoveryClient +from devices.discovery.serial_devices import Device, SerialScanner, serial_from_json +from PyQt6 import QtCore + +if typing.TYPE_CHECKING: + from .device_configurator import DeviceConfigManager + +_logger = logging.getLogger(__name__) + + +# pylint: disable-next=too-many-instance-attributes +class DeviceConfigWatcher(QtCore.QObject): + """Applies device templates to printer.cfg as the daemon reports devices.""" + + config_changed: typing.ClassVar[QtCore.pyqtSignal] = QtCore.pyqtSignal( + list, name="device-config-changed" + ) + confirmation_required: typing.ClassVar[QtCore.pyqtSignal] = QtCore.pyqtSignal( + list, name="device-config-confirmation-required" + ) + + # Events inside this window are applied together: a burst of udev + # activity, or a daemon misbehaving and flooding events, costs one + # printer.cfg read/write instead of one per message on the GUI thread. + COALESCE_MS = 300 + # /dev/serial/by-id poll interval while the daemon is unreachable. + POLL_MS = 2000 + + def __init__( + self, + manager: DeviceConfigManager, + client: DeviceDiscoveryClient | None = None, + parent: QtCore.QObject | None = None, + ) -> None: + super().__init__(parent) + self._manager = manager + self._pending: dict[str, Device] = {} + # Serial devices plugged in right now, by by-id name. + self._present: dict[str, Device] = {} + # Profiles the UI is asking about, and the ones the user declined + # (asked again only once their device is unplugged and replugged). + self._asking: set[str] = set() + self._declined: set[str] = set() + self._flush_timer = QtCore.QTimer(self) + self._flush_timer.setSingleShot(True) + self._flush_timer.setInterval(self.COALESCE_MS) + self._flush_timer.timeout.connect(self._flush) + self._poll_timer = QtCore.QTimer(self) + self._poll_timer.setInterval(self.POLL_MS) + self._poll_timer.timeout.connect(self._poll) + self._client = client if client is not None else DeviceDiscoveryClient() + # The client emits from its reader thread; these queue onto the + # thread this watcher lives in (the GUI thread), so file writes and + # the UI prompt happen there. + self._client.device_added.connect(self._on_device_added) + self._client.device_removed.connect(self._on_device_removed) + self._client.snapshot_ready.connect(self._on_snapshot) + self._client.daemon_connected.connect(self._on_daemon_connected) + self._client.daemon_disconnected.connect(self._on_daemon_disconnected) + + def start(self) -> None: + """Start listening to the daemon (signals are already connected).""" + self._client.start() + + def stop(self) -> None: + """Stop listening.""" + self._flush_timer.stop() + self._poll_timer.stop() + self._client.stop() + + # Slots never raise: an exception escaping a Qt slot reaches CrashHandler, + # which exits BlocksScreen. A broken daemon or template must cost at most + # a log line, never the screen. + + @QtCore.pyqtSlot(dict) + def _on_device_added(self, device: dict) -> None: + self._queue([device]) + + @QtCore.pyqtSlot(list) + def _on_snapshot(self, devices: list) -> None: + # Normally a no-op (boot sync already applied the same devices), but + # covers devices plugged in during startup or while the daemon restarted. + self._present.clear() + self._queue(devices) + self._forget_declined() + + @QtCore.pyqtSlot(dict) + def _on_device_removed(self, device: dict) -> None: + name = device.get("symlink_name") if isinstance(device, dict) else None + _logger.info( + "Device removed: %s (printer.cfg is reconciled at next boot)", + name or (device.get("name") if isinstance(device, dict) else device), + ) + if name: + self._present.pop(name, None) + self._pending.pop(name, None) + self._forget_declined() + + @QtCore.pyqtSlot() + def _on_daemon_connected(self) -> None: + # The daemon's snapshot follows and takes over from the poll. + if self._poll_timer.isActive(): + _logger.info("device_discoveryd reachable again, stopping by-id polling") + self._poll_timer.stop() + + @QtCore.pyqtSlot() + def _on_daemon_disconnected(self) -> None: + if not self._poll_timer.isActive(): + _logger.info("device_discoveryd unreachable, polling /dev/serial/by-id") + self._poll() + self._poll_timer.start() + + def _poll(self) -> None: + try: + current = {d.symlink_name: d for d in SerialScanner.legacy_scan()} + except Exception: # pylint: disable=broad-except + _logger.exception("Polling /dev/serial/by-id failed") + return + for name in set(self._present) - set(current): + self._on_device_removed({"symlink_name": name}) + added = [d for n, d in current.items() if n not in self._present] + if added: + self._add_devices(added) + + def _queue(self, raw_devices: list) -> None: + try: + devices = [d for d in serial_from_json(raw_devices) if d.symlink_name] + except Exception: # pylint: disable=broad-except + _logger.exception("Ignoring unusable device report") + return + self._add_devices(devices) + + def _add_devices(self, devices: list[Device]) -> None: + for dev in devices: + if dev.symlink_name not in self._present: + self._log_device(dev) + self._present[dev.symlink_name] = dev + self._pending[dev.symlink_name] = dev + if self._pending and not self._flush_timer.isActive(): + self._flush_timer.start() + + def _log_device(self, dev: Device) -> None: + """Log a newly seen serial device and whether a profile recognises it. + + The one place that says why a board did or didn't trigger anything: + its USB ID (from the daemon or sysfs) and the profiles that match. + """ + try: + matches = self._manager.matching_profiles(dev) + except Exception: # pylint: disable=broad-except + _logger.exception("Matching device profiles failed") + matches = [] + _logger.info( + "Serial device seen: %s path=%s usb_id=%s:%s profiles=%s", + dev.symlink_name, + dev.device_path or "-", + dev.vid_hex or "-", + dev.pid_hex or "-", + ", ".join(matches) or "none", + ) + + def _forget_declined(self) -> None: + """Drop declines (and open questions) whose device is no longer plugged in.""" + if not self._declined and not self._asking: + return + try: + still = set( + self._manager.pending_confirmation(list(self._present.values())) + ) + except Exception: # pylint: disable=broad-except + _logger.exception("Checking pending device confirmations failed") + return + self._declined &= still + self._asking &= still + + def _flush(self) -> None: + devices = list(self._pending.values()) + self._pending.clear() + if not devices: + return + self._apply(devices) + try: + waiting = self._manager.pending_confirmation(list(self._present.values())) + except Exception: # pylint: disable=broad-except + _logger.exception("Checking pending device confirmations failed") + return + ask = [n for n in waiting if n not in self._declined and n not in self._asking] + skipped = sorted(set(waiting) - set(ask)) + if skipped: + _logger.info( + "Not asking again for %s (declined or already asking)", + ", ".join(skipped), + ) + if ask: + self._asking.update(ask) + _logger.info("Device templates waiting for the user: %s", ", ".join(ask)) + self.confirmation_required.emit(ask) + + def _apply(self, devices: list[Device]) -> None: + try: + changed = self._manager.apply_devices(devices) + except Exception: # pylint: disable=broad-except + _logger.exception("Applying device templates failed") + return + if changed: + _logger.info("Device templates applied: %s", ", ".join(changed)) + self.config_changed.emit(changed) + + @QtCore.pyqtSlot(list, name="accept-device-config") + def accept(self, names: list) -> None: + """The user accepted these profiles: remember that and apply them now.""" + self._asking.difference_update(names) + self._declined.difference_update(names) + try: + self._manager.accept(names) + except Exception: # pylint: disable=broad-except + _logger.exception("Recording accepted device templates failed") + return + _logger.info("User accepted device templates: %s", ", ".join(names)) + self._apply(list(self._present.values())) + + @QtCore.pyqtSlot(list, name="decline-device-config") + def decline(self, names: list) -> None: + """The user declined these profiles: leave printer.cfg alone until replug.""" + self._asking.difference_update(names) + self._declined.update(names) + _logger.info("User declined device templates: %s", ", ".join(names)) diff --git a/BlocksScreen/tools/serial_scanner.py b/BlocksScreen/tools/serial_scanner.py deleted file mode 100644 index 9efd79de..00000000 --- a/BlocksScreen/tools/serial_scanner.py +++ /dev/null @@ -1,134 +0,0 @@ -import os -import logging -import glob -from dataclasses import dataclass -from enum import Enum - -_logger = logging.getLogger(__name__) - - -class FirmwareState(Enum): - Klipper = "Klipper" - Katapult = "Katapult" - Unflashed = "Unflashed" - Unknown = "Unknown" - - -@dataclass -class Device: - name: str = "" - manufacturer: str = "" - product: str = "" - serial_number: str = "" - mcu_type: str = "" - firmware: FirmwareState = FirmwareState.Unknown - - symlink_name: str = "" - device_path: str = "" - interface: str = "" - - @property - def symlink(self) -> str: - """Full /dev/serial/by-id path""" - return f"/dev/serial/by-id/{self.symlink_name}" - - @property - def is_klipper(self) -> bool: - return self.firmware == FirmwareState.Klipper - - @property - def is_katapult(self) -> bool: - return self.firmware == FirmwareState.Katapult - - @property - def is_unflashed(self) -> bool: - return self.firmware == FirmwareState.Unflashed - - def __repr__(self) -> str: - return f" {self.device_path}> {self.symlink_name} " - - -class SerialScanner: - SERIAL_PATH = "/dev/serial/by-id/" - - def _detect_firmware(self, name: str) -> FirmwareState: - if "Klipper" in name: - return FirmwareState.Klipper - if "Katapult" in name: - return FirmwareState.Katapult - if "CanBoot" in name: - return FirmwareState.Katapult - - if any(x in name for x in ("1a86", "FTDI", "Silicon_Labs", "Prolific")): - return FirmwareState.Unflashed - - return FirmwareState.Unknown - - def extract_mcu(self, product: str) -> str: - """Extract mcu information""" - return product.split("_")[0] if product else "" - - def _parse_symlink(self, symlink_name: str, resolved: str) -> Device: - - d = Device() - - d.symlink_name = symlink_name - d.device_path = resolved - d.firmware = self._detect_firmware(symlink_name) - - s = symlink_name - if s.startswith("usb-"): - s = s[4:] - - if "-if" in s: - if_pos = s.rfind("-if") - d.interface = s[if_pos + 3 :].split("-")[0] - s = s[:if_pos] - - parts = s.split("_") - if len(parts) >= 3: - d.manufacturer = parts[0] - d.serial_number = parts[-1] - d.product = "_".join(parts[1:-1]) - elif len(parts) == 2: - d.manufacturer = parts[0] - d.product = parts[1] - else: - d.manufacturer = s - - if d.firmware in (FirmwareState.Klipper, FirmwareState.Katapult): - d.mcu_type = self.extract_mcu(d.product) - - d.name = f"{d.manufacturer} {d.mcu_type or d.product}".strip() - - return d - - def scan(self) -> list[Device]: - """Scan for serial devices compatible with klipper - Equivalent to ls /dev/serial/by-id/* - """ - if not os.path.exists(self.SERIAL_PATH): - return [] - - devs = [] - for path in glob.glob(f"{self.SERIAL_PATH}/*"): - try: - symlink_name = os.path.basename(path) - resolved = os.path.realpath(path) - devs.append(self._parse_symlink(symlink_name, resolved)) - except OSError as e: - _logger.info("[SerialScanner] Skipping: %s: %s" % (path, e)) - - return devs - - def scan_klipper(self) -> list[Device]: - """Scans for Klipper serial devices""" - return [d for d in self.scan() if d.is_klipper] - - def scan_katapult(self) -> list[Device]: - """Scans for katapult serial devices""" - return [d for d in self.scan() if d.is_katapult] - - def scan_unflashed(self) -> list[Device]: - """Scans for unflashed serial devices""" - return [d for d in self.scan() if d.is_unflashed] diff --git a/scripts/install-updater.sh b/scripts/install-updater.sh index 24ba3f64..cfdf55db 100755 --- a/scripts/install-updater.sh +++ b/scripts/install-updater.sh @@ -57,8 +57,7 @@ echo "$SERVICE" | sudo tee /etc/systemd/system/.BlocksScreen-updater.service.new sudo mv -Tf /etc/systemd/system/.BlocksScreen-updater.service.new /etc/systemd/system/BlocksScreen-updater.service sudo systemctl daemon-reload sudo systemctl enable BlocksScreen-updater.service -sudo systemctl restart BlocksScreen-updater.service || true -echo_ok "BlocksScreen-updater service installed and started" +echo_ok "BlocksScreen-updater service installed (restarted at the end)" echo_info "Installing Spoolman service unit (inactive until the updater provisions it) ..." SPOOLMAN_SVC=$(cat "$SCRIPT_PATH/Spoolman.service") @@ -260,4 +259,51 @@ echo_info "Pre-generating framebuffer splash cache ..." _as_blocks "$BSENV/bin/python3.11" "$SCRIPT_PATH/bs-splash.py" --precompute 2>/dev/null || true echo_ok "Splash cache ready" +# First-time setup of a DeviceDiscovery clone the updater provisioned (the same +# steps as that repo's scripts/install.sh, which needs sudo the hook lacks). +# Later updates only rebuild, through updater/hooks/DeviceDiscovery.sh. +_DD_REPO="$_BSENV_HOME/DeviceDiscovery" +_DD_UNIT="device-discoveryd.service" +_setup_device_discovery() { + # Called in an `||` context, where set -e is off: every step checks itself. + if [[ "$(cat "$_DD_REPO/.install-mode" 2>/dev/null)" == "prebuilt" ]]; then + sudo apt-get install -y --no-install-recommends \ + curl ca-certificates libusb-1.0-0 libudev1 || return 1 + else + sudo apt-get install -y --no-install-recommends \ + build-essential cmake pkg-config \ + libusb-1.0-0-dev libudev-dev nlohmann-json3-dev || return 1 + fi + # As blocks, so build/ and bin/ stay writable by later updater hook runs. + sudo -u "$_BSENV_USER" "$_DD_REPO/scripts/build.sh" || return 1 + if [[ "$(systemctl show --property=LoadState --value "$_DD_UNIT")" != "loaded" ]]; then + sudo systemctl link "$_DD_REPO/systemd/$_DD_UNIT" || return 1 + sudo systemctl daemon-reload || return 1 + fi + sudo systemctl enable "$_DD_UNIT" || return 1 + sudo systemctl restart "$_DD_UNIT" || return 1 +} +if [[ -d "$_DD_REPO/.git" ]]; then + if [[ -x "$_DD_REPO/bin/device_discoveryd" ]] && + [[ "$(systemctl show --property=LoadState --value "$_DD_UNIT")" == "loaded" ]]; then + echo_ok "DeviceDiscovery already set up" + else + echo_info "Setting up DeviceDiscovery (first build can take several minutes) ..." + if _setup_device_discovery; then + echo_ok "DeviceDiscovery installed and started" + else + # Non-fatal: the next updater run defers to this script again. + echo_error "DeviceDiscovery setup failed, retried on the next update" + fi + fi +fi + +# Restart the daemon last, --no-block, so it starts after everything above is in +# place and after this run ends. The restarted daemon may provision components a +# self-update added and touch the deploy flag again; restarting earlier could +# let that happen mid-run, and BlocksScreen-deploy.service's ExecStartPost would +# then delete the flag and drop the follow-up setup. +sudo systemctl --no-block restart BlocksScreen-updater.service || true +echo_ok "BlocksScreen-updater restart queued" + echo_ok "BlocksScreen updater setup complete" diff --git a/tests/discovery/__init__.py b/tests/discovery/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/tests/discovery/conftest.py b/tests/discovery/conftest.py new file mode 100644 index 00000000..32cfc1f8 --- /dev/null +++ b/tests/discovery/conftest.py @@ -0,0 +1,67 @@ +"""Shared fixtures: a fake device_discoveryd on a short-path Unix socket.""" + +import json +import os +import socket +import sys +import tempfile +import threading + +import pytest + +# App modules import top-level packages (`from tools...`, `from devices...`) +# with BlocksScreen/ on sys.path, as BlocksScreen.py sets it up. Appended, +# not prepended: first on the path, BlocksScreen/BlocksScreen.py (the entry +# script) would shadow the `BlocksScreen` package for every other test. +_BS_DIR = os.path.abspath( + os.path.join(os.path.dirname(__file__), "..", "..", "BlocksScreen") +) +if _BS_DIR not in sys.path: + sys.path.append(_BS_DIR) + + +@pytest.fixture(autouse=True) +def _no_host_sysfs(tmp_path, monkeypatch): + """Keep the USB-ID sysfs lookup off the test machine's real /sys.""" + from devices.discovery import serial_devices + + monkeypatch.setattr(serial_devices, "SYSFS_TTY_PATH", str(tmp_path / "sys-tty")) + + +@pytest.fixture +def sock_path(): + # Not tmp_path: AF_UNIX paths are limited to 108 bytes. + d = tempfile.mkdtemp(prefix="dd-", dir="/tmp") + path = os.path.join(d, "s.sock") + yield path + if os.path.exists(path): + os.unlink(path) + os.rmdir(d) + + +class FakeDaemon: + """Accepts one client and sends it the given lines.""" + + def __init__(self, path, messages): + self._srv = socket.socket(socket.AF_UNIX, socket.SOCK_STREAM) + self._srv.bind(path) + self._srv.listen(1) + self._payload = b"".join( + (m if isinstance(m, bytes) else json.dumps(m).encode()) + b"\n" + for m in messages + ) + self._conn = None + self._thread = threading.Thread(target=self._serve, daemon=True) + self._thread.start() + + def _serve(self): + self._conn, _ = self._srv.accept() + # Split mid-line to exercise the client's line buffering. + half = len(self._payload) // 2 + self._conn.sendall(self._payload[:half]) + self._conn.sendall(self._payload[half:]) + + def close(self): + if self._conn: + self._conn.close() + self._srv.close() diff --git a/tests/discovery/test_amu_end_to_end_unit.py b/tests/discovery/test_amu_end_to_end_unit.py new file mode 100644 index 00000000..a0cda585 --- /dev/null +++ b/tests/discovery/test_amu_end_to_end_unit.py @@ -0,0 +1,204 @@ +"""AMU plug-in, end to end, with the real DeviceConfigManager and watcher. + +Walks the whole checklist: the daemon reports a serial device whose by-id name +says nothing about AMU, its USB ID (0x1d50:0xb001, USB_IDS.md) is read from +sysfs, the user is asked, printer.cfg only changes after they accept, the +restart request follows, and a reboot with the AMU still plugged keeps the +config without asking again. +""" + +import os +import shutil + +import pytest + +from devices.discovery import serial_devices as sd +from devices.discovery.client import DeviceDiscoveryClient +from tools.configuration_manager.device_configurator import ( + DeviceConfigManager, + DeviceConsent, +) +from tools.configuration_manager.device_profiles import load_profiles +from tools.configuration_manager.device_watcher import DeviceConfigWatcher + +from .conftest import FakeDaemon + +SYMLINK = "usb-Klipper_stm32h723xx_2B0012000851333235363233-if00" +AMU_REPORT = { + "connection": "Serial", + "firmware": "Klipper", + "symlink_name": SYMLINK, + "device_path": "/dev/ttyACM1", + "vendor_id": 0, # what device_discoveryd sends for Serial entries + "product_id": 0, +} + +# Fallback when no RF50-Klipper checkout is around: same shape as its printer.cfg. +MINI_PRINTER_CFG = """\ +#[include config/variant_mmu/base/*.cfg] +[include config/variant_sync_single/*.cfg ] + +[mcu] +canbus_uuid: + +[mcu pi] +serial: /tmp/klipper_host_mcu + +[printer] +kinematics: corexy +""" + + +def _rf50_printer_cfg(): + for d in ( + os.environ.get("RF50_KLIPPER_DIR"), + os.path.expanduser("~/RF50-Klipper"), + os.path.expanduser("~/github/RF50-Klipper"), + ): + if d and os.path.isfile(os.path.join(d, "printer.cfg")): + return os.path.join(d, "printer.cfg") + return None + + +class _Section: + def __init__(self, values): + self._values = values + + def get(self, key, default=None): + return self._values.get(key, default) + + def getlists(self, key, default=None): + return default + + +class _Config: + def __init__(self, values): + self._section = _Section(values) + + def get_section(self, name, fallback=None): + return self._section + + +@pytest.fixture +def machine(tmp_path, monkeypatch): + repo = tmp_path / "RF50-Klipper" + config_dir = tmp_path / "printer_data" / "config" + (repo / ".git").mkdir(parents=True) + config_dir.mkdir(parents=True) + src = _rf50_printer_cfg() + if src: + shutil.copy(src, repo / "printer.cfg") + else: + (repo / "printer.cfg").write_text(MINI_PRINTER_CFG) + + # sysfs: ttyACM1 -> USB interface -> USB device 1d50:b001 + usb_dev = tmp_path / "sys" / "devices" / "usb1" / "1-1" + (usb_dev / "1-1:1.0").mkdir(parents=True) + (usb_dev / "idVendor").write_text("1d50\n") + (usb_dev / "idProduct").write_text("b001\n") + tty = tmp_path / "sys" / "class" / "tty" / "ttyACM1" + tty.mkdir(parents=True) + (tty / "device").symlink_to(usb_dev / "1-1:1.0") + monkeypatch.setattr(sd, "SYSFS_TTY_PATH", str(tmp_path / "sys" / "class" / "tty")) + + plugged = [] + monkeypatch.setattr( + DeviceConfigManager, + "scan_devices", + staticmethod(lambda: [sd.device_from_json(r) for r in plugged]), + ) + consent_path = tmp_path / "home" / ".config" / "blockscreen" / "consent.json" + + def boot(): + return DeviceConfigManager( + _Config( + { + "config_repo": str(repo), + "config_dir": str(config_dir), + "backup_dir": str(tmp_path / "backup"), + "copy_files": None, + } + ), + profiles=load_profiles(override=None), + consent=DeviceConsent(consent_path), + ) + + return boot, plugged, config_dir / "printer.cfg" + + +def test_amu_plug_in_end_to_end(qtbot, sock_path, machine): + boot, plugged, printer_cfg = machine + manager = boot() # first boot, nothing plugged + repo_text = printer_cfg.read_text() + assert "[mcu mmu]" not in repo_text + + daemon = FakeDaemon( + sock_path, + [ + {"type": "hello", "proto": 1, "pid": 1}, + {"type": "snapshot", "devices": []}, + {"type": "added", "device": AMU_REPORT}, + ], + ) + watcher = DeviceConfigWatcher(manager, client=DeviceDiscoveryClient(sock_path)) + try: + # 1+2: plug-in noticed and identified as AMU by USB ID -> user is asked + with qtbot.waitSignal(watcher.confirmation_required, timeout=5000) as ask: + watcher.start() + assert ask.args == [["amu"]] + assert printer_cfg.read_text() == repo_text # nothing written yet + + # 3: accepting writes the Happy Hare layout; 4: restart is requested + with qtbot.waitSignal(watcher.config_changed, timeout=2000) as changed: + watcher.accept(["amu"]) + assert changed.args == [["amu"]] + finally: + watcher.stop() + daemon.close() + + text = printer_cfg.read_text() + assert f"[mcu mmu]\nserial: /dev/serial/by-id/{SYMLINK}\n" in text + lines = text.splitlines() + assert "[include config/variant_mmu/base/*.cfg]" in lines + assert any( + ln.startswith("#[include config/variant_sync_single/*.cfg") for ln in lines + ) + # the serial comes after the include so it overrides mmu.cfg's placeholder + assert text.index("[include config/variant_mmu/base/*.cfg]") < text.index( + "[mcu mmu]" + ) + + # 5: reboot with the AMU still plugged - same file, no question, no restart + plugged.append(AMU_REPORT) + rebooted = boot() + assert printer_cfg.read_text() == text + assert rebooted.boot_changed is False + assert rebooted.pending_confirmation( + [sd.device_from_json(AMU_REPORT)] + ) == [] + + # unplugged while off: reverted at boot, and flagged for a Klipper restart + plugged.clear() + rebooted = boot() + assert printer_cfg.read_text() == repo_text + assert rebooted.boot_changed is True + + +def test_decline_writes_nothing(qtbot, sock_path, machine): + boot, _, printer_cfg = machine + manager = boot() + before = printer_cfg.read_text() + daemon = FakeDaemon( + sock_path, + [{"type": "hello", "proto": 1, "pid": 1}, {"type": "added", "device": AMU_REPORT}], + ) + watcher = DeviceConfigWatcher(manager, client=DeviceDiscoveryClient(sock_path)) + try: + with qtbot.waitSignal(watcher.confirmation_required, timeout=5000): + watcher.start() + with qtbot.assertNotEmitted(watcher.config_changed, wait=500): + watcher.decline(["amu"]) + finally: + watcher.stop() + daemon.close() + assert printer_cfg.read_text() == before diff --git a/tests/discovery/test_device_watcher_unit.py b/tests/discovery/test_device_watcher_unit.py new file mode 100644 index 00000000..398f8922 --- /dev/null +++ b/tests/discovery/test_device_watcher_unit.py @@ -0,0 +1,339 @@ +"""DeviceConfigWatcher: daemon events -> DeviceConfigManager.apply_devices -> config_changed.""" + +import pytest + +from devices.discovery.client import DeviceDiscoveryClient +from tools.configuration_manager.device_watcher import DeviceConfigWatcher + +from .conftest import FakeDaemon + +AMU = { + "connection": "Serial", + "firmware": "Klipper", + "symlink_name": "usb-Klipper_stm32f446xx_AMU0001-if00", +} +WEBCAM = {"connection": "USB", "usb_class": "Video", "name": "Chicony HD Webcam"} + + +class FakeManager: + """Records apply_devices calls; 'changes' the config once per profile.""" + + def __init__(self): + self.calls = [] + self._applied = set() + + def apply_devices(self, devices): + self.calls.append([d.symlink_name for d in devices]) + changed = [] + for d in devices: + if "AMU" in d.symlink_name and "amu" not in self._applied: + self._applied.add("amu") + changed.append("amu") + return changed + + def pending_confirmation(self, devices): + return [] + + +@pytest.fixture +def watcher_for(sock_path): + started = [] + + def _make(messages): + daemon = FakeDaemon(sock_path, messages) + manager = FakeManager() + watcher = DeviceConfigWatcher(manager, client=DeviceDiscoveryClient(sock_path)) + started.append((daemon, watcher)) + return watcher, manager + + yield _make + for daemon, watcher in started: + watcher.stop() + daemon.close() + + +def test_hotplug_amu_emits_once(qtbot, watcher_for): + watcher, manager = watcher_for( + [ + {"type": "hello", "proto": 1, "pid": 1}, + {"type": "snapshot", "devices": []}, + {"type": "added", "device": WEBCAM}, + {"type": "added", "device": AMU}, + {"type": "added", "device": AMU}, # e.g. replug before restart + ] + ) + with qtbot.waitSignal(watcher.config_changed, timeout=5000) as blocker: + watcher.start() + assert blocker.args == [["amu"]] + # The burst is coalesced into one apply; USB-only entries and the empty + # snapshot never reach the manager. + assert manager.calls == [[AMU["symlink_name"]]] + + +def test_snapshot_devices_are_applied(qtbot, watcher_for): + watcher, manager = watcher_for( + [ + {"type": "hello", "proto": 1, "pid": 1}, + {"type": "snapshot", "devices": [WEBCAM, AMU]}, + ] + ) + with qtbot.waitSignal(watcher.config_changed, timeout=5000) as blocker: + watcher.start() + assert blocker.args == [["amu"]] + assert manager.calls == [[AMU["symlink_name"]]] + + +def test_removed_is_only_logged(qtbot, watcher_for): + watcher, manager = watcher_for( + [ + {"type": "hello", "proto": 1, "pid": 1}, + {"type": "removed", "device": AMU}, + ] + ) + with qtbot.assertNotEmitted(watcher.config_changed, wait=500): + watcher.start() + assert manager.calls == [] + + +class ExplodingManager: + def apply_devices(self, devices): + raise RuntimeError("template bug") + + def pending_confirmation(self, devices): + raise RuntimeError("template bug") + + +def test_manager_error_does_not_escape_the_slot(qtbot, sock_path): + # pytest-qt fails the test if an exception escapes a Qt slot, which in + # the app would reach CrashHandler and exit BlocksScreen. + daemon = FakeDaemon( + sock_path, + [{"type": "hello", "proto": 1, "pid": 1}, {"type": "added", "device": AMU}], + ) + watcher = DeviceConfigWatcher( + ExplodingManager(), client=DeviceDiscoveryClient(sock_path) + ) + try: + with qtbot.assertNotEmitted(watcher.config_changed, wait=1000): + watcher.start() + finally: + watcher.stop() + daemon.close() + + +# What the daemon reports for an AMU on its own USB ID (USB_IDS.md): the +# by-id name carries no "AMU", only the serial entry's vendor/product ID does. +AMU_SERIAL_BY_ID = { + "connection": "Serial", + "firmware": "Klipper", + "name": "Klipper stm32h723xx", + "symlink_name": "usb-Klipper_stm32h723xx_2B0012000851333235363233-if00", + "device_path": "/dev/ttyACM1", + "vendor_id": 0x1D50, + "product_id": 0xB001, +} +AMU_USB_BY_ID = { + "connection": "USB", + "usb_class": "CDCSerial", + "vendor_id": 0x1D50, + "product_id": 0xB001, +} +PRINTER_CFG = "[printer]\nkinematics: corexy\n\n[mcu]\nserial: /dev/ttyACM0\n" + + +class ProfileManager: + """apply_devices through the real bundled profiles, on in-memory printer.cfg.""" + + def __init__(self, accepted=("amu",)): + from tools.configuration_manager.device_profiles import load_profiles + + self.profiles = load_profiles(override=None) + self.accepted = set(accepted) + self.text = PRINTER_CFG + + def _allowed(self): + return [p for p in self.profiles if not p.confirm or p.name in self.accepted] + + def apply_devices(self, devices): + from tools.configuration_manager.device_profiles import ( + apply_device_profiles, + ) + + self.text, changed = apply_device_profiles(self.text, self._allowed(), devices) + return changed + + def pending_confirmation(self, devices): + from tools.configuration_manager.device_profiles import ( + apply_device_profiles, + match_devices, + ) + + waiting = [p for p in self.profiles if p.confirm and p.name not in self.accepted] + return [ + p.name + for p, d in match_devices(waiting, devices) + if apply_device_profiles(self.text, [p], [d])[1] + ] + + def accept(self, names): + self.accepted.update(names) + + +def test_amu_identified_by_usb_id_updates_config(qtbot, sock_path): + daemon = FakeDaemon( + sock_path, + [ + {"type": "hello", "proto": 1, "pid": 1}, + {"type": "snapshot", "devices": []}, + {"type": "added", "device": AMU_SERIAL_BY_ID}, + {"type": "added", "device": AMU_USB_BY_ID}, + ], + ) + manager = ProfileManager() + watcher = DeviceConfigWatcher(manager, client=DeviceDiscoveryClient(sock_path)) + try: + with qtbot.waitSignal(watcher.config_changed, timeout=5000) as blocker: + watcher.start() + finally: + watcher.stop() + daemon.close() + assert blocker.args == [["amu"]] + assert "[mcu mmu]" in manager.text + assert f"serial: /dev/serial/by-id/{AMU_SERIAL_BY_ID['symlink_name']}" in manager.text + + +def test_amu_without_reported_id_is_not_recognised(qtbot, sock_path): + # No ID from the daemon and none in sysfs (see the autouse fixture), and + # a by-id name without "AMU": nothing identifies it as an AMU. + daemon = FakeDaemon( + sock_path, + [ + {"type": "hello", "proto": 1, "pid": 1}, + { + "type": "added", + "device": {**AMU_SERIAL_BY_ID, "vendor_id": 0, "product_id": 0}, + }, + ], + ) + manager = ProfileManager() + watcher = DeviceConfigWatcher(manager, client=DeviceDiscoveryClient(sock_path)) + try: + with qtbot.assertNotEmitted(watcher.config_changed, wait=800): + watcher.start() + finally: + watcher.stop() + daemon.close() + assert manager.text == PRINTER_CFG + + +class TestConfirmation: + """AMU (confirm: true): ask first, write only after accept().""" + + MESSAGES = [ + {"type": "hello", "proto": 1, "pid": 1}, + {"type": "snapshot", "devices": []}, + {"type": "added", "device": AMU_SERIAL_BY_ID}, + ] + + @pytest.fixture + def setup(self, sock_path): + made = [] + + def _make(messages=None): + daemon = FakeDaemon(sock_path, messages or self.MESSAGES) + manager = ProfileManager(accepted=()) + watcher = DeviceConfigWatcher( + manager, client=DeviceDiscoveryClient(sock_path) + ) + made.append((daemon, watcher)) + return watcher, manager + + yield _make + for daemon, watcher in made: + watcher.stop() + daemon.close() + + def test_asks_before_writing(self, qtbot, setup): + watcher, manager = setup() + with qtbot.assertNotEmitted(watcher.config_changed, wait=0): + with qtbot.waitSignal(watcher.confirmation_required, timeout=5000) as b: + watcher.start() + assert b.args == [["amu"]] + assert manager.text == PRINTER_CFG + + def test_accept_writes_and_emits_config_changed(self, qtbot, setup): + watcher, manager = setup() + with qtbot.waitSignal(watcher.confirmation_required, timeout=5000): + watcher.start() + with qtbot.waitSignal(watcher.config_changed, timeout=1000) as b: + watcher.accept(["amu"]) + assert b.args == [["amu"]] + assert "[mcu mmu]" in manager.text + assert manager.accepted == {"amu"} + + def test_decline_leaves_config_and_does_not_ask_again(self, qtbot, setup): + watcher, manager = setup() + with qtbot.waitSignal(watcher.confirmation_required, timeout=5000): + watcher.start() + watcher.decline(["amu"]) + # A later snapshot (daemon reconnect) with the same AMU: no new question. + with qtbot.assertNotEmitted(watcher.confirmation_required, wait=600): + watcher._on_snapshot([AMU_SERIAL_BY_ID]) + assert manager.text == PRINTER_CFG + + def test_replug_after_decline_asks_again(self, qtbot, setup): + watcher, manager = setup() + with qtbot.waitSignal(watcher.confirmation_required, timeout=5000): + watcher.start() + watcher.decline(["amu"]) + watcher._on_device_removed(AMU_SERIAL_BY_ID) + with qtbot.waitSignal(watcher.confirmation_required, timeout=2000) as b: + watcher._on_device_added(AMU_SERIAL_BY_ID) + assert b.args == [["amu"]] + + def test_no_second_question_while_one_is_open(self, qtbot, setup): + watcher, _ = setup() + with qtbot.waitSignal(watcher.confirmation_required, timeout=5000): + watcher.start() + with qtbot.assertNotEmitted(watcher.confirmation_required, wait=600): + watcher._on_device_added(AMU_SERIAL_BY_ID) + + +class TestPollFallback: + """No daemon: /dev/serial/by-id is polled so hotplug still works.""" + + def test_polls_when_daemon_unreachable(self, qtbot, sock_path, monkeypatch): + from devices.discovery import serial_devices as sd + + found = [] + monkeypatch.setattr(sd.SerialScanner, "legacy_scan", staticmethod(lambda: found)) + manager = ProfileManager() + watcher = DeviceConfigWatcher(manager, client=DeviceDiscoveryClient(sock_path)) + monkeypatch.setattr(watcher, "POLL_MS", 50) + watcher._poll_timer.setInterval(50) + try: + watcher.start() # nothing listens on sock_path + qtbot.waitUntil(watcher._poll_timer.isActive, timeout=5000) + found.append( + sd.Device( + symlink_name=AMU_SERIAL_BY_ID["symlink_name"], + vendor_id=0x1D50, + product_id=0xB001, + ) + ) + with qtbot.waitSignal(watcher.config_changed, timeout=3000) as b: + pass + finally: + watcher.stop() + assert b.args == [["amu"]] + assert "[mcu mmu]" in manager.text + + def test_daemon_connect_stops_polling(self, qtbot, sock_path): + watcher = DeviceConfigWatcher( + ProfileManager(), client=DeviceDiscoveryClient(sock_path) + ) + watcher._on_daemon_disconnected() + assert watcher._poll_timer.isActive() + watcher._on_daemon_connected() + assert not watcher._poll_timer.isActive() + watcher.stop() diff --git a/tests/discovery/test_discovery_client_unit.py b/tests/discovery/test_discovery_client_unit.py new file mode 100644 index 00000000..0fe7ccc6 --- /dev/null +++ b/tests/discovery/test_discovery_client_unit.py @@ -0,0 +1,254 @@ +"""DeviceDiscoveryClient against a fake device_discoveryd socket. + +Signals are connected with DirectConnection so they fire on the client's +reader thread and land in a queue - no Qt event loop needed. +""" + +import json +import logging +import queue +import time + +import pytest +from PyQt6 import QtCore + +from devices.discovery import client as dd_client +from devices.discovery.client import DeviceDiscoveryClient + +from .conftest import FakeDaemon + +DIRECT = QtCore.Qt.ConnectionType.DirectConnection +TIMEOUT = 5.0 + + +def _collect(client): + events = queue.Queue() + client.snapshot_ready.connect(lambda v: events.put(("snapshot", v)), DIRECT) + client.device_added.connect(lambda v: events.put(("added", v)), DIRECT) + client.device_removed.connect(lambda v: events.put(("removed", v)), DIRECT) + client.daemon_connected.connect(lambda: events.put(("connected", None)), DIRECT) + return events + + +def _next(events): + return events.get(timeout=TIMEOUT) + + +class TestDefaultSocketPath: + def test_env_var_wins(self, monkeypatch): + monkeypatch.setenv("DEVICE_DISCOVERY_SOCKET", "/x/y.sock") + assert dd_client.default_socket_path() == "/x/y.sock" + + def test_systemd_path_when_present(self, monkeypatch): + monkeypatch.delenv("DEVICE_DISCOVERY_SOCKET", raising=False) + monkeypatch.setattr( + dd_client.os.path, "exists", lambda p: p == dd_client.SYSTEMD_SOCKET_PATH + ) + assert dd_client.default_socket_path() == dd_client.SYSTEMD_SOCKET_PATH + + def test_xdg_fallback(self, monkeypatch): + monkeypatch.delenv("DEVICE_DISCOVERY_SOCKET", raising=False) + monkeypatch.setenv("XDG_RUNTIME_DIR", "/run/user/1000") + monkeypatch.setattr(dd_client.os.path, "exists", lambda p: False) + assert ( + dd_client.default_socket_path() + == "/run/user/1000/blockscreen/device_discovery.sock" + ) + + def test_no_tmp_fallback(self, monkeypatch): + monkeypatch.delenv("DEVICE_DISCOVERY_SOCKET", raising=False) + monkeypatch.delenv("XDG_RUNTIME_DIR", raising=False) + monkeypatch.setattr(dd_client.os.path, "exists", lambda p: False) + assert dd_client.default_socket_path() == dd_client.SYSTEMD_SOCKET_PATH + + +class TestLateDaemon: + def test_socket_created_after_start_is_found(self, sock_path, tmp_path, monkeypatch): + # BlocksScreen can start before device-discoveryd has bound its socket + # (Type=simple). The client must look the path up again on each retry + # instead of sticking with the $XDG_RUNTIME_DIR fallback it saw first. + monkeypatch.delenv("DEVICE_DISCOVERY_SOCKET", raising=False) + monkeypatch.setenv("XDG_RUNTIME_DIR", str(tmp_path)) + monkeypatch.setattr(dd_client, "SYSTEMD_SOCKET_PATH", sock_path) + client = DeviceDiscoveryClient() + events = _collect(client) + client.start() + daemon = None + try: + time.sleep(0.3) # first attempt fails: no socket anywhere yet + daemon = FakeDaemon( + sock_path, + [{"type": "hello", "proto": 1, "pid": 1}, {"type": "snapshot", "devices": []}], + ) + assert _next(events) == ("connected", None) # next retry, ~2 s later + assert _next(events) == ("snapshot", []) + finally: + client.stop() + if daemon: + daemon.close() + + +class TestProtocol: + def test_snapshot_added_removed(self, sock_path): + dev = {"name": "Klipper stm32h723xx", "connection": "Serial"} + daemon = FakeDaemon( + sock_path, + [ + {"type": "hello", "proto": 1, "pid": 1}, + {"type": "snapshot", "devices": [dev]}, + {"type": "added", "device": dev}, + {"type": "removed", "device": dev}, + ], + ) + client = DeviceDiscoveryClient(sock_path) + events = _collect(client) + client.start() + try: + assert _next(events) == ("connected", None) + assert _next(events) == ("snapshot", [dev]) + assert _next(events) == ("added", dev) + assert _next(events) == ("removed", dev) + finally: + client.stop() + daemon.close() + + def test_malformed_and_unknown_lines_are_skipped(self, sock_path): + daemon = FakeDaemon( + sock_path, + [ + b"not json", + {"type": "future_message"}, + {"type": "snapshot", "devices": []}, + ], + ) + client = DeviceDiscoveryClient(sock_path) + events = _collect(client) + client.start() + try: + assert _next(events) == ("connected", None) + assert _next(events) == ("snapshot", []) + finally: + client.stop() + daemon.close() + + def test_protocol_mismatch_warns(self, caplog): + client = DeviceDiscoveryClient("/nonexistent") + with caplog.at_level(logging.WARNING, logger=dd_client.__name__): + client._dispatch(json.dumps({"type": "hello", "proto": 99}).encode()) + assert "protocol 99" in caplog.text + + def test_stop_without_daemon_returns(self, sock_path): + client = DeviceDiscoveryClient(sock_path) # nothing listening + client.start() + client.stop() + assert not client._thread.is_alive() + + def test_stop_before_start(self): + DeviceDiscoveryClient("/nonexistent").stop() + + +@pytest.fixture +def thread_exceptions(monkeypatch): + """Exceptions that reached threading.excepthook (CrashHandler exits on these).""" + import threading + + seen = [] + monkeypatch.setattr(threading, "excepthook", lambda args: seen.append(args)) + return seen + + +BAD_LINES = [ + b"not json", + b"\xff\xfe bad utf-8", + b"[" * 100000, # deep nesting -> RecursionError in json + b"42", + b"[1, 2]", + b'{"no": "type"}', + b'{"type": 7}', + b'{"type": "added"}', + b'{"type": "added", "device": "x"}', + b'{"type": "removed", "device": null}', + b'{"type": "snapshot"}', + b'{"type": "snapshot", "devices": {"a": 1}}', + b'{"type": "hello", "proto": "banana"}', +] + + +class TestBrokenDaemon: + def test_bad_messages_are_skipped_and_reading_continues( + self, sock_path, thread_exceptions + ): + good = {"name": "ok", "connection": "Serial"} + daemon = FakeDaemon( + sock_path, + BAD_LINES + + [ + {"type": "snapshot", "devices": [1, "x", good]}, + {"type": "added", "device": good}, + ], + ) + client = DeviceDiscoveryClient(sock_path) + events = _collect(client) + client.start() + try: + assert _next(events) == ("connected", None) + assert _next(events) == ("snapshot", [good]) # non-dicts filtered + assert _next(events) == ("added", good) + assert client._thread.is_alive() + finally: + client.stop() + daemon.close() + assert thread_exceptions == [] + + def test_oversized_line_drops_the_connection(self, sock_path, thread_exceptions): + daemon = FakeDaemon(sock_path, [b"x" * (dd_client._MAX_LINE + 70000)]) + client = DeviceDiscoveryClient(sock_path) + gone = queue.Queue() + client.daemon_disconnected.connect(lambda: gone.put(True), DIRECT) + client.start() + try: + assert gone.get(timeout=TIMEOUT) + assert client._thread.is_alive() # reconnecting, not dead + finally: + client.stop() + daemon.close() + assert thread_exceptions == [] + + def test_unexpected_error_keeps_the_thread_alive( + self, sock_path, monkeypatch, thread_exceptions + ): + calls = [] + + def boom(self, path): + calls.append(1) + raise RuntimeError("bug") + + monkeypatch.setattr(DeviceDiscoveryClient, "_connect_and_read", boom) + monkeypatch.setattr(dd_client, "_RECONNECT_DELAYS", (0.05,)) + client = DeviceDiscoveryClient(sock_path) + client.start() + try: + deadline = time.monotonic() + TIMEOUT + while len(calls) < 3 and time.monotonic() < deadline: + time.sleep(0.02) + assert len(calls) >= 3 and client._thread.is_alive() + finally: + client.stop() + assert thread_exceptions == [] + + @pytest.mark.parametrize("line", BAD_LINES) + def test_fetch_snapshot_never_raises(self, sock_path, line): + daemon = FakeDaemon(sock_path, [line]) + try: + assert dd_client.fetch_snapshot(sock_path, timeout=0.5) is None + finally: + daemon.close() + + def test_fetch_snapshot_hung_daemon_is_bounded(self, sock_path): + daemon = FakeDaemon(sock_path, []) # accepts, never sends + try: + t0 = time.monotonic() + assert dd_client.fetch_snapshot(sock_path, timeout=0.5) is None + assert time.monotonic() - t0 < 1.5 + finally: + daemon.close() diff --git a/tests/discovery/test_serial_devices_unit.py b/tests/discovery/test_serial_devices_unit.py new file mode 100644 index 00000000..d49097e2 --- /dev/null +++ b/tests/discovery/test_serial_devices_unit.py @@ -0,0 +1,289 @@ +"""devices.discovery.serial_devices: daemon scan with a /dev/serial/by-id fallback.""" + +import os + +import pytest + +from devices.discovery import client as dd_client +from devices.discovery import serial_devices as sd + +from .conftest import FakeDaemon + +KLIPPER = { + "connection": "Serial", + "firmware": "Klipper", + "name": "Klipper stm32h723xx", + "manufacturer": "Klipper", + "product": "stm32h723xx", + "serial_number": "29003A000851323235363233", + "mcu_type": "stm32h723xx", + "symlink_name": "usb-Klipper_stm32h723xx_29003A000851323235363233-if00", + "device_path": "/dev/ttyACM0", + "interface": "00", + "usb_class": "Unknown", + "vendor_id": 0, +} +KATAPULT = { + **KLIPPER, + "firmware": "Katapult", + "name": "Katapult rp2040", + "symlink_name": "usb-Katapult_rp2040_E6625C05E7-if00", +} +CH340 = { + **KLIPPER, + "firmware": "Unflashed", + "name": "1a86 USB", + "mcu_type": "", + "symlink_name": "usb-1a86_USB_Serial-if00-port0", +} +BEACON = { + **KLIPPER, + "firmware": "Unknown", + "name": "Beacon Beacon_RevH", + "mcu_type": "", + "symlink_name": "usb-Beacon_Beacon_RevH_ABC123-if00", +} +WEBCAM = {"connection": "USB", "usb_class": "Video", "name": "Chicony HD Webcam"} + + +def _hello(proto=1): + return {"type": "hello", "proto": proto, "pid": 1} + + +@pytest.fixture +def daemon(sock_path): + started = [] + + def _start(messages): + d = FakeDaemon(sock_path, messages) + started.append(d) + return d + + yield _start + for d in started: + d.close() + + +class TestDeviceFromJson: + def test_maps_every_field(self): + dev = sd.device_from_json(KLIPPER) + assert dev.name == "Klipper stm32h723xx" + assert dev.firmware is sd.FirmwareState.Klipper + assert dev.mcu_type == "stm32h723xx" + assert dev.interface == "00" + assert dev.device_path == "/dev/ttyACM0" + assert dev.symlink == "/dev/serial/by-id/" + KLIPPER["symlink_name"] + assert dev.is_klipper and not dev.is_katapult and not dev.is_unflashed + + def test_unknown_or_missing_firmware(self): + assert sd.device_from_json({"firmware": "Bogus"}).firmware is ( + sd.FirmwareState.Unknown + ) + assert sd.device_from_json({}).firmware is sd.FirmwareState.Unknown + + def test_missing_usb_ids_default_to_zero(self): + dev = sd.device_from_json(KLIPPER) # KLIPPER["vendor_id"] is 0 + assert dev.vendor_id == 0 + assert dev.product_id == 0 + assert dev.vid_hex == "" and dev.pid_hex == "" + + def test_usb_ids_are_parsed(self): + dev = sd.device_from_json( + {**KLIPPER, "vendor_id": 0x1D50, "product_id": 0x614E} + ) + assert dev.vendor_id == 0x1D50 + assert dev.product_id == 0x614E + assert dev.vid_hex == "0x1d50" + assert dev.pid_hex == "0x614e" + + def test_garbage_usb_ids_default_to_zero(self): + dev = sd.device_from_json({**KLIPPER, "vendor_id": "not-a-number"}) + assert dev.vendor_id == 0 + + def test_repr_matches_legacy_format(self): + dev = sd.device_from_json(KLIPPER) + assert repr(dev) == ( + " /dev/ttyACM0> " + f"{KLIPPER['symlink_name']} " + ) + + +class TestScanViaDaemon: + def test_only_serial_entries(self, daemon, sock_path): + daemon([_hello(), {"type": "snapshot", "devices": [KLIPPER, WEBCAM, BEACON]}]) + devs = sd.SerialScanner(sock_path).scan() + assert [d.symlink_name for d in devs] == [ + KLIPPER["symlink_name"], + BEACON["symlink_name"], + ] + + @pytest.mark.parametrize( + ("method", "expected"), + [ + ("scan_klipper", KLIPPER), + ("scan_katapult", KATAPULT), + ("scan_unflashed", CH340), + ], + ) + def test_filters(self, daemon, sock_path, method, expected): + # Unknown (BEACON) must not count as unflashed - legacy semantics. + daemon( + [ + _hello(), + {"type": "snapshot", "devices": [KLIPPER, KATAPULT, CH340, BEACON]}, + ] + ) + devs = getattr(sd.SerialScanner(sock_path), method)() + assert [d.symlink_name for d in devs] == [expected["symlink_name"]] + + def test_empty_snapshot_is_not_a_fallback(self, daemon, sock_path, monkeypatch): + daemon([_hello(), {"type": "snapshot", "devices": []}]) + monkeypatch.setattr(sd.SerialScanner, "legacy_scan", staticmethod(pytest.fail)) + assert sd.SerialScanner(sock_path).scan() == [] + + +@pytest.fixture +def legacy_by_id(tmp_path, monkeypatch): + """Point the by-id scan at a fake /dev/serial/by-id.""" + by_id = tmp_path / "by-id" + by_id.mkdir() + for name in (KLIPPER["symlink_name"], BEACON["symlink_name"]): + os.symlink("/dev/null", by_id / name) + monkeypatch.setattr(sd, "SERIAL_BY_ID_PATH", f"{by_id}/") + return by_id + + +class TestFallback: + def test_no_daemon_uses_legacy_scan(self, sock_path, legacy_by_id): + devs = sd.SerialScanner(sock_path).scan() # nothing listening + by_name = {d.symlink_name: d for d in devs} + assert set(by_name) == {KLIPPER["symlink_name"], BEACON["symlink_name"]} + klipper = by_name[KLIPPER["symlink_name"]] + assert isinstance(klipper, sd.Device) + assert klipper.firmware is sd.FirmwareState.Klipper + assert klipper.mcu_type == "stm32h723xx" + + def test_protocol_mismatch_uses_legacy_scan(self, daemon, sock_path, legacy_by_id): + daemon([_hello(proto=99), {"type": "snapshot", "devices": [WEBCAM]}]) + devs = sd.SerialScanner(sock_path).scan() + assert len(devs) == 2 + + def test_silent_daemon_times_out(self, daemon, sock_path, legacy_by_id): + daemon([]) # accepts, never sends + devs = sd.SerialScanner(sock_path, timeout=0.3).scan() + assert len(devs) == 2 + + def test_no_by_id_dir_is_empty(self, tmp_path, monkeypatch): + monkeypatch.setattr(sd, "SERIAL_BY_ID_PATH", f"{tmp_path}/by-id/") + assert sd.SerialScanner.legacy_scan() == [] + + +@pytest.mark.parametrize( + ("name", "expected"), + [ + ( + "usb-Klipper_rp2040_my_board_E66-if01", + ("Klipper", "rp2040_my_board", "E66", "rp2040", "01", "Klipper rp2040"), + ), + ( + "usb-CanBoot_stm32f446xx_123-if00", + ( + "CanBoot", + "stm32f446xx", + "123", + "stm32f446xx", + "00", + "CanBoot stm32f446xx", + ), + ), + ( + "usb-1a86_USB_Serial-if00-port0", + ("1a86", "USB", "Serial", "", "00", "1a86 USB"), + ), + ("usb-Foo_Bar", ("Foo", "Bar", "", "", "", "Foo Bar")), + ("plainname", ("plainname", "", "", "", "", "plainname")), + ], +) +def test_device_from_symlink(name, expected) -> None: + dev = sd.device_from_symlink(name, "/dev/null") + assert ( + dev.manufacturer, + dev.product, + dev.serial_number, + dev.mcu_type, + dev.interface, + dev.name, + ) == expected + + +class TestFetchSnapshot: + def test_returns_devices(self, daemon, sock_path): + daemon([_hello(), {"type": "snapshot", "devices": [KLIPPER]}]) + assert dd_client.fetch_snapshot(sock_path) == [KLIPPER] + + def test_garbage_returns_none(self, daemon, sock_path): + daemon([b"not json"]) + assert dd_client.fetch_snapshot(sock_path, timeout=1.0) is None + + def test_missing_socket_returns_none(self, sock_path): + assert dd_client.fetch_snapshot(sock_path) is None + + +class TestSysfsUsbIds: + """IDs read from sysfs when the daemon reports none (it never does for Serial).""" + + @pytest.fixture + def sysfs(self, tmp_path, monkeypatch): + # /sys/class/tty/ttyACM1/device -> .../usb1/1-1/1-1:1.0 (interface); + # idVendor/idProduct live on the parent USB device 1-1. + usb_dev = tmp_path / "devices" / "usb1" / "1-1" + iface = usb_dev / "1-1:1.0" + iface.mkdir(parents=True) + (usb_dev / "idVendor").write_text("1d50\n") + (usb_dev / "idProduct").write_text("b001\n") + tty = tmp_path / "class" / "tty" / "ttyACM1" + tty.mkdir(parents=True) + (tty / "device").symlink_to(iface) + monkeypatch.setattr(sd, "SYSFS_TTY_PATH", str(tmp_path / "class" / "tty")) + return tmp_path + + def test_daemon_serial_entry_gets_ids_from_sysfs(self, sysfs): + dev = sd.device_from_json( + { + "connection": "Serial", + "symlink_name": "usb-Klipper_stm32h723xx_2B00-if00", + "device_path": "/dev/ttyACM1", + "vendor_id": 0, + "product_id": 0, + } + ) + assert (dev.vendor_id, dev.product_id) == (0x1D50, 0xB001) + + def test_reported_ids_win_over_sysfs(self, sysfs): + dev = sd.device_from_json( + {"device_path": "/dev/ttyACM1", "vendor_id": 0x1234, "product_id": 1} + ) + assert (dev.vendor_id, dev.product_id) == (0x1234, 1) + + def test_unknown_tty_stays_zero(self, sysfs): + dev = sd.device_from_json({"device_path": "/dev/ttyACM9"}) + assert (dev.vendor_id, dev.product_id) == (0, 0) + + def test_non_tty_path_is_ignored(self, sysfs): + assert sd.usb_ids_from_sysfs(sd.Device(device_path="/etc/passwd")) == (0, 0) + + def test_amu_matched_by_id_without_amu_in_name(self, sysfs): + from tools.configuration_manager.device_profiles import load_profiles + + amu = next(p for p in load_profiles(override=None) if p.name == "amu") + dev = sd.device_from_json( + { + "symlink_name": "usb-Klipper_stm32h723xx_2B00-if00", + "device_path": "/dev/ttyACM1", + } + ) + assert amu.matches(dev) + stock = sd.Device( + symlink_name="usb-Klipper_AMU_stock-if00", vendor_id=0x1D50, product_id=0x614E + ) + assert not amu.matches(stock) # stock Klipper ID: ID mismatch is final diff --git a/tests/panels/test_main_window_unit.py b/tests/panels/test_main_window_unit.py index 4409f0b7..0e80fe17 100644 --- a/tests/panels/test_main_window_unit.py +++ b/tests/panels/test_main_window_unit.py @@ -11,7 +11,7 @@ def _make_fake_window( *, manual_restart_pending: bool = False, ws_connected: bool = True ) -> SimpleNamespace: - return SimpleNamespace( + win = SimpleNamespace( _klippy_ready=False, _update_in_progress=False, _klipper_auto_restart_pending=False, @@ -23,7 +23,11 @@ def _make_fake_window( loadwidget=MagicMock(), loadscreen=MagicMock(), ws=MagicMock(connected=ws_connected), + _try_device_config_restart=MagicMock(), + _prompt_device_confirmation=MagicMock(), ) + win._begin_klipper_restart = MethodType(MainWindow._begin_klipper_restart, win) + return win class TestKlippyDisconnectedAutoRestartGuard: @@ -129,3 +133,170 @@ def test_reset_tab_indexes_holds_job_status_when_paused(self) -> None: win = _make_nav_window("paused") win.reset_tab_indexes() win.printPanel.setCurrentIndex.assert_called_once_with(_JOB_STATUS_IDX) + + +def _make_device_window(**state) -> SimpleNamespace: + """Fake window running the real device-config prompt/restart methods.""" + win = SimpleNamespace( + _klippy_state="ready", + _print_state="standby", + _print_state_known=True, + print_status="idle", + _popup_toggle=False, + _update_in_progress=False, + _klipper_auto_restart_pending=False, + _device_restart_pending=False, + _pending_device_confirm=[], + _device_confirm_names=[], + _klipper_restart_timeout=MagicMock(), + loadwidget=MagicMock(), + loadscreen=MagicMock(), + ws=MagicMock(), + device_config_accepted=MagicMock(), + device_config_declined=MagicMock(), + _show_device_confirmation=MagicMock(), + ) + win.__dict__.update(state) + for name in ( + "_is_job_active", + "request_klipper_restart", + "_device_restart_blocked", + "_try_device_config_restart", + "_begin_klipper_restart", + "on_device_config_changed", + "on_device_confirmation_required", + "_device_prompt_blocked", + "_prompt_device_confirmation", + "_on_device_config_accept", + "_on_device_config_decline", + ): + setattr(win, name, MethodType(getattr(MainWindow, name), win)) + return win + + +class TestDeviceConfigRestart: + def test_restarts_klipper_service_when_idle(self) -> None: + win = _make_device_window() + win.ws.api.restart_service.return_value = True + win.on_device_config_changed(["amu"]) + win.ws.api.restart_service.assert_called_once_with("klipper") + assert win._device_restart_pending is False + # marked like the auto-restart so "disconnected" doesn't restart again + assert win._klipper_auto_restart_pending is True + + @pytest.mark.parametrize( + "state", + [ + {"_print_state": "printing"}, + {"_print_state": "paused"}, + {"print_status": "printing"}, + {"_klippy_state": None}, + {"_print_state_known": False}, + {"_update_in_progress": True}, + {"_klipper_auto_restart_pending": True}, + ], + ) + def test_deferred_while_unsafe(self, state) -> None: + win = _make_device_window(**state) + win.request_klipper_restart() + win.ws.api.restart_service.assert_not_called() + assert win._device_restart_pending is True + + def test_klipper_not_ready_needs_no_print_state(self) -> None: + win = _make_device_window(_klippy_state="error", _print_state_known=False) + win.ws.api.restart_service.return_value = True + win.request_klipper_restart() + win.ws.api.restart_service.assert_called_once_with("klipper") + + def test_retried_after_moonraker_was_disconnected(self) -> None: + win = _make_device_window() + win.ws.api.restart_service.return_value = False + win.request_klipper_restart() + assert win._device_restart_pending is True + win.ws.api.restart_service.return_value = True + win._try_device_config_restart() # e.g. on websocket connected + assert win._device_restart_pending is False + assert win.ws.api.restart_service.call_count == 2 + + def test_deferred_restart_runs_when_print_ends(self) -> None: + win = _make_device_window(_print_state="printing") + win.request_klipper_restart() + win.ws.api.restart_service.assert_not_called() + win._print_state = "complete" + win.ws.api.restart_service.return_value = True + win._try_device_config_restart() + win.ws.api.restart_service.assert_called_once_with("klipper") + + +class TestDeviceConfirmationPrompt: + def test_prompt_shown_when_idle(self) -> None: + win = _make_device_window() + win.on_device_confirmation_required(["amu"]) + win._show_device_confirmation.assert_called_once() + + @pytest.mark.parametrize( + "state", + [ + {"_print_state": "printing"}, + {"_popup_toggle": True}, + {"_update_in_progress": True}, + ], + ) + def test_prompt_held_while_blocked(self, state) -> None: + win = _make_device_window(**state) + win.on_device_confirmation_required(["amu"]) + win._show_device_confirmation.assert_not_called() + assert win._pending_device_confirm == ["amu"] + + def test_accept_and_decline_emit_the_asked_names(self) -> None: + win = _make_device_window(_device_confirm_names=["amu"]) + win._on_device_config_accept() + win.device_config_accepted.emit.assert_called_once_with(["amu"]) + assert win._device_confirm_names == [] + win._device_confirm_names = ["amu"] + win._on_device_config_decline() + win.device_config_declined.emit.assert_called_once_with(["amu"]) + + +def test_real_confirmation_popup_opens_and_accepts(qtbot, monkeypatch) -> None: + import importlib.util + import pathlib + import sys + + from PyQt6 import QtWidgets + + # conftest stubs lib.panels.widgets.Common.basePopup; load the real widget here. + path = ( + pathlib.Path(__file__).parents[2] + / "BlocksScreen/lib/panels/widgets/Common/basePopup.py" + ) + spec = importlib.util.spec_from_file_location("_real_basePopup", path) + real = importlib.util.module_from_spec(spec) + spec.loader.exec_module(real) + monkeypatch.setattr(sys.modules[MainWindow.__module__], "BasePopup", real.BasePopup) + + class _Host(QtWidgets.QMainWindow): + # BasePopup needs a real widget parent; borrow the real method. + _show_device_confirmation = MainWindow._show_device_confirmation + + host = _Host() + qtbot.addWidget(host) + host._device_config_popup = None + host._device_confirm_names = [] + host._pending_device_confirm = ["amu"] + host._on_device_config_accept = MagicMock() + host._on_device_config_decline = MagicMock() + + host._show_device_confirmation() + popup = host._device_config_popup + assert popup.isVisible() + assert host._device_confirm_names == ["amu"] + assert host._pending_device_confirm == [] + # another device while it's open: same popup, names appended, not replaced + host._pending_device_confirm = ["beacon"] + host._show_device_confirmation() + assert host._device_config_popup is popup + assert host._device_confirm_names == ["amu", "beacon"] + popup.accept() + host._on_device_config_accept.assert_called_once() + host._on_device_config_decline.assert_not_called() diff --git a/tests/updater/test_executor_unit.py b/tests/updater/test_executor_unit.py index c5b250e8..eb7c2c89 100644 --- a/tests/updater/test_executor_unit.py +++ b/tests/updater/test_executor_unit.py @@ -45,6 +45,7 @@ klipper_printing, restart_service, restart_service_noblock, + service_unit_missing, run_hook, wait_for_http_ready, ) @@ -1149,6 +1150,33 @@ async def test_false_when_import_subprocess_fails(self, tmp_path): assert await verify_updater_importable(tmp_path) is False +class TestServiceUnitMissing: + @pytest.mark.asyncio + @pytest.mark.parametrize( + ("returncode", "stdout", "expected"), + [ + (0, b"not-found\n", True), + (0, b"loaded\n", False), + (0, b"masked\n", False), + (1, b"", False), # query failed: not a definite "missing" + ], + ) + async def test_only_definite_not_found_is_missing( + self, returncode, stdout, expected + ): + proc = _make_proc(returncode=returncode, stdout=stdout) + with patch( + "asyncio.create_subprocess_exec", new_callable=AsyncMock, return_value=proc + ) as mock_exec: + assert await service_unit_missing("device-discoveryd.service") is expected + assert "--property=LoadState" in mock_exec.call_args[0] + + @pytest.mark.asyncio + async def test_invalid_name_never_queries(self): + with patch("asyncio.create_subprocess_exec") as mock_exec: + assert await service_unit_missing("bad;name.service") is False + mock_exec.assert_not_called() + class TestRunHook: """run_hook: resolve under hooks dir, pass timeout, reject traversal.""" diff --git a/tests/updater/test_hooks_unit.py b/tests/updater/test_hooks_unit.py new file mode 100644 index 00000000..d52c553d --- /dev/null +++ b/tests/updater/test_hooks_unit.py @@ -0,0 +1,68 @@ +"""Tests for the per-component shell hooks in updater/hooks/.""" + +from __future__ import annotations + +import os +import subprocess +from pathlib import Path + +import pytest + +HOOK = Path(__file__).resolve().parents[2] / "updater" / "hooks" / "DeviceDiscovery.sh" + + +def _run_hook(tmp_path: Path, repo: Path, load_state: str) -> subprocess.CompletedProcess: + """Run the hook with a stub systemctl that reports load_state.""" + stub_bin = tmp_path / "stub-bin" + stub_bin.mkdir() + systemctl = stub_bin / "systemctl" + systemctl.write_text(f"#!/bin/bash\necho {load_state}\n") + systemctl.chmod(0o755) + env = { + "PATH": f"{stub_bin}:{os.environ['PATH']}", + "COMPONENT_PATH": str(repo), + "BS_UPDATER_SELF_UPDATE": "1", + "BS_UPDATER_RESTART_SENTINEL": str(tmp_path / "run" / "sentinel"), + } + return subprocess.run( + ["bash", str(HOOK)], env=env, capture_output=True, text=True, check=False + ) + + +@pytest.fixture +def repo(tmp_path: Path) -> Path: + repo = tmp_path / "DeviceDiscovery" + (repo / "scripts").mkdir(parents=True) + build = repo / "scripts" / "build.sh" + build.write_text(f"#!/bin/bash\ntouch {repo}/built\n") + build.chmod(0o755) + return repo + + +def _install_binary(repo: Path) -> None: + (repo / "bin").mkdir() + binary = repo / "bin" / "device_discoveryd" + binary.write_text("") + binary.chmod(0o755) + + +class TestDeviceDiscoveryHook: + def test_fresh_clone_defers_setup_to_install_updater(self, tmp_path, repo): + result = _run_hook(tmp_path, repo, "not-found") + assert result.returncode == 0, result.stderr + assert (tmp_path / "run" / "sentinel").read_text().split() == ["install"] + assert not (repo / "built").exists() + + def test_binary_without_unit_still_defers(self, tmp_path, repo): + _install_binary(repo) + result = _run_hook(tmp_path, repo, "not-found") + assert result.returncode == 0, result.stderr + assert (tmp_path / "run" / "sentinel").exists() + assert not (repo / "built").exists() + + def test_set_up_machine_rebuilds(self, tmp_path, repo): + _install_binary(repo) + result = _run_hook(tmp_path, repo, "loaded") + assert result.returncode == 0, result.stderr + assert (repo / "built").exists() + assert not (tmp_path / "run" / "sentinel").exists() diff --git a/tests/updater/test_service_unit.py b/tests/updater/test_service_unit.py index 043769a8..3622f745 100644 --- a/tests/updater/test_service_unit.py +++ b/tests/updater/test_service_unit.py @@ -13,6 +13,13 @@ @pytest.fixture(autouse=True) +def units_installed(): + """Report every systemd unit as installed so restart-failure tests don't + depend on which units exist on the machine running them.""" + with patch( + "updater.service.service_unit_missing", AsyncMock(return_value=False) + ) as mock: + yield mock def _isolate_inflight(tmp_path_factory, monkeypatch): """Keep the in-flight marker, state and history out of the real cache.""" marker = tmp_path_factory.mktemp("inflight") / "updater_inflight.json" @@ -1330,6 +1337,36 @@ async def fake_restart(name): done = {c.args[0]: c.args[1] for c in cb.on_component_done.call_args_list} assert done == {"c0": True, "c1": False} + @pytest.mark.asyncio + async def test_restart_of_uninstalled_unit_does_not_abort( + self, tmp_path, units_installed + ): + # A component cloned before its unit was set up (install-updater still + # pending) must not revert every other component in the batch. + cb = MagicMock() + svc, comps = self._svc(tmp_path, cb, n=2, service="device-discoveryd.service") + units_installed.return_value = True + with ( + patch( + "updater.service.UpdateService._stage_component", + return_value=(True, ""), + ), + patch("updater.service.git_reset_to_hash") as mr, + patch( + "updater.service.UpdateService._install_dependencies", + return_value=(True, ""), + ), + patch("updater.service.run_hook", return_value=(True, "")), + patch("updater.service.restart_service", return_value=(False, "no unit")), + patch("updater.service.wait_for_service_active") as mock_wait, + patch.object(UpdateService, "_apply_deferred_restart", new=AsyncMock()), + ): + await svc._run_git_batch(comps) + mr.assert_not_called() + mock_wait.assert_not_called() + assert all(c.args[1] is True for c in cb.on_component_done.call_args_list) + cb.on_error.assert_not_called() + @pytest.mark.asyncio async def test_cancel_mid_batch_reverts_staged(self, tmp_path): cb = MagicMock() @@ -1982,6 +2019,41 @@ async def test_provision_skips_restart_when_hook_started_service(self, tmp_path) assert ok is True mock_restart.assert_not_called() + @pytest.mark.asyncio + async def test_provision_with_uninstalled_unit_defers_setup( + self, tmp_path, units_installed + ): + # First clone: the hook deferred setup to install-updater, so the unit + # does not exist yet. Provisioning succeeds, keeps the clone, and applies + # the deferred install (touches the deploy flag). + comp = self._comp(tmp_path, service="newcomp.service") + cb = MagicMock() + units_installed.return_value = True + deferred = AsyncMock() + with ( + patch("updater.service.git_clone", return_value=(True, "")), + patch("updater.service.git_get_hash", return_value="newhash"), + patch( + "updater.service.UpdateService._install_dependencies", + return_value=(True, ""), + ), + patch("updater.service.run_hook", return_value=(True, "")), + patch("updater.service.restart_service", return_value=(False, "no unit")), + patch("updater.service.wait_for_service_active") as mock_wait, + patch("updater.service.enable_service") as mock_enable, + patch("updater.service.shutil.rmtree") as mock_rmtree, + patch.object(UpdateService, "_apply_deferred_restart", new=deferred), + ): + svc = UpdateService(callback=cb) + svc._components = [comp] + ok = await svc.update_component("newcomp") + assert ok is True + mock_wait.assert_not_called() + mock_enable.assert_not_called() # install-updater enables it + mock_rmtree.assert_not_called() + deferred.assert_awaited_once() + cb.on_component_done.assert_called_once_with("newcomp", True) + @pytest.mark.asyncio async def test_check_status_reports_needs_install(self, tmp_path): comp = self._comp(tmp_path) diff --git a/tests/util/__init__.py b/tests/util/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/tests/util/test_configurator_unit.py b/tests/util/test_configurator_unit.py new file mode 100644 index 00000000..e67dd1ef --- /dev/null +++ b/tests/util/test_configurator_unit.py @@ -0,0 +1,861 @@ +"""Unit tests for BlocksScreen.tools.configuration_manager. + +Tests module-level functions and ConfigManager methods using tmp_path +for all filesystem operations, no Qt dependencies needed. +""" + +import configparser +import hashlib +import logging +import os +import pathlib +import shutil +import subprocess +import sys +import threading +from unittest.mock import MagicMock, patch + +import pytest + +# configurator.py imports top-level packages (`from devices...`) the way the +# app runs it, with BlocksScreen/ on sys.path. Appended so the `BlocksScreen` +# package isn't shadowed by the BlocksScreen/BlocksScreen.py entry script. +_BS_DIR = os.path.abspath( + os.path.join(os.path.dirname(__file__), "..", "..", "BlocksScreen") +) +if _BS_DIR not in sys.path: + sys.path.append(_BS_DIR) + +from BlocksScreen.tools.configuration_manager.configurator import ( # noqa: E402 + SV_CONFIG_MARKER, + ConfigManager, + check_broken_symlinks, + copy_file_simple, + ensure_dir, + get_file_checksum, + is_git_repo, + resolve_symlink, +) + + +# ========================================================================= +# Helpers +# ========================================================================= + + +def _write(path: pathlib.Path, content: str) -> pathlib.Path: + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text(content, encoding="utf-8") + return path + + +def _make_manager(tmp_path: pathlib.Path, **attrs) -> ConfigManager: + """Create a ConfigManager bypassing the heavy __init__ side effects. + + Builds repo / config / backup dirs underneath tmp_path. + Override any attribute via **attrs. + """ + repo = tmp_path / "repo" + config_dir = tmp_path / "config" + backup = tmp_path / "backup" + repo.mkdir(parents=True, exist_ok=True) + config_dir.mkdir(parents=True, exist_ok=True) + backup.mkdir(parents=True, exist_ok=True) + + mock_cfg = MagicMock() + mock_section = MagicMock() + mock_section.get.side_effect = ( + lambda key, default="": { + "config_repo": str(repo), + "config_dir": str(config_dir), + "backup_dir": str(backup), + "variant": "RF50", + }.get(key, default) + ) + mock_cfg.get_section.return_value = mock_section + + inst = object.__new__(ConfigManager) + inst.config = mock_section + inst.mergeLock = threading.Lock() + inst.repo = repo + inst.config_dir = config_dir + inst.backup_dir = backup + inst.config_variant = "RF50" + inst.cpy_files = ["printer.cfg", "variables.cfg", "BlocksScreen.cfg"] + inst.repo_fi_name = {} + inst.repo_fi_relpath = {} + inst.config_fi_name = {} + inst.config_fi_relpath = {} + + for k, v in attrs.items(): + setattr(inst, k, v) + + return inst + + +@pytest.fixture +def mock_home(tmp_path, monkeypatch): + """Make Path.home() point inside tmp_path so relative_to(home) works.""" + home = tmp_path / "home" / "user" + home.mkdir(parents=True) + monkeypatch.setattr(pathlib.Path, "home", lambda _cls: home) + return home + + +# ========================================================================= +# Module-level helpers +# ========================================================================= + + +class TestIsGitRepo: + def test_not_exist(self, tmp_path): + assert is_git_repo(tmp_path / "nope") is False + + def test_is_not_a_dir(self, tmp_path): + f = _write(tmp_path / "file.txt", "hi") + assert is_git_repo(f) is False + + def test_dir_no_git(self, tmp_path): + d = tmp_path / "plain_dir" + d.mkdir() + assert is_git_repo(d) is False + + def test_is_git(self, tmp_path): + d = tmp_path / "myrepo" + d.mkdir() + subprocess.run(["git", "init"], cwd=str(d), capture_output=True) + assert is_git_repo(d) is True + + +class TestEnsureDir: + def test_creates(self, tmp_path): + target = tmp_path / "a" / "b" + result = ensure_dir(target) + assert result == target + assert target.is_dir() + + def test_already_exists(self, tmp_path): + target = tmp_path / "exists" + target.mkdir() + result = ensure_dir(target) + assert result == target + + def test_returns_path_even_on_failure(self, tmp_path): + target = tmp_path / "sub" / "file.txt" + target.parent.mkdir() + target.write_text("x") + result = ensure_dir(target) + assert result == target + + +class TestCheckBrokenSymlinks: + def test_empty_dir(self, tmp_path): + assert check_broken_symlinks(tmp_path) == [] + + def test_no_symlinks(self, tmp_path): + _write(tmp_path / "a.cfg", "a") + assert check_broken_symlinks(tmp_path) == [] + + def test_valid_symlink(self, tmp_path): + real = _write(tmp_path / "real.txt", "content") + link = tmp_path / "link.txt" + link.symlink_to(real) + broken = check_broken_symlinks(tmp_path) + assert link not in broken + + def test_broken_symlink(self, tmp_path): + link = tmp_path / "broken" + link.symlink_to(tmp_path / "ghost") + broken = check_broken_symlinks(tmp_path) + assert link in broken + assert len(broken) == 1 + + def test_mixed(self, tmp_path): + real = _write(tmp_path / "real.txt", "ok") + valid = tmp_path / "valid_link" + valid.symlink_to(real) + broken = tmp_path / "broken_link" + broken.symlink_to(tmp_path / "nowhere") + result = check_broken_symlinks(tmp_path) + assert valid not in result + assert broken in result + + def test_not_exist(self, tmp_path): + assert check_broken_symlinks(tmp_path / "ghost") == [] + + +class TestResolveSymlink: + def test_symlink_found(self, tmp_path): + real = _write(tmp_path / "source.cfg", "x") + link = tmp_path / "configs" / "source.cfg" + link.parent.mkdir() + link.symlink_to(real) + assert resolve_symlink(real, {link.resolve().as_posix()}) is True + + def test_symlink_not_found(self, tmp_path): + real = _write(tmp_path / "source.cfg", "x") + other = _write(tmp_path / "other.cfg", "x") + assert resolve_symlink(real, {other.resolve().as_posix()}) is False + + def test_empty_set(self, tmp_path): + assert resolve_symlink(_write(tmp_path / "a.cfg", "x"), set()) is False + + def test_symlinked_file_resolves_to_its_target(self, tmp_path): + real = _write(tmp_path / "original.cfg", "content") + deep = tmp_path / "sub" / "original.cfg" + deep.parent.mkdir() + deep.symlink_to(real) + assert resolve_symlink(deep, {real.resolve().as_posix()}) is True + + +class TestGetFileChecksum: + def test_file_exists(self, tmp_path): + f = _write(tmp_path / "a.cfg", "hello world") + h = get_file_checksum(f) + assert isinstance(h, str) + assert len(h) == 64 + assert h == hashlib.sha256(b"hello world").hexdigest() + + def test_file_not_found(self, tmp_path): + with pytest.raises(FileNotFoundError): + get_file_checksum(tmp_path / "ghost") + + def test_identical_files_same_checksum(self, tmp_path): + a = _write(tmp_path / "a.cfg", "same") + b = _write(tmp_path / "b.cfg", "same") + assert get_file_checksum(a) == get_file_checksum(b) + + def test_different_files_different_checksum(self, tmp_path): + a = _write(tmp_path / "a.cfg", "foo") + b = _write(tmp_path / "b.cfg", "bar") + assert get_file_checksum(a) != get_file_checksum(b) + + def test_empty_file(self, tmp_path): + f = tmp_path / "empty.cfg" + f.touch() + h = get_file_checksum(f) + assert h == hashlib.sha256(b"").hexdigest() + + +class TestCopyFileSimple: + def test_copies_to_dir(self, tmp_path): + src = _write(tmp_path / "src.cfg", "content") + dest = tmp_path / "output" + dest.mkdir() + result = copy_file_simple(src, dest) + assert (dest / "src.cfg").is_file() + assert (dest / "src.cfg").read_text() == "content" + assert result == str(dest / "src.cfg") + + def test_src_missing(self, tmp_path): + src = tmp_path / "ghost.cfg" + dest = tmp_path / "out" + dest.mkdir() + with pytest.raises(FileNotFoundError): + copy_file_simple(src, dest) + + def test_copies_to_file_path(self, tmp_path): + src = _write(tmp_path / "src.cfg", "data") + dest = tmp_path / "output" + dest.mkdir() + copy_file_simple(src, dest / "renamed.cfg") + assert (dest / "renamed.cfg").is_file() + + +# ========================================================================= +# _build_file_index +# ========================================================================= + + +class TestBuildFileIndex: + def test_empty(self, tmp_path): + mgr = _make_manager(tmp_path) + n, r = mgr._build_file_index(tmp_path / "nonexistent") + assert n == {} + assert r == {} + + def test_flat_files(self, tmp_path): + mgr = _make_manager(tmp_path) + _write(tmp_path / "a.cfg", "a") + _write(tmp_path / "b.cfg", "b") + n, r = mgr._build_file_index(tmp_path) + assert set(n.keys()) == {"a.cfg", "b.cfg"} + assert set(r.keys()) == {"a.cfg", "b.cfg"} + + def test_subdirectory(self, tmp_path): + mgr = _make_manager(tmp_path) + _write(tmp_path / "sub" / "deep.cfg", "x") + n, r = mgr._build_file_index(tmp_path) + assert "deep.cfg" in n + assert "sub/deep.cfg" in r + + def test_duplicate_filenames(self, tmp_path): + mgr = _make_manager(tmp_path) + _write(tmp_path / "a.cfg", "v1") + _write(tmp_path / "sub" / "a.cfg", "v2") + n, r = mgr._build_file_index(tmp_path) + assert len(n["a.cfg"]) == 2 + assert "a.cfg" in r + assert "sub/a.cfg" in r + + def test_ignores_git(self, tmp_path): + mgr = _make_manager(tmp_path) + (tmp_path / ".git").mkdir() + _write(tmp_path / ".git" / "config", "[core]") + _write(tmp_path / "real.cfg", "ok") + n, r = mgr._build_file_index(tmp_path) + assert "config" not in n + assert "real.cfg" in n + + def test_ignores_directories(self, tmp_path): + mgr = _make_manager(tmp_path) + _write(tmp_path / "sub" / "file.cfg", "x") + n, r = mgr._build_file_index(tmp_path) + assert "sub" not in n + assert "sub/file.cfg" in r + + +# ========================================================================= +# _get_missing_symlinks +# ========================================================================= + + +class TestGetMissingSymlinks: + def test_all_present(self, tmp_path): + mgr = _make_manager(tmp_path) + repo = mgr.repo + cfg_dir = mgr.config_dir + src = _write(repo / "printer.cfg", "x") + (cfg_dir / "printer.cfg").symlink_to(src) + mgr.cpy_files = [] + result = mgr._get_missing_symlinks(cfg_dir, repo) + assert result == [] + + def test_some_missing(self, tmp_path): + mgr = _make_manager(tmp_path) + repo = mgr.repo + cfg_dir = mgr.config_dir + src_a = _write(repo / "a.cfg", "a") + _write(repo / "b.cfg", "b") + (cfg_dir / "a.cfg").symlink_to(src_a) + mgr.cpy_files = [] + result = mgr._get_missing_symlinks(cfg_dir, repo) + assert len(result) == 1 + assert result[0].name == "b.cfg" + + def test_none_present(self, tmp_path): + mgr = _make_manager(tmp_path) + repo = mgr.repo + cfg_dir = mgr.config_dir + _write(repo / "a.cfg", "a") + _write(repo / "b.cfg", "b") + mgr.cpy_files = [] + result = mgr._get_missing_symlinks(cfg_dir, repo) + assert len(result) == 2 + + def test_excludes_copy_files(self, tmp_path): + mgr = _make_manager(tmp_path) + repo = mgr.repo + cfg_dir = mgr.config_dir + _write(repo / "printer.cfg", "x") + _write(repo / "variables.cfg", "y") + _write(repo / "other.cfg", "z") + mgr.cpy_files = ["printer.cfg", "variables.cfg"] + result = mgr._get_missing_symlinks(cfg_dir, repo) + names = [p.name for p in result] + assert "printer.cfg" not in names + assert "variables.cfg" not in names + assert "other.cfg" in names + + def test_excludes_git(self, tmp_path): + mgr = _make_manager(tmp_path) + repo = mgr.repo + cfg_dir = mgr.config_dir + (repo / ".git").mkdir() + _write(repo / ".git" / "head.cfg", "x") + _write(repo / "real.cfg", "y") + mgr.cpy_files = [] + result = mgr._get_missing_symlinks(cfg_dir, repo) + assert any(p.name == "real.cfg" for p in result) + assert not any(".git" in p.parts for p in result) + + def test_root_not_exist(self, tmp_path): + mgr = _make_manager(tmp_path) + repo = mgr.repo + with pytest.raises(NotADirectoryError): + mgr._get_missing_symlinks(tmp_path / "ghost", repo) + + def test_repo_not_exist(self, tmp_path): + mgr = _make_manager(tmp_path) + cfg_dir = mgr.config_dir + with pytest.raises(NotADirectoryError): + mgr._get_missing_symlinks(cfg_dir, tmp_path / "ghost") + + +# ========================================================================= +# _symlink_config +# ========================================================================= + + +class TestSymlinkConfig: + def test_creates_symlinks(self, tmp_path, mock_home): + mgr = _make_manager(tmp_path, repo=mock_home / "repo") + mgr.config_dir = mock_home / "config" + mgr.config_dir.mkdir(parents=True, exist_ok=True) + src = _write(mock_home / "repo" / "a.cfg", "x") + mgr._symlink_config([src]) + target = mgr.config_dir / "a.cfg" + assert target.is_symlink() + assert target.resolve() == src.resolve() + + def test_skips_existing_valid(self, tmp_path, mock_home): + mgr = _make_manager(tmp_path, repo=mock_home / "repo") + mgr.config_dir = mock_home / "config" + mgr.config_dir.mkdir(parents=True, exist_ok=True) + src = _write(mock_home / "repo" / "a.cfg", "x") + existing = mgr.config_dir / "a.cfg" + existing.symlink_to(src) + mgr._symlink_config([src]) + assert existing.is_symlink() + assert existing.resolve() == src.resolve() + + def test_creates_parent_dirs(self, tmp_path, mock_home): + mgr = _make_manager(tmp_path, repo=mock_home / "repo") + mgr.config_dir = mock_home / "config" + mgr.config_dir.mkdir(parents=True, exist_ok=True) + src = _write(mock_home / "repo" / "sub" / "deep.cfg", "x") + mgr._symlink_config([src]) + target = mgr.config_dir / "sub" / "deep.cfg" + assert target.is_symlink() + assert target.resolve() == src.resolve() + + +# ========================================================================= +# merge_cfg — configparser merge (no marker, variables.cfg style) +# ========================================================================= + + +class TestMergeCfgParser: + def test_all_options_present_no_change(self, tmp_path): + mgr = _make_manager(tmp_path) + src = _write(tmp_path / "vars.cfg", "[global]\nopt_a: 1\nopt_b: 2\n") + tgt = _write(tmp_path / "vars_target.cfg", "[global]\nopt_a: 1\nopt_b: 2\n") + assert mgr.merge_cfg(src, tgt) is True + result = configparser.ConfigParser() + result.read_string(tgt.read_text()) + assert result.get("global", "opt_a") == "1" + assert result.get("global", "opt_b") == "2" + + def test_adds_missing_option(self, tmp_path): + mgr = _make_manager(tmp_path) + src = _write(tmp_path / "vars.cfg", "[global]\nopt_a: 1\nopt_b: 2\n") + tgt = _write(tmp_path / "vars_target.cfg", "[global]\nopt_a: 1\n") + assert mgr.merge_cfg(src, tgt) is True + result = configparser.ConfigParser() + result.read_string(tgt.read_text()) + assert result.get("global", "opt_a") == "1" + assert result.get("global", "opt_b") == "2" + + def test_keeps_existing_value(self, tmp_path): + mgr = _make_manager(tmp_path) + src = _write(tmp_path / "vars.cfg", "[global]\nopt_a: from_repo\n") + tgt = _write(tmp_path / "vars_target.cfg", "[global]\nopt_a: user_value\n") + assert mgr.merge_cfg(src, tgt) is True + result = configparser.ConfigParser() + result.read_string(tgt.read_text()) + assert result.get("global", "opt_a") == "user_value" + + def test_multiple_sections(self, tmp_path): + mgr = _make_manager(tmp_path) + src = _write( + tmp_path / "vars.cfg", + "[section_a]\nkey_a: repo_a\n[section_b]\nkey_b: repo_b\n", + ) + tgt = _write( + tmp_path / "vars_target.cfg", + "[section_a]\nkey_a: existing_a\n", + ) + assert mgr.merge_cfg(src, tgt) is True + result = configparser.ConfigParser() + result.read_string(tgt.read_text()) + assert result.get("section_a", "key_a") == "existing_a" + assert result.get("section_b", "key_b") == "repo_b" + + def test_target_not_exist_fails(self, tmp_path): + mgr = _make_manager(tmp_path) + src = _write(tmp_path / "vars.cfg", "[g]\nk: v\n") + tgt = tmp_path / "vars_ghost.cfg" + assert mgr.merge_cfg(src, tgt) is False + + def test_target_empty(self, tmp_path): + mgr = _make_manager(tmp_path) + src = _write(tmp_path / "vars.cfg", "[g]\nk: v\n") + tgt = tmp_path / "vars_target.cfg" + tgt.parent.mkdir(parents=True, exist_ok=True) + tgt.touch() + assert mgr.merge_cfg(src, tgt) is True + result = configparser.ConfigParser() + result.read_string(tgt.read_text()) + assert result.get("g", "k") == "v" + + def test_empty_string_value(self, tmp_path): + mgr = _make_manager(tmp_path) + src = _write(tmp_path / "vars.cfg", "[g]\nk:\n") + tgt = _write(tmp_path / "vars_target.cfg", "[g]\n") + assert mgr.merge_cfg(src, tgt) is True + result = configparser.ConfigParser() + result.read_string(tgt.read_text()) + assert result.get("g", "k") == "" + + def test_src_no_sections(self, tmp_path): + mgr = _make_manager(tmp_path) + src = _write(tmp_path / "vars.cfg", "# just a comment\n") + tgt = _write(tmp_path / "vars_target.cfg", "[g]\nk: v\n") + assert mgr.merge_cfg(src, tgt) is True + result = configparser.ConfigParser() + result.read_string(tgt.read_text()) + assert result.get("g", "k") == "v" + + def test_src_empty_file(self, tmp_path): + mgr = _make_manager(tmp_path) + src = tmp_path / "vars.cfg" + src.parent.mkdir(parents=True, exist_ok=True) + src.touch() + tgt = _write(tmp_path / "vars_target.cfg", "[g]\nk: v\n") + assert mgr.merge_cfg(src, tgt) is True + + +# ========================================================================= +# merge_cfg — marker-aware (printer.cfg, SAVE_CONFIG) +# ========================================================================= + + +class TestMergeCfgMarker: + def test_target_has_marker_preserved(self, tmp_path): + mgr = _make_manager(tmp_path) + src = _write( + tmp_path / "src_printer.cfg", + "[stepper_x]\nstep_pin: PC2\n[extruder]\nheater_pin: PA2\n", + ) + tgt = _write( + tmp_path / "tgt_printer.cfg", + "[stepper_x]\nstep_pin: PC2\n" + + SV_CONFIG_MARKER + "\n" + + "#*# saved_value: 123\n", + ) + assert mgr.merge_cfg(src, target_file=tgt, marker=SV_CONFIG_MARKER) is True + merged = tgt.read_text() + assert "[extruder]" in merged + assert SV_CONFIG_MARKER in merged + assert "#*# saved_value: 123" in merged + + def test_target_no_marker_copies_source(self, tmp_path): + mgr = _make_manager(tmp_path) + src = _write( + tmp_path / "src_printer.cfg", + "[section]\nopt: from_repo\n", + ) + tgt = _write( + tmp_path / "tgt_printer.cfg", + "[section]\nopt: from_target\n", + ) + assert mgr.merge_cfg(src, target_file=tgt, marker=SV_CONFIG_MARKER) is True + result = configparser.ConfigParser() + result.read_string(tgt.read_text()) + assert result.get("section", "opt") == "from_repo" + + def test_src_with_marker_target_also_with_marker(self, tmp_path): + mgr = _make_manager(tmp_path) + src = _write( + tmp_path / "src_printer.cfg", + "[header]\nval: 1\n" + + SV_CONFIG_MARKER + + "\n" + + "#*# [probe]\n#*# z_offset = 1.0\n#*# speed = 5\n#*#\n" + + "#*# [bed_mesh default]\n#*# version = 1\n", + ) + tgt = _write( + tmp_path / "tgt_printer.cfg", + "[header]\nval: 1\n" + + SV_CONFIG_MARKER + + "\n" + + "#*# [probe]\n#*# z_offset = 0.5\n", + ) + assert mgr.merge_cfg(src, target_file=tgt, marker=SV_CONFIG_MARKER) is True + lines = tgt.read_text().splitlines() + assert "#*# z_offset = 0.5" in lines and "#*# z_offset = 1.0" not in lines + # unindented, else Klipper reads it as a continuation of z_offset + assert "#*# speed = 5" in lines + assert "#*# [bed_mesh default]" in lines + + def test_marker_at_start_of_target(self, tmp_path): + mgr = _make_manager(tmp_path) + src = _write(tmp_path / "src_printer.cfg", "[header]\nval: 1\n") + tgt = _write( + tmp_path / "tgt_printer.cfg", + SV_CONFIG_MARKER + "\n#*# preserved: yes\n", + ) + assert mgr.merge_cfg(src, target_file=tgt, marker=SV_CONFIG_MARKER) is True + merged = tgt.read_text() + assert merged.startswith("[header]") + assert SV_CONFIG_MARKER in merged + assert "#*# preserved: yes" in merged + + def test_marker_only_no_save_block(self, tmp_path): + mgr = _make_manager(tmp_path) + src = _write(tmp_path / "src_printer.cfg", "[header]\nval: 1\n") + tgt = _write( + tmp_path / "tgt_printer.cfg", + SV_CONFIG_MARKER + "\n", + ) + assert mgr.merge_cfg(src, target_file=tgt, marker=SV_CONFIG_MARKER) is True + merged = tgt.read_text() + assert SV_CONFIG_MARKER in merged + assert merged.strip().endswith(SV_CONFIG_MARKER) + + def test_no_marker_argument_uses_configparser(self, tmp_path): + mgr = _make_manager(tmp_path) + src = _write(tmp_path / "src.cfg", "[g]\nk: repo_val\n") + tgt = _write(tmp_path / "tgt.cfg", "[g]\nk: tgt_val\n") + assert mgr.merge_cfg(src, tgt) is True + result = configparser.ConfigParser() + result.read_string(tgt.read_text()) + assert result.get("g", "k") == "tgt_val" + + def test_marker_provided_but_target_missing_fails(self, tmp_path): + mgr = _make_manager(tmp_path) + src = _write(tmp_path / "src_printer.cfg", "[g]\nk: v\n") + tgt = tmp_path / "tgt_ghost.cfg" + assert mgr.merge_cfg(src, target_file=tgt, marker=SV_CONFIG_MARKER) is False + + def test_multiple_markers_first_used(self, tmp_path): + mgr = _make_manager(tmp_path) + src = _write(tmp_path / "src_printer.cfg", "[header]\n") + tgt = _write( + tmp_path / "tgt_printer.cfg", + "[g]\nk: v\n" + + SV_CONFIG_MARKER + "\n" + + "#*# a: 1\n" + + SV_CONFIG_MARKER + "\n" + + "#*# b: 2\n", + ) + assert mgr.merge_cfg(src, target_file=tgt, marker=SV_CONFIG_MARKER) is True + merged = tgt.read_text() + idx = merged.index(SV_CONFIG_MARKER) + after_first_marker = merged[idx:] + assert "#*# a: 1" in after_first_marker + assert "#*# b: 2" in after_first_marker + + def test_src_has_marker_target_does_not(self, tmp_path): + mgr = _make_manager(tmp_path) + src = _write( + tmp_path / "src_printer.cfg", + "[header]\nval: 1\n" + + SV_CONFIG_MARKER + "\n" + + "#*# saved: val\n", + ) + tgt = _write( + tmp_path / "tgt_printer.cfg", + "[header]\nval: 1\n", + ) + assert mgr.merge_cfg(src, target_file=tgt, marker=SV_CONFIG_MARKER) is True + merged = tgt.read_text() + # target has no SAVE_CONFIG yet — copy source verbatim, preserving everything + assert "[header]" in merged + assert SV_CONFIG_MARKER in merged + assert "#*# saved: val" in merged + + + def test_mcu_section_preserved_from_target(self, tmp_path): + mgr = _make_manager(tmp_path) + src = _write( + tmp_path / "src_printer.cfg", + "[stepper_x]\nstep_pin: PC2\n" + "[mcu]\nserial: /dev/ttyACM0\n" + "[extruder]\nheater_pin: PA2\n", + ) + tgt = _write( + tmp_path / "tgt_printer.cfg", + "[stepper_x]\nstep_pin: PC2\n" + "[mcu]\nserial: /dev/ttyACM1\n" + "[extruder]\nheater_pin: PA2\n" + + SV_CONFIG_MARKER + "\n" + + "#*# saved: val\n", + ) + assert mgr.merge_cfg(src, target_file=tgt, marker=SV_CONFIG_MARKER) is True + merged = tgt.read_text() + assert "[mcu]" in merged + assert "serial: /dev/ttyACM1" in merged + assert "serial: /dev/ttyACM0" not in merged + assert SV_CONFIG_MARKER in merged + + def test_mcu_removed_when_missing_in_source(self, tmp_path): + mgr = _make_manager(tmp_path) + src = _write( + tmp_path / "src_printer.cfg", + "[stepper_x]\nstep_pin: PC2\n" + "[extruder]\nheater_pin: PA2\n", + ) + tgt = _write( + tmp_path / "tgt_printer.cfg", + "[stepper_x]\nstep_pin: PC2\n" + "[mcu pi]\nserial: /tmp/klipper_host_mcu\n" + "[extruder]\nheater_pin: PA2\n" + + SV_CONFIG_MARKER + "\n" + + "#*# saved: val\n", + ) + assert mgr.merge_cfg(src, target_file=tgt, marker=SV_CONFIG_MARKER) is True + merged = tgt.read_text() + assert "[mcu pi]" not in merged + assert "/tmp/klipper_host_mcu" not in merged + assert SV_CONFIG_MARKER in merged + + def test_multiple_mcu_sections_match_by_name(self, tmp_path): + mgr = _make_manager(tmp_path) + src = _write( + tmp_path / "src_printer.cfg", + "[mcu]\nserial: /dev/ttyACM0\n" + "[mcu Toolhead]\ncanbus_uuid: abc\n", + ) + tgt = _write( + tmp_path / "tgt_printer.cfg", + "[mcu]\nserial: /dev/ttyACM1\n" + "[mcu Toolhead]\ncanbus_uuid: def\n" + "[mcu pi]\nserial: /tmp/klipper_host_mcu\n" + + SV_CONFIG_MARKER + "\n" + + "#*# saved: val\n", + ) + assert mgr.merge_cfg(src, target_file=tgt, marker=SV_CONFIG_MARKER) is True + merged = tgt.read_text() + assert "[mcu]" in merged + assert "[mcu Toolhead]" in merged + assert "[mcu pi]" not in merged + assert "serial: /dev/ttyACM1" in merged + assert "canbus_uuid: def" in merged + + def test_mcu_preserves_all_target_options(self, tmp_path): + mgr = _make_manager(tmp_path) + src = _write( + tmp_path / "src_printer.cfg", + "[mcu]\nserial: /dev/ttyACM0\nbaud: 250000\n" + "[mcu Toolhead]\ncanbus_uuid: 123\n", + ) + tgt = _write( + tmp_path / "tgt_printer.cfg", + "[mcu]\nserial: /dev/ttyACM1\n" + "[mcu Toolhead]\ncanbus_uuid: 456\n" + "[mcu pi]\nserial: /tmp/klipper_host_mcu\n" + + SV_CONFIG_MARKER + "\n" + + "#*# saved: val\n", + ) + assert mgr.merge_cfg(src, target_file=tgt, marker=SV_CONFIG_MARKER) is True + merged = tgt.read_text() + assert "[mcu]" in merged + assert "[mcu Toolhead]" in merged + assert "[mcu pi]" not in merged + assert "serial: /dev/ttyACM1" in merged + assert "serial: /dev/ttyACM0" not in merged + assert "canbus_uuid: 456" in merged + assert "canbus_uuid: 123" not in merged + assert "/tmp/klipper_host_mcu" not in merged + + +# ========================================================================= +# _cpy_cfg_files +# ========================================================================= + + +class TestCpyCfgFiles: + def test_no_cpy_files_returns_immediately(self, tmp_path): + mgr = _make_manager(tmp_path, cpy_files=[]) + mgr._cpy_cfg_files() + + def test_file_not_in_repo_skipped(self, tmp_path, caplog): + caplog.set_level(logging.INFO) + mgr = _make_manager(tmp_path, cpy_files=["missing.cfg"]) + mgr.repo_fi_name = {} + mgr._cpy_cfg_files() + assert "not exist in repository" in caplog.text + + def test_file_not_in_config_copied(self, tmp_path, mock_home): + mgr = _make_manager(tmp_path, cpy_files=["printer.cfg"]) + mgr.config_dir = mock_home / "config" + mgr.config_dir.mkdir(parents=True, exist_ok=True) + src = _write(mgr.repo / "printer.cfg", "content") + # The method uses relative_to(home) — put repo under home too + home = mock_home + mgr.repo = home / "repo" + mgr.repo.mkdir(parents=True, exist_ok=True) + src = _write(mgr.repo / "printer.cfg", "content") + mgr.repo_fi_name = {"printer.cfg": [src]} + mgr.config_fi_name = {} + mgr._cpy_cfg_files() + target = mgr.config_dir / "printer.cfg" + assert target.is_file() + assert target.read_text() == "content" + + def test_file_identical_skipped(self, tmp_path): + mgr = _make_manager(tmp_path, cpy_files=["printer.cfg"]) + src = _write(mgr.repo / "printer.cfg", "same") + tgt = _write(mgr.config_dir / "printer.cfg", "same") + mgr.repo_fi_name = {"printer.cfg": [src]} + mgr.config_fi_name = {"printer.cfg": [tgt]} + mgr._cpy_cfg_files() + assert tgt.read_text() == "same" + + def test_printer_cfg_differs_merge_called(self, tmp_path): + mgr = _make_manager(tmp_path, cpy_files=["printer.cfg"]) + src = _write(mgr.repo / "printer.cfg", "[header]\nopt: repo\n") + tgt = _write( + mgr.config_dir / "printer.cfg", + "[header]\nopt: target\n" + + SV_CONFIG_MARKER + "\n" + + "#*# user: saved\n", + ) + mgr.repo_fi_name = {"printer.cfg": [src]} + mgr.config_fi_name = {"printer.cfg": [tgt]} + mgr._cpy_cfg_files() + merged = tgt.read_text() + assert "[header]" in merged + assert SV_CONFIG_MARKER in merged + assert "#*# user: saved" in merged + + def test_variables_cfg_differs_merge_called(self, tmp_path): + mgr = _make_manager(tmp_path, cpy_files=["variables.cfg"]) + src = _write(mgr.repo / "variables.cfg", "[g]\nopt_a: repo\nopt_b: repo\n") + tgt = _write(mgr.config_dir / "variables.cfg", "[g]\nopt_a: existing\n") + mgr.repo_fi_name = {"variables.cfg": [src]} + mgr.config_fi_name = {"variables.cfg": [tgt]} + mgr._cpy_cfg_files() + result = configparser.ConfigParser() + result.read_string(tgt.read_text()) + assert result.get("g", "opt_a") == "existing" + assert result.get("g", "opt_b") == "repo" + + +# ========================================================================= +# sync — smoke test +# ========================================================================= + + +class TestSync: + def test_sync_roundtrip(self, tmp_path, mock_home): + mgr = _make_manager(tmp_path) + mgr.config_dir = mock_home / "config" + mgr.config_dir.mkdir(parents=True, exist_ok=True) + mgr.repo = mock_home / "repo" + mgr.repo.mkdir(parents=True, exist_ok=True) + _write(mgr.repo / "printer.cfg", "[g]\nk: repo\n") + _write(mgr.repo / "other.cfg", "[g]\no: v\n") + mgr.repo_fi_name, mgr.repo_fi_relpath = mgr._build_file_index(mgr.repo) + mgr.config_fi_name, mgr.config_fi_relpath = mgr._build_file_index( + mgr.config_dir + ) + mgr.sync() + + def test_sync_handles_missing_repo(self, tmp_path, caplog): + caplog.set_level(logging.ERROR) + mgr = _make_manager(tmp_path) + mgr.repo = tmp_path / "ghost_repo" + mgr.sync() + assert len(caplog.records) > 0 diff --git a/tests/util/test_device_configurator_unit.py b/tests/util/test_device_configurator_unit.py new file mode 100644 index 00000000..b4cd40fd --- /dev/null +++ b/tests/util/test_device_configurator_unit.py @@ -0,0 +1,296 @@ +"""DeviceConfigManager: boot-time sync with device templates, and hotplug apply.""" + +import os +import pathlib +import sys + +import pytest + +_BS_DIR = os.path.abspath( + os.path.join(os.path.dirname(__file__), "..", "..", "BlocksScreen") +) +if _BS_DIR not in sys.path: + sys.path.append(_BS_DIR) + +from devices.discovery.serial_devices import Device # noqa: E402 +from tools.configuration_manager import device_configurator as dc # noqa: E402 +from tools.configuration_manager.device_configurator import ( # noqa: E402 + DeviceConfigManager, + DeviceConsent, +) +from tools.configuration_manager.device_profiles import load_profiles # noqa: E402 + +from .test_device_profiles_unit import AMU, BEACON, PRINTER_CFG # noqa: E402 + +VARIABLES_CFG = "[Variables]\nspeed = 100\n" + + +class _Section: + def __init__(self, values): + self._values = values + + def get(self, key, default=None): + return self._values.get(key, default) + + def getlists(self, key, default=None): + return default + + +class _Config: + def __init__(self, values): + self._section = _Section(values) + + def get_section(self, name, fallback=None): + return self._section + + +@pytest.fixture +def dirs(tmp_path): + repo = tmp_path / "RF50-Klipper" + config_dir = tmp_path / "config" + (repo / ".git").mkdir(parents=True) + config_dir.mkdir() + (repo / "printer.cfg").write_text(PRINTER_CFG) + (repo / "variables.cfg").write_text(VARIABLES_CFG) + return repo, config_dir, tmp_path / "backup" + + +@pytest.fixture +def consent_path(tmp_path): + return tmp_path / "state" / "device_consent.json" + + +def _accept_amu(consent_path): + DeviceConsent(consent_path).accept(["amu"]) + + +def _boot(dirs, monkeypatch, devices, consent_path=None, accepted=True): + """Construct the manager like the app does at startup (runs sync()). + + accepted: the user already said yes to the AMU prompt on an earlier run + (the default, so the layout tests below see the AMU template applied). + """ + repo, config_dir, backup = dirs + if consent_path is None: + consent_path = repo.parent / "state" / "device_consent.json" + if accepted: + _accept_amu(consent_path) + monkeypatch.setattr( + DeviceConfigManager, "scan_devices", staticmethod(lambda: list(devices)) + ) + return DeviceConfigManager( + _Config( + { + "config_repo": str(repo), + "config_dir": str(config_dir), + "backup_dir": str(backup), + "copy_files": None, + } + ), + profiles=load_profiles(override=None), + consent=DeviceConsent(consent_path), + ) + + +def _printer_cfg(dirs) -> str: + return (dirs[1] / "printer.cfg").read_text() + + +def _assert_amu_layout(text: str): + assert f"[mcu mmu]\nserial: {AMU.symlink}\n" in text + assert "\n[include config/variant_mmu/base/*.cfg]\n" in text + assert "\n#[include config/variant_sync_single/*.cfg ]\n" in text + + +def _assert_repo_layout(text: str): + assert "[mcu mmu]" not in text + assert "\n#[include config/variant_mmu/base/*.cfg]\n" in text + assert "\n[include config/variant_sync_single/*.cfg ]\n" in text + + +class TestBoot: + def test_fresh_machine_with_amu(self, dirs, monkeypatch): + _boot(dirs, monkeypatch, [AMU, BEACON]) + text = _printer_cfg(dirs) + _assert_amu_layout(text) + assert f"[beacon]\nserial: {BEACON.symlink}\n" in text + + def test_fresh_machine_without_devices_equals_repo(self, dirs, monkeypatch): + _boot(dirs, monkeypatch, []) + assert _printer_cfg(dirs) == PRINTER_CFG + + def test_amu_unplugged_since_last_boot_is_reverted(self, dirs, monkeypatch): + _boot(dirs, monkeypatch, [AMU, BEACON]) + _assert_amu_layout(_printer_cfg(dirs)) + _boot(dirs, monkeypatch, [BEACON]) + text = _printer_cfg(dirs) + _assert_repo_layout(text) + # byte-for-byte the repo file apart from the beacon serial - no + # banner or blank line lost around where [mcu mmu] was + assert text == PRINTER_CFG.replace( + "[beacon]\nserial:\n", f"[beacon]\nserial: {BEACON.symlink}\n" + ) + + def test_repo_changes_still_sync_with_amu(self, dirs, monkeypatch): + repo = dirs[0] + _boot(dirs, monkeypatch, [AMU]) + (repo / "printer.cfg").write_text( + PRINTER_CFG.replace( + "kinematics: corexy", "kinematics: corexy\nmax_velocity: 500" + ) + ) + _boot(dirs, monkeypatch, [AMU]) + text = _printer_cfg(dirs) + assert "max_velocity: 500" in text + _assert_amu_layout(text) + assert text.count("[mcu mmu]") == 1 + + def test_templates_only_touch_printer_cfg(self, dirs, monkeypatch): + repo, config_dir, _ = dirs + (config_dir / "variables.cfg").write_text("[Variables]\nspeed = 50\n") + _boot(dirs, monkeypatch, [AMU, BEACON]) + variables = (config_dir / "variables.cfg").read_text() + assert "AMU" not in variables and "beacon" not in variables.lower() + + +class TestApplyDevices: + def test_hotplug_amu(self, dirs, monkeypatch): + manager = _boot(dirs, monkeypatch, []) + assert manager.apply_devices([AMU]) == ["amu"] + _assert_amu_layout(_printer_cfg(dirs)) + assert manager.apply_devices([AMU]) == [] # already applied + + def test_unknown_device_changes_nothing(self, dirs, monkeypatch): + manager = _boot(dirs, monkeypatch, []) + before = _printer_cfg(dirs) + assert ( + manager.apply_devices( + [Device(symlink_name="usb-1a86_USB_Serial-if00-port0")] + ) + == [] + ) + assert _printer_cfg(dirs) == before + + def test_missing_printer_cfg(self, dirs, monkeypatch): + manager = _boot(dirs, monkeypatch, []) + (dirs[1] / "printer.cfg").unlink() + assert manager.apply_devices([AMU]) == [] + assert not pathlib.Path(dirs[1] / "printer.cfg").exists() + + +class TestConfirmation: + """The AMU template asks first: nothing is written before the user accepts.""" + + def test_boot_leaves_unaccepted_amu_alone(self, dirs, monkeypatch, consent_path): + manager = _boot(dirs, monkeypatch, [AMU], consent_path, accepted=False) + assert _printer_cfg(dirs) == PRINTER_CFG + assert manager.pending_confirmation([AMU]) == ["amu"] + + def test_hotplug_unaccepted_amu_is_not_applied( + self, dirs, monkeypatch, consent_path + ): + manager = _boot(dirs, monkeypatch, [], consent_path, accepted=False) + assert manager.apply_devices([AMU]) == [] + assert _printer_cfg(dirs) == PRINTER_CFG + assert manager.pending_confirmation([AMU]) == ["amu"] + + def test_beacon_needs_no_confirmation(self, dirs, monkeypatch, consent_path): + manager = _boot(dirs, monkeypatch, [], consent_path, accepted=False) + assert manager.pending_confirmation([BEACON]) == [] + assert manager.apply_devices([BEACON]) == ["beacon"] + + def test_accept_applies_and_persists_across_reboot( + self, dirs, monkeypatch, consent_path + ): + manager = _boot(dirs, monkeypatch, [], consent_path, accepted=False) + manager.accept(["amu"]) + assert manager.apply_devices([AMU]) == ["amu"] + _assert_amu_layout(_printer_cfg(dirs)) + assert manager.pending_confirmation([AMU]) == [] + # Reboot with the AMU still plugged: consent was saved, config stays, + # and printer.cfg is not rewritten. + before = _printer_cfg(dirs) + rebooted = _boot(dirs, monkeypatch, [AMU], consent_path, accepted=False) + assert _printer_cfg(dirs) == before + assert rebooted.boot_changed is False + assert rebooted.pending_confirmation([AMU]) == [] + + def test_consent_file_survives_garbage(self, consent_path): + consent_path.parent.mkdir(parents=True) + consent_path.write_text("{not json") + consent = DeviceConsent(consent_path) + assert not consent.is_accepted("amu") + consent.accept(["amu"]) + assert DeviceConsent(consent_path).is_accepted("amu") + + +class TestBootChanged: + def test_unchanged_boot(self, dirs, monkeypatch): + _boot(dirs, monkeypatch, [AMU]) + assert _boot(dirs, monkeypatch, [AMU]).boot_changed is False + + def test_amu_plugged_while_off(self, dirs, monkeypatch): + _boot(dirs, monkeypatch, []) + assert _boot(dirs, monkeypatch, [AMU]).boot_changed is True + + def test_amu_removed_while_off(self, dirs, monkeypatch): + _boot(dirs, monkeypatch, [AMU]) + assert _boot(dirs, monkeypatch, []).boot_changed is True + + +class TestScanDevices: + def test_by_id_listing_fills_in_what_the_daemon_missed(self, monkeypatch): + other = Device(symlink_name="usb-Klipper_stm32h723xx_29003A00-if00") + # daemon answered, without the AMU + monkeypatch.setattr(dc.SerialScanner, "daemon_scan", lambda self: [other]) + monkeypatch.setattr( + dc.SerialScanner, "legacy_scan", staticmethod(lambda: [other, AMU]) + ) + names = [d.symlink_name for d in DeviceConfigManager.scan_devices()] + assert names == [other.symlink_name, AMU.symlink_name] + + def test_daemon_down_lists_by_id_once(self, monkeypatch): + calls = [] + monkeypatch.setattr(dc.SerialScanner, "daemon_scan", lambda self: None) + monkeypatch.setattr( + dc.SerialScanner, + "legacy_scan", + staticmethod(lambda: calls.append(1) or [AMU]), + ) + names = [d.symlink_name for d in DeviceConfigManager.scan_devices()] + assert names == [AMU.symlink_name] and calls == [1] + + def test_scan_errors_never_raise(self, monkeypatch): + def boom(*_): + raise RuntimeError("scan bug") + + monkeypatch.setattr(dc.SerialScanner, "daemon_scan", boom) + monkeypatch.setattr(dc.SerialScanner, "legacy_scan", staticmethod(boom)) + assert DeviceConfigManager.scan_devices() == [] + + +class TestAtomicWrite: + def test_keeps_mode_and_symlink(self, tmp_path): + real = tmp_path / "real.cfg" + real.write_text("old\n") + os.chmod(real, 0o640) + link = tmp_path / "printer.cfg" + link.symlink_to(real) + dc.atomic_write_text(link, "new\n") + assert link.is_symlink() + assert real.read_text() == "new\n" + assert real.stat().st_mode & 0o777 == 0o640 + assert [p.name for p in tmp_path.iterdir() if p.name.startswith(".")] == [] + + def test_failed_write_leaves_file_untouched(self, tmp_path, monkeypatch): + target = tmp_path / "printer.cfg" + target.write_text("old\n") + + def fail(*_): + raise OSError("disk full") + + monkeypatch.setattr(dc.os, "replace", fail) + with pytest.raises(OSError): + dc.atomic_write_text(target, "new\n") + assert target.read_text() == "old\n" + assert [p.name for p in tmp_path.iterdir()] == ["printer.cfg"] diff --git a/tests/util/test_device_profiles_unit.py b/tests/util/test_device_profiles_unit.py new file mode 100644 index 00000000..7b998824 --- /dev/null +++ b/tests/util/test_device_profiles_unit.py @@ -0,0 +1,439 @@ +"""Device templates (tools/configuration_manager/device_profiles.py): loading and text transforms.""" + +import os +import re +import sys +import textwrap + +import pytest + +# App modules import top-level packages; appended so the `BlocksScreen` +# package isn't shadowed by the BlocksScreen/BlocksScreen.py entry script. +_BS_DIR = os.path.abspath( + os.path.join(os.path.dirname(__file__), "..", "..", "BlocksScreen") +) +if _BS_DIR not in sys.path: + sys.path.append(_BS_DIR) + +from devices.discovery.serial_devices import Device # noqa: E402 +from tools.configuration_manager import device_profiles as dp # noqa: E402 +from tools.configuration_manager.configurator import SV_CONFIG_MARKER # noqa: E402 + +BEACON = Device(symlink_name="usb-Beacon_Beacon_RevH_ABC123-if00") +AMU = Device(symlink_name="usb-Klipper_stm32f446xx_AMU0001-if00") +KLIPPER = Device(symlink_name="usb-Klipper_stm32h723xx_29003A00-if00") + +# Shaped like RF50-Klipper's printer.cfg: banners around the variant +# includes, a trailing space inside one include, [mcu]/[beacon]/[mcu pi]. +PRINTER_CFG = textwrap.dedent( + f"""\ + [include hardware/*.cfg] + ############################################################################################## + ####################################### Amu System ##################################### + ############################################################################################## + #[include config/variant_mmu/base/*.cfg] + ############################################################################################## + ####################################### Normal system ##################################### + ############################################################################################## + [include config/variant_sync_single/*.cfg ] + ############################################################################################## + + [mcu] + canbus_uuid: + canbus_interface: can0 + + [mcu Toolhead] # In the RF50 case it's the EBB-36 + canbus_uuid: + + [beacon] + serial: + + [mcu pi] + serial: /tmp/klipper_host_mcu + + ############################################################################################## + ######################################### KINEMATICS ######################################### + [printer] + kinematics: corexy + + {SV_CONFIG_MARKER} + #*# DO NOT EDIT THIS BLOCK OR BELOW. The contents are auto-generated. + #*# + #*# [beacon model default] + #*# model_coef = 1.0 + """ +) + + +@pytest.fixture +def profiles(): + return dp.load_profiles(override=None) + + +def _by_name(profiles, name): + return next(p for p in profiles if p.name == name) + + +class TestLoadProfiles: + def test_bundled(self, profiles): + assert [p.name for p in profiles] == ["beacon", "amu"] + amu = _by_name(profiles, "amu") + assert amu.section == "mcu mmu" + assert amu.confirm is True + assert _by_name(profiles, "beacon").confirm is False + assert amu.enable_includes == ("config/variant_mmu/base/*.cfg",) + assert amu.disable_includes == ("config/variant_sync_single/*.cfg",) + + def _override(self, tmp_path, body, mode=0o644): + path = tmp_path / "blockscreen_devices.yaml" + path.write_text(textwrap.dedent(body)) + os.chmod(path, mode) + return path + + def test_override_merges_by_name_and_adds(self, tmp_path): + path = self._override( + tmp_path, + """\ + profiles: + - name: amu + match: 'MyAMU' + - name: fps + match: 'FPS' + section: mcu fps + options: {serial: '${serial}'} + """, + ) + profiles = dp.load_profiles(override=path) + amu = _by_name(profiles, "amu") + assert amu.match.pattern == "MyAMU" + assert amu.section == "mcu mmu" # untouched keys come from the bundle + assert _by_name(profiles, "fps").section == "mcu fps" + + def test_override_can_disable(self, tmp_path): + path = self._override(tmp_path, "profiles: [{name: beacon, enabled: false}]\n") + assert [p.name for p in dp.load_profiles(override=path)] == ["amu"] + + def test_group_writable_override_is_ignored(self, tmp_path): + path = self._override( + tmp_path, "profiles: [{name: beacon, enabled: false}]\n", mode=0o664 + ) + assert [p.name for p in dp.load_profiles(override=path)] == ["beacon", "amu"] + + def test_invalid_entries_are_skipped(self, tmp_path): + path = self._override( + tmp_path, + """\ + profiles: + - name: nosection + match: 'x' + - name: badregex + match: '(' + section: foo + """, + ) + assert [p.name for p in dp.load_profiles(override=path)] == ["beacon", "amu"] + + def test_missing_bundle_gives_no_profiles(self, tmp_path): + assert dp.load_profiles(bundled=tmp_path / "nope.yaml", override=None) == [] + + def test_vendor_id_accepts_hex_string_and_int(self, tmp_path): + path = self._override( + tmp_path, + """\ + profiles: + - name: amu + vendor_id: '0x1d50' + product_id: 24577 + """, + ) + amu = _by_name(dp.load_profiles(override=path), "amu") + assert amu.vendor_id == 0x1D50 + assert amu.product_id == 24577 # == 0x6001 + + def test_product_id_without_vendor_id_is_rejected(self, tmp_path): + path = self._override( + tmp_path, + """\ + profiles: + - name: fps + match: 'FPS' + section: mcu fps + product_id: '0x6001' + """, + ) + assert [p.name for p in dp.load_profiles(override=path)] == ["beacon", "amu"] + + def test_bad_vendor_id_string_is_rejected(self, tmp_path): + path = self._override( + tmp_path, + """\ + profiles: + - name: fps + match: 'FPS' + section: mcu fps + vendor_id: 'nonsense' + """, + ) + assert [p.name for p in dp.load_profiles(override=path)] == ["beacon", "amu"] + + def test_duplicate_ids_across_profiles_are_logged(self, tmp_path, caplog): + path = self._override( + tmp_path, + """\ + profiles: + - name: fps + match: 'FPS' + section: mcu fps + vendor_id: '0x1d50' + product_id: '0xb001' + """, + ) + with caplog.at_level("ERROR"): + profiles = dp.load_profiles(override=path) + # Both profiles are still loaded - misconfiguration is surfaced, not silently fixed. + assert {p.name for p in profiles} == {"beacon", "amu", "fps"} + assert "amu" in caplog.text and "fps" in caplog.text and "0xb001" in caplog.text + + +class TestMatch: + def test_case_insensitive_on_by_id_name(self, profiles): + pairs = dp.match_devices(profiles, [KLIPPER, AMU, BEACON]) + assert [(p.name, d.symlink_name) for p, d in pairs] == [ + ("beacon", BEACON.symlink_name), + ("amu", AMU.symlink_name), + ] + + def test_unknown_devices_match_nothing(self, profiles): + assert dp.match_devices(profiles, [KLIPPER]) == [] + + def test_two_devices_matching_one_profile_uses_first_and_logs( + self, profiles, caplog + ): + amu2 = Device(symlink_name="usb-Klipper_stm32f446xx_AMU0002-if00") + with caplog.at_level("WARNING"): + pairs = dp.match_devices(profiles, [AMU, amu2]) + assert [(p.name, d.symlink_name) for p, d in pairs] == [ + ("amu", AMU.symlink_name) + ] + assert "amu" in caplog.text + assert AMU.symlink_name in caplog.text and amu2.symlink_name in caplog.text + + +class TestMatchByUsbId: + PROFILE = dp.DeviceProfile( + name="amu", + match=re.compile("AMU"), + section="mcu AMU", + vendor_id=0x1D50, + product_id=0x6001, + ) + + def test_matches_on_id_even_without_symlink_hint(self): + # No "AMU" anywhere in the symlink - only the USB IDs identify it. + device = Device( + symlink_name="usb-Klipper_stm32f446xx_XYZ-if00", + vendor_id=0x1D50, + product_id=0x6001, + ) + assert self.PROFILE.matches(device) + + def test_wrong_product_id_does_not_match_even_with_symlink_hint(self): + # Regex would match ("AMU" in the name), but the ID says otherwise - + # ID wins once the device reports one, so a mislabeled/spoofed + # symlink can't masquerade as the real board. + device = Device( + symlink_name="usb-Klipper_stm32f446xx_AMU0001-if00", + vendor_id=0x1D50, + product_id=0x9999, + ) + assert not self.PROFILE.matches(device) + + def test_wrong_vendor_id_does_not_match(self): + device = Device( + symlink_name="usb-Klipper_stm32f446xx_AMU0001-if00", + vendor_id=0x0483, + product_id=0x6001, + ) + assert not self.PROFILE.matches(device) + + def test_falls_back_to_regex_when_device_reports_no_id(self): + # device.vendor_id == 0: legacy scanner, or a daemon that doesn't + # send USB IDs yet - unchanged regex behaviour. + assert self.PROFILE.matches(AMU) + assert not self.PROFILE.matches(KLIPPER) + + def test_profile_without_vendor_id_ignores_device_ids(self): + profile = dp.DeviceProfile( + name="amu", match=re.compile("AMU"), section="mcu AMU" + ) + device = Device( + symlink_name=KLIPPER.symlink_name, vendor_id=0x1D50, product_id=0x6001 + ) + assert not profile.matches( + device + ) # no "AMU" in the symlink, no vendor_id to check + + def test_vendor_id_only_profile_accepts_any_product_id(self): + profile = dp.DeviceProfile( + name="amu", match=re.compile("AMU"), section="mcu AMU", vendor_id=0x1D50 + ) + device = Device(symlink_name="whatever", vendor_id=0x1D50, product_id=0xBEEF) + assert profile.matches(device) + + +class TestApply: + def test_amu(self, profiles): + out = dp.apply_profile(PRINTER_CFG, _by_name(profiles, "amu"), AMU) + assert "\n[include config/variant_mmu/base/*.cfg]\n" in out + assert "\n#[include config/variant_sync_single/*.cfg ]\n" in out + # banners around the includes are untouched + assert out.count("#" * 94) == PRINTER_CFG.count("#" * 94) + # new section at a block boundary, just before the last [mcu ...] + assert ( + "[beacon]\nserial:\n\n" + "[mcu mmu]\nserial: /dev/serial/by-id/" + "usb-Klipper_stm32f446xx_AMU0001-if00\n\n" + "[mcu pi]\nserial: /tmp/klipper_host_mcu\n\n####" + ) in out + + def test_beacon_fills_existing_section(self, profiles): + out = dp.apply_profile(PRINTER_CFG, _by_name(profiles, "beacon"), BEACON) + assert ( + "[beacon]\nserial: /dev/serial/by-id/usb-Beacon_Beacon_RevH_ABC123-if00\n" + in out + ) + assert out.count("[beacon]") == 1 + + def test_beacon_replaces_stale_serial(self, profiles): + stale = PRINTER_CFG.replace( + "[beacon]\nserial:\n", "[beacon]\nserial: /dev/serial/by-id/old # note\n" + ) + out = dp.apply_profile(stale, _by_name(profiles, "beacon"), BEACON) + assert "old" not in out + assert BEACON.symlink in out + + @pytest.mark.parametrize("devices", [[AMU], [BEACON], [AMU, BEACON]]) + def test_idempotent(self, profiles, devices): + once, changed = dp.apply_device_profiles(PRINTER_CFG, profiles, devices) + twice, changed_again = dp.apply_device_profiles(once, profiles, devices) + assert changed and once != PRINTER_CFG + assert twice == once and changed_again == [] + + def test_save_config_block_untouched(self, profiles): + cfg = ( + PRINTER_CFG + + "#*# [mcu mmu]\n#*# [include config/variant_sync_single/*.cfg]\n" + ) + out, _ = dp.apply_device_profiles(cfg, profiles, [AMU]) + tail = cfg[cfg.index(SV_CONFIG_MARKER) :] + assert out.endswith(tail) + assert out.count("[mcu mmu]") == 2 # one new section + the untouched #*# line + + def test_no_marker_and_no_mcu_appends(self, profiles): + out = dp.apply_profile( + "[printer]\nkinematics: corexy", _by_name(profiles, "amu"), AMU + ) + assert out.endswith(f"\n[mcu mmu]\nserial: {AMU.symlink}\n") + + def test_template_placeholders(self): + profile = dp.DeviceProfile( + name="x", + match=re.compile("AMU"), + section="mcu x", + options=(("serial", "${serial}"), ("note", "${name} ${unknown}")), + ) + dev = Device(symlink_name=AMU.symlink_name, name="Klipper stm32f446xx") + opts = dict(profile.render_options(dev)) + assert opts == {"serial": AMU.symlink, "note": "Klipper stm32f446xx ${unknown}"} + + def test_device_name_cannot_inject_a_config_line(self): + # device.name is built from the device's own USB manufacturer/product + # descriptor strings - fully attacker/corruption-controlled. A + # newline there must never become a newline in printer.cfg. + profile = dp.DeviceProfile( + name="x", + match=re.compile("AMU"), + section="mcu x", + options=(("note", "${name}"),), + ) + dev = Device( + symlink_name=AMU.symlink_name, + name="Evil\n[virtual_sdcard]\npath: /root\n#", + ) + opts = dict(profile.render_options(dev)) + assert "\n" not in opts["note"] + assert opts["note"] == "Evil[virtual_sdcard]path: /root#" + + def test_apply_profile_with_corrupt_device_name_stays_one_line(self): + profile = dp.DeviceProfile( + name="x", + match=re.compile("AMU"), + section="mcu x", + options=(("note", "${name}"),), + ) + dev = Device( + symlink_name=AMU.symlink_name, name="Bad\n[printer]\nkinematics: none" + ) + out = dp.apply_profile("[printer]\nkinematics: corexy", profile, dev) + lines = out.splitlines() + # Injected text lands inline in the note value's line - never as its + # own standalone line/section. + assert "note: Bad[printer]kinematics: none" in lines + assert lines.count("[printer]") == 1 + assert "kinematics: none" not in lines + assert "kinematics: corexy" in lines + + +class TestSetInclude: + def test_round_trip(self): + path = "config/variant_sync_single/*.cfg" + off = dp.set_include(PRINTER_CFG, path, False) + assert off != PRINTER_CFG + assert dp.set_include(off, path, True) == PRINTER_CFG + + def test_banner_line_above_is_not_treated_as_comment(self): + # regression: `#*` + `\s*` used to swallow the "####" line above + text = "#####\n[include a.cfg]\n" + assert dp.set_include(text, "a.cfg", False) == "#####\n#[include a.cfg]\n" + assert dp.set_include("#####\n#[include a.cfg]\n", "a.cfg", True) == text + + +def _rf50_repo(): + for candidate in ( + os.environ.get("RF50_KLIPPER_DIR"), + os.path.expanduser("~/RF50-Klipper"), + os.path.expanduser("~/github/RF50-Klipper"), + ): + if candidate and os.path.isfile(os.path.join(candidate, "printer.cfg")): + return candidate + return None + + +@pytest.mark.skipif(_rf50_repo() is None, reason="RF50-Klipper checkout not found") +class TestRF50Layout: + """The AMU template against the real RF50-Klipper files it has to fit.""" + + def test_amu_serial_lands_in_the_mcu_happy_hare_uses(self, profiles): + repo = _rf50_repo() + amu = _by_name(profiles, "amu") + mmu_cfg = open( + os.path.join(repo, "config", "variant_mmu", "base", "mmu.cfg"), + encoding="utf-8", + ).read() + mcu_sections = re.findall(r"^\[(mcu\s+[^\]]+)\]", mmu_cfg, re.MULTILINE) + assert amu.section in [" ".join(s.split()) for s in mcu_sections] + + def test_apply_to_real_printer_cfg(self, profiles): + repo = _rf50_repo() + text = open(os.path.join(repo, "printer.cfg"), encoding="utf-8").read() + amu = _by_name(profiles, "amu") + out = dp.apply_profile(text, amu, AMU) + assert re.search(r"^\[include config/variant_mmu/base/\*\.cfg\]", out, re.M) + assert re.search( + r"^#\[include config/variant_sync_single/\*\.cfg\s*\]", out, re.M + ) + header = out[: out.index(f"[{amu.section}]")] + # after the include, so its serial overrides mmu.cfg's placeholder + assert "[include config/variant_mmu/base/*.cfg]" in header + assert f"[{amu.section}]\nserial: {AMU.symlink}\n" in out + assert dp.apply_profile(out, amu, AMU) == out diff --git a/tests/widgets/test_sigterm_shutdown.py b/tests/widgets/test_sigterm_shutdown.py index b5d4250b..f01ce2c5 100644 --- a/tests/widgets/test_sigterm_shutdown.py +++ b/tests/widgets/test_sigterm_shutdown.py @@ -61,7 +61,12 @@ def _exception_hook(*_a: object, **_k: object) -> None: tools = _stub("tools") tools.__path__ = [] # type: ignore[attr-defined] - _stub("tools.configuration_manager", ConfigManager=MagicMock()) + _stub( + "tools.configuration_manager", + ConfigManager=MagicMock(), + DeviceConfigManager=MagicMock(), + DeviceConfigWatcher=MagicMock(), + ) saved = {k: sys.modules.get(k) for k in stubs} sys.modules.update(stubs) diff --git a/updater/components.yaml b/updater/components.yaml index 69acd1a0..cb6a945b 100644 --- a/updater/components.yaml +++ b/updater/components.yaml @@ -25,7 +25,7 @@ components: type: git path: ~/RF50-Klipper url: https://github.com/BlocksTechnology/RF50-Klipper - branch: Release-2.0 + branch: feat/amu-configs service: klipper.service reset_mode: hard order: 10 @@ -40,6 +40,19 @@ components: reset_mode: hard order: 13 + # Device discovery daemon (BlocksTechnology/DeviceDiscovery). Cloned on + # machines that lack it; the hook then defers first-time setup (apt deps, + # full build, systemd unit - root) to scripts/install-updater.sh. + - name: DeviceDiscovery + type: git + path: ~/DeviceDiscovery + url: https://github.com/BlocksTechnology/DeviceDiscovery.git + install_if_missing: true + branch: main + service: device-discoveryd.service + reset_mode: hard + order: 15 + - name: katapult type: git path: ~/katapult @@ -72,7 +85,7 @@ components: - name: BlocksScreen type: git path: ~/BlocksScreen - branch: Release-2.0 + branch: dev url: https://github.com/BlocksTechnology/BlocksScreen service: BlocksScreen.service reset_mode: hard diff --git a/updater/executor.py b/updater/executor.py index 32d5f09f..50c049b0 100644 --- a/updater/executor.py +++ b/updater/executor.py @@ -1029,6 +1029,22 @@ async def wait_for_service_active(name: str, timeout: float = 90.0) -> bool: return False +async def service_unit_missing(name: str) -> bool: + """Return True only when systemd reports the unit file does not exist. + + A component can be cloned before its unit is installed (first-time setup + runs later, as root, via install-updater.sh). Anything short of a definite + 'not-found' (query failure, masked, invalid name) returns False so the + caller keeps treating the service as real and restart failures still count. + """ + if not _SERVICE_RE.match(name): + return False + ok, out = await _run( + [SYSTEMCTL, "show", "--property=LoadState", "--value", name], timeout=10.0 + ) + return ok and out.strip() == "not-found" + + def _http_probe(url: str) -> bool: """One blocking GET; True only on a 2xx from the loopback health URL.""" parts = urlsplit(url) diff --git a/updater/hooks/DeviceDiscovery.sh b/updater/hooks/DeviceDiscovery.sh new file mode 100755 index 00000000..5ecfa0af --- /dev/null +++ b/updater/hooks/DeviceDiscovery.sh @@ -0,0 +1,29 @@ +#!/bin/bash +# Runs after a DeviceDiscovery git update or first clone (via updater daemon). +# Rebuilds the daemon (build.sh is incremental and daemon-only, well inside +# run_hook's HOOK_TIMEOUT); the updater then restarts device-discoveryd.service. +# +# First-time setup (apt build deps, systemd unit) needs root, which hooks do +# not have, so it is not done here: the hook asks for install-updater.sh, which +# runs as root once the update batch is done. +set -euo pipefail + +if [ -z "${COMPONENT_PATH:-}" ]; then + exit 0 +fi + +UNIT="device-discoveryd.service" + +if [ ! -x "$COMPONENT_PATH/bin/device_discoveryd" ] || \ + [ "$(systemctl show --property=LoadState --value "$UNIT" 2>/dev/null)" != "loaded" ]; then + if [ -n "${BS_UPDATER_SELF_UPDATE:-}" ] && [ -n "${BS_UPDATER_RESTART_SENTINEL:-}" ]; then + mkdir -p "$(dirname "$BS_UPDATER_RESTART_SENTINEL")" 2>/dev/null || true + echo "install" >>"$BS_UPDATER_RESTART_SENTINEL" + echo "[hook:DeviceDiscovery] not set up yet: deferring first-time setup to install-updater" + else + echo "[hook:DeviceDiscovery] not set up yet: run install-updater.sh or $COMPONENT_PATH/scripts/install.sh" + fi + exit 0 +fi + +exec "$COMPONENT_PATH/scripts/build.sh" diff --git a/updater/service.py b/updater/service.py index 0038af19..e88ca72c 100644 --- a/updater/service.py +++ b/updater/service.py @@ -54,6 +54,7 @@ restart_service, restart_service_noblock, run_hook, + service_unit_missing, verify_updater_importable, wait_for_http_ready, wait_for_service_active, @@ -1776,6 +1777,13 @@ async def _provision_restart_service( """Restart+health-check+enable the provisioned service; fail reason or None.""" if not component.service: return None + if await service_unit_missing(component.service): + self._log.info( + "%s: %s not installed yet, setup deferred to install-updater", + component.name, + component.service, + ) + return None if await is_service_active(component.service): # The hook already started it (enable --now): a restart would start it twice. ok = await self._await_health(component.service, component.health_url) @@ -1846,6 +1854,9 @@ async def _provision_component(self, component: ComponentConfig) -> bool: await self._set_provision_failed(component.name, False) self._history("install_success", component.name, new_hash=new_hash[:12]) self._cb("on_component_done", component.name, True) + # Provisioning runs after any git batch, so a setup the hook deferred + # to the sentinel is applied here rather than by the batch. + await self._apply_deferred_restart() return True except asyncio.CancelledError: self._log.warning( @@ -2055,6 +2066,15 @@ async def _restart_one(self, service: str, health_url: str | None = None) -> boo self._log.info("restarting %s and waiting for active", service) ok, err = await restart_service(service) if not ok: + if await service_unit_missing(service): + # Cloned but not set up yet (first-time setup failed or is still + # pending): reverting code cannot fix a missing unit, and aborting + # would block every other component in the batch. + self._log.warning( + "%s is not installed, skipping restart until its setup runs", + service, + ) + return True self._log.error("restart %s failed: %s", service, err) return False if not await wait_for_service_active(service, timeout=90.0):