Skip to content

Update new-matter-lock driver #1911

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.

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

Merged
merged 4 commits into from
Mar 24, 2025

Conversation

HunsupJung
Copy link
Collaborator

@HunsupJung HunsupJung commented Feb 5, 2025

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

  1. Issue
    User is allowed to lock or unlock from the device card if door lock status is unlatched, but user is not allowed to do the same from Plugin.

  2. Solution
    Add Visible Condition to the profiles that have unlatched status

  3. Changes
    Add Visible Condition to the profiles that have unlatched status
    Add some profiles for unlatch feature

Summary of Completed Tests

Copy link

github-actions bot commented Feb 5, 2025

Duplicate profile check: Passed - no duplicate profiles detected.

Copy link

github-actions bot commented Feb 5, 2025

Channel deleted.

Copy link

github-actions bot commented Feb 5, 2025

Test Results

   64 files  ±0    415 suites  ±0   0s ⏱️ ±0s
2 109 tests +9  2 109 ✅ +9  0 💤 ±0  0 ❌ ±0 
3 601 runs  +9  3 601 ✅ +9  0 💤 ±0  0 ❌ ±0 

Results for commit b3091c7. ± Comparison against base commit 3dd1b23.

This pull request removes 3 and adds 12 tests. Note that renamed tests count towards both.
Test profile change when BatChargeLevel and BatPercentRemaining attributes are available
Test profile change when BatChargeLevel attribute is available
Test profile change when attributes related to BAT feature is not available.
Test lock profile change when BatChargeLevel and BatPercentRemaining attributes are available
Test lock profile change when BatChargeLevel attribute is available
Test lock profile change when attributes related to BAT feature is not available.
Test lock-unlatch profile change when BatChargeLevel and BatPercentRemaining attributes are available
Test lock-unlatch profile change when BatChargeLevel attribute is available
Test lock-unlatch profile change when attributes related to BAT feature is not available.
Test lock-user-pin profile change when BatChargeLevel and BatPercentRemaining attributes are available
Test lock-user-pin profile change when BatChargeLevel attribute is available
Test lock-user-pin profile change when attributes related to BAT feature is not available.
Test lock-user-pin-schedule-unlatch profile change when BatChargeLevel and BatPercentRemaining attributes are available
…

♻️ This comment has been updated with latest results.

Copy link

github-actions bot commented Feb 5, 2025

File Coverage
All files 90%
/home/runner/work/SmartThingsEdgeDrivers/SmartThingsEdgeDrivers/drivers/SmartThings/matter-lock/src/lock_utils.lua 98%
/home/runner/work/SmartThingsEdgeDrivers/SmartThingsEdgeDrivers/drivers/SmartThings/matter-lock/src/init.lua 91%
/home/runner/work/SmartThingsEdgeDrivers/SmartThingsEdgeDrivers/drivers/SmartThings/matter-lock/src/new-matter-lock/init.lua 90%

Minimum allowed coverage is 90%

Generated by 🐒 cobertura-action against b3091c7

@HunsupJung HunsupJung force-pushed the fix/unmatching-visible-status-in-matter-lock branch from ad6655a to 6ebe1a5 Compare February 5, 2025 09:49
@hcarter-775
Copy link
Contributor

Can you add a unit test to ensure the battery profiling works as expected for these new profiles?

@hcarter-775
Copy link
Contributor

@HunsupJung
Copy link
Collaborator Author

@hcarter-775
Yes, I will close this PR.
#1880

@HunsupJung HunsupJung force-pushed the fix/unmatching-visible-status-in-matter-lock branch 2 times, most recently from 4a8a036 to ac1c841 Compare February 25, 2025 11:35
@HunsupJung
Copy link
Collaborator Author

@hcarter-775
When I check the result of "Run driver tests", the error occurs as like below

    lua: .../Project/hub-core/lib/scripting-engine/lua_libs/yaml.lua:287: expected comma
    stack traceback:
    	[C]: in function 'error'
    	.../Project/hub-core/lib/scripting-engine/lua_libs/yaml.lua:287: in method 'expect'
    	.../Project/hub-core/lib/scripting-engine/lua_libs/yaml.lua:537: in method 'parseInlineHash'
    	.../Project/hub-core/lib/scripting-engine/lua_libs/yaml.lua:347: in method 'parse'
    	.../Project/hub-core/lib/scripting-engine/lua_libs/yaml.lua:516: in method 'parseHash'
    	.../Project/hub-core/lib/scripting-engine/lua_libs/yaml.lua:351: in method 'parse'
    	.../Project/hub-core/lib/scripting-engine/lua_libs/yaml.lua:560: in method 'parseList'
    	.../Project/hub-core/lib/scripting-engine/lua_libs/yaml.lua:345: in method 'parse'
    	.../Project/hub-core/lib/scripting-engine/lua_libs/yaml.lua:516: in method 'parseHash'
    	.../Project/hub-core/lib/scripting-engine/lua_libs/yaml.lua:351: in method 'parse'
    	.../Project/hub-core/lib/scripting-engine/lua_libs/yaml.lua:516: in method 'parseHash'
    	.../Project/hub-core/lib/scripting-engine/lua_libs/yaml.lua:351: in method 'parse'
    	.../Project/hub-core/lib/scripting-engine/lua_libs/yaml.lua:516: in method 'parseHash'
    	.../Project/hub-core/lib/scripting-engine/lua_libs/yaml.lua:351: in function <.../Project/hub-core/lib/scripting-engine/lua_libs/yaml.lua:320>
    	(...tail calls...)
    	...lib/scripting-engine/lua_libs/integration_test/utils.lua:111: in function 'integration_test.utils.get_profile_definition'
    	...atter-lock/src/test/test_matter_lock_unlatch_battery.lua:72: in main chunk
    	[C]: in ?

