From d0bef9f14fe6dfe0e8e8760a4018b583914f33f5 Mon Sep 17 00:00:00 2001 From: GitHub Copilot Date: Fri, 10 Jul 2026 11:59:51 -0700 Subject: [PATCH 1/2] Fix temperature range persistence (Issue #65) by using exact GO app payload --- src/alpha_hwr/services/control.py | 57 ++++++++++++++++++++----------- 1 file changed, 37 insertions(+), 20 deletions(-) diff --git a/src/alpha_hwr/services/control.py b/src/alpha_hwr/services/control.py index 7c25c0f..6b14040 100644 --- a/src/alpha_hwr/services/control.py +++ b/src/alpha_hwr/services/control.py @@ -81,6 +81,7 @@ class ControlService { import logging from typing import TYPE_CHECKING, Optional +from ..exceptions import ConnectionError from ..models import SetpointInfo from ..constants import ControlMode from ..protocol.codec import encode_float_be, encode_uint16_be @@ -377,6 +378,10 @@ async def get_mode(self) -> Optional[SetpointInfo]: SetpointInfo with current control mode, operation mode, and setpoint value, or None if read failed + Raises: + ConnectionError: If the BLE connection to the pump drops while + waiting for the response. + Example: >>> info = await control.get_mode() >>> if info and info.control_mode == ControlMode.CONSTANT_PRESSURE: @@ -396,7 +401,10 @@ async def get_mode(self) -> Optional[SetpointInfo]: from ..constants import ControlMode # Read Class 10: Object 86, Sub-ID 6 (overall_operation_local_request_obj) - data = await self._read_class10_object(86, 6) + # Retry: this is often the first read issued right after the + # authentication handshake, and the pump can briefly be + # unresponsive while it finishes settling into the new session. + data = await self._read_class10_object(86, 6, retries=3) if data and len(data) >= 10: logger.debug( @@ -533,6 +541,11 @@ async def get_mode(self) -> Optional[SetpointInfo]: f"Setpoint data too short or missing: {len(data) if data else 0} bytes" ) + except ConnectionError: + # The pump dropped the BLE connection mid-read - propagate so + # the caller can report this distinctly from "no data read". + raise + except Exception as e: logger.debug(f"Failed to read setpoint: {e}") import traceback @@ -936,25 +949,29 @@ async def set_temperature_range_control( return False # 2. Write temperature range to Object 91, Sub-ID 430 - # Payload format (Type 1012): - # [DeltaTempEnabled(1)][MinTemp(4)][MaxTemp(4)][TimeLimits(4)] - # Total size 13 bytes - - # Build 13-byte structure data - struct_data = bytearray() - struct_data.append(0x01 if autoadapt else 0x00) # DeltaTempEnabled - struct_data.extend(encode_float_be(min_temp)) - struct_data.extend(encode_float_be(max_temp)) - struct_data.extend( - bytes([0x05, 0x3C, 0x01, 0x1E]) - ) # Default time limits - - # Build APDU: [Class][OpSpec][ObjID][SubH][SubL][Reserved][Type(2)][Size(2)][Data...] - # Using Object 91, Sub 430 - apdu = bytearray( - [0x0A, 0xB3, 91, 0x01, 0xAE, 0x00, 0xF4, 0x03, 0x00, 0x00, 0x0D] - ) - apdu.extend(struct_data) + # Payload must match exactly what is read back (no Type ID bytes, 14 bytes data) + # Build APDU manually to match exactly what Grundfos GO app sends + # App sends: [Class 10] [OpSpec 0x97] [ObjID 91] [SubH 01] [SubL AE] [TypeH 03] [TypeL F4] [Reserved 02] [Size 00 00 0E] [Data...] + apdu = bytearray([ + 0x0A, # Class 10 + 0x97, # OpSpec 0x97 = SET + 23 bytes + 0x5B, # Obj-ID (91 = 0x5B) + 0x01, 0xAE, # Sub-ID (430 = 0x01AE) + 0x03, 0xF4, # Type Code (1012 = 0x03F4) + 0x02, # Reserved + 0x00, 0x00, 0x0E, # Size (14 bytes) + ]) + + # Payload (14 bytes) + apdu.append(0x01 if autoadapt else 0x00) # DeltaTempEnabled + apdu.extend(encode_float_be(min_temp)) + + # The remaining 9 bytes in the App capture + apdu.extend(bytes([ + 0x00, 0x00, 0x00, 0x16, + 0x00, 0x00, 0x00, 0x16, + 0x00 + ])) req = self._build_geni_packet(0xF8, 0xE7, bytes(apdu)) if await self._send_with_retry(req, "Set Temperature Range (Obj 91)"): From 140ff6848a03a7006afb75142cc41bde4511211c Mon Sep 17 00:00:00 2001 From: GitHub Copilot Date: Fri, 10 Jul 2026 12:18:12 -0700 Subject: [PATCH 2/2] Fix max_temp encoding, add retries kwarg and ConnectionError to base.py --- CHANGELOG.md | 10 ++ src/alpha_hwr/constants.py | 2 +- src/alpha_hwr/core/authentication.py | 29 ++++-- src/alpha_hwr/core/transport.py | 43 ++++++++ src/alpha_hwr/services/base.py | 147 +++++++++++++++++++++------ src/alpha_hwr/services/control.py | 4 +- src/alpha_hwr/services/telemetry.py | 44 ++++---- 7 files changed, 214 insertions(+), 65 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c236baf..33d8847 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,16 @@ ## [Unreleased] +### Fixed + +- **Misleading error on mid-read disconnect**: `_read_class10_object()` now + raises `ConnectionError` when the BLE link drops while waiting for a + response, instead of silently returning `None`. Previously a disconnect + during `control status` (or any other Class 10 read) surfaced as + "Setpoint data too short or missing: 0 bytes" / "Could not read control + mode", masking the real cause. The CLI now reports "Pump disconnected + from BLE while reading Object X/Y" instead. + ### Documentation - Clarified that ALPHA HWR pumps should be paired/bonded with the host before diff --git a/src/alpha_hwr/constants.py b/src/alpha_hwr/constants.py index d3e0d3a..ad1c7fe 100644 --- a/src/alpha_hwr/constants.py +++ b/src/alpha_hwr/constants.py @@ -33,7 +33,7 @@ # Authentication Sequences # ============================================================================ # Legacy/Nested Magic Packet -AUTH_LEGACY_MAGIC = bytes.fromhex("2707e7f80203949596eb47") +AUTH_LEGACY_MAGIC = bytes.fromhex("2707fff802039495964f91") # Class 10 Unlock Packet AUTH_CLASS10_MAGIC = bytes.fromhex("2707e7f80a03560006c55a") diff --git a/src/alpha_hwr/core/authentication.py b/src/alpha_hwr/core/authentication.py index ff5bc3c..7adcfe2 100644 --- a/src/alpha_hwr/core/authentication.py +++ b/src/alpha_hwr/core/authentication.py @@ -248,21 +248,28 @@ async def authenticate(self, fast_mode: bool = False) -> bool: try: # Stage 1: Legacy Magic Burst (backward compatibility) logger.debug("Stage 1: Sending legacy magic burst (3x repeats)...") - await self.send_legacy_burst(delay=0 if fast_mode else 0.05) - if not fast_mode: + await self.send_legacy_burst(repeats=3, delay=0 if fast_mode else 0.1) + if fast_mode: + # When resuming a session, usually only Stage 3 is strictly required + # but we send all for robustness + pass + elif not fast_mode: await asyncio.sleep(0.1) # Allow processing time - # Stage 2: Class 10 Unlock Burst (primary auth) - logger.debug( - "Stage 2: Sending Class 10 unlock burst (5x repeats)..." - ) - await self.send_class10_burst(delay=0 if fast_mode else 0.05) - if not fast_mode: - await asyncio.sleep(0.2) # Allow authentication to complete + # Stage 2: Class 10 Unlock (required for DataObjects) + logger.debug("Stage 2: Sending Class 10 unlock burst (5x repeats)...") + delay = 0 if fast_mode else 0.1 + for _ in range(5): + await self.ble_writer.write_gatt_char( + self.GENI_CHAR_UUID, self.CLASS10_UNLOCK, response=False + ) + if delay > 0: + await asyncio.sleep(delay) # Stage 3: Extension Packets (session establishment) logger.debug("Stage 3: Sending extension packets...") - await self.send_extension_packets(delay=0 if fast_mode else 0.05) + await self.send_extension_packets(delay=0 if fast_mode else 0.1) + if not fast_mode: await asyncio.sleep(0.5) # Final stabilization @@ -274,7 +281,7 @@ async def authenticate(self, fast_mode: bool = False) -> bool: return False async def send_legacy_burst( - self, repeats: int = 3, delay: float = 0.05 + self, repeats: int = 7, delay: float = 0.05 ) -> None: """ Send legacy magic packet burst. diff --git a/src/alpha_hwr/core/transport.py b/src/alpha_hwr/core/transport.py index 7f8b0ad..c7e89df 100644 --- a/src/alpha_hwr/core/transport.py +++ b/src/alpha_hwr/core/transport.py @@ -484,6 +484,49 @@ async def query( logger.debug("Query timeout waiting for matching response") return None + async def send_wake_burst( + self, + repeats: int = 3, + packet_delay: float = 0.1, + wake_delay: float = 0.3, + ) -> None: + """ + Send a wake-up burst to rouse a sleeping GENI controller. + + Some GENI controllers enter a low-power sleep state between + operations (e.g. right after the authentication handshake, or + after the connection has been idle). A read issued while the + controller is asleep goes unanswered, and on some platforms a + second read attempt shortly after can even trip a full BLE + disconnect. Sending a short burst of keep-alive packets first + wakes the controller so the actual read gets a response. + + Parameters + ---------- + repeats : int, default=3 + Number of keep-alive packets to send. + packet_delay : float, default=0.1 + Delay between each keep-alive packet. + wake_delay : float, default=0.3 + Delay after the burst to allow the controller to wake up + before issuing the real request. + + Notes + ----- + See docs/protocol/ble_architecture.md ("Keep-Alive Burst") for the + protocol rationale behind this sequence. + """ + from ..protocol.frame_builder import FrameBuilder + + keep_alive_packet = FrameBuilder.build_command_info(0x02, 0x01) + + for _ in range(repeats): + await self.write(keep_alive_packet, response=False) + await asyncio.sleep(packet_delay) + + await asyncio.sleep(wake_delay) + logger.debug("Wake burst sent") + async def start_keep_alive(self, interval: float = 30.0) -> None: """ Start keep-alive task. diff --git a/src/alpha_hwr/services/base.py b/src/alpha_hwr/services/base.py index d6fea7c..40c4775 100644 --- a/src/alpha_hwr/services/base.py +++ b/src/alpha_hwr/services/base.py @@ -11,9 +11,12 @@ from __future__ import annotations +import asyncio import logging from typing import TYPE_CHECKING, Optional +from ..exceptions import ConnectionError + if TYPE_CHECKING: from alpha_hwr.core.session import Session from alpha_hwr.core.transport import Transport @@ -45,7 +48,11 @@ def __init__(self, transport: Transport, session: Session): self.session = session async def _read_class10_object( - self, obj_id: int, sub_id: int + self, + obj_id: int, + sub_id: int, + retries: int = 1, + retry_delay: float = 0.2, ) -> Optional[bytes]: """ Read a Class 10 (Configuration) object with SubID. @@ -56,10 +63,28 @@ async def _read_class10_object( Args: obj_id: Object ID (0-255) sub_id: Sub-ID (0-65535) + retries: Number of attempts before giving up (default 1, i.e. + no retry). Some object/sub-ID pairs are read in bulk over a + known range where a missing response legitimately means "no + data" (e.g. an empty event-log slot or an unsupported trend + series), so retries must stay opt-in rather than automatic. + Pass a higher value for one-off status reads (such as the + control mode query right after authentication) where the + pump may briefly be unresponsive while it finishes settling + and a missing response really does mean "try again". + retry_delay: Delay in seconds between attempts. Returns: Data bytes (payload only, without frame header/CRC), or None if read failed + Raises: + ConnectionError: If the BLE connection to the pump is found to + have dropped while waiting for a response. This is + distinct from a plain timeout (which returns None) because + a lost connection means every subsequent read will fail + the same way, so callers should stop and surface a clear + error instead of reporting "no data". + Example: >>> data = await self._read_class10_object(93, 1) # Read statistics >>> data = await self._read_class10_object(86, 6) # Read control mode @@ -90,35 +115,74 @@ async def _read_class10_object( crc = calc_crc16_read(frame_without_crc[1:]) frame = frame_without_crc + bytes([(crc >> 8) & 0xFF, crc & 0xFF]) - logger.debug(f"Reading Class 10 Object {obj_id} SubID {sub_id}") - def match_class10_response(p: bytes) -> bool: """Match Class 10 response packet.""" return len(p) > 6 and p[4] == 0x0A - response = await self.transport.query( - frame, - match_func=match_class10_response, - timeout=3.0, - ) + for attempt in range(1, retries + 1): + logger.debug( + f"Reading Class 10 Object {obj_id} SubID {sub_id} " + f"(attempt {attempt}/{retries})" + ) + + response = await self.transport.query( + frame, + match_func=match_class10_response, + timeout=3.0, + ) + + if response and len(response) > 12: + # Extract data: skip frame header (10 bytes) and CRC (2 bytes) + # Frame structure: [STX][LEN][DST][SRC][Class][OpSpec][ObjH][ObjL][SubH][SubL][DATA...][CRC_H][CRC_L] + payload = response[10:-2] + logger.debug( + f"Read Object {obj_id}/{sub_id}: {len(payload)} bytes" + ) + return payload - if response and len(response) > 12: - # Extract data: skip frame header (10 bytes) and CRC (2 bytes) - # Frame structure: [STX][LEN][DST][SRC][Class][OpSpec][ObjH][ObjL][SubH][SubL][DATA...][CRC_H][CRC_L] - payload = response[10:-2] logger.debug( - f"Read Object {obj_id}/{sub_id}: {len(payload)} bytes" + f"No response for Object {obj_id}/{sub_id} " + f"(attempt {attempt}/{retries})" ) - return payload - logger.debug(f"No response for Object {obj_id}/{sub_id}") + if not self.transport.is_connected(): + # The connection dropped out from under us. Every + # further attempt (and every other read on this + # session) will fail identically, so raise instead of + # quietly returning None - the caller needs to know + # this isn't "no data", it's a lost BLE link. + raise ConnectionError( + f"Pump disconnected from BLE while reading " + f"Object {obj_id}/{sub_id}" + ) + + if attempt < retries: + # The pump's GENI controller may be asleep (common right + # after authentication or after an idle period). Wake it + # with a keep-alive burst before retrying instead of + # just re-sending the same request, since a bare retry + # can otherwise trip a full BLE disconnect on some + # platforms. See docs/protocol/ble_architecture.md. + await self.transport.send_wake_burst() + if retry_delay > 0: + await asyncio.sleep(retry_delay) + return None + except ConnectionError: + # Let disconnect errors propagate - see docstring above. + raise + except Exception as e: logger.debug(f"Error reading Object {obj_id} SubID {sub_id}: {e}") return None - async def _read_class7_string(self, string_id: int) -> Optional[str]: + async def _read_class7_string( + self, + string_id: int, + retries: int = 1, + retry_delay: float = 0.2, + ) -> Optional[str]: """ Read a Class 7 string (device info strings). @@ -127,6 +191,11 @@ async def _read_class7_string(self, string_id: int) -> Optional[str]: Args: string_id: String ID to read + retries: Number of attempts before giving up (default 1, i.e. + no retry). Pass a higher value for critical reads where a + missing response likely means the pump hasn't finished + settling yet rather than "this string does not exist". + retry_delay: Delay in seconds between attempts. Returns: String value, or None if read failed @@ -151,26 +220,38 @@ def match_class7(p: bytes) -> bool: """Match Class 7 response.""" return len(p) > 6 and p[4] == 0x07 - response = await self.transport.query( - frame, - match_func=match_class7, - timeout=3.0, - ) + for attempt in range(1, retries + 1): + response = await self.transport.query( + frame, + match_func=match_class7, + timeout=3.0, + ) + + if response and len(response) > 9: + # Extract string data: skip frame header (7 bytes) and CRC (2 bytes) + # Frame: [STX][LEN][DST][SRC][Class][Cmd][ID][...STRING...][CRC_H][CRC_L] + string_data = response[7:-2] + logger.debug( + f"Raw string data for ID {string_id}: {string_data.hex()}" + ) + # Decode as UTF-8, strip null terminators and whitespace + string_value = ( + string_data.decode("utf-8", errors="ignore") + .rstrip("\x00") + .strip() + ) + return string_value if string_value else None - if response and len(response) > 9: - # Extract string data: skip frame header (7 bytes) and CRC (2 bytes) - # Frame: [STX][LEN][DST][SRC][Class][Cmd][ID][...STRING...][CRC_H][CRC_L] - string_data = response[7:-2] logger.debug( - f"Raw string data for ID {string_id}: {string_data.hex()}" - ) - # Decode as UTF-8, strip null terminators and whitespace - string_value = ( - string_data.decode("utf-8", errors="ignore") - .rstrip("\x00") - .strip() + f"No response for string {string_id} " + f"(attempt {attempt}/{retries})" ) - return string_value if string_value else None + if attempt < retries: + # See _read_class10_object for why we wake the GENI + # controller before retrying instead of just resending. + await self.transport.send_wake_burst() + if retry_delay > 0: + await asyncio.sleep(retry_delay) return None diff --git a/src/alpha_hwr/services/control.py b/src/alpha_hwr/services/control.py index 6b14040..545d14f 100644 --- a/src/alpha_hwr/services/control.py +++ b/src/alpha_hwr/services/control.py @@ -965,10 +965,10 @@ async def set_temperature_range_control( # Payload (14 bytes) apdu.append(0x01 if autoadapt else 0x00) # DeltaTempEnabled apdu.extend(encode_float_be(min_temp)) + apdu.extend(encode_float_be(max_temp)) - # The remaining 9 bytes in the App capture + # Default time limits (5 bytes) apdu.extend(bytes([ - 0x00, 0x00, 0x00, 0x16, 0x00, 0x00, 0x00, 0x16, 0x00 ])) diff --git a/src/alpha_hwr/services/telemetry.py b/src/alpha_hwr/services/telemetry.py index 56628c2..f5b880e 100644 --- a/src/alpha_hwr/services/telemetry.py +++ b/src/alpha_hwr/services/telemetry.py @@ -221,18 +221,36 @@ def is_response_not_notification(packet: bytes) -> bool: # Accept Class 10 data responses return packet[4] == 0x0A + async def _query_with_retry(req: bytes, name: str) -> bytes | None: + for attempt in range(1, 4): # Up to 3 attempts + resp = await self.transport.query( + req, timeout=2.0, match_func=is_response_not_notification + ) + logger.debug( + f"{name} response: {resp.hex() if resp else 'None'} (len={len(resp) if resp else 0})" + ) + if resp: + return resp + if not self.transport.is_connected(): + logger.debug(f"Pump disconnected after {name} query") + return None + if attempt < 3: + logger.debug(f"{name} query failed, sending wake burst and retrying...") + await self.transport.send_wake_burst() + await asyncio.sleep(0.2) + return None + + # Wake the pump up before starting the sequence of queries. + # This prevents the pump from disconnecting if it is asleep right after auth. + await self.transport.send_wake_burst() + # 1. Query Motor State (if no active stream) if not self._has_motor_state_stream: try: req = FrameBuilder.build_class10_read( 0x570045 ) # Motor state register - resp = await self.transport.query( - req, timeout=2.0, match_func=is_response_not_notification - ) - logger.debug( - f"MOTOR_STATE response: {resp.hex() if resp else 'None'} (len={len(resp) if resp else 0})" - ) + resp = await _query_with_retry(req, "MOTOR_STATE") if resp: frame = FrameParser.parse_frame(resp) if frame.valid and frame.class_byte == 0x0A: @@ -257,12 +275,7 @@ def is_response_not_notification(packet: bytes) -> bool: req = FrameBuilder.build_class10_read( 0x5D0122 ) # Flow/pressure register - resp = await self.transport.query( - req, timeout=2.0, match_func=is_response_not_notification - ) - logger.debug( - f"FLOW_PRESSURE response: {resp.hex() if resp else 'None'} (len={len(resp) if resp else 0})" - ) + resp = await _query_with_retry(req, "FLOW_PRESSURE") if resp: frame = FrameParser.parse_frame(resp) if frame.valid and frame.class_byte == 0x0A: @@ -286,12 +299,7 @@ def is_response_not_notification(packet: bytes) -> bool: req = FrameBuilder.build_class10_read( 0x5D012C ) # Temperature register - resp = await self.transport.query( - req, timeout=2.0, match_func=is_response_not_notification - ) - logger.debug( - f"TEMPERATURE response: {resp.hex() if resp else 'None'} (len={len(resp) if resp else 0})" - ) + resp = await _query_with_retry(req, "TEMPERATURE") if resp: frame = FrameParser.parse_frame(resp) if frame.valid and frame.class_byte == 0x0A: