Zigbee Lock: Support SLGA Migration by hcarter-775 · Pull Request #2956 · SmartThingsCommunity/SmartThingsEdgeDrivers

hcarter-775 · 2026-05-11T17:12:25Z

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 a kwikset lock.

Various Unit tests added.

github-actions · 2026-05-11T17:13:12Z

Duplicate profile check: Passed - no duplicate profiles detected.

github-actions · 2026-05-11T17:13:56Z

Invitation URL:
https://bestow-regional.api.smartthings.com/invite/QLMOrymVRRj3

github-actions · 2026-05-11T17:15:04Z

Test Results

72 files ± 0 516 suites +6 0s ⏱️ ±0s
2 983 tests +125 2 983 ✅ +125 0 💤 ±0 0 ❌ ±0
4 850 runs +122 4 850 ✅ +122 0 💤 ±0 0 ❌ ±0

Results for commit 1ee5d47. ± Comparison against base commit 853b02b.

This pull request removes 2 and adds 127 tests. Note that renamed tests count towards both.

Device added data lock codes population, device response produces no events
Device init with new data should populate fields

Device called 'migrate' command
Device init function handler
Late PIN_CODE_ADDED after addCredential: user and credential already exist; complete no-op
Late PIN_CODE_ADDED when both user and credential already exist: both add_entry calls are no-ops
Late PIN_CODE_CHANGED after updateCredential: not handled by notification handler, no events emitted
Late PIN_CODE_DELETED after deleteCredential: credential and user already removed; no-op
Max user code number report should be handled for migrated locks
ProgrammingEventNotification PIN_CODE_ADDED for different user while busy is processed as manual event
ProgrammingEventNotification PIN_CODE_ADDED while addCredential is in flight acts as failsafe
ProgrammingEventNotification PIN_CODE_ADDED while not busy syncs user and credential entries
…

♻️ This comment has been updated with latest results.

github-actions · 2026-05-11T17:15:06Z

File	Coverage
All files	`94%`	✅
/home/runner/work/SmartThingsEdgeDrivers/SmartThingsEdgeDrivers/drivers/SmartThings/zigbee-lock/src/lock_handlers/zigbee_responses.lua	`89%`	❌
/home/runner/work/SmartThingsEdgeDrivers/SmartThingsEdgeDrivers/drivers/SmartThings/zigbee-lock/src/lock_handlers/capabilities.lua	`90%`	✅
/home/runner/work/SmartThingsEdgeDrivers/SmartThingsEdgeDrivers/drivers/SmartThings/zigbee-lock/src/lazy_load_subdriver.lua	`57%`	❌
/home/runner/work/SmartThingsEdgeDrivers/SmartThingsEdgeDrivers/drivers/SmartThings/zigbee-lock/src/configurations.lua	`95%`	✅
/home/runner/work/SmartThingsEdgeDrivers/SmartThingsEdgeDrivers/drivers/SmartThings/zigbee-lock/src/init.lua	`99%`	✅
/home/runner/work/SmartThingsEdgeDrivers/SmartThingsEdgeDrivers/drivers/SmartThings/zigbee-lock/src/legacy-handlers/yale/init.lua	`92%`	✅
/home/runner/work/SmartThingsEdgeDrivers/SmartThingsEdgeDrivers/drivers/SmartThings/zigbee-lock/src/lock_utils/tables.lua	`98%`	✅
/home/runner/work/SmartThingsEdgeDrivers/SmartThingsEdgeDrivers/drivers/SmartThings/zigbee-lock/src/samsungsds/init.lua	`89%`	❌
/home/runner/work/SmartThingsEdgeDrivers/SmartThingsEdgeDrivers/drivers/SmartThings/zigbee-lock/src/legacy-handlers/init.lua	`95%`	✅

Minimum allowed coverage is 90%

Generated by 🐒 cobertura-action against 1ee5d47

tpmanley · 2026-05-12T13:30:53Z

