Skip to content

CHAD-16364 & CHAD-16365: Add lockUsers and lockCredentials to profiles and also implement lockCodes:migrate functionality. #2416

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

CHAD-16364 & CHAD-16365: Add lockUsers and lockCredentials to profiles and also implement lockCodes:migrate functionality. #2416

wants to merge 3 commits into from

Conversation

pegor-karoglanian
Copy link

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

@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@github-actions GitHub Actions
Copy link

Duplicate profile check: Passed - no duplicate profiles detected.

@github-actions GitHub Actions
Copy link

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

@github-actions GitHub Actions
Copy link

github-actions bot commented Sep 19, 2025 •
edited
Loading

Test Results

   71 files    454 suites   0s ⏱️
2 346 tests 2 346 ✅ 0 💤 0 ❌
3 955 runs  3 955 ✅ 0 💤 0 ❌

Results for commit c248678.

♻️ This comment has been updated with latest results.

@github-actions GitHub Actions
Copy link

github-actions bot commented Sep 19, 2025 •
edited
Loading

zigbee-lock_coverage.xml

File Coverage
All files 94% ✅
/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 94% ✅
/home/runner/work/SmartThingsEdgeDrivers/SmartThingsEdgeDrivers/drivers/SmartThings/zigbee-lock/src/lock_utils.lua 98% ✅
/home/runner/work/SmartThingsEdgeDrivers/SmartThingsEdgeDrivers/drivers/SmartThings/zigbee-lock/src/yale/init.lua 81% ❌

zwave-lock_coverage.xml

File Coverage
All files 88% ❌
/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 94% ✅
/home/runner/work/SmartThingsEdgeDrivers/SmartThingsEdgeDrivers/drivers/SmartThings/zigbee-lock/src/lock_utils.lua 98% ✅
/home/runner/work/SmartThingsEdgeDrivers/SmartThingsEdgeDrivers/drivers/SmartThings/zigbee-lock/src/yale/init.lua 81% ❌
/home/runner/work/SmartThingsEdgeDrivers/SmartThingsEdgeDrivers/drivers/SmartThings/zwave-lock/src/keywe-lock/init.lua 78% ❌
/home/runner/work/SmartThingsEdgeDrivers/SmartThingsEdgeDrivers/drivers/SmartThings/zwave-lock/src/samsung-lock/init.lua 94% ✅
/home/runner/work/SmartThingsEdgeDrivers/SmartThingsEdgeDrivers/drivers/SmartThings/zwave-lock/src/init.lua 83% ❌
/home/runner/work/SmartThingsEdgeDrivers/SmartThingsEdgeDrivers/drivers/SmartThings/zwave-lock/src/schlage-lock/init.lua 95% ✅
/home/runner/work/SmartThingsEdgeDrivers/SmartThingsEdgeDrivers/drivers/SmartThings/zwave-lock/src/zwave-alarm-v1-lock/init.lua 90% ✅
/home/runner/work/SmartThingsEdgeDrivers/SmartThingsEdgeDrivers/drivers/SmartThings/zwave-lock/src/apiv6_bugfix/init.lua 87% ❌

Minimum allowed coverage is 90%

Generated by 🐒 cobertura-action against c248678

greens
greens reviewed Sep 19, 2025
View reviewed changes
greens
greens reviewed Sep 19, 2025
View reviewed changes
@pegor-karoglanian pegor-karoglanian force-pushed the pegor/edgedrivers/lock_changes branch 2 times, most recently from b8168d4 to 36fa094 Compare September 29, 2025 20:25
@pegor-karoglanian pegor-karoglanian marked this pull request as ready for review September 29, 2025 20:26
@pegor-karoglanian pegor-karoglanian force-pushed the pegor/edgedrivers/lock_changes branch from 36fa094 to b303430 Compare September 29, 2025 20:47
greens
greens reviewed Sep 29, 2025
View reviewed changes
greens
greens reviewed Sep 29, 2025
View reviewed changes
@pegor-karoglanian pegor-karoglanian force-pushed the pegor/edgedrivers/lock_changes branch from 8c6dcd8 to 39cfed8 Compare September 30, 2025 19:32
@pegor-karoglanian pegor-karoglanian force-pushed the pegor/edgedrivers/lock_changes branch from 39cfed8 to 88d8d99 Compare September 30, 2025 19:32
@pegor-karoglanian pegor-karoglanian force-pushed the pegor/edgedrivers/lock_changes branch from ecdc2be to c248678 Compare September 30, 2025 22:29
tpmanley
tpmanley reviewed Sep 30, 2025
View reviewed changes
drivers/SmartThings/zigbee-lock/src/init.lua

table.sort(ordered_codes)
for index, code_slot in ipairs(ordered_codes) do
table.insert(lock_users, {userIndex = index, userType = "guest", userName = lock_codes[code_slot]})
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Zigbee supports other user types, and IIRC some locks like Yale do use them. It doesn't look like lockCodes kept track of that information, but could we try to get the type with the Get User Type command?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My gut says that we should keep it simple for the migration (i.e. limit device communication), but perhaps handle it in the new capability handlers.

greens
greens reviewed Sep 30, 2025
View reviewed changes
drivers/SmartThings/zwave-lock/src/init.lua
end

for code in pairs(lock_codes) do
table.insert(ordered_codes, code)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: spacing again

greens
greens reviewed Sep 30, 2025
View reviewed changes
drivers/SmartThings/zwave-lock/src/init.lua
Comment on lines +174 to +175
local min_code_len = device:get_latest_state("main", capabilities.lockCodes.ID, capabilities.lockCodes.minCodeLength.NAME)
local max_code_len = device:get_latest_state("main", capabilities.lockCodes.ID, capabilities.lockCodes.maxCodeLength.NAME)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the 4th (optional) argument to get_latest_state is a default value.

you could save yourself a few lines

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@greens greens greens left review comments

@tpmanley tpmanley tpmanley left review comments

@wkhenon wkhenon Awaiting requested review from wkhenon

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@pegor-karoglanian @CLAassistant @greens @tpmanley