From b2602e8aad8760cb1472f0119265db0c79e0640e Mon Sep 17 00:00:00 2001 From: raman325 <7243222+raman325@users.noreply.github.com> Date: Mon, 2 Mar 2026 16:02:23 -0500 Subject: [PATCH] Gate operations on device availability (dead-node check) Rename is_connection_up/async_is_connection_up/async_internal_is_connection_up to is_integration_connected/async_is_integration_connected/ async_internal_is_integration_connected to clarify they check integration connectivity, not device availability. Add is_device_available() and async_is_device_available() to BaseLock with default True. Override in ZWaveJSLock to return False when node status is DEAD, preventing doomed RF commands to dead nodes. Gate _execute_rate_limited() on device availability after the existing integration connectivity check, raising LockDisconnected when the device is not available. Co-Authored-By: Claude Opus 4.6 --- AGENTS.md | 4 +- .../lock_code_manager/__init__.py | 2 +- .../lock_code_manager/coordinator.py | 2 +- .../lock_code_manager/providers/_base.py | 33 ++++++--- .../lock_code_manager/providers/virtual.py | 2 +- .../lock_code_manager/providers/zwave_js.py | 15 ++-- tests/common.py | 4 +- tests/providers/test_base.py | 44 +++++++++++- tests/providers/test_virtual.py | 2 +- tests/providers/test_zwave_js.py | 70 +++++++++++++++++-- tests/test_coordinator.py | 4 +- tests/test_exceptions.py | 4 +- 12 files changed, 153 insertions(+), 33 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 6637e5e3..f9fbdf49 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -49,7 +49,7 @@ entities. - `zwave_js.py`: Z-Wave JS lock implementation - `virtual.py`: Virtual lock implementation for testing - Each provider implements: `async_get_usercodes()`, `async_set_usercode()`, `async_clear_usercode()`, - `async_is_connection_up()`, `async_hard_refresh_codes()` + `async_is_integration_connected()`, `async_hard_refresh_codes()` - Providers listen for lock-specific events and translate them to LCM events via `async_fire_code_slot_event()` **Coordinator** (`coordinator.py`) @@ -237,7 +237,7 @@ yarn watch # Watch mode for development 2. Subclass `BaseLock` from `providers/_base.py` 3. Implement required abstract methods: - `domain` property: return integration domain string - - `async_is_connection_up()`: check if lock is reachable + - `async_is_integration_connected()`: check if integration is connected - `async_get_usercodes()`: return dict of slot→code mappings - `async_set_usercode()`: program a code to a slot - `async_clear_usercode()`: remove code from slot diff --git a/custom_components/lock_code_manager/__init__.py b/custom_components/lock_code_manager/__init__.py index 9e468215..8a9ba1e0 100644 --- a/custom_components/lock_code_manager/__init__.py +++ b/custom_components/lock_code_manager/__init__.py @@ -516,7 +516,7 @@ async def async_update_listener( added_locks.append(lock) # Check if lock is connected (but don't wait - entity creation doesn't require it) - if not await lock.async_internal_is_connection_up(): + if not await lock.async_internal_is_integration_connected(): _LOGGER.debug( "%s (%s): Lock %s is not connected yet. Entities will be created " "but will be unavailable until the lock comes online. This is normal " diff --git a/custom_components/lock_code_manager/coordinator.py b/custom_components/lock_code_manager/coordinator.py index ee02ed72..aee4c620 100644 --- a/custom_components/lock_code_manager/coordinator.py +++ b/custom_components/lock_code_manager/coordinator.py @@ -128,7 +128,7 @@ async def _async_drift_check(self, now: datetime) -> None: async def _async_connection_check(self, now: datetime) -> None: """Poll connection state so providers can resubscribe on reconnect.""" try: - await self._lock.async_internal_is_connection_up() + await self._lock.async_internal_is_integration_connected() except LockCodeManagerError as err: _LOGGER.debug( "Connection check failed for %s: %s", self._lock.lock.entity_id, err diff --git a/custom_components/lock_code_manager/providers/_base.py b/custom_components/lock_code_manager/providers/_base.py index aae05ede..5d967393 100644 --- a/custom_components/lock_code_manager/providers/_base.py +++ b/custom_components/lock_code_manager/providers/_base.py @@ -75,7 +75,7 @@ class BaseLock: - Set hard_refresh_interval = None to disable 4. Poll connection state: - - Periodic async_internal_is_connection_up() at connection_check_interval + - Periodic async_internal_is_integration_connected() at connection_check_interval - Helps detect reconnects for integrations without config entry state signals - Set connection_check_interval = None to disable @@ -144,11 +144,16 @@ async def _execute_rate_limited( **kwargs: Any, ) -> Any: """Execute operation with connection check, serialization, and delay.""" - if not await self.async_internal_is_connection_up(): + if not await self.async_internal_is_integration_connected(): raise LockDisconnected( f"Cannot {_OPERATION_MESSAGES[operation_type]} {self.lock.entity_id} - lock not connected" ) + if not await self.async_is_device_available(): + raise LockDisconnected( + f"Cannot {_OPERATION_MESSAGES[operation_type]} {self.lock.entity_id} - device not available" + ) + async with self._aio_lock: elapsed = time.monotonic() - self._last_operation_time if elapsed < self._min_operation_delay: @@ -362,10 +367,18 @@ async def async_unload(self, remove_permanently: bool) -> None: await self.hass.async_add_executor_job(self.unload, remove_permanently) - def is_connection_up(self) -> bool: - """Return whether connection to lock is up.""" + def is_integration_connected(self) -> bool: + """Return whether the integration's client/driver/broker is connected.""" raise NotImplementedError() + def is_device_available(self) -> bool: + """Return whether the physical device is available for commands.""" + return True + + async def async_is_device_available(self) -> bool: + """Return whether the physical device is available for commands.""" + return await self._async_executor_call(self.is_device_available) + def _setup_config_entry_state_listener(self) -> None: """Listen for provider config entry state changes to resubscribe.""" lock_entry = self.lock_config_entry @@ -399,14 +412,14 @@ def _handle_state_change() -> None: _handle_state_change ) - async def async_is_connection_up(self) -> bool: - """Return whether connection to lock is up.""" - return await self._async_executor_call(self.is_connection_up) + async def async_is_integration_connected(self) -> bool: + """Return whether the integration's client/driver/broker is connected.""" + return await self._async_executor_call(self.is_integration_connected) @final - async def async_internal_is_connection_up(self) -> bool: - """Return whether connection to lock is up.""" - is_up = await self.async_is_connection_up() + async def async_internal_is_integration_connected(self) -> bool: + """Return whether the integration's client/driver/broker is connected.""" + is_up = await self.async_is_integration_connected() lock_entry = self.lock_config_entry if self.supports_push and lock_entry: # Only react to connection transitions when the config entry is loaded. diff --git a/custom_components/lock_code_manager/providers/virtual.py b/custom_components/lock_code_manager/providers/virtual.py index c9ccda6c..64ffd037 100644 --- a/custom_components/lock_code_manager/providers/virtual.py +++ b/custom_components/lock_code_manager/providers/virtual.py @@ -54,7 +54,7 @@ async def async_unload(self, remove_permanently: bool) -> None: else: await self._store.async_save(self._data) - async def async_is_connection_up(self) -> bool: + async def async_is_integration_connected(self) -> bool: """Return whether connection to lock is up.""" return True diff --git a/custom_components/lock_code_manager/providers/zwave_js.py b/custom_components/lock_code_manager/providers/zwave_js.py index d999086a..fb22622d 100644 --- a/custom_components/lock_code_manager/providers/zwave_js.py +++ b/custom_components/lock_code_manager/providers/zwave_js.py @@ -10,7 +10,7 @@ from typing import Any from zwave_js_server.client import Client -from zwave_js_server.const import CommandClass +from zwave_js_server.const import CommandClass, NodeStatus from zwave_js_server.const.command_class.lock import ( ATTR_CODE_SLOT, ATTR_IN_USE, @@ -470,11 +470,18 @@ async def async_unload(self, remove_permanently: bool) -> None: self._listeners.clear() await super().async_unload(remove_permanently) - async def async_is_connection_up(self) -> bool: - """Return whether connection to lock is up.""" + async def async_is_integration_connected(self) -> bool: + """Return whether the Z-Wave JS client is connected.""" ready, _reason = self._get_client_state() return ready + async def async_is_device_available(self) -> bool: + """Return whether the Z-Wave node is available for commands.""" + try: + return self.node.status != NodeStatus.DEAD + except Exception: # noqa: BLE001 + return False + async def async_hard_refresh_codes(self) -> dict[int, int | str]: """ Perform hard refresh and return all codes. @@ -600,7 +607,7 @@ async def async_get_usercodes(self) -> dict[int, int | str]: } data: dict[int, int | str] = {} - if not await self.async_is_connection_up(): + if not await self.async_is_integration_connected(): raise LockDisconnected slots = self._get_usercodes_from_cache() diff --git a/tests/common.py b/tests/common.py index 896a8e4a..8b5d02cc 100644 --- a/tests/common.py +++ b/tests/common.py @@ -87,8 +87,8 @@ def set_connected(self, connected: bool) -> None: """Set connection state for testing.""" self._connected = connected - def is_connection_up(self) -> bool: - """Return whether connection to lock is up.""" + def is_integration_connected(self) -> bool: + """Return whether the integration's client/driver/broker is connected.""" return self._connected def hard_refresh_codes(self) -> None: diff --git a/tests/providers/test_base.py b/tests/providers/test_base.py index a8a31a3b..740c8b6d 100644 --- a/tests/providers/test_base.py +++ b/tests/providers/test_base.py @@ -87,9 +87,9 @@ async def test_base(hass: HomeAssistant): with pytest.raises(NotImplementedError): assert lock.domain with pytest.raises(NotImplementedError): - await lock.async_internal_is_connection_up() + await lock.async_internal_is_integration_connected() # Note: hard_refresh, set, and clear operations now check connection first, - # so they raise NotImplementedError from is_connection_up() instead of + # so they raise NotImplementedError from is_integration_connected() instead of # the expected error from the unimplemented method with pytest.raises(NotImplementedError): await lock.async_internal_hard_refresh_codes() @@ -644,3 +644,43 @@ async def test_config_entry_state_listener_ignores_same_state( assert lock.unsubscribe_calls == 0 await hass.config_entries.async_unload(lcm_config_entry.entry_id) + + +async def test_is_device_available_default_returns_true(hass: HomeAssistant): + """Test that base class is_device_available() returns True by default.""" + entity_reg = er.async_get(hass) + config_entry = MockConfigEntry(domain=DOMAIN) + config_entry.add_to_hass(hass) + + lock_entity = entity_reg.async_get_or_create( + "lock", + "test", + "test_lock_device_available", + config_entry=config_entry, + ) + + lock = BaseLock( + hass, + dr.async_get(hass), + entity_reg, + config_entry, + lock_entity, + ) + + # Default implementation returns True + assert lock.is_device_available() is True + assert await lock.async_is_device_available() is True + + +async def test_execute_rate_limited_raises_when_device_not_available( + hass: HomeAssistant, + mock_lock_config_entry, + lock_code_manager_config_entry, +): + """Test that _execute_rate_limited raises LockDisconnected when device not available.""" + lock_provider = lock_code_manager_config_entry.runtime_data.locks[LOCK_1_ENTITY_ID] + + # Device is not available but integration is connected + with patch.object(lock_provider, "async_is_device_available", return_value=False): + with pytest.raises(LockDisconnected, match="device not available"): + await lock_provider.async_internal_set_usercode(2, "9999", "test") diff --git a/tests/providers/test_virtual.py b/tests/providers/test_virtual.py index ac918ff9..19b9633f 100644 --- a/tests/providers/test_virtual.py +++ b/tests/providers/test_virtual.py @@ -35,7 +35,7 @@ async def test_door_lock(hass: HomeAssistant): assert await lock.async_setup(config_entry) is None assert lock.usercode_scan_interval == timedelta(minutes=1) assert lock.domain == "virtual" - assert await lock.async_internal_is_connection_up() + assert await lock.async_internal_is_integration_connected() assert lock._data == {} await lock.async_internal_hard_refresh_codes() assert lock._data == {} diff --git a/tests/providers/test_zwave_js.py b/tests/providers/test_zwave_js.py index 08b7c458..a3061ab6 100644 --- a/tests/providers/test_zwave_js.py +++ b/tests/providers/test_zwave_js.py @@ -7,7 +7,7 @@ import pytest from pytest_homeassistant_custom_component.common import MockConfigEntry -from zwave_js_server.const import CommandClass +from zwave_js_server.const import CommandClass, NodeStatus from zwave_js_server.const.command_class.lock import ( LOCK_USERCODE_STATUS_PROPERTY, CodeSlotStatus, @@ -151,17 +151,17 @@ async def test_node_property( # Connection tests -async def test_is_connection_up_when_loaded( +async def test_is_integration_connected_when_loaded( hass: HomeAssistant, zwave_js_lock: ZWaveJSLock, zwave_integration: MockConfigEntry, ) -> None: """Test connection is up when config entry is loaded and client connected.""" assert zwave_integration.state == ConfigEntryState.LOADED - assert await zwave_js_lock.async_is_connection_up() is True + assert await zwave_js_lock.async_is_integration_connected() is True -async def test_is_connection_down_when_not_loaded( +async def test_is_integration_not_connected_when_not_loaded( hass: HomeAssistant, zwave_js_lock: ZWaveJSLock, zwave_integration: MockConfigEntry, @@ -171,7 +171,7 @@ async def test_is_connection_down_when_not_loaded( await hass.async_block_till_done() assert zwave_integration.state != ConfigEntryState.LOADED - assert await zwave_js_lock.async_is_connection_up() is False + assert await zwave_js_lock.async_is_integration_connected() is False # Usercode tests @@ -1918,3 +1918,63 @@ async def test_push_update_user_id_status_available_clears_when_slot_inactive( zwave_js_lock.unsubscribe_push_updates() await zwave_js_lock.async_unload(False) + + +# Device availability tests + + +async def test_is_device_available_returns_true_when_alive( + zwave_js_lock: ZWaveJSLock, + lock_schlage_be469: Node, +) -> None: + """Test that async_is_device_available returns True when node is ALIVE.""" + with patch.object( + type(lock_schlage_be469), + "status", + new_callable=lambda: property(lambda self: NodeStatus.ALIVE), + ): + assert await zwave_js_lock.async_is_device_available() is True + + +async def test_is_device_available_returns_true_when_asleep( + zwave_js_lock: ZWaveJSLock, + lock_schlage_be469: Node, +) -> None: + """Test that async_is_device_available returns True when node is ASLEEP.""" + with patch.object( + type(lock_schlage_be469), + "status", + new_callable=lambda: property(lambda self: NodeStatus.ASLEEP), + ): + assert await zwave_js_lock.async_is_device_available() is True + + +async def test_is_device_available_returns_false_when_dead( + zwave_js_lock: ZWaveJSLock, + lock_schlage_be469: Node, +) -> None: + """Test that async_is_device_available returns False when node is DEAD.""" + with patch.object( + type(lock_schlage_be469), + "status", + new_callable=lambda: property(lambda self: NodeStatus.DEAD), + ): + assert await zwave_js_lock.async_is_device_available() is False + + +async def test_is_device_available_returns_false_on_exception( + hass: HomeAssistant, + zwave_js_lock: ZWaveJSLock, + zwave_integration: MockConfigEntry, +) -> None: + """Test that async_is_device_available returns False when node access raises.""" + + def raise_error(self): + raise RuntimeError("node gone") + + with patch.object( + type(zwave_js_lock), + "node", + new_callable=lambda: property(raise_error), + ): + assert await zwave_js_lock.async_is_device_available() is False diff --git a/tests/test_coordinator.py b/tests/test_coordinator.py index a04a5dab..e31dec37 100644 --- a/tests/test_coordinator.py +++ b/tests/test_coordinator.py @@ -36,8 +36,8 @@ def hard_refresh_interval(self) -> timedelta | None: """Return configurable hard refresh interval.""" return self._hard_refresh_interval - def is_connection_up(self) -> bool: - """Return whether connection to lock is up.""" + def is_integration_connected(self) -> bool: + """Return whether the integration's client/driver/broker is connected.""" return self._is_connected def hard_refresh_codes(self) -> dict[int, int | str]: diff --git a/tests/test_exceptions.py b/tests/test_exceptions.py index 71d1fad6..79136711 100644 --- a/tests/test_exceptions.py +++ b/tests/test_exceptions.py @@ -27,8 +27,8 @@ def domain(self) -> str: """Return integration domain.""" return "test" - def is_connection_up(self) -> bool: - """Return whether connection to lock is up.""" + def is_integration_connected(self) -> bool: + """Return whether the integration's client/driver/broker is connected.""" return True