Fix the issues found in the review of 1.0.2 - #8
Merged
Merged
Conversation
A third review, this one of 1.0.2, found one behaviour that was worse than the original library's and a handful of small things. Nothing in the wire format changed. This fixes them. Tested on 3.13 and 3.14, 261 tests, oracle fixtures unchanged, and live against an RM4 Pro under -X dev. Technical details: - When re-authentication after an expired-key answer fails (a device locked in the app, say), the request's own reply is returned, so the caller sees the same AuthorizationError or ConnectionClosedError the original library raised, not an AuthenticationError from the retry. The failure is logged at debug. - The auth generation is read under the request lock, so a request queued behind auth() cannot observe a stale one and skip a needed re-auth. - aclose() racing an endpoint open no longer leaks the new socket; the open notices the close and raises EndpointClosedError. - A new capture window re-checks for a rival claimant after giving the finalizer its turn. - CapturedSignal.pulses and ParsedPacket.pulses are tuples, so the frozen dataclasses are hashable. - check_error unpacks with "<h"; the locks are created in __init__; send_packet accepts bytearray; setup() goes through a public send_setup_packet() helper; the CLI closes its device. - README: re-auth contract and worst case (three timeouts), A2 and Hysen HY02/HY03 in the device list, hello response mac is bytes. - Version 1.0.3.
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.
I had 1.0.2 reviewed the same way as the last two. Nothing in how the library talks to devices changed, and the review confirmed that again.
It did find an issue when a device's session key expires and the device has since been locked in the app, the library tried to log in again, failed, and raised the login failure instead of the error the device gave the original request. The original library never tried to log in again, so a program written against it, including Home Assistant, handles the first error and may have never seen the second. That is fixed, along with some smaller items.
Tested on Python 3.13 and 3.14, 261 tests, plus a live run against an RM4 Pro.
Coding assistant's notes on this change: