Add Dynamic Battery Support for Base-Lock Profiled Door Lock Devices #1660

hcarter-775 · 2024-10-01T15:55:02Z

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

Jira ticket showing mis-profiling of a bridged door lock: https://smartthings.atlassian.net/browse/MTR-821
Further, there is a general ticket on adding dynamic profiling to this driver: https://smartthings.atlassian.net/browse/CHAD-13948

This logic will permit dynamic profiling on the base-lock profile depending on the Power Source cluster, and will ignore devices that do not auto-profile as base-lock.

Summary of Completed Tests

Unit tests
Bridged Switchbot device - Passed
VDA Door Lock - Passed
VDA Door Lock (Lock Codes) - Passed
Yale Assure Matter Lock - Passed

github-actions · 2024-10-01T15:55:34Z

Duplicate profile check: Passed - no duplicate profiles detected.

github-actions · 2024-10-01T15:56:35Z

Channel deleted.

github-actions · 2024-10-01T15:58:34Z

File	Coverage
All files	`91%`	✅
/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 e0179b2

github-actions · 2024-10-01T15:59:29Z

Test Results

64 files 400 suites 0s ⏱️
1 988 tests 1 988 ✅ 0 💤 0 ❌
3 429 runs 3 429 ✅ 0 💤 0 ❌

Results for commit e0179b2.

♻️ This comment has been updated with latest results.

drivers/SmartThings/matter-lock/src/init.lua

nickolas-deboom · 2024-10-01T18:55:07Z

This looks good to me, but I was just thinking that one potential drawback with this approach is that each PR for new WWST device fingerprints would also have to include an update to the list of fingerprinted devices in do_configure (the one I mentioned in my other comment for example). Maybe that is no big deal but I thought I'd mention it

hcarter-775 · 2024-10-01T19:15:29Z

Yeah @nickolas-deboom I agree, that's definitely a downside. Unfortunately, there's not really any better method in place of solving this issue atm. I figured we would inform Alissa of this in the next Risk Analysis meeting and then you and I can help monitor this as well until we put a better solution in place.

hcarter-775 · 2024-10-01T19:19:13Z

@nickolas-deboom I also think this example might get some more proactive desire to fix this situation in a larger way

hcarter-775 · 2024-10-01T19:28:20Z

And I just thought of this, but another point of note here is that there is a new and improved matter Lock driver coming soon, so this issue may be extremely temporary.

nickolas-deboom · 2024-10-01T19:33:25Z

Those are good points, and I can't think of a better way to handle this either, so I think this is a good solution for now, especially given it might be temporary since there are improvements coming to this driver as you mentioned.

ctowns

This looks good, however, there are multiple other places in the matter-lock driver that update the device's profile, for example:

SmartThingsEdgeDrivers/drivers/SmartThings/matter-lock/src/init.lua

Line 573 in 44087fd

device:try_update_metadata({profile = "lock-without-codes"})

SmartThingsEdgeDrivers/drivers/SmartThings/matter-lock/src/init.lua

Line 200 in 44087fd

    
           device:try_update_metadata({profile = "base-lock", provisioning_state = "PROVISIONED"})

SmartThingsEdgeDrivers/drivers/SmartThings/matter-lock/src/init.lua

Line 213 in 44087fd

    
           device:try_update_metadata({profile = "nonfunctional-lock", provisioning_state = "NONFUNCTIONAL"})

Two of the instances have to do with setting a profile for a non-functional lock, however in the device_added function, I think we'd also need to do something similar in the case that the device is going to be switched to the lock-without-codes profile. Essentially, you'd have to just add a check that the device would switch to lock-without-codes-batteryLevel/nobattery depending on what it supports like you have done above.

Also, when recovering from a non-functional state, we'd probably want to copy what you've done and make sure the device joins to base-lock plus whatever is appropriate for it's battery situation.

ctowns · 2024-10-01T20:38:49Z

Also before merging, this change should be tested with both the new bridged the device as well as regression tested against an existing lock.

hcarter-775 · 2024-10-01T20:43:57Z

Yes, I agreee Cooper. I will add the dynamic battery profiling logic to those places as well. And I have already reached out on the affected ticket to get some testing with the new device.

ctowns

Looks good! I would suggest waiting for the new matter lock PR to merge first and then rebasing these changes on top of it. This PR should be easier to rebase and we want the new matter lock PR to be as frictionless as possible a this point.

Huangxiangjie

If device Power Source 0x002F Features is BAT, But do not support 0x000D BatTimeRemaining

Huangxiangjie · 2024-11-12T08:57:45Z

drivers/SmartThings/matter-lock/src/init.lua

