Fix entity creation blocking and revert defensive UI changes#602
Fix entity creation blocking and revert defensive UI changes#602
Conversation
Problem: Config entries with locks and code slots were not generating any entities. Investigation revealed a blocking wait loop that prevented entity creation when Z-Wave JS wasn't ready during startup. Root Cause: __init__.py contained an infinite while loop (lines 448-462) that waited for lock connections before proceeding. Since Z-Wave JS often loads after Lock Code Manager during HA startup, this caused entity creation to hang indefinitely. Why PRs #534 and #594 Were Wrong: These PRs made UI entities optional and added defensive code to handle missing entities. This was the WRONG solution because: 1. It masked the real problem (entities not being created) 2. Made the UI fail silently instead of revealing the bug 3. Treated a symptom as expected behavior 4. Made debugging harder by hiding the root cause The Correct Solution: 1. Remove blocking wait loop - entities should be created immediately 2. Add connection checks at the OPERATION level, not entity creation level 3. Graceful error handling when operations fail due to lock not ready 4. Revert all defensive UI changes so it fails fast if entities missing Changes Made: __init__.py: - Removed infinite while loop waiting for lock connection - Changed to single connection check with DEBUG log - Wrapped coordinator first refresh in try/except - Entities now create immediately regardless of lock state providers/_base.py: - Added connection checks in async_internal_set_usercode() - Added connection checks in async_internal_clear_usercode() - Raise LockDisconnected if lock not ready during operations binary_sensor.py: - Added connection check in async_update() before sync operations - Wrapped set_usercode in try/except with DEBUG logging - Wrapped clear_usercode in try/except with DEBUG logging ts/types.ts: - Reverted SlotMapping.pinActiveEntity back to required - Reverted SlotMapping.codeEventEntity back to required ts/generate-view.ts: - Removed optional chaining on pinActiveEntity.entity_id - Removed optional chaining on codeEventEntity.entity_id - Removed conditional rendering logic - Added non-null assertion to pinActiveEntity assignment CLAUDE.md: - Added comprehensive documentation of the issue - Explained why PRs #534/#594 were wrong - Documented the correct solution - Added lessons learned Behavior After Fix: - Entities create immediately during startup - Entities show as unavailable until lock is ready (expected) - Operations fail gracefully with DEBUG logs if lock not connected - UI fails fast if entities genuinely missing (reveals bugs) - No more infinite wait loops blocking entity creation Key Lessons: 1. Never mask symptoms with defensive code - fix root causes 2. Entity creation should never block on external service readiness 3. Connection checks belong at operation level, not setup level 4. Fail fast in development to reveal bugs early 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Changed all 'except Exception' clauses to 'except HomeAssistantError' for better exception handling specificity. This avoids catching system-level exceptions like KeyboardInterrupt and SystemExit, while still catching all Home Assistant-related errors including our custom exceptions (LockDisconnected, etc.) which inherit from HomeAssistantError. Changes: - binary_sensor.py: Added HomeAssistantError import, changed two exception handlers in set/clear usercode operations - __init__.py: Changed exception handler for coordinator first refresh 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Changed from catching broad HomeAssistantError to catching only the specific exceptions we expect in each context: binary_sensor.py: - Now catches only LockDisconnected during set/clear usercode operations - This is the specific exception raised when locks aren't connected during startup - Any other errors will now bubble up and be visible at ERROR level, making unexpected issues more discoverable __init__.py: - Now catches only UpdateFailed during coordinator first refresh - This is the specific exception coordinators raise when data fetch fails - More precise error handling without hiding unexpected failures Benefits: - Unexpected errors are now visible instead of silently caught - More maintainable - clear what errors we're handling and why - Follows Python best practice of catching specific exceptions - Makes debugging easier when unexpected issues occur 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Removed redundant connection checks and improved logging:
binary_sensor.py:
- Removed redundant connection check in async_update() (line 238)
- The check was duplicated - async_internal_set/clear_usercode already
check connection and raise LockDisconnected
- Try/except blocks handle disconnection, so early check unnecessary
- Simplifies conditional logic
- Changed ERROR to INFO log level for out-of-sync codes (line 242)
- Syncing out-of-sync codes is normal behavior, not an error
- ERROR level would confuse users and clutter logs
- INFO level is appropriate for informational sync operations
generate-view.ts:
- Removed diagnostic console.debug logging (lines 38-44)
- This was added during troubleshooting and is no longer needed
- Reduces console noise in production
lock-code-manager-strategy.js:
- Rebuilt from TypeScript source
Benefits:
- Cleaner code with less redundancy
- More appropriate log levels
- Single responsibility for connection checking (in the operation methods)
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <noreply@anthropic.com>
| self.lock.lock.entity_id, | ||
| self.slot_num, | ||
| ) | ||
| except LockDisconnected as err: |
There was a problem hiding this comment.
this could put locks out of sync. We need a graceful way to handle this
| self.lock.lock.entity_id, | ||
| self.slot_num, | ||
| ) | ||
| except LockDisconnected as err: |
There was a problem hiding this comment.
this could put locks out of sync. We need a graceful way to handle this
There was a problem hiding this comment.
Pull Request Overview
This PR fixes a critical entity creation blocking issue where no entities would be generated during config entry setup due to a startup race condition with Z-Wave JS. The integration was blocking indefinitely waiting for locks to connect before creating entities, preventing the entity creation flow from completing. The fix removes the blocking wait loop, adds proper connection checks in provider methods, handles exceptions gracefully during sync operations, and reverts previous defensive UI changes that were masking the root cause.
- Removed blocking wait loop that prevented entity creation during Z-Wave JS initialization
- Added connection checks with
LockDisconnectedexception handling in provider methods - Improved error handling and logging for usercode sync operations
- Reverted defensive TypeScript changes that made entities optional, allowing UI to fail fast if truly missing
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 16 comments.
Show a summary per file
| File | Description |
|---|---|
| custom_components/lock_code_manager/init.py | Removed blocking wait loop, added graceful connection check with try/except for coordinator refresh |
| custom_components/lock_code_manager/providers/_base.py | Added connection checks before set/clear usercode operations with LockDisconnected exception |
| custom_components/lock_code_manager/binary_sensor.py | Added error handling for sync operations and improved log messaging |
| ts/types.ts | Reverted optional entity types to required (removed ? from interface) |
| ts/generate-view.ts | Reverted conditional rendering and optional chaining, cleaned up formatting |
| custom_components/lock_code_manager/www/lock-code-manager-strategy.js | Rebuilt minified JavaScript from TypeScript sources |
| CLAUDE.md | Added comprehensive documentation of the issue, root cause, and fix |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: raman325 <7243222+raman325@users.noreply.github.com> Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: raman325 <7243222+raman325@users.noreply.github.com> Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: raman325 <7243222+raman325@users.noreply.github.com> Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
This commit addresses TypeScript code quality and fixes test failures: TypeScript Improvements: - Use safer .find() pattern for codeEventEntity instead of mutable assignment - Add non-null assertions (!) to both codeEventEntity and pinActiveEntity - Makes code more consistent and explicitly documents that entities always exist - Aligns with type definitions where these entities are required (non-optional) Test Fixes: - Fix KeyError during test teardown in tests/common.py mock methods - Add defensive checks for lock data existence in set_usercode(), clear_usercode(), and get_usercodes() - Update tests/_base/test_provider.py to expect NotImplementedError for abstract methods with connection checks - All 26 tests now pass successfully Documentation: - Add high priority TODO #1: "Fix Locks Out of Sync Issue" - Comprehensive analysis of auto-sync logic issues - Proposed solutions including manual sync service and better state change detection - Renumbered all subsequent TODOs (1-11) Files Changed: - ts/generate-view.ts: Safer entity finding with non-null assertions - custom_components/lock_code_manager/www/lock-code-manager-strategy.js: Rebuilt from TypeScript - tests/common.py: Defensive mock methods handle missing data during teardown - tests/_base/test_provider.py: Updated test expectations for abstract methods - CLAUDE.md: Added high priority TODO and renumbered existing items 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #602 +/- ##
==========================================
+ Coverage 90.31% 90.39% +0.07%
==========================================
Files 20 20
Lines 1260 1280 +20
==========================================
+ Hits 1138 1157 +19
- Misses 122 123 +1
🚀 New features to boost your workflow:
|
Eslint automatically removes non-null assertions (!), so removing them to keep the repository clean after lint runs in CI. The types are still enforced at compile time through the SlotMapping interface which declares these fields as required. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
This commit adds comprehensive test coverage for the new LockDisconnected exception handling code paths introduced to handle disconnected locks during sync operations. Test Coverage Added: Provider Tests (tests/_base/test_provider.py): - test_set_usercode_when_disconnected: Verifies async_internal_set_usercode raises LockDisconnected when lock is disconnected - test_clear_usercode_when_disconnected: Verifies async_internal_clear_usercode raises LockDisconnected when lock is disconnected Binary Sensor Tests (tests/test_binary_sensor.py): - test_handles_disconnected_lock_on_set: Verifies binary sensor handles LockDisconnected exception when setting usercode fails, with proper logging - test_handles_disconnected_lock_on_clear: Verifies binary sensor handles LockDisconnected exception when clearing usercode fails, with proper logging - test_syncs_when_lock_reconnects: Verifies eventual consistency - sync fails while disconnected, then succeeds after lock reconnects Test Infrastructure (tests/common.py): - Added __init__ to MockLCMLock to initialize _connected flag - Added set_connected() method to simulate disconnected locks for testing - Modified is_connection_up() to return _connected state Code Coverage: These tests cover the following new code paths: - providers/_base.py lines 168-171, 186-189 (connection checks) - binary_sensor.py lines 237, 309-329, 338-356 (exception handling) All tests verify: - Proper exception raising/handling - Debug logging output - No service calls made when disconnected - Data integrity (codes not changed when operations fail) - Eventual consistency after reconnection 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Proposed change
Config entries with locks and code slots were not generating any entities. Investigation revealed a blocking wait loop that prevented entity creation when Z-Wave JS wasn't ready during startup. init.py contained an infinite while loop (lines 448-462) that waited for lock connections before proceeding. Since Z-Wave JS often loads after Lock Code Manager during HA startup, this caused entity creation to hang indefinitely. PRs #534 and #594 were wrong because they were focused on the UI instead of the integration itself.
The Correct Solution:
Type of change
Additional information
Claude stuff
Problem: Config entries with locks and code slots were not generating any entities. Investigation revealed a blocking wait loop that prevented entity creation when Z-Wave JS wasn't ready during startup.
Root Cause: init.py contained an infinite while loop (lines 448-462) that waited for lock connections before proceeding. Since Z-Wave JS often loads after Lock Code Manager during HA startup, this caused entity creation to hang indefinitely.
Why PRs #534 and #594 Were Wrong:
These PRs made UI entities optional and added defensive code to handle missing entities. This was the WRONG solution because:
The Correct Solution:
CLAUDE.md:
Behavior After Fix:
Key Lessons:
🤖 Generated with Claude Code