feat: Z-Wave JS UI (MQTT) lock provider - #1466
Merged
Merged
Conversation
Entire-Checkpoint: eb367b3b1060
Entire-Checkpoint: f2adf83d22c2
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Entire-Checkpoint: c4b6ca3bd017
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…S rename Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Entire-Checkpoint: d8eecebeaa5b
Give the zwave-js-ui provider its addressing: the device registry identifier yields (home id, node id), and the lock entity's MQTT discovery state_topic yields the gateway prefix and the node topic. Door Lock state values carry no propertyKey, so the state topic always ends with exactly cc/endpoint/property; anything shorter is a MANUAL-gateway custom topic and stays unresolvable rather than guessed. There is no command_topic fallback because the command topic addresses targetMode, not currentMode. Adds the provider test package's conftest -- discovery payload builder, discovery helper, and an LCM-config-entry fixture -- that the remaining zwave-js-ui tasks build on. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Adds pure MQTT value-payload unwrapping (raw/{time,value}/full-valueId
shapes) and User Code CC get-result projection to SlotCredential, mirroring
the reasoning already used by the zigbee2mqtt provider so unreadable codes
never get misread as empty and reprogrammed forever.
Resolve the gateway api base by listening for retained ZWAVE_GATEWAY-* client statuses under the lock's own topic prefix, disambiguating several gateways on one broker by asking each for its home id. A primary and a secondary controller reporting the same home id fail loud rather than tiebreak, so nothing is ever programmed through a controller the user did not ask about. The api client correlates responses by adding a nonce to the request payload, which zwave-js-ui echoes verbatim under the response's origin -- the api response topic is shared with the zwave-js-ui UI and any other automation, so an uncorrelated read would cross wires with them. Also tighten the value-topic threshold: a gateway-built topic needs five segments (prefix, node, and the three value segments), so a four-segment topic is a MANUAL custom topic and no longer yields a guessed prefix. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ient Home Assistant registers an MQTT subscription locally on the spot but defers the wire SUBSCRIBE behind a ~0.1s debouncer, while a publish goes out immediately. Subscribing per api call and publishing in the same breath therefore lost every zwave-js-ui response that arrived before the broker had been told to route the topic -- and those responses are unretained, so they were gone for good. Tests could not see it: the mocked broker dispatches locally and synchronously. Establish one persistent <prefix>/_CLIENTS/+/api/+ subscription at the start of gateway resolution, before the discovery window, which is long enough to outlast the debounce. Calls now register a nonce in a pending-call registry that the single response handler resolves, and refuse outright if the subscription is not live rather than subscribing and racing. Also: route subscribe refusals to LockDisconnected so a reloading MQTT integration does not escape as a bare HomeAssistantError; drop the cached gateway base on a disconnect so a renamed or replaced gateway is rediscovered instead of sticking until reload; and reject a non-integer homeid during disambiguation, since True == 1 would let a JSON boolean match a home id of 1. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…scribe ordering The api response subscription was registered in the base class's push-unsub bucket, which is released only when supports_push is true. This provider inherits supports_push=False, so async_unload skipped it and every reload orphaned a wildcard subscription holding a dead provider. The base's own _register_push_unsub docstring says listeners with a different lifecycle must be tracked separately, and this one is the api transport, alive for as long as the provider is. Move it to a dedicated field released by an idempotent helper, called from both teardown_push_subscription (keeping the reconnect re-resolve) and an async_unload override, mirroring zwave_js.py's handling of its own provider-lifetime listeners. Also pin the ordering the previous commit relied on: a test now asserts the response subscription is already live while the discovery window is open, so moving the subscribe block after the window fails loudly instead of silently reopening the race it was written to close. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Reads, writes, clears, and the advertised User Code capacity, all built on the correlated api client: one sendCommand round trip per operation on the node's User Code Command Class. The three connectivity guards every public operation shares are one helper rather than the inline repetition Zigbee2MQTT carries. A read that the api refuses becomes unreadable rather than empty, so a transient refusal cannot tell sync a slot was cleared; a disconnect propagates instead, because only reaching the caller runs the reconnect path. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Subscribe to the node's whole value tree and classify User Code CC value publications and Access Control keypad notifications out of it, in both the VALUEID and NAMED topic spellings the gateway can be configured for. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The capability probe is api traffic like any other, so a disabled MQTT integration was surfacing as a call timeout rather than a disconnect. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
zwave_js_ui's _resolve_state_topic was a near-verbatim copy of zigbee2mqtt's _resolve_device_topic, and async_is_device_available was byte-identical between the two. Both move to providers/_util.py as resolve_discovery_payload() and entity_state_is_available(). The walk stops at the payload; the topic-field extraction stays in each provider, because that is where the two genuinely disagree -- zigbee2mqtt falls back to command_topic minus /set, which names the same device, while zwave-js-ui's command topic addresses targetMode rather than currentMode and must never be stripped into a state topic. Each provider now carries a comment saying so. async_is_device_available goes to _util.py rather than becoming the BaseLock default: the base default is `return True`, and seam relies on it (tests/providers/test_seam.py documents that choice explicitly), so changing the default would silently alter every provider that doesn't override. A free function called by the two providers that want this behavior has no blast radius outside them. Also corrects stale prose in both providers: usercode_scan_interval is inert while supports_push is true (coordinator.py sets update_interval to None and nothing else reads the property), so the recurring visit that re-ensures a drifted push subscription is the hourly hard refresh, not a 5-minute poll. The property stays -- it is the cadence the coordinator would use if push were ever unsupported or disabled. No behavior change. Both providers' full test directories pass unchanged. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
test_unusable_discovery_data_resolves_nothing took an `entities: object` param that meant "the whole entities list" for one case and "the payload value" for the other four, leaving a reader to infer which from the runtime type. Split into an entity-absent test and a payload test parametrized on `discovery_data_override`, so each case says what it is. Adds the combined-errors options-flow test: one submission carrying both an unclaimed mqtt lock and invalid users data must render both errors together. The accumulation contract (different keys, errors.update twice rather than a short-circuit) was only verifiable by reading the flow. Verified by mutation -- short-circuiting after the lock check fails the new test and nothing else. Also corrects test_push's prose about which recurring visit re-ensures a drifted subscription, matching the provider comment it describes. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Drop the leading underscores from the three module constants that had them; nothing else in the file is spelled that way. Keep the ``_published_code`` docstring on one word for the thing it returns, and hoist the "already subscribed to the right topic" predicate the two subscribe paths were each spelling out. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The one-time unmanaged-code sweep builds a throwaway provider per lock and reads it. An MQTT provider subscribes to its lock's topics on the way to answering that read, and nothing ever tore the throwaway down -- so after the migration that instance went on firing code slot events next to the entry's real provider, doubling every keypad event until Home Assistant restarted. Found by the zwave-js-ui end-to-end test, which saw one keypad unlock arrive twice. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Drive a full LCM config entry over a discovered zwave-js-ui lock: the gateway resolves through the real retained-status path, the configured codes are programmed and read back through a stand-in User Code table, and pushes, keypad events, disabling a slot, and unload all go through the same seams a user's setup would. The api-driven design lets these assert the exact sendCommand envelope each operation arrives as, which the Zigbee2MQTT equivalent cannot. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
github-actions
Bot
added
python
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1466 +/- ##
==========================================
+ Coverage 99.07% 99.13% +0.05%
==========================================
Files 62 64 +2
Lines 7714 8222 +508
Branches 520 520
==========================================
+ Hits 7643 8151 +508
Misses 71 71
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
A push provider's coordinator is constructed with no update interval, on the premise that the device tells us when something changes. That premise does not cover the first load: the coordinator still has to poll once to seed itself, and if that poll raises, `async_get_usercodes` turns it into `UpdateFailed` and nothing ever schedules another one. The recovery probe that `_apply_backoff` already has cannot help -- it only starts once the breaker trips, and tripping it takes repeated polls that, with no timer, never happen. The lock's entities stay unavailable until the integration is reloaded, and the reload re-runs the same first load into the same wall. A lock that is merely asleep at startup hits this routinely: FLiRS battery locks answer on their own wake schedule, and Home Assistant's own startup is exactly when everything else is competing for the mesh. zigbee2mqtt is affected identically. So while `_original_update_interval` is None and no refresh has ever succeeded, a failure schedules one at the base backoff cadence. It is an `elif` on the breaker's arm so the escalating backoff wins the moment the breaker trips, and `_reset_backoff` restores the push cadence on the first success. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
A FLiRS (battery) lock answers on its own wake schedule, so a single command can legitimately take the better part of a minute. Abandoning it at ten seconds does not cancel it -- zwave-js-ui keeps working the queued command, and LCM's retry stacks a duplicate behind it. On live Kwikset FLiRS meshes that was already fatal at fifteen seconds: the gateway's queue filled faster than the lock could drain it and stopped answering anything. So the mesh budget goes to 60s. Widening it uniformly would be a regression for gateway discovery, where each candidate is asked getInfo and a dead client would now cost a minute apiece. getInfo is assembled from zwave-js-ui's own cached driver state and puts nothing on the mesh (ZwaveClient.getInfo), so it gets its own five-second budget, threaded in as an optional argument on `_async_api_call_at`. `getUsersCount` does NOT qualify despite the name: it is a User Code Command Class UsersNumberGet addressed to the node (node-zwave-js UserCodeCCAPI.getUsersCount), so it waits on the same wake schedule as a code read and keeps the mesh budget. Also widens this provider's inter-operation pacing to 5s. Every api call lands on zwave-js-ui's single command queue, which it shares with its own UI and every other MQTT client; pacing our own operations wider costs nothing on a healthy mesh and keeps LCM from being the client that fills it. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The status collector recorded any `_CLIENTS/ZWAVE_GATEWAY-*` topic segment
it saw, without reading the payload. zwave-js-ui publishes that topic
retained (`MqttClient.updateClientStatus`) AND registers a retained
`{"value": false}` on it as its last will (`MqttClient._init`), so a
gateway that crashed or was decommissioned leaves a status behind forever.
Being the only status on the prefix is the worst case, not the safest one:
the single-candidate path binds it outright, with no getInfo to expose
that nobody is home, so every api call times out, drops the cached base,
and rediscovers the same corpse.
The payload is the ordinary value shape, so `_unwrap_mqtt_value` handles
the wrapped and raw forms alike, and a truthiness test is all that is
needed. This is the one place in the module that does not also reject a
boolean: here `True` is the online signal itself rather than a JSON type
confusion, and the comment says so.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
A lock configured to withhold its user codes answers a read with one
asterisk per digit instead of the digits. Both paths that turn a published
`userCode` into a credential took that at face value: the api projection
made it `known("****")` and the push classifier confirmed the slot with
it.
That is not a wrong value, it is a value that can never be right. Sync
compares the coordinator's credential against the configured Personal
Identification Number, so a masked code is a mismatch on every tick
forever -- the slot is reprogrammed, the lock answers with the mask again,
and the loop never converges.
Withheld belongs in the state that already means "occupied, contents
unknown": unreadable from the api projection, and nothing at all from the
push classifier. Only an all-asterisk code counts; asterisks mixed with
digits are not a shape any lock produces, and guessing at partial masking
would discard a code that is really there.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
zwave-js-ui builds its Home Assistant device identifier as `UID_DISCOVERY_PREFIX + homeHex + '_node' + node.id`, and that prefix is `process.env.UID_DISCOVERY_PREFIX` -- `zwavejs2mqtt_` is only its default (Gateway.ts:36). Anchoring the pattern on the default rejected every lock behind a gateway whose operator had renamed it, and rejected it at the config flow: an identifier nobody claims resolves to no provider at all, so the lock could not be added, with nothing on screen to explain why. The head is now unconstrained and the tail carries the whole burden of not over-claiming -- a hex home id, `_node`, digits, end of string -- so `somebridge_1`, a bare `_node5`, and an identifier with anything trailing the node id all still fail to match. That makes the dispatch order in `resolve_provider_class` load-bearing rather than incidental: Zigbee2MQTT's prefix is fixed while ours is only a tail, so a Zigbee2MQTT address ending in a zwave-js-ui-shaped tail now matches both rules and only the ordering keeps it with the right provider. Commented, and pinned by a test that fails if the two checks are swapped. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ng as data A poll where the lock refused every single slot returned a list of unreadable credentials, which the coordinator cannot tell from a poll that succeeded: it resets the connectivity breaker, un-suspends every slot, and lets them all re-fail on the next tick. That is a seconds-scale oscillation flipping every managed slot in and out of sync. The coordinator already guards the shape where the read *raises* (async_get_usercodes), but it has nothing to guard this one with. The distinction that matters is not "did the credential come back readable" -- a lock configured to withhold codes answers every request and is perfectly healthy, and so is a slot whose status is Disabled. It is "did the lock answer at all". So `_async_read_slot` now returns None for an operation-level refusal and a credential for anything the lock actually described; both still reach the coordinator as unreadable, but only the refusals decide whether the read was worth anything. All refusals raises LockDisconnected and leaves the backoff timer in charge of the next attempt. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…opic zwave-js-ui writes its own client status topic into the `availability` list of every entity it discovers (Gateway.ts `setDiscoveryAvailability`), so the payload a lock arrives on already says which gateway owns it. That makes it the authoritative binding, and it is now the primary path: no traffic, no discovery window, and -- unlike the retained-status scan -- it is per lock, so two gateways sharing a topic prefix resolve correctly instead of forcing a getInfo interrogation of each (or an outright refusal when both answer for the same home id). The list is scanned for a topic of that shape rather than indexed; the other two entries are the node's status and the driver's, and their order is the gateway's business. The prefix comes from that topic too, deliberately not from the state topic: a MANUAL gateway publishes wherever the user pointed it, so the state topic's first segment is a naming choice rather than a prefix. The scan stays as the fallback for a payload carrying no such entry. Moving the api response subscription: Establishing it inside `_async_resolve_api_base` only ever worked because resolution held a multi-second discovery window, which happened to outlast Home Assistant's MQTT subscribe debouncer. The fast path has no window, so that placement would race again -- a publish goes out immediately, the wire SUBSCRIBE does not, and zwave-js-ui sends api responses unretained. It now happens in `async_setup`, seconds before the coordinator's first poll. Resolution refuses to proceed without a live subscription, arming one for the next attempt so the coordinator's backoff carries the retry. Tests: the discovery payload builder now emits the real three-entry availability list (and `async_discover_zui_lock` replays the availability burst, without which `availability_mode: all` leaves the entity unavailable), so the whole suite exercises the path production takes. The scan tests move to a `zui_scan_lock_provider` fixture built from a payload with no availability entry, which is the shape that fallback is for. The e2e suite no longer needs its retained-status replay fixture at all. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
A lock whose discovery state topic is a MANUAL gateway's custom shape gives up no node address, so it cannot be subscribed to. It was refused outright -- `async_is_integration_connected` required the node topic -- which cost the whole lock to save the half of it that does not work. With the gateway now bound from the availability topic, every api operation on such a lock works: reads, writes, capacity. Worse, `supports_push` was a constant True. Had connectivity simply been loosened, the coordinator would have dropped its update interval for a lock that has no push either, leaving it with no data path at all. So it is derived: true exactly when the node topic resolves, and `supports_code_slot_events` follows it (keypad events are Notification Command Class publications on the same subscription). The coordinator reads it once at construction, which happens after `async_setup`, and the answer comes from Home Assistant's MQTT discovery data -- already loaded. - Connectivity is now "MQTT is up, the identifier parses, and a gateway prefix resolves". The identifier requirement stays: something has to prove this is a zwave-js-ui device, and nothing is inferred from a name. - `async_setup` skips the node subscription instead of raising, at info per the log-levels convention -- a MANUAL gateway is a configuration, not a fault. - `async_get_users` skips the drift re-ensure for an api-only lock rather than logging a failure it can do nothing about on every poll. - `async_unload` is overridden to release both subscriptions. The base only tears the push subscription down when `supports_push` says so, and an api-only lock says no while its api transport is live all the same. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
A lock configured to withhold its user codes answers a read with one
asterisk per digit instead of the digits. Zigbee2MQTT publishes that
verbatim, and both paths that turn a published code into a credential took
it at face value: the `users` projection made it `known("****")`, and so did
the `pin_code` response that resolves a pending slot read.
That is not a wrong value, it is a value that can never be right. Sync
compares the coordinator's credential against the configured Personal
Identification Number, so a masked code is a mismatch on every tick forever
-- the slot is reprogrammed, the lock answers with the mask again, and the
loop never converges.
Withheld belongs in the state that already means "occupied, contents
unknown", which is exactly where this provider already puts an `enabled`
user whose `pin_code` field `expose_pin` hides. Masking only decides
anything where the code would otherwise be known: a user the lock is not
accepting is already incomparable, so a disabled masked user lands in the
same place it did before.
The asterisk test is the one the zwave_js_ui provider already applies, so it
moves to providers/_util.py rather than being written twice. Only an
all-asterisk code counts; asterisks mixed with digits are not a shape any
lock produces, and guessing at partial masking would discard a code that is
really there.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…asquerading as data A poll where not one GET reached the lock returned a list of unreadable credentials, which the coordinator cannot tell from a poll that succeeded: it resets the connectivity breaker, un-suspends every slot, and lets them all re-fail on the next tick. That is a seconds-scale oscillation flipping every managed slot in and out of sync. The coordinator already guards the shape where the read *raises* (async_get_usercodes), but it has nothing to guard this one with. A dead broker reaches this read: every gate above it -- the MQTT config entry being enabled, the topic resolving from discovery data, the lock entity's state -- answers from Home Assistant's own configuration rather than from the wire, so none of them notices that nothing is being carried. Then every slot fails on publish or times out waiting, and the result reads as a healthy lock full of unreadable slots. The distinction that matters is not "did the credential come back readable" -- a lock configured to withhold codes answers every request and is perfectly healthy, and so is a user the lock reports as disabled. It is "did anything come back". So `_async_read_slot` now returns None when the request never left or nothing answered it, and a credential for anything the bridge actually described; both still reach the coordinator as unreadable, but only the silences decide whether the read was worth anything. All silent raises LockDisconnected and leaves the backoff timer in charge of the next attempt. One silent slot among answered ones is unchanged: it stays unreadable, on purpose. This provider asks one index per round trip, and a lock that drops one request out of several is a weak link, not a lost transport. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… prefix `_async_ensure_api_response_subscription` was idempotent on "is one set" rather than on "does it cover the right topic", which `_node_subscription_current` has been drift-aware about all along. An operator who changes zwave-js-ui's MQTT prefix republishes discovery, so the new prefix resolves and every publish goes there -- while the subscription stays on the old one. The response is published where nothing is listening, the call times out, the timeout drops `_api_base`, resolution re-derives the same new prefix, and the guard passes again because a topic is set. The loop sustains itself. A push lock eventually falls out of it when a connection transition tears the subscription down. An api-only lock -- a MANUAL gateway whose custom state topic carries no node address -- never does: that teardown is gated on `supports_push`, which such a lock derives as False. It stays stranded until the integration is reloaded, and the reload re-resolves into the same wall. So the guard now compares against the topic the prefix implies and releases before resubscribing, and `_async_resolve_api_base` ensures the subscription rather than merely checking it -- that resolve is the only place a lock with no push lifecycle passes through often enough to notice the prefix moved. Refusing the attempt that resubscribes is the same debounce-settling rule a first subscription already obeys: Home Assistant defers the wire SUBSCRIBE while a publish goes out immediately, and zwave-js-ui sends api responses unretained. A prefix that will not resolve right now still leaves a live subscription alone. Transiently missing discovery data is not a prefix that moved, and tearing down over it would drop traffic for a lock that never changed address. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…lock Push is derived per lock from whether its discovery state topic yields a node address, so a single config entry can hold a structured-topic lock and a MANUAL-gateway lock at once. Every consequence of that was reachable but untested: the two locks take different subscription paths, share one api response wildcard on the same prefix, run their coordinators at different cadences, and are released by one unload. The correlation case is the one that needed care. Both locks subscribe to `<prefix>/_CLIENTS/+/api/+`, so every response reaches both handlers and the echoed nonce is the only thing telling them apart -- but a stand-in gateway that answers each request before the next goes out never puts two nonces in play, and a test written that way passes with the nonce ignored entirely. So the stand-in holds both requests until both calls are waiting, then answers them in the opposite order, and each node reports its own id as its capacity so a crossed wire is a wrong number rather than a silent pass. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Zigbee2MQTT and zwave-js-ui speak different protocols to different radios, but they reach them the same way: through a bridge Home Assistant's MQTT integration already talks to, at topics the bridge published in its own discovery payload. Everything that hardening has added lately has therefore had to be added twice, in parallel -- most recently the all-transport-failed read, which landed as two implementations of one invariant a week apart. Two copies of a rule is one copy plus a place for it to drift. `providers/_mqtt.py` holds the three decisions that are about the transport rather than the protocol: * `_async_ensure_operational`, the preamble every public operation runs first. `require_device` carries the one real difference: Zigbee2MQTT hands a write to a bridge that will queue it for a sleeping device, while a zwave-js-ui write is an api round trip the node itself has to answer. * `_async_read_slots`, the sequential per-slot read and the all-transport-failed raise. Providers supply a `_async_read_slot` that returns None for silence and a credential for anything the lock described, plus the phrase naming what silence means on their bridge -- "was refused" for a gateway declining a command, "failed to reach the lock" for a GET that never got an answer. * `async_is_device_available`, identical in both. Deliberately not hoisted: subscription lifecycles (a per-node value tree versus a per-device topic, and zwave-js-ui's api transport with its own lifetime), the payload projections, the api client, and the poll cadences -- whose intervals coincide today for unrelated reasons. Two things resisted. Zigbee2MQTT names the wrong-bridge misconfiguration before falling back to "Lock not connected", which is now a `_raise_not_connected` hook rather than three inline calls; it is undecorated because `@callback` costs mypy the `NoReturn` inference. And the two providers phrase their all-failed message differently, so the base owns the frame and the caller owns the noun. Behaviour is unchanged: both provider test directories pass with three patch targets moved (the preamble's MQTT-enabled check is bound in `_mqtt` now, not in each provider, which still binds its own for the subscription paths) and one message match updated. The one thing that was not pinned is now: nothing tested that a Zigbee2MQTT write skips the device-availability check a read makes, so `require_device` survived being ignored. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… the call Resolution refused any call whose api response subscription had just been created, on the assumption that the coordinator's backoff would retry. Only a provider that lives long enough to be retried gets that: the config flow's allocation reads, the option flow's capacity check, and the unmanaged sweep each build a provider, make exactly one call, and drop it. Every attempt to add a zwave-js-ui lock through the user interface therefore failed with occupancy_unknown, and the sweep skipped zwave-js-ui locks permanently. Every subscribe now waits SUBSCRIBE_SETTLE_DELAY before returning, so a subscription is settled by the time the call that created it publishes into it. Putting the wait at the subscribe rather than at the caller also closes the setup-time race: async_setup subscribed and the coordinator's first poll published milliseconds later, inside Home Assistant's SUBSCRIBE_COOLDOWN, so the reply was lost and the lock stalled for a whole timeout budget. The fail-loud path is kept for the one case that is not a timing problem: no subscription could be established at all. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Three allocation sites built a provider, read it, and dropped it. An MQTT provider subscribes to its lock's topics on the way to answering, and a zwave-js-ui one also opens its gateway's api channel, so every config flow attempt left another live subscription behind -- each one reporting the same keypad press again, from an instance nothing owns. Only the unmanaged sweep had a teardown, hand-rolled in a finally. All four now go through one borrowed_lock_instance context manager, so a new query site cannot forget. Disposal is unsubscribe_push_updates, not async_unload. Unload means "this lock is leaving the entry" and providers act on that: the virtual lock writes its store back, which from a borrowed instance that never read one would erase every code in it. Releasing the transport is the whole of what a borrowed instance acquired -- and unsubscribe_push_updates is ungated at the base, so it reaches the zwave-js-ui api transport even on an api-only lock that reports no push support. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The retained-status scan bound a lone candidate outright, on the reasoning that getInfo only exists to break a tie. Filtering dead gateways out at the status changed what "lone candidate" means: a user whose own controller is down, sharing a broker with a neighbouring network, is left with that network's gateway as the only one answering on the prefix. Binding it wrote this lock's Personal Identification Numbers into whatever the other network calls node 20. Every candidate is now asked which network it runs, and a mismatch or a silence refuses by name. The availability fast path still skips the check and now says why in the code: that topic came out of the discovery payload that created this very lock entity, so the gateway named it about itself. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
_release_api_subscription cancels every pending call's future, and the wait only caught TimeoutError, so the CancelledError escaped the provider. Being a BaseException it slipped past the coordinator's and the sync layer's error handling entirely and surfaced as an unhandled task cancellation, for what is simply a lost transport. The two cancellations that reach this point are told apart by whether this task itself is being cancelled. Ours -- the release -- becomes LockDisconnected. An outer one, Home Assistant shutting down or the entry unloading, is never converted: swallowing it would stall the teardown that asked for it. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Every existing claimed-mqtt-lock flow test stopped at choose_path, one step short of the only place a config flow talks to the lock. Submitting a user count is what builds a throwaway provider and reads for free numbers, and nothing covered it -- which is why a provider change that made those reads refuse could make a zwave-js-ui lock unaddable through the user interface with the whole suite green. Both bridged providers now run that step: reverting the settle-wait makes the zwave-js-ui one fail with occupancy_unknown at the code_slot step. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…nything The shared MQTT read raised LockDisconnected whenever every slot it asked about came back silent, which for a one-user entry is one lost reply. On a lossy mesh that is routine -- issue #1397 had a node dropping roughly half its responses -- so the same lock, losing at the same rate, tripped the connectivity breaker for a household with one user and polled on untroubled for a household with two. How many people live there is not evidence about the transport. Rule chosen: raise only when two or more slots were read and all of them were silent. The alternative considered was to keep raising at n=1 for failures that prove the transport is dead, but neither bridge reports one cleanly here -- zwave-js-ui's silence is the gateway explicitly refusing the command, which proves the transport is alive, and Zigbee2MQTT folds a refused publish and a lost reply into the same None before the shared loop sees either. Dead transports are still caught by the operational preamble every read runs first, whose entity availability follows the bridge's own status topic, and by writes, which fail on their own path at any slot count. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
A lock whose provider cannot be resolved is popped from the entry during setup: no entities appear for it, nothing is written to it, and the only trace was one line in the log. Selection-time validation refuses that choice today, so the entries this happens to are the ones configured before the check existed -- exactly the users who never chose it knowingly and never see the log. A lock_dropped repair now names the entity and quotes what refused it, and is dismissed when the lock sets up or leaves the entry. Removing it from the entry is what the repair advises, so that path had to work: it read the missing instance out of runtime data first, and the KeyError -- swallowed by the fire-and-forget update listener -- abandoned the rest of the update. The read was dead code. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ady loaded Provider setup is skipped when the lock's integration reports itself not connected, and the only thing that re-ran it was that integration reaching LOADED. An MQTT bridge is loaded long before Lock Code Manager on a cold boot, and what is late is its discovery data, not its entry -- so the transition never came. The lock stayed un-set-up for the rest of the run: never validated, its provider's async_setup never called, and sync, which gates writes on that, never wrote another code to it. The 30-second connection check now runs the same integration-loaded path when it finds a reachable lock whose setup was deferred. A dedicated flag arms it rather than provider_setup_succeeded, which is also False for a lock whose setup ran and failed: that lock has a diagnosis and its own revalidation path, and re-probing its capabilities every thirty seconds would put a read on the wire for a lock already known to be degraded. The flag is cleared by setup running, so this fires once either way. Two triggers now spawn the reconnect, so the spawn (and its superseding of an in-flight task) is factored into one place and stays in the single slot async_unload cancels. Deferred, and commented at the coordinator: supports_push is still snapshot when the coordinator is built, so a lock that gains push from late discovery keeps polling until the next reload. Push itself works -- the retry re-runs the provider's subscribe -- so this is a redundant poll, not a lost update, and re-deriving it live means reaching into an interval whose breaker backoff, cold-start probe, and restore-on-recovery arms each own it at different times. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…estion ``_gateway_prefix`` asked its two sources in turn, and each walked Home Assistant's MQTT debug data -- a walk that rebuilds the debug state of every MQTT entity on the device. It runs on every connectivity check and several times per operation, so a gateway whose availability list names no client status paid for it twice. The two derivations become pure functions over one payload, read once. ``_resolve_state_topic``, ``_prefix_and_node_topic`` and ``_gateway_from_availability`` keep their signatures and their single walk apiece. Also records why ``_async_ensure_operational`` repeats two of the checks ``_execute_rate_limited`` just made: the unmanaged-code sweep and slot allocation reach the provider's operations without that wrapper. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Two changes to the fallback that finds a gateway by scanning a prefix's retained client statuses. The getInfo probes now run concurrently. They are answered from each gateway's own cached driver state and put nothing on anybody's mesh, so asking in turn only meant paying the local budget once per gateway that had stopped answering before reaching the live one. Arbitration is unchanged: a home id still has to match exactly one candidate. The result is now shared across provider instances, keyed by ``(prefix, home hex)`` in ``hass.data`` and serialized behind a mutex. The scan answers a question about a gateway, not about a lock, and every lock behind one gateway asks it together -- their coordinators refresh on the same tick. A disconnect that drops the instance binding drops the shared answer with it, so nobody replays an address that stopped answering; a failed scan caches nothing. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…e cadences Zigbee2MQTT and zwave-js-ui had the same forty-five lines of ``setup_push_subscription`` twice, differing only in which topic they resolve and which subscribe they run. Both now hand those two things to ``BaseMqttLock._schedule_push_subscription``, which holds the policy: keep a working subscription when the topic goes transiently unresolvable, refuse when there is nothing to fall back on, and run the provider's own idempotent subscribe in a task because the caller is synchronous and cannot be raised at. The explicit "MQTT is disabled" gate goes with it rather than being hoisted. Both providers' subscribe already refuses on that, so it arrives as an ordinary deferral instead of being asked twice. The poll cadences and the one-line hard refresh move to the base too. The module docstring had claimed their agreement was coincidence; it is not. A read on either bridge costs a round trip per slot, which is what sets the interval, and neither has a cache for a hard refresh to go behind, which is why it is the ordinary read. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…e way twice ``userIdStatus`` was compared against two hand-written ints that restate ``CodeSlotStatus`` from zwave-js-server -- the same enum the zwave_js provider already imports for the same field. The boolean guard stays: ``CodeSlotStatus.ENABLED`` is an int, so ``True == 1`` is still the trap it was. Three sites also spelled the withheld-code check inline as ``code == "*" * len(code)``, which ``_util.is_masked_code`` exists to say. That helper answers False for an empty string where the inline form answered True, so both sites that could see one keep an explicit emptiness test in front of it. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The provider register listed neither ``zwave_js_ui.py`` nor ``zha.py``, and said nothing about ``BaseMqttLock`` -- which is now the class a new MQTT-bridged provider is supposed to start from, so the step that says "subclass BaseLock" says which one. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
``_unclaimed_mqtt_locks`` had one caller that did nothing but test its result for emptiness, and ``entity_state_is_available`` had one caller that was already the docstring's subject -- the MQTT base's device availability. Both read better where they are used, and the availability one can now say which bridges it is deferring to. ``resolve_discovery_payload``'s scan for its own entity becomes a ``next``, which is what the loop's match-then-return was spelling. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
5 tasks
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.
Proposed change
Adds a provider for Z-Wave locks bridged into Home Assistant through zwave-js-ui's MQTT discovery gateway (locks that appear as
mqttintegration entities), requested in #1145 — plus a hardening pass informed by @erocm123's live-hardware branch, applied to both MQTT providers.Design: API-driven, not value-topic scraping. All reads/writes go through zwave-js-ui's MQTT api (
<prefix>/_CLIENTS/ZWAVE_GATEWAY-<name>/api/sendCommand→ User Code CC), correlated by a nonce echoed in the response'sorigin. Value topics are consumed only as opportunistic push. Key mechanics:availabilitylist, so the provider binds its gateway straight from the discovery payload — no scan, works per-lock across multiple gateways (including same-prefix primary/secondary setups). A retained-status scan withgetInfohome-id disambiguation remains as fallback; gateways whose retained status (or LWT) reports offline are never bound.supports_pushderived (push + events when the node topic resolves; coordinator polling otherwise). CustomUID_DISCOVERY_PREFIXidentifiers are accepted.mqttplatform is centralized (resolve_provider_class, device-identifier keyed). Unclaimed MQTT locks are rejected at config-flow submit time instead of being accepted and sitting permanently disconnected — an improvement for the existing Zigbee2MQTT provider too.async_subscribeis not live on return; the provider holds a persistent prefix-scoped response subscription established during setup, drift-aware across gateway prefix changes, with mutation-verified regression tests. Mocked-broker tests structurally cannot catch this class of bug.Hardening (from comparing against the live-tested zwave-mqtt branch — credit @erocm123):
fix(coordinator): a push provider whose initial refresh fails now retries instead of stranding until reload (core-layer; benefits Zigbee2MQTT too).getInfo, 5s inter-operation pacing."****") project unreadable, not known — prevents a permanent reprogramming storm on masking locks. Applied to both MQTT providers.LockDisconnectedinstead of returning an all-unreadable list that masquerades as a healthy poll and resets the breaker. Applied to both MQTT providers (operation refusals on zui; dead-broker silence on z2m).BaseMqttLockintermediate base (providers/_mqtt.py): the operational-guard preamble, the transport-distinguishing read loop, and availability — so the invariants exist once. Both providers' behavior provably unchanged (mutations in the shared loop fail tests in both directories).Also fixes two released bugs surfaced along the way: the unmanaged-codes sweep never tore down the throwaway providers it builds (duplicate events after migration until restart), and the coordinator initial-load stranding above.
Type of change
Additional information
{userId}) and raw-payload string quoting (source says always JSON-quoted).🤖 Generated with Claude Code
Review round (post-hardening)
An eight-angle review of the full PR surfaced four merge blockers — all fixed here with fail-first tests: fresh provider instances' first api call could never succeed (which made zui locks unconfigurable through the UI and is why the flow now runs through allocation in tests), throwaway providers built for config-flow/allocation questions leaked subscriptions (now context-managed via
borrowed_lock_instance), a sole gateway candidate was bound without home-id verification (cross-network write hazard), andCancelledErrorescaped the provider contract. Also fixed: the all-slots-failed raise now needs ≥2 slots (single-user entries on lossy meshes keep main's behavior), users upgrading with an unclaimed MQTT lock get alock_droppedrepair issue instead of a silent pop, deferred provider setup retries from the connection check (cold-boot discovery race), plus an efficiency/dedup/docs cleanup tier (shared per-gateway scan cache, parallel getInfo probes, one discovery walk per question,CodeSlotStatusenum,is_masked_codeat all sites,BaseMqttLockabsorbing the remaining duplication, AGENTS.md + README refreshed). Known deferral, documented in code: a lock that gains push support from late-arriving discovery keeps a redundant poll until reload (push itself works).