fix(matter): fall back to slot-only userName when lock rejects canonical tag - #1246
Merged
Merged
Conversation
Production regression on raman.office's Matter lock: even with the 4.0.1 MatterError catch, set_lock_user keeps failing with ``InteractionModelError: InvalidCommand (0x85)`` because the lock's firmware restricts userName to alphanumeric characters and rejects the colons in the canonical ``lcm:<slot>:`` tag. The Matter spec itself (1.4, DoorLock 5.2.4.36) does not restrict userName charset, but many lock firmwares add this constraint silently. Without the fallback, 4.0.1 routes the error through LockOperationFailed -> retry -> circuit breaker -> slot_suspended. The lock keeps rejecting the same name, the breaker keeps tripping, no PIN ever gets written. Fix: on MatterError, retry once with the slot-only fallback ``str(slot)``. The tolerant parser added in #1238 already recognizes digit-only names as the slot binding (originally for length-constrained locks); this just exercises the same path for charset-constrained ones. The slot identity survives subsequent reads because parse_tag("1") returns (1, ""). Per-lock divergence is handled correctly. Each provider runs its own find-or-create-by-tag against its own lock independently. If lock A accepts ``lcm:1:Raman`` and lock B falls back to ``1``, both still project to LCM slot 1 via the tolerant parser. The only visible divergence is the lock's own keypad display (firmware-side); LCM's own UI is unaffected because slot display names come from slot config, not the lock-side name. Applied to both UPDATE (rename) and CREATE (allocate) paths: * UPDATE: try canonical -> on MatterError, try slot-only -> on second failure, tolerate (the user record is still valid; only the rename was lost; subsequent credential write proceeds against the existing user_index). * CREATE: try canonical -> on MatterError, try slot-only -> on second failure, raise LockOperationFailed (we need an allocated user_index for the credential write). Regression tests cover the four cases: fallback succeeds on UPDATE, fallback succeeds on CREATE, both fail on UPDATE (tolerated), both fail on CREATE (LockOperationFailed). The existing test_set_user_update_tolerates_* tests now exercise the "both attempts fail" branch with their original error type, with assertions updated from assert_called_once() to call_count == 2. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
github-actions
Bot
added
python
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1246 +/- ##
=======================================
Coverage 96.98% 96.99%
=======================================
Files 53 53
Lines 6171 6181 +10
Branches 461 461
=======================================
+ Hits 5985 5995 +10
Misses 186 186
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
Contributor
There was a problem hiding this comment.
Pull request overview
This PR addresses a production regression in the Matter provider where some lock firmwares reject the canonical lcm:<slot>: username tag due to non-alphanumeric character restrictions, causing set_lock_user to fail repeatedly and preventing PIN writes. The change introduces a one-time retry using a slot-only username (str(slot)) so the tolerant tag parser can still recover the slot identity on subsequent reads.
Changes:
- Matter provider: on
set_lock_userfailure, retry once with slot-only username for both UPDATE (tolerate if both fail) and CREATE (raiseLockOperationFailedonly after both fail). - Tests: add new CREATE/UPDATE fallback scenarios and update existing tolerance tests to reflect the new two-attempt behavior.
- Logging: add diagnostic logs around canonical rejection and fallback behavior.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
custom_components/lock_code_manager/providers/matter.py |
Adds slot-only fallback retry for set_lock_user on UPDATE and CREATE paths. |
tests/providers/matter/test_provider.py |
Adds/updates tests to cover fallback success and both-attempts-fail behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+451
to
+468
| slot_only_name = str(slot) | ||
| try: | ||
| result = await set_lock_user( | ||
| client, | ||
| node, | ||
| user_index=None, | ||
| user_name=slot_only_name, | ||
| ) | ||
| except MatterError as fallback_err: | ||
| # Second attempt also failed -- the lock is rejecting more | ||
| # than just the charset. Surface as LockOperationFailed so | ||
| # the seam routes through retry rather than the catchall | ||
| # suspend path (which would create a spurious repair issue). | ||
| raise LockOperationFailed( | ||
| f"Matter set_lock_user failed for {self.lock.entity_id} " | ||
| f"on both canonical and slot-only fallback names: " | ||
| f"canonical={err}, slot-only={fallback_err}" | ||
| ) from fallback_err |
Comment on lines
+2399
to
+2401
| user = User(user_id=1, name="lcm:1:Alice") | ||
| with ( | ||
| patch.object( |
Comment on lines
+2471
to
+2473
| user = User(user_id=2, name="lcm:2:Updated Name") | ||
| with ( | ||
| patch.object( |
Comment on lines
+414
to
+417
| LOGGER.info( | ||
| "Lock %s: lock rejected canonical tag %r for slot %s " | ||
| "(%s); renamed to slot-only %r so the slot binding " | ||
| "survives the read", |
8 tasks
raman325
added a commit
that referenced
this pull request
Jun 11, 2026
… caplog level (#1248) Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Proposed change
Production regression on a real Matter lock (`lock.raman_office`): even after the 4.0.1 `MatterError` catch (#1245), `set_lock_user` keeps failing with `InteractionModelError: InvalidCommand (0x85)`. The lock firmware silently restricts `userName` to alphanumeric characters and rejects the colons in the canonical `lcm::` tag.
Matter spec position: 1.4 Application Cluster spec section 5.2.4.36 SetUser command defines `UserName` as type `string` (UTF-8, max 10 chars) with no explicit charset restriction. The restriction is enforced by individual lock firmwares (Yale, Aqara, etc. have been documented to do this).
Without the fallback: 4.0.1's `MatterError → LockOperationFailed` mapping routes through retry → circuit breaker → `slot_suspended_*` repair issue. The lock keeps rejecting the same name forever; no PIN ever gets written.
Fix
On `MatterError` from `set_lock_user`, retry once with the slot-only fallback (`str(slot)`). The tolerant parser added in #1238 already recognizes digit-only names as the slot binding (originally for length-constrained locks); this just exercises the same path for charset-constrained ones. `parse_tag("1")` returns `(1, "")` — the slot identity survives subsequent reads.
Applied to both paths:
Cross-lock divergence
raman asked: if lock A in the config entry accepts `lcm:1:Raman` and lock B falls back to `1`, is that handled correctly?
Yes. Each provider runs its own find-or-create-by-tag against its own lock independently. Both lock-side names project to LCM slot 1 via the tolerant parser:
Subsequent reads of either lock find the right user via `_find_user_index_for_slot(1)`. The only visible divergence is the lock's own keypad/firmware display (lock A shows "Raman", lock B shows "1"). LCM's own UI is unaffected because slot display names come from slot config, not the lock-side userName.
Type of change
Test plan
🤖 Generated with Claude Code