Skip to content

Slot-clear verification waits are serialized across the whole refresh loop #801

Description

@tykeal

Split out of the review of #800, which is fine to merge as-is — this is pre-existing architecture that PR merely makes visible.

Background

#800 adds a bounded poll to _async_clear_legacy_usercode() in custom_components/keymaster/providers/zwave_js.py, because a legacy User Code CC lock acknowledges a clear command before it reports the slot's new value. Verification now waits up to LEGACY_CLEAR_VERIFY_TIMEOUT (10 s) for the lock's report before declaring the clear failed.

That is the right fix for the race. The concern is where the wait sits.

The problem

The coordinator refresh loop is fully serialized at two levels:

  • async_refresh_all_locks iterates every lock and awaits each one (coordinator.py:2521-2527)
  • _update_code_slots iterates every code slot and awaits each one (coordinator.py:2670-2675), then retries every OUT_OF_SYNC slot again further down

_update_slot / _sync_pin / _sync_child_code_slot_pin all funnel into clear_pin_from_lock → async_clear_usercode. So a slot that fails to report cleared costs its full verification timeout, and those costs add up in sequence across every slot and every lock.

With update_interval at 60 s (coordinator.py:254), a handful of slots that never report can exceed the entire update interval, at which point Home Assistant logs "Update of keymaster took longer than the scheduled update interval" and every other lock is stuck behind the wait. The parent/child fan-out multiplies this: a parent with many children re-syncs child slots in the same serialized pass.

What limits the impact today

  • async_clear_usercode returns early for dead nodes via _is_node_alive(), so a fully dead lock never enters the poll.
  • get_usercode() is a cached, synchronous read. The poll generates no Z-Wave traffic — the cost is purely wall-clock.

Why it is still reachable

_is_node_alive() deliberately treats ASLEEP battery nodes as alive, since they wake periodically (providers/zwave_js.py:475-487). An asleep battery lock is precisely the node that will not report within the timeout, so it enters the poll and pays the full cost on every refresh cycle.

Possible directions

Not a recommendation, just the options as they look from here:

  • Cap the total verification budget per refresh pass rather than per slot, so N slow slots cannot cost N × timeout.
  • Let verification fail fast and lean on the existing OUT_OF_SYNC + QUICK_REFRESH_SECONDS / PIN_SET_GRACE_SECONDS reconciliation, which already retries — making the in-line wait a short UX smoothing rather than the mechanism of record.
  • Run per-lock refreshes concurrently, which is a much larger change and interacts with the shared save path.

Worth noting the same serialization applies to any future provider that adds an in-line wait, so a general answer may be better than a zwave_js-specific one.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions