Fix the issues found in the second review of 1.0.1 - #7
Merged
Conversation
A second, adversarial review of 1.0.1 and a re-test of the first review's findings turned up three real defects in the transport and capture code, none in the wire format. This fixes them and takes the smaller items along. Tested on 3.13 and 3.14, 257 tests, oracle fixtures unchanged. Technical details: - Reply matching remembers every recently used counter, not only timed-out ones, so the second answer to a resent request is dropped instead of being taken as the next request's reply. - auth() resets the session and installs the new key under the request lock, so a queued request is never framed with id 0. - A new capture window gives asyncio's finalizer a turn to close a dropped generator, then refuses if the old window is still alive, instead of taking it from a paused consumer. A refused attempt no longer displaces the live window. - An undecodable returned packet is logged and skipped; the window re-arms. - EndpointClosedError (-4013), a subclass of ConnectionClosedError, for aclose() during a request. - hello() closes the scan generator; TimeoutError spelling; unused protocol future removed; debug logging on device and remote. - README: Closing section, Timing section with the bench numbers, Python support note, return-value differences from 0.19.0. - Version 1.0.2.
No behaviour change: 257 tests pass before and after, the oracle fixtures are byte-identical, and repr/str output is unchanged. This is the one-time formatting pass the pyproject comment promised once the async port landed. Technical details: - ruff format over the tree; CI now runs ruff format --check. - Lint set widened from E9/F/I to E, W, F, I, UP, B, ASYNC, RUF. Safe autofixes applied (typing modernised to the 3.13 spellings, f-strings for the percent formatting in repr/str and the exceptions). - Ignored with a reason in pyproject: ASYNC109 (protocol timeout, not a cancellation scope), RUF012 and E721 (inherited class tables and the exception __eq__), RUF006 in tests (helper tasks fired on purpose), and E501 in the three upstream modules with long example payloads. - zip() calls carry an explicit strict=; hello()'s first-reply loop carries a noqa with its reason.
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.1 looked at again, this time with someone deliberately trying to break it, and the first reviewer re-tested their own findings against it. Between them they found three real problems, all in the parts we added rather than in how the library talks to devices. This fixes them.
The main one is a cousin of the 1.0.1 bug. When the library resends a request after a second of silence and the device answers both copies, the second answer could still be mistaken for the reply to whatever came next. The other two: a re-authentication could let a queued request go out with a blank session, and a capture window could be taken away from a program that was still using it. The bench numbers for the timing fix are now in the README, and the code is formatted with the project's own tool, checked in CI.
Tested on Python 3.13 and 3.14, 257 tests, plus a live run against an RM4 Pro.
Coding assistant's notes on this change: