Add new-matter-lock driver

[HunsupJung]

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

Adds a new sub-driver with updates to the door lock functionality. Leverages new capabilities and some new design architecture for the Aqara door locks.

Summary of Completed Tests

Completed Testing 9/27
Completed Testing 10/18

[github-actions]

Duplicate profile check: Passed - no duplicate profiles detected.

[github-actions]

Channel deleted.

[github-actions]

Test Results

   64 files    397 suites   0s ⏱️
1 974 tests 1 974 ✅ 0 💤 0 ❌
3 409 runs  3 409 ✅ 0 💤 0 ❌

Results for commit 37709e8.

♻️ This comment has been updated with latest results.

[github-actions]

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

Minimum allowed coverage is 90%

Generated by 🐒 cobertura-action against 37709e8

[tpmanley]

I'm still working my way through testing this. Can you add unit tests as well?

[lelandblue]

Hello @HunsupJung before merging this code , please provide tested results as communicated via slack.

[HunsupJung]

@lelandblue @ctowns
I would like to merge this patch before the next release. I hope you can help me to meet the schedule.

[ctowns]

@HunsupJung could you add more unit tests for the new-matter-lock driver? Currently the code coverage for this driver is very low, so we should add additional test cases to increase the code coverage. You should be able to copy the test cases from the test_matter_lock_cota.lua and test_matter_lock_codes.lua files as a example, and make the necessary changes to account for the new capabilities (such as using lockAlarm instead of tamperAlert, and including the new capabilities such as `lockCredentials'). The structure should be similar to what is done in the other test files, but you will need to swap in the new capabilities. If you need additional help with this, please let me know and I can help!

[HunsupJung]

@ctowns
Okay, I will update unit test for new door lock driver tomorrow.

[hcarter-775]

@lelandblue I think I added the testing you requested that's blocking the merge. Was there anything else you needed for this?

[tpmanley]

When I test with a lock that uses COTA and the initial attempt to set the COTA PIN fails with a "duplicate" error, then it gets stuck in a loop logging this message and failing to ever retry setting the COTA PIN.

[ctowns]

@HunsupJung I understand how you are using these fields to save information to be used in a different function, but could you help me understand why we need to do it this way? Do the commands themselves not have the relevant information that we can use in the handler? It seems this is the case for things like the ClearUser command, but do we really need to save all of this information or can be pull some of it from the command args and ib elements?

Also, it seems like there could be issues if two commands that rely on a field like USER_INDEX are processed at the same time. For example, if one command sets USER_INDEX to 1 and then before we handle that response, another command sets USER_INDEX to 2 , the original value would be overwritten and could cause problems in handling the first command that expected USER_INDEX to be 1. Are there cases where this could happen?

[HunsupJung]

Also, it seems like there could be issues if two commands that rely on a field like USER_INDEX are processed at the same time. For example, if one command sets USER_INDEX to 1 and then before we handle that response, another command sets USER_INDEX to 2 , the original value would be overwritten and could cause problems in handling the first command that expected USER_INDEX to be 1. Are there cases where this could happen?

Therefore, I used busy_state so that only one command is proceed at a time.

[tpmanley]

Should this be "notFound" instead of "failure"?

[tpmanley]

I guess not because "notFound" is not a valid value for the statusCode property in the capability

[HunsupJung]

Yes, there's no notFound value for capability

[ctowns]

Thank you for writing the new unit tests @HunsupJung ! I think this looks like it is ready to go.

One last thing - were you able to run the regression tests listed here with the new code changes? I know you have been doing a lot of testing over the past few days, so I think many of these cases have been covered but I just want to make sure the testing is good to go.

Otherwise, I think this PR is ready to go. Thank you for your hard work on this 💪 !

[HunsupJung]

@ctowns
Thank you for checking. I updated the regression tests here.
It's all thanks to the help of you and the US team.

[ctowns]

Thank you for your hard work on this @HunsupJung !

Testing results provided, team should continue to review results.