fix: add timeout and fallback for lock_info probe#306
Merged
Conversation
… retry loop lock_info() reads GATT Device Information characteristics with no timeout, causing hangs when the lock drops the connection during reads. Since lock_info is required before any update can proceed, a failing probe creates an infinite reconnect-probe-disconnect loop. - Add 10s timeout around the entire lock_info() probe - Handle individual BleakError per characteristic so partial reads succeed - Read model first since it drives battery and door sense behavior - Fall back to default LockInfo in _update() when probe fails entirely
There was a problem hiding this comment.
Pull request overview
This PR hardens the BLE lock_info() probe and the PushLock._update() flow so update cycles can continue even when device-information GATT reads hang or fail, avoiding repeated reconnect/probe loops.
Changes:
- Add a timeout + per-characteristic error handling in
Lock.lock_info()and read the model characteristic first. - Add
_probe_lock_info()inPushLockto fall back to a defaultLockInfowhen probing fails, allowing_update()to proceed. - Add unit tests covering success, partial failure, total failure, timeout behavior, model-first ordering, and
_update()fallback behavior.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
src/yalexs_ble/lock.py |
Adds LOCK_INFO_TIMEOUT and updates lock_info() to read model first, apply a timeout, and tolerate per-read BleakErrors. |
src/yalexs_ble/push.py |
Adds _probe_lock_info() and uses it from _update() to avoid update failures when probing lock info fails. |
tests/test_lock.py |
Adds coverage for lock_info() success/partial failure/all-fail/timeout and verifies characteristic read order. |
tests/test_push.py |
Adds coverage to ensure _update() continues when lock_info() raises TimeoutError or BleakError. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Skip missing characteristics instead of aborting entire probe - Use empty model default so door_sense=False when model is unknown - Fix docstring wording - Add test for missing characteristic handling
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.
Summary
lock_info()GATT reads to prevent indefinite hangsBleakErrorper characteristic so partial reads return partial resultsLockInfoin_update()when the entire probe fails, breaking the infinite reconnect-probe-disconnect loopProblem
lock_info()reads 4 Device Information Service characteristics (read_gatt_char) with no timeout. When the lock drops the BLE connection during these reads (e.g. ATT error 19),read_gatt_charhangs until the remote disconnects (~30s). Since_lock_infois required before any_update()can proceed, every update cycle reconnects and retries the probe, creating an infinite loop that makes the lock completely unresponsive.Test plan
test_lock_info_success— all reads succeedtest_lock_info_partial_failure— one read fails, others succeed with partial resultstest_lock_info_all_reads_fail— all reads fail, returns all defaultstest_lock_info_timeout— reads hang, 5s timeout firestest_lock_info_reads_model_first— model is read before other characteristicstest_update_continues_when_lock_info_probe_fails—TimeoutErrorfallback with MAC as serialtest_update_continues_when_lock_info_probe_bleak_error—BleakErrorfallback with MAC as serial