[Aqara/Locks] Improvement of credential info management #2712

seojune79 · 2026-01-16T04:32:06Z

Check all that apply

Type of Change

WWST Certification Request
- If this is your first time contributing code:
  - I have reviewed the README.md file
  - I have reviewed the CODE_OF_CONDUCT.md file
  - I have signed the CLA
- I plan on entering a WWST Certification Request or have entered a request through the WWST Certification console at developer.smartthings.com
Bug fix
New feature
Refactor

Checklist

I have performed a self-review of my code
I have commented my code in hard-to-understand areas
I have verified my changes by testing with a device or have communicated a plan for testing
I am adding new behavior, such as adding a sub-driver, and have added and run new unit tests to cover the new behavior

Description of Change

Summary of Completed Tests

…eplacement.

github-actions · 2026-01-16T04:33:54Z

Test Results

71 files 483 suites 0s ⏱️
2 493 tests 2 493 ✅ 0 💤 0 ❌
4 279 runs 4 279 ✅ 0 💤 0 ❌

Results for commit c7a5f13.

♻️ This comment has been updated with latest results.

github-actions · 2026-01-16T04:33:55Z

File	Coverage
All files	`57%`	❌
/home/runner/work/SmartThingsEdgeDrivers/SmartThingsEdgeDrivers/drivers/Aqara/aqara-lock/src/init.lua	`53%`	❌
/home/runner/work/SmartThingsEdgeDrivers/SmartThingsEdgeDrivers/drivers/Aqara/aqara-lock/src/credential_utils.lua	`66%`	❌

Minimum allowed coverage is 90%

Generated by 🐒 cobertura-action against c7a5f13

kdbang · 2026-01-19T01:50:17Z

linter fail

Checking drivers/Aqara/aqara-lock/src/credential_utils.lua 1 warning

    drivers/Aqara/aqara-lock/src/credential_utils.lua:99:1: (W611) line contains only whitespace

seojune79 · 2026-01-19T03:32:40Z

linter fail

Checking drivers/Aqara/aqara-lock/src/credential_utils.lua 1 warning

    drivers/Aqara/aqara-lock/src/credential_utils.lua:99:1: (W611) line contains only whitespace

I have removed the whitespace.

cjswedes · 2026-01-20T20:31:40Z

drivers/Aqara/aqara-lock/src/credential_utils.lua

 local HOST_COUNT = "__host_count"
+local PERSIST_DATA = "__persist_area"
+
+credential_utils.attrCopy = function(table)


I believe this is an unnecessary function. utils.deep_copy is already doing this recursively for the whole table.

Thank you for your feedback. I have revised the implementation to manage data at the table unit level now.

drivers/Aqara/aqara-lock/src/credential_utils.lua

cjswedes · 2026-01-20T20:36:54Z

drivers/Aqara/aqara-lock/src/credential_utils.lua

+  local credentialInfoTable = device:get_field(PERSIST_DATA)
+  if credentialInfoTable ~= nil then
+    device:emit_event(lockCredentialInfo.credentialInfo(credentialInfoTable, { visibility = { displayed = false } }))
+  end


Should there be an else clause where we call backup_data in the case that there is nothing persisted? I think this will help ensure existing devices have the state for this capability persisted.

Could you please elaborate on this part a bit more? Are you suggesting that adding a check for backup_data would be beneficial if capability information could be deleted during normal use? I'd appreciate further explanation to ensure my understanding is correct.

This sync function is called on device_init. Devices out there right now, do not have PERSIST_DATA set on the device object yet. We want any device that does not yet have this persisted, to have it persisted as soon as we detect that it hasnt been persisted. So my suggestion is, if table is nil, then we should read the current state and persist it in the driver datastore with credential_utils.backup_data. If we do not detect and persist in the device_init handler, then we are waiting for a capability command to come in that will cause the persistence, which may or may not happen before a hub failover.

Thank you for your kind explanation; I now fully understand the situation. As per your suggestion, I have modified the code to read and set the current value if PERSIST_DATA is nil.

cjswedes

This is looking good to me. Has there been any testing done?

seojune79 · 2026-01-28T00:37:27Z

This is looking good to me. Has there been any testing done?

Thank you for your positive feedback!
Regarding testing, while my environment makes it challenging to directly verify the specific scenarios where issues were previously identified, such as hub replacement or main/sub changes, I have confirmed that the code is operating normally and without issues through the following steps:

Registered an Aqara door lock using the SmartThings Aqara-Lock driver and proceeded with user additions and other related functions.
Added specific log code to the current Pull Request (PR) to monitor and confirm normal operation, then packaged and installed this on the hub.
Replaced and utilized the driver via the SmartThings app.
Repeated step 3 for 2-3 cycles to ensure consistency.

(I designed test cases based on the common understanding that the init handler is executed during hub replacement or main/sub changes.)

Based on these checks, I am confident that the added code is unlikely to introduce any unforeseen side effects or malfunctions.

kdbang · 2026-01-28T04:09:09Z

@seojune79 Can you share the invitation link including this PR. I will test switch over scenario.

seojune79 · 2026-01-28T04:40:12Z

Can you share the invitation link including this PR. I will test switch over scenario.

Here is the invitation link and driver information for the switch-over scenario testing:
Invitation Link for Acceptance Server

