Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
106 changes: 91 additions & 15 deletions custom_components/lock_code_manager/providers/zwave_js.py
Original file line number Diff line number Diff line change
Expand Up @@ -492,11 +492,6 @@ async def async_set_credential(
) from err
except HomeAssistantError as err:
key = getattr(err, "translation_key", None)
if key == "credential_rejected_duplicate":
raise DuplicateCodeError(
code_slot=credential.slot,
lock_entity_id=self.lock.entity_id,
) from err
if key == "credential_rejected_unknown":
_LOGGER.debug(
"Lock %s slot %s: driver returned ERROR_UNKNOWN; treating "
Expand All @@ -508,12 +503,28 @@ async def async_set_credential(
credential.slot,
err,
)
# No reconciliation read here: the seam's on-demand
# confirmation read hard-refreshes for every OPTIMISTIC
# write, so a single-slot read would be pure duplication.
return WriteResult.OPTIMISTIC
# Definitive rejection: pre-15.25.2 drivers never re-read the
# slot after a supervised failure, so an already-stale cache
# entry would stay wrong forever (see
# _async_uc_reconcile_value_db).
await self._async_uc_reconcile_value_db(credential.slot)
if key == "credential_rejected_duplicate":
raise DuplicateCodeError(
code_slot=credential.slot,
lock_entity_id=self.lock.entity_id,
) from err
raise CodeRejectedError(
code_slot=credential.slot,
lock_entity_id=self.lock.entity_id,
reason=str(err),
) from err
# Pre-15.25.2 drivers never persist a supervised success to the
# value database (see _async_uc_reconcile_value_db).
await self._async_uc_reconcile_value_db(credential.slot)
return WriteResult.CONFIRMED

async def async_delete_credential(self, ref: CredentialRef) -> bool:
Expand All @@ -531,6 +542,11 @@ async def async_delete_credential(self, ref: CredentialRef) -> bool:
f"delete credential slot {ref.slot} failed: {err}"
) from err
except HomeAssistantError as err:
# Same supervised-failure staleness as the set path (see
# _async_uc_reconcile_value_db). Success needs no read: the
# driver clears its cached User Code CC values on a
# successful delete since 15.24.3 (zwave-js/zwave-js#8866).
await self._async_uc_reconcile_value_db(ref.slot)
raise LockOperationFailed(
f"delete credential slot {ref.slot} failed: {err}"
) from err
Expand Down Expand Up @@ -637,20 +653,34 @@ def _on_credential_deleted(self, event: dict[str, Any]) -> None:
# providers, so without this shim those changes would never reach
# the coordinator and sync could not reconcile them.
#
# zwave-js/zwave-js#8930 (merged, unreleased) fixes this by making
# the access-control API the source of truth for User Code CC. It
# also makes these User Code CC values internal, so once a driver
# with it ships the value events stop arriving and the unified
# events we already subscribe to take over -- the shim goes dormant
# on its own, no LCM change needed. To remove it once the minimum
# supported driver includes #8930:
# zwave-js/zwave-js#8930 (merged to the v16 branch, unreleased)
# fixes this by making the access-control API the source of truth
# for User Code CC. It also makes these User Code CC values
# internal -- and the driver does not emit value events for
# internal value IDs -- so once a driver with it ships the value
# events stop arriving and the unified events we already subscribe
# to take over: the shim goes dormant on its own, no LCM change
# needed.
#
# The shim also carries ``_async_uc_reconcile_value_db``, a
# post-write single-slot read that bridges a second gap fixed in
# driver 15.25.2 (zwave-js/zwave-js#8927): before that, supervised
# User Code CC writes through the unified API never persisted to
# (success) or reconciled (failure) the driver's value database,
# so cached reads served stale slots until the next report.
#
# To remove the whole section once the minimum supported driver
# includes #8930 (which implies #8927):
#
# 1. Delete everything from this comment through
# ``_uc_slot_in_use``.
# ``_async_uc_reconcile_value_db``.
# 2. Delete the ``_node_advertises_user_code_cc`` branch in
# ``setup_push_subscription`` (and its docstring paragraph).
# 3. Delete the shim tests in
# ``tests/providers/zwave_js/test_events.py`` (grep ``_uc_``)
# 3. Delete the ``_async_uc_reconcile_value_db`` call sites in
# ``async_set_credential`` and ``async_delete_credential``.
# 4. Delete the shim tests in
# ``tests/providers/zwave_js/test_events.py`` and
# ``tests/providers/zwave_js/test_provider.py`` (grep ``_uc_``)
# and restore ``_EXPECTED_PUSH_UNSUB_COUNT`` to 3.
# ------------------------------------------------------------------

Expand Down Expand Up @@ -737,6 +767,52 @@ def _uc_slot_in_use(self, code_slot: int) -> bool | None:
return None
return in_use if isinstance(in_use, bool) else None

async def _async_uc_reconcile_value_db(self, code_slot: int) -> None:
"""
Read one slot back from the device so the driver's cache converges.

Pre-15.25.2 drivers never persist a supervised User Code CC write
to the value database (success) and never re-read the slot after a
failure, so every later cached read -- including LCM's own initial
load after a restart -- serves the pre-write state. This fresh
single-slot read through the unified API forces the driver to
query the device: on the User Code CC dispatch path the solicited
report repairs the value database and doubles as a push through
the report shim above; on a User Credential CC lock the driver
dispatches natively and the read is merely redundant.

Best-effort by design: the write already concluded (the caller
returns or raises on its own evidence), so a failed read must not
change the outcome -- the next hard refresh or sync tick reconciles
instead.
"""
if not self._node_advertises_user_code_cc():
return
try:
await self.node.access_control.get_credential(
UserCredentialType.PIN_CODE, code_slot
)
except BaseZwaveJSServerError as err:
Comment thread
raman325 marked this conversation as resolved.
_LOGGER.debug(
"Lock %s slot %s: post-write reconciliation read failed (%s); "
"leaving it to the next hard refresh or sync tick",
self.lock.entity_id,
code_slot,
err,
)
except Exception:
# Broad by design, mirroring the coordinator's confirmation-read
# backstop: two call sites run inside except clauses mapping the
# write outcome to typed errors, and an escaping exception here
# would replace that typed error and derail the seam's handling.
_LOGGER.exception(
"Lock %s slot %s: unexpected error during post-write "
"reconciliation read; leaving it to the next hard refresh "
"or sync tick",
self.lock.entity_id,
code_slot,
)

@callback
def teardown_push_subscription(self) -> None:
"""Unsubscribe from credential change events."""
Expand Down
1 change: 1 addition & 0 deletions tests/providers/zwave_js/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -470,5 +470,6 @@ def mock_access_control(lock_schlage_be469: Node):
ac.get_all_credentials = AsyncMock(return_value=[])
ac.set_credential = AsyncMock(return_value=SetCredentialResult.OK)
ac.delete_credential = AsyncMock(return_value=SetCredentialResult.OK)
ac.get_credential = AsyncMock(return_value=None)
with patch.object(type(lock_schlage_be469), "access_control", ac):
yield ac
224 changes: 224 additions & 0 deletions tests/providers/zwave_js/test_provider.py
Original file line number Diff line number Diff line change
Expand Up @@ -1457,3 +1457,227 @@ async def test_get_client_state_not_ready_when_driver_missing(

assert ready is False
assert "driver not ready" in reason


# ---------------------------------------------------------------------------
# Post-write reconciliation read tests (delete with the report shim; grep _uc_)
# ---------------------------------------------------------------------------


def _pin_credential(slot: int, pin: str) -> Credential:
"""Build a PIN credential for reconciliation-read tests."""
return Credential(
type=CredentialType.PIN, slot=slot, state=SlotCredential.known(pin)
)


async def test_set_credential_success_triggers_reconcile_read(
zwave_js_lock: ZWaveJSLock,
mock_access_control: MagicMock,
mock_lock_helpers: dict,
) -> None:
"""A confirmed set reads the slot back so the driver's value DB converges."""
result = await zwave_js_lock.async_set_credential(
user_id=1,
credential=_pin_credential(2, "5678"),
pin="5678",
name=None,
source="sync",
)

assert result is WriteResult.CONFIRMED
mock_access_control.get_credential.assert_awaited_once_with(
UserCredentialType.PIN_CODE, 2
)


async def test_set_credential_optimistic_skips_reconcile_read(
zwave_js_lock: ZWaveJSLock,
mock_access_control: MagicMock,
mock_lock_helpers: dict,
) -> None:
"""An OPTIMISTIC set skips the read; the seam's confirmation read covers it."""
mock_lock_helpers["async_set_credential"].side_effect = HomeAssistantError(
translation_key="credential_rejected_unknown"
)

result = await zwave_js_lock.async_set_credential(
user_id=1,
credential=_pin_credential(2, "5678"),
pin="5678",
name=None,
source="sync",
)

assert result is WriteResult.OPTIMISTIC
mock_access_control.get_credential.assert_not_called()


async def test_set_credential_rejection_triggers_reconcile_read(
zwave_js_lock: ZWaveJSLock,
mock_access_control: MagicMock,
mock_lock_helpers: dict,
) -> None:
"""A definitive rejection reads the slot back before raising."""
mock_lock_helpers["async_set_credential"].side_effect = HomeAssistantError(
translation_key="credential_rejected_manufacturer_rules"
)

with pytest.raises(CodeRejectedError):
await zwave_js_lock.async_set_credential(
user_id=1,
credential=_pin_credential(4, "2222"),
pin="2222",
name=None,
source="sync",
)

mock_access_control.get_credential.assert_awaited_once_with(
UserCredentialType.PIN_CODE, 4
)


async def test_set_credential_duplicate_triggers_reconcile_read(
zwave_js_lock: ZWaveJSLock,
mock_access_control: MagicMock,
mock_lock_helpers: dict,
) -> None:
"""A duplicate rejection reads the slot back before raising."""
mock_lock_helpers["async_set_credential"].side_effect = HomeAssistantError(
translation_key="credential_rejected_duplicate"
)

with pytest.raises(DuplicateCodeError):
await zwave_js_lock.async_set_credential(
user_id=1,
credential=_pin_credential(3, "1111"),
pin="1111",
name=None,
source="sync",
)

mock_access_control.get_credential.assert_awaited_once_with(
UserCredentialType.PIN_CODE, 3
)


async def test_set_credential_disconnect_skips_reconcile_read(
zwave_js_lock: ZWaveJSLock,
mock_access_control: MagicMock,
mock_lock_helpers: dict,
) -> None:
"""A transient command failure skips the read; the node is unreachable anyway."""
mock_lock_helpers["async_set_credential"].side_effect = FailedZWaveCommand(
"failed", 1, "error"
)

with pytest.raises(LockDisconnected):
await zwave_js_lock.async_set_credential(
user_id=1,
credential=_pin_credential(2, "5678"),
pin="5678",
name=None,
source="sync",
)

mock_access_control.get_credential.assert_not_called()


async def test_set_credential_skips_reconcile_read_without_user_code_cc(
zwave_js_lock: ZWaveJSLock,
mock_access_control: MagicMock,
mock_lock_helpers: dict,
) -> None:
"""A pure User Credential CC lock never needs the reconciliation read."""
with patch.object(ZWaveJSLock, "_node_advertises_user_code_cc", return_value=False):
result = await zwave_js_lock.async_set_credential(
user_id=1,
credential=_pin_credential(2, "5678"),
pin="5678",
name=None,
source="sync",
)

assert result is WriteResult.CONFIRMED
mock_access_control.get_credential.assert_not_called()


async def test_reconcile_read_failure_does_not_change_write_outcome(
zwave_js_lock: ZWaveJSLock,
mock_access_control: MagicMock,
mock_lock_helpers: dict,
) -> None:
"""A failed reconciliation read is swallowed; the write result stands."""
mock_access_control.get_credential.side_effect = FailedZWaveCommand(
"failed", 1, "error"
)

result = await zwave_js_lock.async_set_credential(
user_id=1,
credential=_pin_credential(2, "5678"),
pin="5678",
name=None,
source="sync",
)

assert result is WriteResult.CONFIRMED


async def test_delete_credential_success_skips_reconcile_read(
zwave_js_lock: ZWaveJSLock,
mock_access_control: MagicMock,
mock_lock_helpers: dict,
) -> None:
"""A successful delete needs no read: the driver clears its cache (#8866)."""
result = await zwave_js_lock.async_delete_credential(
CredentialRef(user_id=1, type=CredentialType.PIN, slot=2)
)

assert result is True
mock_access_control.get_credential.assert_not_called()


async def test_delete_credential_failure_triggers_reconcile_read(
zwave_js_lock: ZWaveJSLock,
mock_access_control: MagicMock,
mock_lock_helpers: dict,
) -> None:
"""A failed delete reads the slot back before raising."""
mock_lock_helpers["async_delete_credential"].side_effect = HomeAssistantError(
"delete failed"
)

with pytest.raises(LockOperationFailed):
await zwave_js_lock.async_delete_credential(
CredentialRef(user_id=1, type=CredentialType.PIN, slot=5)
)

mock_access_control.get_credential.assert_awaited_once_with(
UserCredentialType.PIN_CODE, 5
)


async def test_reconcile_read_unexpected_error_does_not_replace_typed_error(
zwave_js_lock: ZWaveJSLock,
mock_access_control: MagicMock,
mock_lock_helpers: dict,
) -> None:
"""An unexpected reconcile-read error must not replace the mapped typed error.

The rejection call sites run inside except clauses; if the read raised
there, the seam would see the raw exception instead of DuplicateCodeError
and mishandle the rejection.
"""
mock_lock_helpers["async_set_credential"].side_effect = HomeAssistantError(
translation_key="credential_rejected_duplicate"
)
mock_access_control.get_credential.side_effect = RuntimeError("boom")

with pytest.raises(DuplicateCodeError):
await zwave_js_lock.async_set_credential(
user_id=1,
credential=_pin_credential(3, "1111"),
pin="1111",
name=None,
source="sync",
)
Loading