Skip to content

fix: autolock timer persistence, race conditions, and throttle gaps#596

Open
raman325 wants to merge 18 commits intoFutureTense:mainfrom
raman325:fix/594-autolock-timer-unknown
Open

fix: autolock timer persistence, race conditions, and throttle gaps#596
raman325 wants to merge 18 commits intoFutureTense:mainfrom
raman325:fix/594-autolock-timer-unknown

Conversation

@raman325
Copy link
Copy Markdown
Collaborator

@raman325 raman325 commented Apr 11, 2026

Summary

Comprehensive fix for the autolock timer: persistence across restarts, callback race conditions, property side effects, switch lifecycle, and throttle gaps.

Breaking change

N/A

Proposed change

1. Timer not surviving restarts / not starting on enable

The autolock timer only started inside _lock_unlocked(), which requires a locked→unlocked transition. Two scenarios left the timer idle:

  • HA restart while lock is unlocked: Timer was not persisted, lost on restart.
  • Enabling autolock while lock is already unlocked: Switch set autolock_enabled = True but didn't start the timer.

Fix: KeymasterTimer now persists end_time and duration to a dedicated HA Store (keymaster.timers). On setup(), it recovers persisted state: expired → fires immediately, active → resumes, absent → idle. The switch also starts the timer when enabling autolock while unlocked.

2. Callback race condition (pre-existing)

The timer scheduled two async_call_later callbacks with the same delay (action + self-cancel). If cancel ran first, it unsubscribed the action — the lock command was never sent.

Fix: Single callback that fires the action first, then cleans up.

3. Property accessor side effects (pre-existing)

is_running, end_time, remaining_seconds called _check_expired() which cancelled callbacks. A coordinator update reading properties after the deadline but before the callback could unsubscribe the action.

Fix: Properties are now pure. is_running (_end_time is not None and _end_time > utcnow()) is the single source of truth; other properties delegate to it. Only the scheduled callback and explicit cancel() mutate state.

4. Disabling autolock doesn't cancel timer

Turning off the autolock switch while the timer was running didn't cancel it — the timer would still fire and lock the door.

Fix: async_turn_off cancels the running timer when disabling autolock.

5. Throttle swallows legitimate unlock after rapid lock→unlock

The 5-second throttle suppressed a second unlock event even when a lock event occurred in between (unlock→lock→unlock within 5s). The autolock timer didn't start on the second unlock.

Fix: Add Throttle.reset() and call it on each state transition to clear the complementary throttle (_lock_locked resets lock_unlocked throttle, and vice versa). Same pattern for door open/close.

6. UTC times

All timer times now use homeassistant.util.dt.utcnow() for consistent UTC storage and comparison.

Type of change

  • Bugfix (non-breaking change which fixes an issue)

Additional information

🤖 Generated with Claude Code