As above error log, There's a problem that lua_libs doesn't support deviceConfig in profile files. We need to update lua_libs to solve this issue.
In my opinion, It would be better that we merge this PR first without additional unit test and then update unit test file after updating lua_libs.

@HunsupJung HunsupJung force-pushed the fix/unmatching-visible-status-in-matter-lock branch from ac1c841 to bb2ccaf Compare March 10, 2025 11:54
@nickolas-deboom
Copy link
Contributor

@hcarter-775 When I check the result of "Run driver tests", the error occurs as like below

    lua: .../Project/hub-core/lib/scripting-engine/lua_libs/yaml.lua:287: expected comma
    stack traceback:
    	[C]: in function 'error'
    	.../Project/hub-core/lib/scripting-engine/lua_libs/yaml.lua:287: in method 'expect'
    	.../Project/hub-core/lib/scripting-engine/lua_libs/yaml.lua:537: in method 'parseInlineHash'
    	.../Project/hub-core/lib/scripting-engine/lua_libs/yaml.lua:347: in method 'parse'
    	.../Project/hub-core/lib/scripting-engine/lua_libs/yaml.lua:516: in method 'parseHash'
    	.../Project/hub-core/lib/scripting-engine/lua_libs/yaml.lua:351: in method 'parse'
    	.../Project/hub-core/lib/scripting-engine/lua_libs/yaml.lua:560: in method 'parseList'
    	.../Project/hub-core/lib/scripting-engine/lua_libs/yaml.lua:345: in method 'parse'
    	.../Project/hub-core/lib/scripting-engine/lua_libs/yaml.lua:516: in method 'parseHash'
    	.../Project/hub-core/lib/scripting-engine/lua_libs/yaml.lua:351: in method 'parse'
    	.../Project/hub-core/lib/scripting-engine/lua_libs/yaml.lua:516: in method 'parseHash'
    	.../Project/hub-core/lib/scripting-engine/lua_libs/yaml.lua:351: in method 'parse'
    	.../Project/hub-core/lib/scripting-engine/lua_libs/yaml.lua:516: in method 'parseHash'
    	.../Project/hub-core/lib/scripting-engine/lua_libs/yaml.lua:351: in function <.../Project/hub-core/lib/scripting-engine/lua_libs/yaml.lua:320>
    	(...tail calls...)
    	...lib/scripting-engine/lua_libs/integration_test/utils.lua:111: in function 'integration_test.utils.get_profile_definition'
    	...atter-lock/src/test/test_matter_lock_unlatch_battery.lua:72: in main chunk
    	[C]: in ?

As above error log, There's a problem that lua_libs doesn't support deviceConfig in profile files. We need to update lua_libs to solve this issue. In my opinion, It would be better that we merge this PR first without additional unit test and then update unit test file after updating lua_libs.

It looks like the issue is from the "version": 1, line within the visibleCondition block in the profiles. Removing that line or adding quotes around the "1" lets the tests pass. So it may just be this line that the lua libs is having trouble parsing with.

@HunsupJung
Copy link
Collaborator Author

@nickolas-deboom
Thank you for solving the error.

@hcarter-775
I added unit test for new profiles. Please review this PR again.

@lelandblue lelandblue requested a review from z-michel March 11, 2025 16:19
version: 1
visibleCondition: {
"capability": "lock",
"version": "1",
Copy link
Contributor

@nickolas-deboom nickolas-deboom Mar 12, 2025

Choose a reason for hiding this comment

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

Hey @HunsupJung , thank you for adding unit tests. Have you tested these changes on a physical lock after making the update to visibleCondition?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, you can see the test result of at following link.
https://smartthings.atlassian.net/wiki/spaces/DLT/pages/3713564956/Nuki+door+lock+testing#Test-to-add-visibleCondition

I added vid/pid of Nuki to this PR on the test. If we want Nuki operates as like the result, vid/pid of Nuki need to be added to new-matter-lock driver.

@HunsupJung HunsupJung merged commit 3e79b08 into main Mar 24, 2025
12 checks passed
@HunsupJung HunsupJung deleted the fix/unmatching-visible-status-in-matter-lock branch March 24, 2025 22:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants