From c4b28509e192425cd1cefcf5494ae67e8fa5c6bf Mon Sep 17 00:00:00 2001 From: raman325 <7243222+raman325@users.noreply.github.com> Date: Tue, 11 Aug 2026 14:35:14 -0400 Subject: [PATCH] fix: treat occupied-but-valueless Z-Wave slots as unreadable, not empty When a lock reports userIdStatus occupied while withholding the code value, node-zwave-js's User Code CC adapter drops the credential from the unified read rather than returning it with no value. The slot then projected to empty(), telling the sync layer the write definitively did not land -- so it rewrote, failed to confirm again, and the circuit breaker suspended a slot whose PIN works on the keypad. Only the OPTIMISTIC write path could loop this way: a CONFIRMED write pops the pending entry and marks the slot verified, so the _last_set_pin escape hatch in calculate_in_sync tolerates an empty read-back. An OPTIMISTIC write stays pending and unverified, and calculate_in_sync returns False at the is_verified gate before ever reaching that hatch. Take userIdStatus as the occupancy authority and the credential as the value, so an occupied slot with no credential is unreadable rather than empty. Applied in the slot projection rather than async_get_users: the user list also drives the write paths (delete-owner resolution and async_set_user's adoption scan), and an occupancy inferred from the value database must not redirect which user a write targets. Co-Authored-By: Claude Opus 5 (1M context) Entire-Checkpoint: 23adf956e0cc --- .../lock_code_manager/providers/zwave_js.py | 57 +++++- tests/providers/zwave_js/test_provider.py | 186 +++++++++++++++++- 2 files changed, 239 insertions(+), 4 deletions(-) diff --git a/custom_components/lock_code_manager/providers/zwave_js.py b/custom_components/lock_code_manager/providers/zwave_js.py index 3167b21d..adceb5c6 100644 --- a/custom_components/lock_code_manager/providers/zwave_js.py +++ b/custom_components/lock_code_manager/providers/zwave_js.py @@ -18,6 +18,7 @@ from zwave_js_server.const import CommandClass, NodeStatus from zwave_js_server.const.command_class.access_control import UserCredentialType from zwave_js_server.const.command_class.lock import ( + ATTR_CODE_SLOT, ATTR_IN_USE, LOCK_USERCODE_PROPERTY, LOCK_USERCODE_STATUS_PROPERTY, @@ -29,7 +30,7 @@ ) from zwave_js_server.exceptions import BaseZwaveJSServerError, NotFoundError from zwave_js_server.model.node import Node -from zwave_js_server.util.lock import get_usercode +from zwave_js_server.util.lock import get_usercode, get_usercodes from homeassistant.components.zwave_js import lock_helpers from homeassistant.components.zwave_js.const import ( @@ -314,6 +315,60 @@ async def async_get_users(self) -> list[User]: ) return list(users_by_id.values()) + async def async_get_usercodes(self) -> dict[int, SlotCredential]: + """ + Project to slots, then rescue occupied slots the unified read left valueless. + + Deliberately overrides the projection rather than ``async_get_users``: + the user list also drives the write paths (delete-owner resolution + and ``async_set_user``'s adoption scan), and an occupancy inferred + from the value database must not redirect which user a write + targets. Read-repair belongs to the read. + """ + codes = await super().async_get_usercodes() + return self._overlay_uc_occupancy(codes) + + def _overlay_uc_occupancy( + self, codes: dict[int, SlotCredential] + ) -> dict[int, SlotCredential]: + """ + Upgrade empty slots that User Code CC reports occupied to ``unreadable``. + + When a lock reports ``userIdStatus`` occupied while withholding the + code itself, node-zwave-js's User Code CC adapter drops the + credential from the unified read rather than returning it with no + value. The slot then projects to ``empty()``, which tells the sync + layer the write definitively did not land -- so it rewrites, fails + to confirm again, and the circuit breaker suspends a slot whose + Personal Identification Number works on the keypad (issue #1397). + + ``userIdStatus`` is the occupancy authority and the credential + supplies only the value, so an occupied slot with no credential is + ``unreadable`` -- "something is here, we cannot read it" -- never + ``empty``. Consequences of that split: + + - A cleared slot reports ``AVAILABLE``, so clears still confirm. + - Only ``empty`` entries are upgraded; a readable credential always + outranks the fallback, so a healthy lock is untouched. + - ``in_use`` is compared against ``True`` explicitly: ``None`` means + the value database holds no status for the slot, which is absence + of evidence, not evidence of occupancy. + + Read-only against the driver's value database (no device traffic), + and only for nodes advertising User Code CC -- a native User + Credential CC lock reports its own credentials and needs no + fallback. + """ + if not self._node_advertises_user_code_cc(): + return codes + for code_slot in get_usercodes(self.node): + slot = code_slot[ATTR_CODE_SLOT] + if code_slot.get(ATTR_IN_USE) is not True: + continue + if codes.get(slot, SlotCredential.empty()).is_empty: + codes[slot] = SlotCredential.unreadable() + return codes + async def async_get_capabilities(self) -> LockCapabilities: """ Report the lock's user/credential capabilities via the unified API. diff --git a/tests/providers/zwave_js/test_provider.py b/tests/providers/zwave_js/test_provider.py index 63786bb9..a7b10b11 100644 --- a/tests/providers/zwave_js/test_provider.py +++ b/tests/providers/zwave_js/test_provider.py @@ -295,12 +295,18 @@ async def test_async_get_usercodes_returns_projection_with_managed_slots( (starting from empty), and overlays any Personal Identification Number credentials returned by async_get_users. With no users on the lock, all managed slots are present and empty. + + Managed slots 3 and 4 are used deliberately: the fixture reports them + ``userIdStatus=AVAILABLE``, so the User Code CC occupancy fallback has + nothing to contribute and the plain empty projection is what's under + test here. Slots 1 and 2 are ENABLED in the fixture and are covered by + ``test_async_get_usercodes_reports_occupied_uc_slot_as_unreadable``. """ lcm_entry = MockConfigEntry( domain=DOMAIN, data={ CONF_LOCKS: [zwave_js_lock.lock.entity_id], - CONF_SLOTS: {"1": {}, "2": {}}, + CONF_SLOTS: {"3": {}, "4": {}}, }, ) lcm_entry.add_to_hass(hass) @@ -310,8 +316,8 @@ async def test_async_get_usercodes_returns_projection_with_managed_slots( codes = await zwave_js_lock.async_get_usercodes() - assert codes[1] == SlotCredential.empty() - assert codes[2] == SlotCredential.empty() + assert codes[3] == SlotCredential.empty() + assert codes[4] == SlotCredential.empty() async def test_async_get_usercodes_overlays_pin_credentials( @@ -325,6 +331,135 @@ async def test_async_get_usercodes_overlays_pin_credentials( When the lock has a user with a PIN credential at slot 1, the result maps slot 1 to the readable credential value. + + Slot 3 stands in for the unoccupied managed slot because the fixture + reports it ``userIdStatus=AVAILABLE``; a readable credential on slot 1 + already outranks the User Code CC occupancy fallback. + """ + lcm_entry = MockConfigEntry( + domain=DOMAIN, + data={ + CONF_LOCKS: [zwave_js_lock.lock.entity_id], + CONF_SLOTS: {"1": {}, "3": {}}, + }, + ) + lcm_entry.add_to_hass(hass) + mock_access_control.get_users_cached.return_value = [ + UserData( + user_id=1, + active=True, + user_type=UserCredentialUserType.GENERAL, + user_name="alice", + ), + ] + mock_access_control.get_all_credentials_cached.return_value = [ + CredentialData( + user_id=1, + type=UserCredentialType.PIN_CODE, + slot=1, + data="9999", + ), + ] + + codes = await zwave_js_lock.async_get_usercodes() + + assert codes[1] == SlotCredential.known("9999") + assert codes[3] == SlotCredential.empty() + + +async def test_async_get_usercodes_reports_occupied_uc_slot_as_unreadable( + hass: HomeAssistant, + zwave_js_lock: ZWaveJSLock, + mock_access_control: MagicMock, + mock_lock_helpers: dict, +) -> None: + """ + Regression test for issue #1397: an occupied slot must not project to empty. + + When a lock reports ``userIdStatus`` occupied but withholds the code + value, node-zwave-js's User Code CC adapter omits the credential from + ``get_all_credentials`` entirely. Projecting that to ``empty()`` tells + the sync layer the write definitively did not land, which drives a + non-converging write/verify loop until the circuit breaker suspends the + slot -- even though the Personal Identification Number works on the + keypad. + + Occupancy comes from ``userIdStatus``; only the *value* is unknown, so + the slot must project to ``unreadable()``. The fixture reports slot 2 + ENABLED, and the unified read here returns the user without any + credential. + """ + lcm_entry = MockConfigEntry( + domain=DOMAIN, + data={ + CONF_LOCKS: [zwave_js_lock.lock.entity_id], + CONF_SLOTS: {"2": {}, "3": {}}, + }, + ) + lcm_entry.add_to_hass(hass) + mock_access_control.get_users_cached.return_value = [ + UserData( + user_id=2, + active=True, + user_type=UserCredentialUserType.GENERAL, + user_name="lcm:2:weshley", + ), + ] + mock_access_control.get_all_credentials_cached.return_value = [] + + codes = await zwave_js_lock.async_get_usercodes() + + assert codes[2] == SlotCredential.unreadable() + # Slot 3 is AVAILABLE on the lock, so a cleared slot still reads empty -- + # otherwise a clear could never confirm. + assert codes[3] == SlotCredential.empty() + + +async def test_async_get_usercodes_reports_occupied_slot_with_no_user_as_unreadable( + hass: HomeAssistant, + zwave_js_lock: ZWaveJSLock, + mock_access_control: MagicMock, + mock_lock_helpers: dict, +) -> None: + """ + An occupied User Code CC slot surfaces even when the unified read reports no user. + + The User Code CC adapter can omit the implicit user as well as the + credential when the value is withheld. The fallback works off the slot + projection rather than the user list precisely so it does not depend on + a user surviving that read. + """ + lcm_entry = MockConfigEntry( + domain=DOMAIN, + data={ + CONF_LOCKS: [zwave_js_lock.lock.entity_id], + CONF_SLOTS: {"1": {}, "3": {}}, + }, + ) + lcm_entry.add_to_hass(hass) + mock_access_control.get_users_cached.return_value = [] + mock_access_control.get_all_credentials_cached.return_value = [] + + codes = await zwave_js_lock.async_get_usercodes() + + assert codes[1] == SlotCredential.unreadable() + assert codes[3] == SlotCredential.empty() + + +async def test_async_get_usercodes_readable_credential_outranks_uc_occupancy( + hass: HomeAssistant, + zwave_js_lock: ZWaveJSLock, + mock_access_control: MagicMock, + mock_lock_helpers: dict, +) -> None: + """ + The unified credential wins over the User Code CC occupancy fallback. + + The fallback only fills the gap where no credential of the requested + type came back. A slot that reports both must keep the readable value, + or every occupied slot on a healthy lock would regress to unreadable + and stop reconciling against the configured Personal Identification + Number. """ lcm_entry = MockConfigEntry( domain=DOMAIN, @@ -341,6 +476,12 @@ async def test_async_get_usercodes_overlays_pin_credentials( user_type=UserCredentialUserType.GENERAL, user_name="alice", ), + UserData( + user_id=2, + active=True, + user_type=UserCredentialUserType.GENERAL, + user_name="bob", + ), ] mock_access_control.get_all_credentials_cached.return_value = [ CredentialData( @@ -349,11 +490,50 @@ async def test_async_get_usercodes_overlays_pin_credentials( slot=1, data="9999", ), + CredentialData( + user_id=2, + type=UserCredentialType.PIN_CODE, + slot=2, + data="1234", + ), ] codes = await zwave_js_lock.async_get_usercodes() assert codes[1] == SlotCredential.known("9999") + assert codes[2] == SlotCredential.known("1234") + + +async def test_async_get_usercodes_skips_uc_occupancy_without_user_code_cc( + hass: HomeAssistant, + zwave_js_lock: ZWaveJSLock, + mock_access_control: MagicMock, + mock_lock_helpers: dict, +) -> None: + """ + A pure User Credential CC lock never consults the User Code CC value database. + + Such a lock reports its own credentials natively, so an empty slot is + genuinely empty. Reading occupancy from User Code CC values that the + node does not even advertise would invent occupancy from stale or + unrelated cache entries -- despite the fixture reporting slots 1 and 2 + ENABLED, both must stay empty here. + """ + lcm_entry = MockConfigEntry( + domain=DOMAIN, + data={ + CONF_LOCKS: [zwave_js_lock.lock.entity_id], + CONF_SLOTS: {"1": {}, "2": {}}, + }, + ) + lcm_entry.add_to_hass(hass) + mock_access_control.get_users_cached.return_value = [] + mock_access_control.get_all_credentials_cached.return_value = [] + + with patch.object(ZWaveJSLock, "_node_advertises_user_code_cc", return_value=False): + codes = await zwave_js_lock.async_get_usercodes() + + assert codes[1] == SlotCredential.empty() assert codes[2] == SlotCredential.empty()