+
+  -- if not fingerprinted, dynamically configure base-lock profile based on Power Source cluster checks
+  local power_source_eps = device:get_endpoints(clusters.PowerSource.ID)
+  local battery_feature_eps = device:get_endpoints(clusters.PowerSource.ID, {feature_bitmap = clusters.PowerSource.types.PowerSourceFeature.BATTERY})


@hcarter-775
I am confused that if device do not have attribute 0x000D BatTimeRemaining, then device will also matched "-batteryLevel". In this case, device will not report BatTimeRemaining, if plugin has problem with device status not updated error, for example like this.

hcarter-775 requested review from tpmanley, ctowns and nickolas-deboom October 1, 2024 15:55

hcarter-775 force-pushed the bridged-lock branch from 5978d4c to 01b03f1 Compare October 1, 2024 15:57

nickolas-deboom reviewed Oct 1, 2024

View reviewed changes

drivers/SmartThings/matter-lock/src/init.lua Outdated Show resolved Hide resolved

hcarter-775 force-pushed the bridged-lock branch from 01b03f1 to 8849b80 Compare October 1, 2024 19:22

nickolas-deboom approved these changes Oct 1, 2024

View reviewed changes

ctowns reviewed Oct 1, 2024

View reviewed changes

hcarter-775 changed the title ~~Add Dynamic Battery Support for Non-WWST Door Lock Devices~~ Add Dynamic Battery Support for Base-Lock Profiled Door Lock Devices Oct 2, 2024

ctowns approved these changes Oct 18, 2024

View reviewed changes

hcarter-775 added 7 commits October 21, 2024 10:13

initial: add do configure logic and edit fingerprint

518b4a2

add: logic to not configure wwst devices

5b60ef2

remove: unused capability definition

e255c96

add: newest fingerprint device

cebeaf4

implement: check for capabilities of base-lock

43a966d

add: new profile, updated battery check logic

b3c94b3

add: info_changed event

ed1dd1b

hcarter-775 force-pushed the bridged-lock branch from ae37c50 to ed1dd1b Compare October 21, 2024 15:13

fix test, remove aqara references

e0179b2

hcarter-775 merged commit b800c9c into main Oct 21, 2024
12 checks passed

hcarter-775 deleted the bridged-lock branch October 21, 2024 15:55

Huangxiangjie reviewed Nov 12, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Dynamic Battery Support for Base-Lock Profiled Door Lock Devices #1660

Add Dynamic Battery Support for Base-Lock Profiled Door Lock Devices #1660

hcarter-775 commented Oct 1, 2024 •

edited

Loading

github-actions bot commented Oct 1, 2024

github-actions bot commented Oct 1, 2024 •

edited

Loading

github-actions bot commented Oct 1, 2024 •

edited

Loading

github-actions bot commented Oct 1, 2024 •

edited

Loading

nickolas-deboom commented Oct 1, 2024 •

edited

Loading

hcarter-775 commented Oct 1, 2024

hcarter-775 commented Oct 1, 2024

hcarter-775 commented Oct 1, 2024

nickolas-deboom commented Oct 1, 2024

ctowns left a comment

ctowns commented Oct 1, 2024

hcarter-775 commented Oct 1, 2024

ctowns left a comment

Huangxiangjie left a comment

Huangxiangjie Nov 12, 2024

Add Dynamic Battery Support for Base-Lock Profiled Door Lock Devices #1660

Add Dynamic Battery Support for Base-Lock Profiled Door Lock Devices #1660

Conversation

hcarter-775 commented Oct 1, 2024 • edited Loading

Type of Change

Checklist

Description of Change

Summary of Completed Tests

github-actions bot commented Oct 1, 2024

github-actions bot commented Oct 1, 2024 • edited Loading

github-actions bot commented Oct 1, 2024 • edited Loading

github-actions bot commented Oct 1, 2024 • edited Loading

Test Results

nickolas-deboom commented Oct 1, 2024 • edited Loading

hcarter-775 commented Oct 1, 2024

hcarter-775 commented Oct 1, 2024

hcarter-775 commented Oct 1, 2024

nickolas-deboom commented Oct 1, 2024

ctowns left a comment

Choose a reason for hiding this comment

ctowns commented Oct 1, 2024

hcarter-775 commented Oct 1, 2024

ctowns left a comment

Choose a reason for hiding this comment

Huangxiangjie left a comment

Choose a reason for hiding this comment

Huangxiangjie Nov 12, 2024

Choose a reason for hiding this comment

hcarter-775 commented Oct 1, 2024 •

edited

Loading

github-actions bot commented Oct 1, 2024 •

edited

Loading

github-actions bot commented Oct 1, 2024 •

edited

Loading

github-actions bot commented Oct 1, 2024 •

edited

Loading

nickolas-deboom commented Oct 1, 2024 •

edited

Loading