diff --git a/CHANGELOG.md b/CHANGELOG.md index 9b60422..c236baf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,9 +1,39 @@ # Changelog -All notable changes to this project will be documented in this file. -The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), -and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [Unreleased] + +### Documentation + +- Clarified that ALPHA HWR pumps should be paired/bonded with the host before + telemetry and control are expected to work reliably. + + +## [0.6.0] - 2026-05-15 + +### Fixed + +- **Authentication extension packet ordering**: `send_extension_packets()` was + sending EXTEND_1 and EXTEND_2 concurrently (via `asyncio.TaskGroup`) with the + wrong submission order. The correct sequence is EXTEND_1 then EXTEND_2 with a + 50ms gap. Parallel or reversed delivery caused premature disconnection (#24). +- **`SetpointInfo.control_mode` type**: Field was typed as bare `int`, causing + `AttributeError` on `.name` access. A `field_validator` now coerces known + values to `ControlMode`; unknown values remain as `int`. +- **Double Pa→m conversion**: `ControlService.get_mode()` already converted + pressure setpoints from Pascals to metres, but `get_display_value()` and + `get_limits_display()` divided by 9806.65 a second time. Both model methods + now return the stored value directly for pressure modes. +- **Disconnection guard in `read_once()`**: Added `transport.is_connected()` + checks before the flow/pressure and temperature queries. A mid-sequence + disconnect now returns partial telemetry instead of raising a `BleakError`. + +### Changed + +- **Minimum Bleak version bumped to 3.0.0** (previously `>=0.19.0`). Bleak 3.0 + removed the `adapter=` keyword argument; the library now uses the Bleak 3.x + `bluez={"adapter": ...}` form on Linux and ignores the argument on other + platforms. Users pinned to Bleak 0.x–2.x must upgrade. ## [0.5.0] - 2026-02-21 diff --git a/README.md b/README.md index 013cce1..1da8fe3 100644 --- a/README.md +++ b/README.md @@ -10,7 +10,7 @@ Python library and CLI for controlling Grundfos ALPHA HWR pumps via Bluetooth Lo ## Features -- Automatic discovery and pairing with ALPHA HWR pumps +- Automatic discovery and guidance for pairing/bonding ALPHA HWR pumps - Stream telemetry data (flow, pressure, power, temperature) - Set pump modes and setpoints with automatic validation - Create and manage time-based operation schedules @@ -27,6 +27,10 @@ pip install alpha-hwr **Requirements:** Python 3.13+ with Bluetooth Low Energy support +> **Important:** Most pumps require the host to be paired/bonded before +> telemetry and control work reliably. If your OS prompts for pairing on first +> connect, accept it before trying to read data. + ## Quick Start ### Command Line diff --git a/docs/getting_started/installation.md b/docs/getting_started/installation.md index 6c6fd59..2b14ccb 100644 --- a/docs/getting_started/installation.md +++ b/docs/getting_started/installation.md @@ -11,6 +11,10 @@ This guide covers how to install the `alpha-hwr` library and its dependencies. * **macOS**: Supported for Control and Telemetry (Schedule download is currently restricted by the OS). * **Windows**: Supported for Control and Telemetry. +**Pairing / Bonding:** The pump usually needs to be paired with the host +before telemetry is reliable. If the OS or Grundfos app prompts you to pair on +first connection, complete that step before testing the library. + ## Installing via pip *Note: The package is not yet on PyPI. Installation is currently from source.* diff --git a/docs/getting_started/quick_start.md b/docs/getting_started/quick_start.md index ebc0862..bac2eb7 100644 --- a/docs/getting_started/quick_start.md +++ b/docs/getting_started/quick_start.md @@ -2,6 +2,10 @@ This guide will help you connect to your Grundfos ALPHA HWR pump, authenticate, and read live telemetry data using Python. +Before you start, make sure the pump is paired/bonded with the laptop or host +you plan to use. On first connection, the operating system may show a pairing +prompt; accept it before expecting telemetry. + ## 1. Scan for the Device First, you need to find the Bluetooth address of your pump. You can use any BLE scanner app (like nRF Connect) or a simple Python script using `bleak`. diff --git a/docs/index.md b/docs/index.md index ed46c1f..5c4556e 100644 --- a/docs/index.md +++ b/docs/index.md @@ -8,5 +8,8 @@ It provides both a high-level Python API and a Command Line Interface (CLI) for This project is **not affiliated with, endorsed by, or associated with Grundfos**. Use this software at your own risk. Incorrect usage of motor control commands could potentially damage hardware, although safety limits in the pump firmware generally prevent this. +Most pumps also need to be paired/bonded with the host before telemetry and +control work reliably. + --- *Version: 0.5.0* diff --git a/docs/protocol/connection.md b/docs/protocol/connection.md index 3576307..0dcac08 100644 --- a/docs/protocol/connection.md +++ b/docs/protocol/connection.md @@ -19,9 +19,9 @@ All communication (commands and telemetry) happens over a single GATT Characteri * **Properties**: `Write`, `Notify` ### Pairing -The device typically requests **Pairing/Bonding** upon connection. +The device requires **Pairing/Bonding** for normal use. * **Level**: `Just Works` (No PIN usually required, though some models might prompt). -* **Requirement**: Bonding is recommended. While some commands might work without it, stable Schedule downloading (HCI layer) and consistent reconnection often rely on a bonded state. +* **Requirement**: Bonding is recommended. While some commands might work without it, telemetry, control, stable Schedule downloading (HCI layer), and consistent reconnection are much more reliable after bonding. ## 2. Authentication Handshake ("Unlock") diff --git a/docs/troubleshooting/service_discovery.md b/docs/troubleshooting/service_discovery.md index 7f75ff6..1744dab 100644 --- a/docs/troubleshooting/service_discovery.md +++ b/docs/troubleshooting/service_discovery.md @@ -189,6 +189,21 @@ void gatt_discover_complete_callback() { 3. Check pump firmware version (see Class 7 ID 50 after auth) 4. Force re-pair: delete bonding data, reconnect +### Issue 1b: Telemetry Works Only After Manual Pairing + +**Symptoms:** +- Authentication appears to succeed, but telemetry is empty or unreliable +- The first successful read happens only after pairing from the OS prompt or + the Grundfos app + +**Cause:** +- The pump has not been bonded with the host yet + +**Solution:** +1. Pair the pump on the host before testing the library +2. Accept any Bluetooth pairing prompt shown by the operating system +3. If needed, remove the old bond and pair again + ### Issue 2: Multiple Pumps Nearby **Symptoms:** diff --git a/pyproject.toml b/pyproject.toml index ef0e3e7..c35ac90 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -51,7 +51,7 @@ classifiers = [ "Operating System :: OS Independent", ] dependencies = [ - "bleak>=0.19.0", + "bleak>=3.0.0", "pydantic>=2.0.0", "pydantic-settings>=2.0.0", "typer>=0.9.0", diff --git a/scripts/verify_hardware.py b/scripts/verify_hardware.py new file mode 100755 index 0000000..569df1a --- /dev/null +++ b/scripts/verify_hardware.py @@ -0,0 +1,217 @@ +#!/usr/bin/env python3 +""" +Hardware verification script for issue #24 regression testing. + +Connects to a local ALPHA HWR pump via BLE, reads telemetry and device +information to verify the fix for: + - Sequential extension packet ordering (EXTEND_1 then EXTEND_2) + - Bleak 3.x adapter handling + - Disconnection guard in read_once (is_connected checks) + +Usage: + # Auto-discover pump: + .venv/bin/python scripts/verify_hardware.py + + # Specify address directly: + .venv/bin/python scripts/verify_hardware.py --address +""" + +from __future__ import annotations + +import asyncio +import logging +import sys +from importlib.metadata import PackageNotFoundError +from importlib.metadata import version as pkg_version + +import typer + +from alpha_hwr.client import AlphaHWRClient + +app = typer.Typer(add_completion=False) + + +def _pkg_ver(name: str) -> str: + try: + return pkg_version(name) + except PackageNotFoundError: + return "unknown" + + +def _setup_logging(verbose: bool) -> None: + level = logging.DEBUG if verbose else logging.INFO + logging.basicConfig( + level=level, + format="%(asctime)s %(name)s %(levelname)s: %(message)s", + ) + if not verbose: + logging.getLogger("bleak").setLevel(logging.WARNING) + + +async def _run(address: str | None, verbose: bool) -> int: + _setup_logging(verbose) + + print( + f"alpha-hwr {_pkg_ver('alpha-hwr')}" + f" | bleak {_pkg_ver('bleak')}" + f" | Python {sys.version.split()[0]}" + ) + print() + + # ------------------------------------------------------------------ # + # Discovery # + # ------------------------------------------------------------------ # + if address is None: + print("Scanning for ALPHA HWR pumps (10 s)...") + devices = await AlphaHWRClient.discover(timeout=10.0) + if not devices: + print( + "ERROR: No ALPHA HWR pumps found." + " Is the device powered on and in range?" + ) + return 1 + print(f"Found {len(devices)} pump(s):") + for d in devices: + print(f" - {d.name or 'Unknown'} [{d.address}]") + address = devices[0].address + print(f"\nUsing first device: {address}") + else: + print(f"Using specified address: {address}") + + print() + + # ------------------------------------------------------------------ # + # Connection + verification # + # ------------------------------------------------------------------ # + passed: list[str] = [] + failed: list[str] = [] + + try: + async with AlphaHWRClient(address) as client: + # Services are guaranteed non-None inside the context manager. + assert client.device_info is not None + assert client.telemetry is not None + assert client.control is not None + + print("Connected and authenticated successfully.") + passed.append("Connection and authentication") + + # -- Firmware / device info --------------------------------- # + print("\n--- Device Information ---") + try: + info = await client.device_info.read_info() + if info: + print(f" Product name : {info.product_name or 'N/A'}") + print(f" BLE firmware : {info.ble_version or 'N/A'}") + if info.ble_version: + passed.append(f"Firmware detected: {info.ble_version}") + else: + failed.append("BLE firmware version not returned") + else: + print(" (no device info returned)") + failed.append("read_info returned None") + except Exception as exc: + print(f" ERROR reading device info: {exc}") + failed.append(f"Device info error: {exc}") + + # -- Telemetry ---------------------------------------------- # + print("\n--- Telemetry ---") + try: + telemetry = await client.telemetry.read_once() + if telemetry is not None: + print(f" Flow : {telemetry.flow_m3h} m3/h") + print(f" Head : {telemetry.head_m} m") + print(f" Power : {telemetry.power_w} W") + if telemetry.flow_m3h is not None: + passed.append("Telemetry flow_m3h populated") + else: + failed.append("flow_m3h is None (possible regression)") + if telemetry.head_m is not None: + passed.append("Telemetry head_m populated") + else: + failed.append("head_m is None (possible regression)") + else: + print(" Telemetry returned None") + failed.append("read_once returned None") + except Exception as exc: + print(f" ERROR reading telemetry: {exc}") + failed.append(f"Telemetry error: {exc}") + + # -- Control mode ------------------------------------------- # + print("\n--- Control Mode ---") + try: + mode_info = await client.control.get_mode() + if mode_info: + from alpha_hwr.constants import ControlMode + + if isinstance(mode_info.control_mode, ControlMode): + mode_name = mode_info.control_mode.name + else: + mode_name = f"unknown({mode_info.control_mode})" + print(f" Mode : {mode_name}") + value, unit = mode_info.get_display_value() + print(f" Setpoint: {value:.2f} {unit}") + passed.append("Control mode read") + else: + print(" (no control mode data)") + failed.append("get_mode returned None") + except Exception as exc: + print(f" ERROR reading control mode: {exc}") + failed.append(f"Control mode error: {exc}") + + except Exception as exc: + print(f"\nFATAL: Could not connect/authenticate: {exc}") + failed.append(f"Connection failed: {exc}") + logging.exception("Connection error") + + # ------------------------------------------------------------------ # + # Summary # + # ------------------------------------------------------------------ # + print("\n" + "=" * 50) + print("VERIFICATION SUMMARY") + print("=" * 50) + for item in passed: + print(f" PASS {item}") + for item in failed: + print(f" FAIL {item}") + + regression_keywords = [ + "Service Discovery", + "BleakError", + "regression", + "None", + ] + regressions = [ + f for f in failed if any(k in f for k in regression_keywords) + ] + if regressions: + print("\nPotential regressions detected (see FAIL lines above).") + return 2 + + if failed: + print("\nSome checks failed - review output above.") + return 1 + + print("\nAll checks passed. No regressions detected.") + return 0 + + +@app.command() +def main( + address: str | None = typer.Option( + None, + "--address", + "-a", + help="BLE device address. Auto-discovers if omitted.", + ), + verbose: bool = typer.Option( + False, "--verbose", "-v", help="Enable debug logging." + ), +) -> None: + """Verify hardware connectivity and firmware for issue #24 regression.""" + exit_code = asyncio.run(_run(address, verbose)) + raise SystemExit(exit_code) + + +if __name__ == "__main__": + app() diff --git a/src/alpha_hwr/client.py b/src/alpha_hwr/client.py index 12af3ef..d086b90 100644 --- a/src/alpha_hwr/client.py +++ b/src/alpha_hwr/client.py @@ -85,6 +85,7 @@ class AlphaHWRClient { from __future__ import annotations import logging +import sys from types import TracebackType from typing import Self @@ -285,7 +286,12 @@ async def connect( # Create BLE client if self.address is None: raise ValueError("BLE device address is not set") - self._bleak_client = BleakClient(self.address, adapter=self.adapter) + if self.adapter and sys.platform.startswith("linux"): + self._bleak_client = BleakClient( + self.address, bluez={"adapter": self.adapter} + ) + else: + self._bleak_client = BleakClient(self.address) # Connect to device await self._bleak_client.connect(timeout=timeout) diff --git a/src/alpha_hwr/core/authentication.py b/src/alpha_hwr/core/authentication.py index 19dcf99..c15f118 100644 --- a/src/alpha_hwr/core/authentication.py +++ b/src/alpha_hwr/core/authentication.py @@ -221,7 +221,7 @@ async def authenticate(self, fast_mode: bool = False) -> bool: This method executes the three-stage authentication sequence: 1. Legacy magic burst (3x repeats, 50ms intervals) 2. Class 10 unlock burst (5x repeats, 50ms intervals) - 3. Extension packets (2 packets in parallel) + 3. Extension packets (EXTEND_1 then EXTEND_2, 50ms apart) Timing Considerations --------------------- @@ -230,7 +230,10 @@ async def authenticate(self, fast_mode: bool = False) -> bool: - Total sequence time: ~1 second Args: - fast_mode: If True, skips delays for testing purposes. + fast_mode: If True, skips all delays (inter-packet and + inter-stage). Intended for unit tests only; do not use + against real hardware as the pump requires the timing + gaps to process each stage. Returns: bool @@ -258,7 +261,7 @@ async def authenticate(self, fast_mode: bool = False) -> bool: # Stage 3: Extension Packets (session establishment) logger.debug("Stage 3: Sending extension packets...") - await self.send_extension_packets() + await self.send_extension_packets(delay=0 if fast_mode else 0.05) if not fast_mode: await asyncio.sleep(0.5) # Final stabilization @@ -325,7 +328,7 @@ async def send_class10_burst( if delay > 0: await asyncio.sleep(delay) - async def send_extension_packets(self) -> None: + async def send_extension_packets(self, delay: float = 0.05) -> None: """ Send authentication extension packets. @@ -333,24 +336,30 @@ async def send_extension_packets(self) -> None: complete the handshake and establish the session. These packets may negotiate capabilities or extend the authentication timeout. + Parameters + ---------- + delay : float, default=0.05 + Delay in seconds between EXTEND_1 and EXTEND_2. Required for + the pump to process each packet before the next arrives. + Pass 0 only in unit tests (via fast_mode=True on authenticate()). + Notes ----- - - Both packets sent in parallel for efficiency - - Order may not matter, but we maintain observed sequence - - These packets "seal" the authentication + - Packets MUST be sent sequentially: EXTEND_1 then EXTEND_2. + - A 50ms gap between them is required for the pump to process + EXTEND_1 before EXTEND_2 arrives. Sending them in parallel + causes premature disconnection (issue #24). + - Order is empirically established from packet captures; see + docs/protocol/packet_traces/02_authentication.md. """ - async with asyncio.TaskGroup() as tg: - # Send both extension packets in parallel - tg.create_task( - self.ble_writer.write_gatt_char( - self.GENI_CHAR_UUID, self.EXTEND_2, response=False - ) - ) - tg.create_task( - self.ble_writer.write_gatt_char( - self.GENI_CHAR_UUID, self.EXTEND_1, response=False - ) - ) + await self.ble_writer.write_gatt_char( + self.GENI_CHAR_UUID, self.EXTEND_1, response=False + ) + if delay > 0: + await asyncio.sleep(delay) + await self.ble_writer.write_gatt_char( + self.GENI_CHAR_UUID, self.EXTEND_2, response=False + ) # ========================================================================== @@ -421,9 +430,10 @@ def calc_crc16(data: bytes) -> int: sleep(50ms) sleep(200ms) - # Stage 3: Extensions - ble.write(GENI_UUID, EXTEND_2) + # Stage 3: Extensions (sequential, EXTEND_1 before EXTEND_2) ble.write(GENI_UUID, EXTEND_1) + sleep(50ms) + ble.write(GENI_UUID, EXTEND_2) sleep(500ms) ``` diff --git a/src/alpha_hwr/models.py b/src/alpha_hwr/models.py index 143b134..62f4429 100644 --- a/src/alpha_hwr/models.py +++ b/src/alpha_hwr/models.py @@ -1,16 +1,16 @@ from __future__ import annotations from datetime import datetime, time +from typing import ClassVar from pydantic import BaseModel, ConfigDict, Field, field_validator - -from typing import ClassVar from alpha_hwr.constants import ( + FACTOR_CELSIUS_TO_FAHRENHEIT, FACTOR_M3H_TO_GPM, FACTOR_M_TO_FT, FACTOR_M_TO_PSI, - FACTOR_CELSIUS_TO_FAHRENHEIT, + ControlMode, ) @@ -137,9 +137,16 @@ class SetpointInfo(BaseModel): model_config: ClassVar[ConfigDict] = ConfigDict(frozen=True) - control_mode: int = Field(description="Active control mode ID") + control_mode: ControlMode | int = Field( + description="Active control mode ID" + ) operation_mode: int = Field(description="Operation mode") - setpoint: float = Field(description="Current setpoint value (raw)") + setpoint: float = Field( + description=( + "Current setpoint in user-facing units: " + "m for pressure, m³/h for flow, RPM for speed, °C for temperature" + ) + ) min_setpoint: float | None = Field( default=None, description="Minimum allowed setpoint" ) @@ -163,6 +170,19 @@ class SetpointInfo(BaseModel): description="True if Delta Temperature control (AutoAdapt) is enabled", ) + @field_validator("control_mode", mode="before") + @classmethod + def _coerce_control_mode(cls, v: object) -> ControlMode | int: + """Coerce raw int to ControlMode enum where possible.""" + if not isinstance(v, int): + raise ValueError( + f"control_mode must be an int, got {type(v).__name__}" + ) + try: + return ControlMode(v) + except ValueError: + return v + def get_display_value(self) -> tuple[float, str]: """ Get setpoint value with appropriate unit based on control mode. @@ -171,16 +191,17 @@ def get_display_value(self) -> tuple[float, str]: Tuple of (value, unit_string) Notes: - - CONSTANT_PRESSURE (0): Returns meters (m) - raw value is in Pascals, converted to m H2O - - PROPORTIONAL_PRESSURE (1): Returns meters (m) - raw value is in Pascals - - CONSTANT_FLOW (8): Returns m³/h - raw value is already in m³/h - - CONSTANT_SPEED (2): Returns RPM - raw value is already in RPM - - CONSTANT_TEMPERATURE (6): Returns °C - raw value is already in °C - - Others: Returns raw value with "units" + All setpoint values are stored in user-facing units (the + ControlService converts raw pump units before storing): + + - Pressure modes: stored in meters of water column (m H2O) + - Flow modes: stored in m³/h + - Speed mode: stored in RPM + - Temperature modes: stored in °C """ - from .constants import ControlMode - # Pressure modes (Pascals meters of water column) + # Pressure modes: stored in meters of water column (ControlService + # converts from raw Pa before storing in this field). if self.control_mode in ( ControlMode.CONSTANT_PRESSURE, ControlMode.PROPORTIONAL_PRESSURE, @@ -190,9 +211,7 @@ def get_display_value(self) -> tuple[float, str]: ControlMode.AUTO_ADAPT_UNDERFLOOR, ControlMode.AUTO_ADAPT_RADIATOR_AND_UNDERFLOOR, ): - # Convert Pascals to meters of water column (1 m H2O ≈ 9806.65 Pa) - meters = self.setpoint / 9806.65 - return (meters, "m") + return (self.setpoint, "m") # Flow modes (already in m³/h) elif self.control_mode in ( @@ -233,9 +252,7 @@ def get_limits_display( if self.min_setpoint is None or self.max_setpoint is None: return None - from .constants import ControlMode - - # Use same conversion logic as get_display_value() + # Pressure modes: stored in meters (same convention as get_display_value) if self.control_mode in ( ControlMode.CONSTANT_PRESSURE, ControlMode.PROPORTIONAL_PRESSURE, @@ -245,9 +262,10 @@ def get_limits_display( ControlMode.AUTO_ADAPT_UNDERFLOOR, ControlMode.AUTO_ADAPT_RADIATOR_AND_UNDERFLOOR, ): - min_val = self.min_setpoint / 9806.65 - max_val = self.max_setpoint / 9806.65 - return ((min_val, "m"), (max_val, "m")) + return ( + (self.min_setpoint, "m"), + (self.max_setpoint, "m"), + ) elif self.control_mode in ( ControlMode.CONSTANT_FLOW, diff --git a/src/alpha_hwr/services/control.py b/src/alpha_hwr/services/control.py index 4e15bae..7c25c0f 100644 --- a/src/alpha_hwr/services/control.py +++ b/src/alpha_hwr/services/control.py @@ -426,11 +426,18 @@ async def get_mode(self) -> Optional[SetpointInfo]: ">f", data[offset + 3 : offset + 7] )[0] - # Convert pressure setpoints from Pascals back to meters (standard for ALPHA HWR) - # Modes: CONSTANT_PRESSURE (0), PROPORTIONAL_PRESSURE (2) + # Convert pressure setpoints from Pascals to meters of + # water column (standard for ALPHA HWR). All pressure + # control modes send Pa on the wire; we convert here so + # SetpointInfo always stores user-facing units (m H2O). if control_mode in ( ControlMode.CONSTANT_PRESSURE, ControlMode.PROPORTIONAL_PRESSURE, + ControlMode.CONSTANT_DIFF_PRESSURE, + ControlMode.PROPORTIONAL_DIFF_PRESSURE, + ControlMode.AUTO_ADAPT_RADIATOR, + ControlMode.AUTO_ADAPT_UNDERFLOOR, + ControlMode.AUTO_ADAPT_RADIATOR_AND_UNDERFLOOR, ): setpoint = setpoint / 9806.65 @@ -492,7 +499,7 @@ async def get_mode(self) -> Optional[SetpointInfo]: ) return SetpointInfo( - control_mode=control_mode, + control_mode=self._current_mode, operation_mode=operation_mode, setpoint=min_temp, # Low temperature min_setpoint=min_temp, @@ -510,7 +517,7 @@ async def get_mode(self) -> Optional[SetpointInfo]: # Fall through to return basic setpoint return SetpointInfo( - control_mode=control_mode, + control_mode=self._current_mode, operation_mode=operation_mode, setpoint=setpoint, is_remote=is_remote, diff --git a/src/alpha_hwr/services/telemetry.py b/src/alpha_hwr/services/telemetry.py index d27a931..56628c2 100644 --- a/src/alpha_hwr/services/telemetry.py +++ b/src/alpha_hwr/services/telemetry.py @@ -250,6 +250,9 @@ def is_response_not_notification(packet: bytes) -> bool: # 2. Query Flow/Pressure (if no active stream) if not self._has_flow_stream: + if not self.transport.is_connected(): + logger.debug("Pump disconnected after motor state query") + return self._telemetry try: req = FrameBuilder.build_class10_read( 0x5D0122 @@ -276,6 +279,9 @@ def is_response_not_notification(packet: bytes) -> bool: await asyncio.sleep(0.05) # 3. Query Temperatures (always poll) + if not self.transport.is_connected(): + logger.debug("Pump disconnected before temperature query") + return self._telemetry try: req = FrameBuilder.build_class10_read( 0x5D012C diff --git a/tests/unit/core/test_authentication.py b/tests/unit/core/test_authentication.py new file mode 100644 index 0000000..7fd0862 --- /dev/null +++ b/tests/unit/core/test_authentication.py @@ -0,0 +1,127 @@ +""" +Unit tests for AuthenticationHandler. + +Covers extension packet ordering and timing requirements introduced +to fix premature disconnection on BLE firmware V06.00.01 (issue #24). +""" + +from __future__ import annotations + +from unittest.mock import AsyncMock, patch + +import pytest + +from alpha_hwr.core.authentication import AuthenticationHandler + + +@pytest.fixture +def mock_writer() -> AsyncMock: + """BLE writer mock that records all write_gatt_char calls.""" + writer = AsyncMock() + writer.write_gatt_char = AsyncMock() + return writer + + +@pytest.fixture +def handler(mock_writer: AsyncMock) -> AuthenticationHandler: + return AuthenticationHandler(mock_writer) + + +# --------------------------------------------------------------------------- +# Extension packet ordering +# --------------------------------------------------------------------------- + + +@pytest.mark.asyncio +async def test_extension_packets_extend1_before_extend2( + handler: AuthenticationHandler, mock_writer: AsyncMock +) -> None: + """EXTEND_1 must be written before EXTEND_2.""" + await handler.send_extension_packets(delay=0) + + calls = mock_writer.write_gatt_char.call_args_list + assert len(calls) == 2 + + first_data = calls[0][0][1] + second_data = calls[1][0][1] + + assert first_data == AuthenticationHandler.EXTEND_1, ( + "EXTEND_1 must be sent first" + ) + assert second_data == AuthenticationHandler.EXTEND_2, ( + "EXTEND_2 must be sent second" + ) + + +@pytest.mark.asyncio +async def test_extension_packets_sleep_between_packets( + handler: AuthenticationHandler, mock_writer: AsyncMock +) -> None: + """A sleep must occur between EXTEND_1 and EXTEND_2 when delay > 0.""" + all_calls: list[tuple[str, object]] = [] + + async def recording_sleep(t: float) -> None: + all_calls.append(("sleep", t)) + + async def recording_write(uuid: str, data: bytes, **kw: object) -> None: + all_calls.append(("write", data)) + + mock_writer.write_gatt_char.side_effect = recording_write + + with patch("alpha_hwr.core.authentication.asyncio.sleep", recording_sleep): + await handler.send_extension_packets(delay=0.05) + + kinds = [k for k, _ in all_calls] + assert kinds == ["write", "sleep", "write"], ( + f"Expected [write, sleep, write] but got {kinds}" + ) + assert all_calls[0][1] == AuthenticationHandler.EXTEND_1 + assert all_calls[2][1] == AuthenticationHandler.EXTEND_2 + + +@pytest.mark.asyncio +async def test_extension_packets_no_sleep_when_delay_zero( + handler: AuthenticationHandler, mock_writer: AsyncMock +) -> None: + """No sleep should be issued when delay=0 (fast_mode / tests).""" + with patch("alpha_hwr.core.authentication.asyncio.sleep") as mock_sleep: + await handler.send_extension_packets(delay=0) + + mock_sleep.assert_not_called() + + +# --------------------------------------------------------------------------- +# authenticate() fast_mode passes delay=0 to send_extension_packets +# --------------------------------------------------------------------------- + + +@pytest.mark.asyncio +async def test_authenticate_fast_mode_skips_extension_sleep( + handler: AuthenticationHandler, +) -> None: + """In fast_mode, no asyncio.sleep calls should be made at all.""" + with patch("alpha_hwr.core.authentication.asyncio.sleep") as mock_sleep: + result = await handler.authenticate(fast_mode=True) + + assert result is True + mock_sleep.assert_not_called() + + +@pytest.mark.asyncio +async def test_authenticate_normal_mode_sleeps_between_extensions( + handler: AuthenticationHandler, +) -> None: + """In normal mode, authenticate() must sleep between extension packets.""" + sleep_calls: list[float] = [] + + async def record_sleep(t: float) -> None: + sleep_calls.append(t) + + with patch("alpha_hwr.core.authentication.asyncio.sleep", record_sleep): + result = await handler.authenticate(fast_mode=False) + + assert result is True + # The 0.05s intra-extension sleep must appear in the call list + assert 0.05 in sleep_calls, ( + f"Expected 0.05s sleep between extension packets, got: {sleep_calls}" + ) diff --git a/tests/unit/core/test_telemetry_disconnect_guard.py b/tests/unit/core/test_telemetry_disconnect_guard.py new file mode 100644 index 0000000..b9a002b --- /dev/null +++ b/tests/unit/core/test_telemetry_disconnect_guard.py @@ -0,0 +1,158 @@ +""" +Unit tests for the disconnection guards in TelemetryService.read_once(). + +Verifies that read_once() returns partial telemetry immediately (without +attempting further queries) when the transport reports a disconnect between +the motor-state query and the flow/pressure or temperature queries. +""" + +from __future__ import annotations + +from unittest.mock import AsyncMock, MagicMock, patch + +import pytest + +from alpha_hwr.core.session import Session +from alpha_hwr.core.transport import Transport +from alpha_hwr.models import TelemetryData +from alpha_hwr.services.telemetry import TelemetryService + +_PATCH_SLEEP = "alpha_hwr.services.telemetry.asyncio.sleep" + + +def _make_service( + *, + connected_after_motor_state: bool = True, + connected_after_flow_pressure: bool = True, +) -> tuple[TelemetryService, MagicMock]: + """Build a TelemetryService with a scripted transport mock. + + Args: + connected_after_motor_state: Value returned by is_connected() before + the flow/pressure query. + connected_after_flow_pressure: Value returned by is_connected() before + the temperature query. + + Returns: + (service, transport_mock) tuple. + """ + transport = MagicMock(spec=Transport) + transport.query = AsyncMock(return_value=None) + # Return connected=True for the initial ensure_connected check, then + # the scripted values for the two guards. + transport.is_connected = MagicMock( + side_effect=[ + connected_after_motor_state, + connected_after_flow_pressure, + ] + ) + + session = MagicMock(spec=Session) + session.ensure_connected = MagicMock() + + service = TelemetryService(transport, session) + # Disable stream flags so the polling branches are exercised + service._has_motor_state_stream = False + service._has_flow_stream = False + return service, transport + + +# --------------------------------------------------------------------------- +# Disconnect after motor-state query (before flow/pressure) +# --------------------------------------------------------------------------- + + +@pytest.mark.asyncio +async def test_disconnect_after_motor_state_skips_flow_query() -> None: + """If disconnected after motor state, flow/pressure query must not run.""" + service, transport = _make_service(connected_after_motor_state=False) + + with patch(_PATCH_SLEEP): + result = await service.read_once() + + assert isinstance(result, TelemetryData) + # query() is called once (motor state) then the guard returns early; + # it must NOT be called for flow/pressure or temperature + assert transport.query.call_count == 1, ( + f"Expected 1 query (motor state only), got {transport.query.call_count}" + ) + + +@pytest.mark.asyncio +async def test_disconnect_after_motor_state_returns_partial_telemetry() -> None: + """Partial telemetry (with None fields) is returned rather than raising.""" + service, _ = _make_service(connected_after_motor_state=False) + + with patch(_PATCH_SLEEP): + result = await service.read_once() + + # flow_m3h and head_m must still be None (not populated) but the call + # must succeed without raising. + assert result.flow_m3h is None + assert result.head_m is None + + +# --------------------------------------------------------------------------- +# Disconnect after flow/pressure query (before temperature) +# --------------------------------------------------------------------------- + + +@pytest.mark.asyncio +async def test_disconnect_before_temperature_skips_temp_query() -> None: + """If disconnected before temperature query, that query must not run.""" + service, transport = _make_service( + connected_after_motor_state=True, + connected_after_flow_pressure=False, + ) + + with patch(_PATCH_SLEEP): + result = await service.read_once() + + assert isinstance(result, TelemetryData) + # motor-state + flow/pressure = 2 calls; temperature must be skipped + assert transport.query.call_count == 2, ( + f"Expected 2 query calls, got {transport.query.call_count}" + ) + + +@pytest.mark.asyncio +async def test_disconnect_before_temperature_returns_partial_telemetry() -> ( + None +): + """Returns without raising when disconnected before temperature query.""" + service, _ = _make_service( + connected_after_motor_state=True, + connected_after_flow_pressure=False, + ) + + with patch(_PATCH_SLEEP): + result = await service.read_once() + + assert result.media_temperature_c is None + + +# --------------------------------------------------------------------------- +# Happy path: all queries succeed when transport stays connected +# --------------------------------------------------------------------------- + + +@pytest.mark.asyncio +async def test_all_connected_runs_all_three_queries() -> None: + """When connected throughout, all three register queries execute.""" + transport = MagicMock(spec=Transport) + transport.query = AsyncMock(return_value=None) + transport.is_connected = MagicMock(return_value=True) + + session = MagicMock(spec=Session) + session.ensure_connected = MagicMock() + + service = TelemetryService(transport, session) + service._has_motor_state_stream = False + service._has_flow_stream = False + + with patch(_PATCH_SLEEP): + await service.read_once() + + assert transport.query.call_count == 3, ( + f"Expected 3 query calls, got {transport.query.call_count}" + )