raman325 and others added 4 commits March 10, 2026 23:20
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* upstream/main:
  chore: bump minimum HA version to 2026.4.0
  fix: stop re-fetching masked usercodes every poll cycle
  feat: add Schlage WiFi lock provider
  fix: auto-dismiss retry lock notifications when lock succeeds
  feat: add Last Used event entity per code slot
  fix: slugify lock name when generating default notification script name
  fix(tests): restore accidentally merged test_get_code_empty_pin
  fix(akuvox): handle X916 user filtering where source_type is None
  test: add tests for `_update_lock_data` early exit conditions
  test: add coverage for reset_code_slot, update_slot_active_state, and _update_child_code_slots
  feat: add Akuvox lock provider with event routing fixes
  style: move imports to module level to fix ruff PLC0415
  feat: Add auto-lock timer sensor with dashboard badge (FutureTense#509)
  test: add ping_node tests for ZWaveJS and base provider
  feat: ping dead node on first failure to attempt recovery
  style: apply ruff formatting fixes and add dead-node backoff tests
  fix: relax dead-node checks on connect/read operations
  fix: skip Z-Wave commands to dead nodes and add exponential backoff
The autolock timer only started inside _lock_unlocked(), which requires
a locked→unlocked state transition. If HA restarts while the lock is
unlocked, or if autolock is enabled while the lock is already unlocked,
the timer never starts and auto-lock doesn't function.

Two fixes:
- _setup_timer: start timer immediately if lock is already unlocked
  with autolock enabled (handles HA restart and add_lock/update_lock)
- switch async_turn_on: start timer when autolock switch is toggled on
  while lock is already unlocked

Fixes FutureTense#594

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings April 11, 2026 02:22
@github-actions github-actions bot added the bugfix Fixes a bug label Apr 11, 2026
@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Apr 11, 2026

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

❌ Patch coverage is 89.41176% with 9 lines in your changes missing coverage. Please review.
✅ Project coverage is 89.84%. Comparing base (cdb4922) to head (72ee58a).
⚠️ Report is 75 commits behind head on main.

Files with missing lines Patch % Lines
custom_components/keymaster/helpers.py 87.14% 9 Missing ⚠️
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #596      +/-   ##
==========================================
+ Coverage   84.14%   89.84%   +5.70%     
==========================================
  Files          10       28      +18     
  Lines         801     3517    +2716     
==========================================
+ Hits          674     3160    +2486     
- Misses        127      357     +230     
Flag Coverage Δ
python 89.84% <89.41%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes the autolock timer not starting (and showing “Unknown”) when Home Assistant restarts or when autolock is enabled while the lock is already in an unlocked state (issue #594).

Changes:

  • Auto-start the autolock timer during coordinator timer setup when autolock is enabled and the lock is already unlocked.
  • Start the autolock timer immediately when the autolock switch is turned on while the lock is currently unlocked.
  • Add/extend unit tests covering the above autolock behaviors (plus two newly added strategy-validation planning/design docs).

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 7 comments.

Show a summary per file
File Description
custom_components/keymaster/coordinator.py Starts autolock timer in _setup_timer() when lock is already unlocked.
custom_components/keymaster/switch.py Starts autolock timer when enabling autolock while already unlocked.
tests/test_switch.py Adds switch tests asserting timer start/no-start behavior based on lock state.
tests/test_coordinator.py Adds _setup_timer() tests for the new auto-start behavior.
docs/plans/2026-03-10-strategy-config-validation.md Adds a strategy config validation implementation plan (appears unrelated to PR scope).
docs/plans/2026-03-10-strategy-config-validation-design.md Adds a strategy config validation design doc (appears unrelated to PR scope).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@raman325 raman325 marked this pull request as draft April 11, 2026 02:54
raman325 and others added 2 commits April 10, 2026 23:09
KeymasterTimer now persists its end_time to a dedicated HA Store
(keymaster.timers) keyed by timer_id. On setup(), the timer recovers
persisted state:
- expired during restart → fire the action immediately
- still active → resume with remaining time
- no entry → idle

This eliminates the heuristic of checking lock_state in _setup_timer,
since the timer's own persistence handles all restart scenarios.

The switch fix (starting timer when autolock is enabled while unlocked)
is retained for the case where no prior timer existed.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Replace dt.now().astimezone() with dt_util.utcnow() throughout the
timer for consistent UTC storage and comparison.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@raman325 raman325 changed the title fix: start autolock timer when lock is already unlocked fix: persist autolock timer to survive restarts and start on enable Apr 11, 2026
The timer previously scheduled two async_call_later callbacks with the
same delay: one for the action and one for self-cancel. If the cancel
callback fired first, it would unsubscribe the action callback before
it could execute, preventing the lock from auto-locking.

Replace with a single callback that fires the action first, then cleans
up the timer state. This eliminates the race condition and ensures the
lock command is always sent when the timer expires.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@raman325 raman325 changed the title fix: persist autolock timer to survive restarts and start on enable fix: persist autolock timer and fix callback race condition Apr 11, 2026
Replace _has_expired()/_check_expired() with a single is_running
property as the source of truth. All other properties (end_time,
remaining_seconds, duration) delegate to is_running rather than
each independently checking expiry. Properties never mutate state.

Consolidate 4 individual expired-property tests into one
comprehensive test that verifies all properties are side-effect-free.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@raman325 raman325 changed the title fix: persist autolock timer and fix callback race condition fix: persist autolock timer, fix callback race, and make properties pure Apr 11, 2026
raman325 and others added 3 commits April 11, 2026 00:04
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Previously, disabling the autolock switch while the timer was running
did not cancel the timer. The timer would still fire and lock the door
even though autolock had been turned off.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The throttle could swallow a legitimate unlock event if it arrived
within 5 seconds of a previous unlock — even if a lock event occurred
in between. This happened because the throttle tracked wall-clock time
per function, with no awareness of intervening state changes.

Add Throttle.reset() and call it on each state transition to clear the
complementary throttle:
  - _lock_unlocked resets lock_locked throttle
  - _lock_locked resets lock_unlocked throttle
  - _door_opened resets door_closed throttle
  - _door_closed resets door_opened throttle

This ensures a rapid unlock→lock→unlock sequence always starts the
autolock timer on the second unlock.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@raman325 raman325 changed the title fix: persist autolock timer, fix callback race, and make properties pure fix: autolock timer persistence, race conditions, and throttle gaps Apr 11, 2026
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@raman325 raman325 marked this pull request as ready for review April 11, 2026 04:25
@raman325 raman325 requested a review from Copilot April 11, 2026 04:26
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

- Push coordinator data after timer resumes from persistence so sensor
  entities reflect the running timer immediately
- Replace Mock(spec=KeymasterLock) with real KeymasterLock instances in
  TestSetupTimer so tests pass the isinstance guard
- Assert action_called in expired timer recovery test to verify the
  action actually fires

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Rename the timer's duration field to total_duration across the timer
class, store persistence, sensor attributes, and all tests. This
distinguishes it from remaining time and makes clear it represents
the original configured duration, not the countdown.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@raman325 raman325 added the breaking-change Changes that will require user intervention label Apr 11, 2026
raman325 and others added 4 commits April 11, 2026 00:48
If the persisted timer entry has a missing or invalid end_time,
setup() would crash with KeyError/TypeError/ValueError during
dt.fromisoformat(). Now catches these, logs a warning, removes
the bad entry, and continues with no timer.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Replace dict[str, Any] with a TimerStoreEntry TypedDict for the timer
store, making the persisted shape explicit: end_time (str) and
duration (int). Tighten duration from int | None to int since it is
always set before persisting.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

breaking-change Changes that will require user intervention bugfix Fixes a bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants