Zigbee Lock: Support SLGA Migration#2939
Open
hcarter-775 wants to merge 6 commits intomainfrom
Open
Conversation
First, I had AI do the following: - Rename subdrivers: using-old-capabilities -> legacy-handlers, promote using-new-capabilities handlers to top-level defaults - Make new capability handling the driver default; legacy-handlers subdriver overrides for pre-migration devices only - Top-level vendor subdrivers (yale, samsungsds, yale-fingerprint-lock) now only activate for migrated devices to avoid double-dispatch - Remove duplicate lock-without-codes top-level subdriver (handled entirely within legacy-handlers since these devices never migrate) - Add init lifecycle handler to legacy-handlers to populate lockCodes state for pre-migration devices on driver restart Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Then, I cleaned up the driver further by fixing the git understanding of what files were made in what year, and by introducing small changes to altogether remove some sub-subdrivers from the legacy subdriver, and vice versa.
|
Duplicate profile check: Passed - no duplicate profiles detected. |
|
Invitation URL: |
Test Results 72 files 510 suites 0s ⏱️ Results for commit ee7c5fc. ♻️ This comment has been updated with latest results. |
|
Minimum allowed coverage is Generated by 🐒 cobertura-action against ee7c5fc |
hcarter-775
requested review from
aleclorimer,
cbaumler,
cjswedes,
ctowns,
nand-nor and
tpmanley
tpmanley
reviewed
May 8, 2026
| local init = function(driver, device) | ||
| lock_utils.reload_tables(device) | ||
| device.thread:call_with_delay(15, function(d) | ||
| reload_all_codes(device) |
Contributor
There was a problem hiding this comment.
Do we have to reload all codes every time or can we put a guard on this so it's only done in the cases where it's needed?
Contributor
Author
There was a problem hiding this comment.
Seems that all of the actual sends in this function are gated on if the cache is actually empty, so I think that's effectively done.
tpmanley
reviewed
May 8, 2026
|
|
||
| if (credential_index == device:get_field(lock_utils.CHECKING_CODE)) then | ||
| -- the credential we're checking has arrived | ||
| local last_slot = device:get_latest_state("main", capabilities.lockCredentials.ID, |
Contributor
There was a problem hiding this comment.
Are we guaranteed to have pinUsersSupported state at this point or do we need to handle the case where it's not set yet?
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
3 participants
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.
Description of Change
Follow-up to #2937, where I've responded to some comments that myself and others have left in order to clarify certain functionality.
Note on structure: The legacy-handlers subdriver keeps some subdrivers that have specific non-conformant behavior that relates specifically to lockCodes. This includes lock-without-codes, which no longer needs to exist in the modern implementation, as well as the yale-fingerprint-lock subdriver, which sets the lockCodes maxCodes state. Similarly, the yale subdriver has lockCodes-specific behavior as well.
However, upon analysis it was clear that the sub-sub-driver yale-bad-battery-reporter subdriver was 1. not unique to yale, and 2. was distinct from any lockCodes behavior, so it was pulled out into the now default subdrivers. Also, the samsungds subdriver had minimal differences between the old and new handling, so I condensed them.
The other primary thing that was removed from the now-legacy-handlers subdriver is the old added handler, for the following 2 reasons. 1. Generally, devices should not be added to this driver any longer, and 2. We now use the added main driver implementation to initially set the migrated keyword to true.
I find the fact that the entire migrated state depends on a keyword set only once in added to be a little precarious, but nothing stands out to me initially at this not working.
Summary of Completed Tests
Migration process, plus lockUsers/lockCredentials functionality, has been tested on-device with two different kwikset locks.
Unit tests added (I renamed some of the previous tests to be
_legacy, to clarify that they are testing pre-migration functionality.