Fix the issues found in the review of 1.0.2 - #843
Closed
DAB-LABS wants to merge 8 commits into
Closed
Conversation
Move packaging to pyproject.toml as the python-broadlink distribution (import name unchanged), require Python 3.13 or newer, and replace the flake8 workflow with ruff and pytest on 3.13 and 3.14 plus an sdist and wheel build. Add a trusted-publishing workflow for version tags. Add tests/oracle: a harness that records the exact request bytes every public method of every device class sends and the result it decodes from canned responses, frozen in fixtures.json (155 cases), plus transport tests for send_packet framing, checksums, auth, discovery, gendevice and setup, and tests for the pure helpers. No library behavior changes; three import blocks were reordered for ruff. Add a README note explaining the fork and a CHANGELOG.
Every method that reaches a device is now a coroutine, with the same names, arguments and return values as before. Discovery, hello and setup are coroutines and xdiscover is an async generator. The packet, CRC and datetime helpers stay synchronous. There is no synchronous compatibility layer. Transport: each device keeps one UDP endpoint (asyncio DatagramProtocol) for its lifetime and serializes requests on it with an asyncio.Lock; the previous code opened a socket per call and declared a lock it never acquired. Retry and timeout behaviour is unchanged. An expired session key is re-authenticated once and the request repeated. async with / aclose() release the endpoint. Device classes are a mechanical port (async def and await); the oracle suite recorded in the previous change passes unchanged, so every method sends the same bytes and decodes the same results as 0.19.0. Transport tests use a fake endpoint and gain cases for lock serialization, endpoint reuse, stale-reply draining and re-auth. The CLI runs under asyncio.run. README and CHANGELOG describe the break. Live-checked against an RM4 Pro: discovery, hello, auth, sensors, concurrent calls, learning primitives, send, and the timeout path.
* Add support for Broadlink RM Max (0xAF8B) Place in the rmpro class which uses <I framing for IR learn/send commands, matching the device's actual protocol. The rm4pro class uses <HI framing which causes "device is locked" errors on this hardware. Co-authored-by: Cursor <cursoragent@cursor.com> * Update __init__.py * Update remote.py * Add an OEM device code for RM mini 3 * support RM mini 3 cmcc version support RM mini 3 cmcc version * Added support for another type of LB26 R1 * Update __init__.py add 0x7d15 SP mini 3-AL * Add support for LEDVANCE SMART+ WIFI CEILING TW 24W (0x6498) * Refactor tick parameter to use constant value * Add tests for TICK constant in remote module Add unit tests for the TICK constant in remote module to ensure accuracy against protocol.md examples and validate behavior with real hardware. * Round pulses to the nearest tick, add issue-reported device IDs, update tests and changelog Follow-ups to the carried-over commits: - pulses_to_data rounds instead of truncating, so a duration that is 0.9 of a tick no longer becomes zero ticks. - TICK gets a docstring explaining the 32768 Hz timebase and the history of the 32.84 value; the import block is sorted for ruff. - 0x4EDA MP1-1K3S2U (mjg59#816) and 0xA57A SP4 (mjg59#758) added from issues, by family; 0x7D15 and 0x27C8 entries tidied into hex order and house style. - cryptography floor raised to 43, the first release with 3.13 wheels (supersedes mjg59#749). - tests/test_helpers.py re-pinned to the new tick and rounding; the old 32.84 pins are gone. - README device list and CHANGELOG updated. --------- Co-authored-by: Alexey Masolov <amasolov@redhat.com> Co-authored-by: Cursor <cursoragent@cursor.com> Co-authored-by: Anil Daoud <anil.daoud@protonmail.com> Co-authored-by: Bartłomiej Nogaś <bart.nogas@gmail.com> Co-authored-by: shuxin <shuxin@users.noreply.github.com> Co-authored-by: techitapart <70172453+techitapart@users.noreply.github.com> Co-authored-by: bbcbbk <44605459+bbcbbk@users.noreply.github.com> Co-authored-by: Felipe Martins Diel <felipe-diel@hotmail.com>
A learning session is arm, poll, wait, re-arm, one code at a time, with a device that leaves learning mode without saying so and a send that ends the session. capture() and capture_rf() own that loop and hand back clean signals, so a consumer subscribes and reads instead of reimplementing the dance (as the remote platform, the receiver PR and others each did). - capture(window, stop_after_first, poll_interval, rearm_interval): async generator yielding CapturedSignal. Re-arms on a timer (default 15 s, under the 25-40 s the device was measured to hold a session) and after any send_data (a send ends the session; both from the bench). One window per device; a second raises CaptureInProgressError. - capture_rf(window, frequency, ...) on the Pro classes: takes the carrier directly, or sweeps for it when not given. The sweep is unreliable on some firmware, so the known-frequency path is primary. - CapturedSignal: device packet, pulses at the corrected tick, kind, repeat, and the RF carrier the packet does not itself record. - pulses_to_data gains kind and repeat; parse_packet is the inverse; SignalKind names the bands. A returned RF packet does not always carry the canonical type byte (an RM4 Pro sends 0xB1 for 433 MHz), so kind is read by band and a capture is tagged from what it armed, never dropped on the byte. - One shared front-end lock and a transmit generation counter already live on the device; capture reads the counter so a concurrent send re-arms the window. Tests drive the loops against a scripted device that models the bench findings; the transport oracle fixtures are unchanged.
- broadlink_cli --learn and --rflearn learn through capture() and capture_rf(), so a session no longer goes deaf when the device times out partway through the 30 s wait. New --window (seconds to listen), --keep (print every code heard) and --repeat (with --send --durations). - cli/README.md: --rfscanlearn was a typo for --rflearn (mjg59#803, mjg59#830); install line is pip install python-broadlink; examples for --frequency, --window, --keep and --repeat. - README: four device calls in the switch examples were missing await; the RF check_frequency example now unpacks the (found, frequency) tuple. - CHANGELOG: 1.0.0 heading, the CLI changes, and the pulses_to_data bytes-instead-of-bytearray note. - tests/test_capture.py timings expressed as multiples of a 30 ms unit (was 10 ms) for headroom on slower CI runners. - Version 1.0.0.
I had 1.0.0 reviewed by an outside party before asking Home Assistant to switch to it. The review confirmed the library talks to devices exactly the way the original did, and found one real bug plus a few small things. This fixes them. No change to the wire format or the public API. Tested on Python 3.13 and 3.14 (252 tests) and live against an RM4 Pro. Technical details: - Replies are matched to their request by the packet counter the device echoes at offset 0x28. A reply for a request that already timed out is dropped; a reply whose counter matches nothing sent is still accepted, so firmware that does not echo the counter keeps working. - Concurrent callers hitting an expired session key share one re-authentication. The logged-out code (-2) now triggers re-auth. - Changing device.host reopens the socket. - aclose() during a request fails it immediately. - capture() treats ReadError (-10) as "nothing yet" alongside StorageError (-5); the CLI inherits this. - An abandoned capture() generator no longer blocks the next one; new capture_active property. - README: broadlink and python-broadlink cannot share an environment. - Changelog: carried-over device commits were squash-merged with Co-authored-by credit; oracle description corrected. - Version 1.0.1.
* Fix the issues found in the second review of 1.0.1 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. * Format with ruff and widen the lint rules 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.
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.
Author
|
Opened against the wrong repository by mistake; this work lives in the DAB-LABS fork. |
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
1 participant
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: