Skip to content
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

Add batteryLevel capability to the matter lock driver #1608

Merged
merged 6 commits into from
Sep 5, 2024

Conversation

hcarter-775
Copy link
Contributor

@hcarter-775 hcarter-775 commented Aug 30, 2024

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: https://smartthings.atlassian.net/browse/CHAD-13915

batteryLevel is a capability that was added for Matter 1.2, and is useful for cases where the BatPercentRemaining attribute of the PowerSource cluster does not exist, but the BatChargeLevel attribute does.

Summary of Completed Tests

A real-life device example: https://smartthings.atlassian.net/browse/WWSTCERT-3507

  • This has been tested and works correctly on-device.
  • A new unit test has been added to ensure this works

@hcarter-775 hcarter-775 changed the title CHAD-12161 Add batteryLevel capability to the drivers CHAD-12161 Add batteryLevel capability to the matter drivers Aug 30, 2024
Copy link

Duplicate profile check: Passed - no duplicate profiles detected.

Copy link

github-actions bot commented Aug 30, 2024

Channel deleted.

Copy link

github-actions bot commented Aug 30, 2024

Test Results

   61 files    379 suites   0s ⏱️
1 852 tests 1 852 ✅ 0 💤 0 ❌
3 208 runs  3 208 ✅ 0 💤 0 ❌

Results for commit fce3333.

♻️ This comment has been updated with latest results.

Copy link

github-actions bot commented Aug 30, 2024

File Coverage
All files 93%
/home/runner/work/SmartThingsEdgeDrivers/SmartThingsEdgeDrivers/drivers/SmartThings/matter-lock/src/aqara-lock/init.lua 96%
/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 97%

Minimum allowed coverage is 90%

Generated by 🐒 cobertura-action against fce3333

@hcarter-775 hcarter-775 changed the title CHAD-12161 Add batteryLevel capability to the matter drivers Add batteryLevel capability to the matter lock driver Sep 4, 2024
@nickolas-deboom
Copy link
Contributor

These changes look good to me except I don't see logic in the driver that would cause a device to join the new profiles based on its supported capabilities - I'm just wondering how the new profiles would get selected?

@hcarter-775
Copy link
Contributor Author

hcarter-775 commented Sep 4, 2024

These changes look good to me except I don't see logic in the driver that would cause a device to join the new profiles based on its supported capabilities - I'm just wondering how the new profiles would get selected?

@nickolas-deboom yeah, right now there's not really any complex configuration logic in matter-lock, so the only way that most profiles in this driver (the new ones included) can be used is with manual WWST fingerprinting. There is some action item open about redoing this logic but for now, that's what we have.

For example, the WWST device that tested with these changes was fingerprinted with one of the new profiles.

Copy link
Contributor

@ctowns ctowns left a comment

Choose a reason for hiding this comment

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

LGTM! Is there a ticket for the eventual dynamic configuration of battLev devices?

@hcarter-775
Copy link
Contributor Author

LGTM! Is there a ticket for the eventual dynamic configuration of battLev devices?

Yes, there is. See this ticket. It's pretty barebones but it's on the docket.

@hcarter-775 hcarter-775 merged commit b9f8bfd into main Sep 5, 2024
12 checks passed
@hcarter-775 hcarter-775 deleted the CHAD-12161-add-batteryLevel-2 branch September 5, 2024 16:51
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.

4 participants