Name: Aqara Acceptance Invite
URL: https://bestow-regionala.api.stacceptance.com/invite/O8n4kEe9ZPrl

Basic Information
────────────────────────────────────────────────────
 Driver Id    c8c7deb2-49da-4d00-9bfb-731e6bd7c480  
 Name         Aqara Lock
 Version      2026-01-28T04:29:16.132505912
 Package Key  aqara-lock
 Description  SmartThings driver for Aqara Doorlock 
 Permissions  zigbee
────────────────────────────────────────────────────

kdbang · 2026-01-28T06:29:07Z

@cjswedes @seojune79 I tested switch over scenario many times with this PR. but I can't find any issue. It seems that the credentialInfo are saved and moved well.

greens · 2026-01-28T17:19:21Z

drivers/Aqara/aqara-lock/src/credential_utils.lua

+        credentialResource[key] = value
+    end
+    return credentialResource


spacing here is inconsistent

I will fix it.

greens

Would now be a good time to add more unit test coverage?

seojune79 · 2026-01-29T01:48:38Z

Would now be a good time to add more unit test coverage?

Yes, I will take this opportunity to give it a try.

seojune79 · 2026-01-30T01:41:48Z

Due to system restrictions, it is not possible to use the shared key arbitrarily, which limits the creation of test cases. I kindly ask for your understanding regarding this matter. I will add test cases once the system has been improved.

greens · 2026-01-30T04:10:53Z

Due to system restrictions, it is not possible to use the shared key arbitrarily, which limits the creation of test cases. I kindly ask for your understanding regarding this matter. I will add test cases once the system has been improved.

Ah yes. Of course. I remember that now.

greens · 2026-01-30T04:11:13Z

Checking drivers/Aqara/aqara-lock/src/test/test_aqara_lock_L100.lua 4 warnings

    drivers/Aqara/aqara-lock/src/test/test_aqara_lock_L100.lua:4:7: (W211) unused variable cluster_base
    drivers/Aqara/aqara-lock/src/test/test_aqara_lock_L100.lua:5:7: (W211) unused variable data_types
    drivers/Aqara/aqara-lock/src/test/test_aqara_lock_L100.lua:20:7: (W211) unused variable PRI_ATTR
    drivers/Aqara/aqara-lock/src/test/test_aqara_lock_L100.lua:21:7: (W211) unused variable MFG_CODE

seojune79 · 2026-01-30T04:24:09Z

Checking drivers/Aqara/aqara-lock/src/test/test_aqara_lock_L100.lua 4 warnings

    drivers/Aqara/aqara-lock/src/test/test_aqara_lock_L100.lua:4:7: (W211) unused variable cluster_base
    drivers/Aqara/aqara-lock/src/test/test_aqara_lock_L100.lua:5:7: (W211) unused variable data_types
    drivers/Aqara/aqara-lock/src/test/test_aqara_lock_L100.lua:20:7: (W211) unused variable PRI_ATTR
    drivers/Aqara/aqara-lock/src/test/test_aqara_lock_L100.lua:21:7: (W211) unused variable MFG_CODE

unused variable

I have removed all unused variables.

seojune79 added 2 commits January 16, 2026 10:17

Update the system to manage credentialInfoTable within the persist area

a8e635c

Improvement of credential info management during Hub switch-over or r…

d7d7b71

…eplacement.

Remove whitespace

478df5e

cjswedes reviewed Jan 20, 2026

View reviewed changes

drivers/Aqara/aqara-lock/src/credential_utils.lua Show resolved Hide resolved

cjswedes reviewed Jan 20, 2026

View reviewed changes

seojune79 added 2 commits January 26, 2026 10:44

Revision of credential data management

eaad1cd

Modify to save the current data if PERSIST_DATA is nil.

4cc53ea

cjswedes reviewed Jan 27, 2026

View reviewed changes

cjswedes requested review from greens and tpmanley January 28, 2026 16:28

cjswedes approved these changes Jan 28, 2026

View reviewed changes

greens reviewed Jan 28, 2026

View reviewed changes

Fix code indentation

a983b4a

Adding test cases

ea494de

Remove the unused variables

c7a5f13

[Aqara/Locks] Improvement of credential info management #2712

Are you sure you want to change the base?

[Aqara/Locks] Improvement of credential info management #2712

Conversation

seojune79 commented Jan 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Type of Change

Checklist

Description of Change

Summary of Completed Tests

Uh oh!

github-actions bot commented Jan 16, 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 Jan 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kdbang commented Jan 19, 2026

Uh oh!

seojune79 commented Jan 19, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

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!

seojune79 commented Jan 28, 2026

Uh oh!

kdbang commented Jan 28, 2026

Uh oh!

seojune79 commented Jan 28, 2026

Uh oh!

kdbang commented Jan 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

greens left a comment

Choose a reason for hiding this comment

Uh oh!

seojune79 commented Jan 29, 2026

Uh oh!

seojune79 commented Jan 30, 2026

Uh oh!

greens commented Jan 30, 2026

Uh oh!

greens commented Jan 30, 2026

Uh oh!

seojune79 commented Jan 30, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

seojune79 commented Jan 16, 2026 •

edited

Loading

github-actions bot commented Jan 16, 2026 •

edited

Loading

github-actions bot commented Jan 16, 2026 •

edited

Loading

kdbang commented Jan 28, 2026 •

edited

Loading