Small items from the review of 1.0.5 - #11
Merged
Merged
Conversation
A sixth review, of 1.0.5, found no defect in the code. This takes its small
items. No change to the wire format or the public API. Tested on 3.13 and
3.14, 274 tests under -X dev with ResourceWarning as an error, oracle
fixtures unchanged, and live against an RM4 Pro.
Technical details:
- A NetworkTimeoutError or EndpointClosedError during the library's own
re-authentication propagates instead of being swallowed in favour of
the device's original expired-key reply, so a network failure is
reported as one.
- _open_endpoint binds to ("0.0.0.0", 0) when neither address is given
(discovery, ping, setup), for the proactor loop on Windows.
- The RM Max comment and the 1.0.0 changelog entry say only what upstream
mjg59#838 showed: mapping per its diff, testers reported "locked" on auth,
listed as reported rather than confirmed.
- The undecodable-capture message is debug, not warning.
- _capture_rf_loop closes its inner generator with aclosing.
- Version 1.0.6.
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.
The sixth review found nothing wrong in the code, so this is a tidy-up round. The one behaviour change is small: if the device says the session key has expired and the library's attempt to log in again then times out, the caller now sees the timeout instead of the device's original message, which is what actually happened.
The rest is a Windows safeguard for the discovery socket, an honest rewording of the RM Max note (the mapping comes from an upstream pull request whose testers reported the device answering "locked", so it is listed as reported rather than confirmed), a log level, and one loop closed the same way as its sibling.
Tested on Python 3.13 and 3.14, 274 tests, plus a live run against an RM4 Pro.
Coding assistant's notes on this change: