Reliability patches (fork): liveness, write retries, RSSI decay, transitions - #1
Merged
Conversation
PlejdMesh.connected returned True whenever a client handle existed, even after the underlying BLE link had silently dropped without the disconnect callback firing. This "stale-link blindness" makes the mesh look connected while every write is quietly discarded (matches hass_plejd #162/#125/#147). Require self._client.is_connected in addition to the handle being set, so connect() will re-establish a dead link and write()/poll() see reality. disconnect() now guards on the raw handle (not self.connected) so a stale client is still torn down and cleared instead of leaked. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
write()/_write() previously returned silently when disconnected and swallowed BleakError into a log line, so a dropped command looked like a success to callers. For an integration that issues frequent small dim adjustments this means state silently diverges from reality. Now write() delegates to a single quick retry: attempt the write; on any failure reconnect the mesh and rewrite exactly once; if that also fails raise the new typed PlejdWriteError. Payloads are kept as raw hex until the moment of writing (_write_once) so the retry re-encrypts against the possibly-new gateway address. Best-effort maintenance callers (poll_buttons, poll_time, broadcast_time) catch and log PlejdWriteError; command callers (light turn_on/turn_off) let it propagate. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
MeshDevice.see() only stored the BLEDevice on the first sighting and kept RSSI as an all-time maximum. As a result gateway selection connected through whichever proxy first advertised a node and never reacted to a node moving, weakening, or a better proxy appearing. Now every advertisement refreshes the BLEDevice handle, and RSSI is decayed exponentially toward -100 dBm with a 5-minute half-life via current_rssi(). connect() ranks candidates by the decayed value so a node that has gone quiet loses priority instead of holding a stale historical-max forever. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The connect path unconditionally did connect->disconnect->sleep(5)->connect before authenticating (a firmware-throttling workaround), holding the write lock for 5-7s on every reconnect and, per pyplejd#23 / hass_plejd#162, actually breaking command delivery to non-gateway devices on some setups. Add a connect_workaround flag (PlejdManager/PlejdMesh, default False). When off we authenticate on the first direct connection and only fall back to the disconnect/reconnect workaround if that first authentication fails; when on we always run the workaround. This mirrors the approach in upstream pyplejd PR thomasloven#23, which we align with. Divergences from thomasloven#23: it is gated behind an explicit flag (so setups that still need the workaround can force it), and the workaround remains reachable automatically via the auth-failure fallback rather than being removed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The Plejd firmware's native dim-speed cannot be driven over the reverse- engineered protocol, so smooth fades must be produced in software. Add a `transition` (seconds) parameter to PlejdLight.turn_on: it starts a cancellable per-device ramp task that steps brightness from the last known level to the target at ~150 ms intervals, each step a normal dim write. The latest command always cancels a previous ramp (_cancel_ramp), so rapid successive adjustments don't stack, and turn_off cancels any active fade. Step generation is factored into the pure, unit-testable ramp_steps() which always finishes exactly on target and collapses redundant duplicate levels. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Room/group mesh addresses are already parsed from cloud data (roomAddress)
but no API exposed them. Add PlejdCloudSite.groups and PlejdManager.groups
yielding {roomId, title, address}, plus an experimental PlejdManager
dim_group(address, dim) that sends a single group output command to a room
address - the same command used for an individual output, addressed to the
group - so an entire room can be actuated with one write instead of one
write per member.
Marked experimental and untested against live hardware.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Add unit tests covering the pure-logic parts touched by the reliability work, plus a GitHub Actions workflow (Python 3.11/3.12) to run them: - test_rssi_decay.py: RSSI exponential decay and BLEDevice handle refresh - test_ramp.py: ramp_steps generation, dedup, and ramp task cancellation - test_write_retry.py: connected-liveness plus write reconnect-retry using a fake Bleak client - test_group.py: experimental group dim command construction Also folds in a small correctness fix in ramp_steps surfaced by the tests: the target level could be appended twice (a redundant duplicate final write); it is now emitted exactly once. No live BLE hardware is exercised - all mesh I/O is faked. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Distinguishes the leiklier reliability fork from upstream 0.21.3. The git tag v0.22.0-fork.1 pins this commit for the hass_plejd fork manifest. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Review round 1 fixes to the connection path: C1 (CRITICAL): remove the call to BleakClient.set_disconnected_callback, which does not exist on bleak 3.x (removed ~0.19; verified absent on bleak 3.0.2) and made every connect() raise AttributeError. The disconnect callback is now passed to establish_connection at connection time, as bleak requires. Also fixes MeshDevice.update() which was defined without `self` and crashed on every successful gateway selection - previously unreached by tests, now exercised by the new regression test. That test drives connect() against a spec'd fake client (not a MagicMock) that has no set_disconnected_callback attribute, so the phantom-API bug cannot regress. Audited all other bleak client calls (write/read_gatt_char, start/ stop_notify, disconnect, is_connected) - all present in bleak 3.x. P2 (HIGH): single-flight connect. A burst of failed writes each called connect() concurrently, causing an establish_connection storm. connect() now serialises through a dedicated lock and re-checks connectivity, so concurrent callers await one attempt. Covered by a two/three-way concurrent connect test. Minor: RSSI decay now uses time.monotonic() instead of datetime.now() so an NTP step cannot over-/under-decay stored RSSI. last_seen remains a wall-clock datetime for the user-facing sensor. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Review round 1 fixes to software transitions: C2 (HIGH): the fade previously ran entirely in a detached ensure_future task, which swallowed PlejdWriteError (so the error-surfacing feature never fired for fades) and produced "Task exception was never retrieved" noise. Now the colour-temperature write and the first brightness step are awaited synchronously in turn_on, so an immediate failure raises to the caller (and is surfaced by Home Assistant). The remaining steps run in the task, whose body catches PlejdWriteError, logs a single warning with device context, and stops - no exception ever escapes the task. C3 (LOW): _cancel_ramp is now async and awaits the cancelled task (suppressing CancelledError) before a new ramp starts, so brief write overlap between an old and new fade is eliminated. Minor: turn_off now accepts `transition` and fades brightness to zero before sending the explicit off command, honouring the declared TRANSITION feature. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The colour-temperature encoder relies on int.to_bytes with a default byteorder (added in 3.11), and the CI matrix already targets 3.11/3.12. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The repo's .gitignore uses a whitelist (/* ignores everything, then un-ignores select paths), so tests/, pytest.ini, tests/requirements.txt and .github/ were silently excluded and an earlier commit only carried a source fix. Whitelist those paths and add the full suite: - tests/test_rssi_decay.py, test_ramp.py, test_write_retry.py, test_group.py (original patches) plus test_connect.py (review round 1: C1 spec'd-fake connect regression + P2 single-flight connect) - pytest.ini, tests/requirements.txt - .github/workflows/test.yml (Python 3.11/3.12) 31 tests, all passing. No live BLE hardware is exercised. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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.
Reliability-focused patches to the Plejd BLE library, motivated by an
integration that issues frequent small brightness adjustments. Each patch is
a self-contained commit intended to be upstreamable individually after a live
soak.
Patches
PlejdMesh.connectednow requiresself._client.is_connected, not just a non-None handle, so a silently-dropped link no longer looks connected (hass_plejd #162/#125/#147).disconnect()guards on the raw handle so a stale client is still cleaned up.write()now does one quick reconnect-and-retry and raises a typedPlejdWriteErroron final failure instead of silently dropping the command. Payloads stay raw hex until write time so a retry re-encrypts against the (possibly new) gateway. Best-effort maintenance writers swallow the error.MeshDevice.see()refreshes the handle every advertisement and decays stored RSSI toward -100 dBm with a 5-minute half-life;connect()ranks candidates by the decayed value.connect→disconnect→sleep(5)→connectfirmware workaround is now behind aconnect_workaroundflag (default off), with automatic fallback to it if a direct authentication fails. Aligns with PR Try direct BLE authentication before double-connect fallback thomasloven/pyplejd#23; divergences: gated behind an explicit flag and reachable automatically via the auth-failure fallback rather than removed.PlejdLight.turn_on(..., transition=...)runs a cancellable per-device brightness ramp (~150 ms steps), latest-command-cancels-previous. Step generation is the pure, testedramp_steps().PlejdManager.groups/dim_group(address, dim)send a single group command to a room mesh address. Marked experimental.Tests
Adds a pytest suite (25 tests) + a GitHub Actions workflow covering RSSI decay, ramp generation/cancellation, connected-liveness, the write reconnect-retry (fake Bleak client), and group command construction. All mesh I/O is faked.
Not tested
No live BLE hardware was available, so none of these paths were exercised against a real Plejd mesh. They are covered by unit tests only and need a live soak before upstreaming.
🤖 Generated with Claude Code
Review round 1 fixes
Adversarial review found blocking defects; fixed on this branch:
BleakClient.set_disconnected_callbackdoes not exist on bleak 3.x (verified absent on 3.0.2), so everyconnect()raisedAttributeErrorand the integration could never connect. Removed the call; the disconnect callback is now passed toestablish_connectionat connection time. Addedtests/test_connect.py::test_connect_uses_only_available_bleak_api, which drivesconnect()against a deliberately spec'd fake client (not a MagicMock, which would auto-create the attribute) that has noset_disconnected_callback. Audited every other client call (write/read_gatt_char,start/stop_notify,disconnect,is_connected) — all present in bleak 3.x. Fix commit:ble: fix connect for bleak 3.x, single-flight connect, monotonic RSSI. That commit also fixesMeshDevice.update(), which was defined withoutselfand crashed on every successful gateway selection (previously unreached by tests).connect(), causing anestablish_connectionstorm.connect()is now single-flight through a dedicated lock and re-checks connectivity, so concurrent callers await one attempt. Covered bytest_connect_is_single_flight. Same commit as C1.ensure_futuretask that swallowedPlejdWriteError(so the error-surfacing feature never fired for fades) and produced "Task exception was never retrieved" noise. Now the colour-temperature write and the first brightness step are awaited synchronously inturn_on, so an immediate failure raises to the caller; the remaining steps run in a task whose body catchesPlejdWriteError, logs a single warning with device context, and stops. No exception escapes the task. Tests:test_first_step_written_synchronously_and_failure_raises,test_mid_ramp_failure_is_contained. Fix commit:light: contain ramp-task errors, await cancellation, fade to off._cancel_rampis now async and awaits the cancelled task (suppressingCancelledError) before a new ramp starts, eliminating brief write overlap between an old and new fade. Same commit as C2.time.monotonic()instead ofdatetime.now()(NTP-safe);last_seenstays a wall-clock datetime for the last-seen sensor.turn_offacceptstransitionand fades to off.MIN_PY_VERSIONbumped to 3.11 to match the CI matrix and the default-byteorderint.to_bytesusage.pytest.iniand CI workflow were silently excluded by the repo's deny-by-default.gitignorein an earlier commit (only a source fix landed). Whitelistedtests/,pytest.iniand.github/and committed the full suite.Test count: 31 passing (was 25). Tag
v0.22.0-fork.2points at the fixed HEAD;v0.22.0-fork.1was left untouched (never reuse a pushed tag).Deviation (explicit):
setup.pykeeps the PEP440-valid0.22.0+fork.1rather than the reviewer-suggested0.22.0-fork.1, because the latter is not a valid PEP440 version and would breakpip install -e .in CI. The integration manifest version remains0.22.0-fork.1. Still no live BLE hardware testing.