Fix remote mode commands#34
Open
eman wants to merge 3 commits into
Open
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR corrects the on-device command bytes used for remote mode control and improves robustness around BLE reads by adding wake/retry behavior and clearer disconnect signaling.
Changes:
- Fix remote mode enable/disable APDUs to use OpSpec
0x81(SET) and send0x08(LOCAL) for disable. - Add wake-burst + retry logic around early-session reads (telemetry polling and selected Class 10 reads) and distinguish mid-read disconnects via
ConnectionError. - Adjust authentication burst timing/bytes and document the behavior change in
CHANGELOG.md.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| src/alpha_hwr/services/telemetry.py | Adds a wake burst before polling and retries queries when the pump is briefly unresponsive. |
| src/alpha_hwr/services/control.py | Updates remote mode command bytes and adds retry to the initial mode read; adjusts temperature-range APDU structure. |
| src/alpha_hwr/services/base.py | Adds opt-in retry support and raises ConnectionError when disconnect is detected mid-read. |
| src/alpha_hwr/core/transport.py | Introduces send_wake_burst() helper for waking a sleeping controller before reads. |
| src/alpha_hwr/core/authentication.py | Tweaks authentication burst behavior/timing and legacy burst defaults. |
| src/alpha_hwr/constants.py | Updates the legacy auth magic packet constant. |
| CHANGELOG.md | Documents the improved error surfaced on mid-read disconnect. |
Comment on lines
+229
to
+231
| logger.debug( | ||
| f"{name} response: {resp.hex() if resp else 'None'} (len={len(resp) if resp else 0})" | ||
| ) |
Comment on lines
+312
to
314
| # Class 3: 03 81 07 | ||
| apdu = bytes([0x03, 0x81, 0x07]) | ||
| cmd = self._build_geni_packet(0xF8, 0xE7, apdu) |
Comment on lines
339
to
+342
| logger.info("Disabling Remote Mode (Auto)...") | ||
|
|
||
| # Class 3: 03 C1 06 | ||
| apdu = bytes([0x03, 0xC1, 0x06]) | ||
| # Class 3: 03 81 08 | ||
| apdu = bytes([0x03, 0x81, 0x08]) |
Comment on lines
+955
to
+974
| 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)) | ||
| apdu.extend(encode_float_be(max_temp)) | ||
|
|
||
| # Default time limits (5 bytes) | ||
| apdu.extend(bytes([ | ||
| 0x00, 0x00, 0x00, 0x16, | ||
| 0x00 | ||
| ])) |
Comment on lines
+252
to
257
| 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 |
Comment on lines
283
to
285
| async def send_legacy_burst( | ||
| self, repeats: int = 3, delay: float = 0.05 | ||
| self, repeats: int = 7, delay: float = 0.05 | ||
| ) -> None: |
Comment on lines
+148
to
+157
| 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}" | ||
| ) |
Comment on lines
+523
to
+528
| 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") |
Comment on lines
35
to
+36
| # Legacy/Nested Magic Packet | ||
| AUTH_LEGACY_MAGIC = bytes.fromhex("2707e7f80203949596eb47") | ||
| AUTH_LEGACY_MAGIC = bytes.fromhex("2707fff802039495964f91") |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes the disable_remote_mode command to send 0x08 (LOCAL) instead of 0x06 (START), and updates the OpSpec to 0x81 (SET) instead of 0xC1 (INFO) for both enable and disable commands.