+
+function CapabilityHandlers.refresh(driver, device, cmd)
+  device:refresh()
+  device:send(LockCluster.attributes.LockState:read(device))


Is the explicit read of LockState needed since I think that'd be covered by refresh?

Not sure, I'd have to look into it. This explicit read is in the current refresh function

I think this is because the LockState handling was added at a later date, so it wasn't in the default immediately.

So we can remove it now?

I'm really not sure. I might lean towards just leaving it as-is

I do not see a reporting configuration for the LockState attribute in the default handler for lock, so we should leave this read in because the default refresh reads the configured attributes.

tpmanley · 2026-05-12T13:48:50Z

+    [4] = "fingerprint",
+    [5] = "bluetooth",
+  }
+  if (op_event_source ~= OpEventSource.KEYPAD and (


The legacy handler doesn't have the parenthesis after the and so the logic is slightly changed. Looking at table 7-29 in ZCL8 it looks like AutoLock, ScheduleLock and ScheduleUnlock should only come from source Manual so it seems like if we were going to do a source check it would be for Manual instead of not Keypad. It's also possible the legacy code was written that way for a reason (like to handle a specific lock behavior). I'm not sure if there would be any harm in just ignoring the source altogether and treating any AutoLock, ScheduleLock and ScheduleUnlock events as "auto". That's slightly more permissive than the legacy code.

makes sense, works for me.

looks like a samsungsds test failed when I made that alteration.

It appears the fingerprint/bluetooth handling is also specific to the Samsung SDS, so I'm going to move the handler as-is to that subdriver, and will make the main driver implementation more generic/permissive.

cjswedes

I am a still a little bit confused on how credentials and users will work in the capabilities for zigbee locks because it seems fragile. The zigbee user_index maps directly to the smart things credentialIndex, and there should be a 1:1 mapping of credentialIndex to userIndex in the lockCredentials and lockUsers capabilities. Is that right? It seems like an artificial constraint, and somewhat unexpected: why even have the lockUsers capability on zigbee locks. It makes sense to me for the app to allow 1 user to have multiple credentials, even for zigbee locks; the users are just something that the ST platform maintains, and the lock is only concerned with credentials.

I really need to play with this in the app to see what the desired user experience is in the new world, but I am not sure I fully understand what the intended user experience is, and I worry we are implementing something that is not what stakeholders want.

cjswedes · 2026-05-12T18:34:18Z

Is it safe to rely on receiving an addCredential/addUser or a deleteCredential/deleteUser in pairs from the app in order to maintain the 1:1 credential to user mapping? If we receive a deleteUser command, we will delete the associated credential on the device and emit the corresponding commandResult, but the opposite is not true; is the split of SoT for the credentials being in the driver and the users in the app intentional?

A successful addCredential leads to an updateUser command, and yes, we can rely on the app sending deleteUser in response to deleteCredential.

One reason there is a distinction between these two is that a credential can be associated with a user we do not want to remove.

cjswedes · 2026-05-12T18:35:14Z

+  device:emit_event(LockUsers.users(users, { visibility = { displayed = false } }))
+  device:emit_event(LockCredentials.credentials(credentials, { visibility = { displayed = false } }))


We only ever emit a lockCredentials.credentials/lockUsers.users attribute event during migration, and rely on the commandResult attribute events to make sure the platform has the correct credentials capability state. It seems a bit brittle to rely on the device responses and commandResult event emissions being handled properly to maintain the lockUser and lockCredential capability state; perhaps on refresh/init we should be emitting the lockCredential.credentials and lockUser.users attributes based on the state we have in the driver.

That is what happens currently, see the table_utils.restore_from_persistent_store(device) function.

hcarter-775 · 2026-05-13T15:16:09Z

Hey @aleclorimer @cjswedes, looks like one of the tests passing now is failing in dev, and as far as I can tell it really shouldn't be. Would one of you mind taking a look?

Specifically, the check device:supports_capability(capabilities.lockCodes) is being evaluated as true after a generate_info_changed event to a profile without that capability.

It's not really an important test, the AI generated it and it's not a path that can actually happen today, but like it technically could

hcarter-775 · 2026-05-13T15:31:44Z

I am a still a little bit confused on how credentials and users will work in the capabilities for zigbee locks because it seems fragile. The zigbee user_index maps directly to the smart things credentialIndex, and there should be a 1:1 mapping of credentialIndex to userIndex in the lockCredentials and lockUsers capabilities. Is that right? It seems like an artificial constraint, and somewhat unexpected: why even have the lockUsers capability on zigbee locks. It makes sense to me for the app to allow 1 user to have multiple credentials, even for zigbee locks; the users are just something that the ST platform maintains, and the lock is only concerned with credentials.

@cjswedes I agree that much of this Users/Credentials state seems a little fragile. I am not 100% sure how to make it less fragile than it already was/is (in Matter Lock for example and in the initial versions of this work), so have mostly endeavored to make the system as clear as possible.

cjswedes · 2026-05-13T21:20:44Z

Hey @aleclorimer @cjswedes, looks like one of the tests passing now is failing in dev, and as far as I can tell it really shouldn't be. Would one of you mind taking a look?

Specifically, the check device:supports_capability(capabilities.lockCodes) is being evaluated as true after a generate_info_changed event to a profile without that capability.

It's not really an important test, the AI generated it and it's not a path that can actually happen today, but like it technically could

The integration test framework, does not actually mock out profile changes on the mock device objects, it just validates that the event was there. I think maybe delete the test since it is not a path that can really happen

cjswedes

I have given this a light test on my home, and an in depth review.

For the test, I started with the old lock driver created some codes, and then did the migration. The only unique thing i did was have non-consecutive codes stored on the device prior to migration, and that was handled well. Subsequent adds re-filled the unused slot.

hcarter-775 added the workinprogress label May 11, 2026

tpmanley reviewed May 12, 2026

View reviewed changes

cjswedes reviewed May 12, 2026

View reviewed changes

CHAD-16364: Migrate Zigbee Lock to lockUsers/lockCredentials

1072439

hcarter-775 force-pushed the re-create/chad-16364 branch from cf83610 to 1072439 Compare May 13, 2026 20:46

hcarter-775 changed the title ~~Latest Zigbee Lock Migration~~ Zigbee Lock: Support SLGA Migration May 13, 2026

hcarter-775 added ai supported AI was used during the making of this work and removed workinprogress labels May 13, 2026

hcarter-775 mentioned this pull request May 13, 2026

Zigbee Lock: Support SLGA Migration #2939

Closed

hcarter-775 requested review from cjswedes, ctowns, nickolas-deboom and tpmanley May 13, 2026 20:51

hcarter-775 added 3 commits May 13, 2026 16:40

remove test that can't actually happen

b296ee2

add driverSwitched failsafe, change user/cred name

2c493f9

remove driverSwitched event

1ee5d47

cjswedes approved these changes May 14, 2026

View reviewed changes

		device:emit_event(LockUsers.users(users, { visibility = { displayed = false } }))
		device:emit_event(LockCredentials.credentials(credentials, { visibility = { displayed = false } }))

Conversation

hcarter-775 commented May 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description of Change

Summary of Completed Tests

Uh oh!

github-actions Bot commented May 11, 2026

Uh oh!

github-actions Bot commented May 11, 2026

Uh oh!

github-actions Bot commented May 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Test Results

Uh oh!

github-actions Bot commented May 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cjswedes left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hcarter-775 commented May 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hcarter-775 commented May 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cjswedes commented May 13, 2026

Uh oh!

cjswedes left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

hcarter-775 commented May 11, 2026 •

edited

Loading

github-actions Bot commented May 11, 2026 •

edited

Loading

github-actions Bot commented May 11, 2026 •

edited

Loading

hcarter-775 commented May 13, 2026 •

edited

Loading

hcarter-775 commented May 13, 2026 •

edited

Loading