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 additional Aqara Light Switch H2 models #1915

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

DongHoon-Ryu
Copy link
Contributor

  • 2 Buttons, 1 Channel (PID=0x1003)
  • 2 Buttons, 2 Channels (PID=0x1004)
  • 4 Buttons, 3 Channels (PID=0x1005)

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

Add additional Aqara Light Switch H2 models

Summary of Completed Tests

Check normal operation through test for existing model

 - 2 Buttons, 1 Channel (PID=0x1003)
 - 2 Buttons, 2 Channels (PID=0x1004)
 - 4 Buttons, 3 Channels (PID=0x1005)
Copy link

github-actions bot commented Feb 6, 2025

Copy link

github-actions bot commented Feb 6, 2025

Test Results

   64 files    409 suites   0s ⏱️
2 033 tests 2 033 ✅ 0 💤 0 ❌
3 512 runs  3 512 ✅ 0 💤 0 ❌

Results for commit 1df18d0.

♻️ This comment has been updated with latest results.

Copy link

github-actions bot commented Feb 6, 2025

File Coverage
All files 94%
/home/runner/work/SmartThingsEdgeDrivers/SmartThingsEdgeDrivers/drivers/SmartThings/matter-switch/src/aqara-cube/init.lua 96%
/home/runner/work/SmartThingsEdgeDrivers/SmartThingsEdgeDrivers/drivers/SmartThings/matter-switch/src/init.lua 96%
/home/runner/work/SmartThingsEdgeDrivers/SmartThingsEdgeDrivers/drivers/SmartThings/matter-switch/src/embedded-cluster-utils.lua 38%
/home/runner/work/SmartThingsEdgeDrivers/SmartThingsEdgeDrivers/drivers/SmartThings/matter-switch/src/eve-energy/init.lua 91%

Minimum allowed coverage is 90%

Generated by 🐒 cobertura-action against 1df18d0

Copy link
Contributor

@lelandblue lelandblue left a comment

Choose a reason for hiding this comment

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

Hey @DongHoon-Ryu I believe in order to make this PR functional, you would need to add fingerprints for these devices as well. I reviewed the fingerprints available in the MAIN branch of our repo and do not see fingerprints for these devices.

Could you please add them as well?

Also please reach out via slack as well to share the testing plans for these changes thank you kindly.

{ vendor_id = 0x115F, product_id = 0x1003, target_profile = "light-power-energy-powerConsumption" }, -- 2 Buttons, 1 Channel
{ vendor_id = 0x115F, product_id = 0x1004, target_profile = "light-power-energy-powerConsumption" }, -- 2 Buttons, 2 Channels
{ vendor_id = 0x115F, product_id = 0x1005, target_profile = "light-power-energy-powerConsumption" }, -- 4 Buttons, 3 Channels
{ vendor_id = 0x115F, product_id = 0x1008, target_profile = "light-power-energy-powerConsumption" }, -- 2 Buttons, 1 Channel
Copy link
Contributor

Choose a reason for hiding this comment

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

Are pids 1003 and 1008 both "2 Button, 1 Channel" devices?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, that's right. According to CSA's Certification, PID 0x1003 is for the US market and PID 0x1008 is for the EU market.

@DongHoon-Ryu
Copy link
Contributor Author

DongHoon-Ryu commented Feb 10, 2025

Hey @DongHoon-Ryu I believe in order to make this PR functional, you would need to add fingerprints for these devices as well. I reviewed the fingerprints available in the MAIN branch of our repo and do not see fingerprints for these devices.

Could you please add them as well?

Also please reach out via slack as well to share the testing plans for these changes thank you kindly.

Hey @lelandblue Aqara Light Switch H2 is implemented to dynamically update profiles during device_init, so there is no need to add fingerprints. Nevertheless, the reason for distinguishing PID was to specify a profile with this capability only for the first switch because there is an electrical sensor device type at endpoint 0 unlike other devices experienced so far. And I will share my testing plan via slack.

@lelandblue lelandblue self-requested a review February 13, 2025 14:55
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