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

Matter Bridge Aqara Cube Support with Custom Capability #1622

Merged
merged 2 commits into from
Oct 2, 2024
Merged

Matter Bridge Aqara Cube Support with Custom Capability #1622

merged 2 commits into from
Oct 2, 2024

Conversation

DongHoon-Ryu
Copy link
Contributor

Currently, the Generic EdgeDriver applied to the Matter Bridge Aqara Cube is implemented by mapping six faces into individual components, making it difficult to check the event occurrence of individual faces on one screen.

This commit wants to improve the problem by mapping the event from Matter Bridge with Aqara Cube Custom Capability developed by Zigbee driver. This means that there is a limitation that the Action Event of the Cube that the Matter Bridge does not give can not be processed.

REQ-15926, REQ-16285, IOTE-4217, IOTE-4266

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

This commit wants to improve the problem by mapping the event from Matter Bridge with Aqara Cube Custom Capability developed by Zigbee driver. This means that there is a limitation that the Action Event of the Cube that the Matter Bridge does not give can not be processed.

Summary of Completed Tests

  • Successful test for Plugin UI update and Dash Board's Device Card status update according to the selection of each face
  • Successful Automation Routine Test for Cube Face
  • Even though the Cube Face is changed, the status is sometimes not updated because the Aqara Matter Bride does not send the event.

Copy link

github-actions bot commented Sep 9, 2024

Duplicate profile check: Passed - no duplicate profiles detected.

Copy link

github-actions bot commented Sep 9, 2024

Test Results

   63 files    396 suites   0s ⏱️
1 936 tests 1 936 ✅ 0 💤 0 ❌
3 360 runs  3 360 ✅ 0 💤 0 ❌

Results for commit 54c2681.

♻️ This comment has been updated with latest results.

Copy link

github-actions bot commented Sep 9, 2024

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

@nickolas-deboom
Copy link
Contributor

Hi @DongHoon-Ryu I went over the changes and left some comments. Outside of that, the changes look good to me!

Copy link

github-actions bot commented Sep 11, 2024

Channel deleted.

Copy link
Contributor

@hcarter-775 hcarter-775 left a comment

Choose a reason for hiding this comment

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

I have those two last newline nits for keeping with our general standards, but otherwise it lgtm 👍 thank you!

DongHoon-Ryu and others added 2 commits October 2, 2024 08:59
Currently, the Generic EdgeDriver applied to the Matter Bridge Aqara Cube is implemented by mapping six faces into individual components,
making it difficult to check the event occurrence of individual faces on one screen.

This commit wants to improve the problem by mapping the event from Matter Bridge with Aqara Cube Custom Capability developed by Zigbee driver.
This means that there is a limitation that the Action Event of the Cube that the Matter Bridge does not give can not be processed.

REQ-15926, REQ-16285, IOTE-4217, IOTE-4266

Signed-off-by: donghoon-ryu <[email protected]>
Comment on lines +155 to +177
local function info_changed(driver, device, event, args)
if device.profile.id ~= args.old_st_store.profile.id
and device:get_field(DEFERRED_CONFIGURE)
and device.network_type ~= device_lib.NETWORK_TYPE_CHILD then

-- At the time of device_added, there is an error that the corresponding capability cannot be found
-- because the profile has not been changed from the generic profile of fingerprint to the Aqara Cube.
reset_thread(device)
device:emit_event(cubeFace.cubeFace("face1Up"))

device:set_field(DEFERRED_CONFIGURE, nil)

-- In the current test framework, the values of device.profile.id and args.old_st_store.profile.id are always the same,
-- so the test coverage cannot be increased. This is why I added DOING_CONFIGURE flag.
device:set_field(DOING_CONFIGURE, true)
end

if device:get_field(DOING_CONFIGURE) then
-- profile has changed, and we deferred setting up our buttons, so do that now
configure_buttons(device)
device:set_field(DOING_CONFIGURE, nil)
end
end
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
local function info_changed(driver, device, event, args)
if device.profile.id ~= args.old_st_store.profile.id
and device:get_field(DEFERRED_CONFIGURE)
and device.network_type ~= device_lib.NETWORK_TYPE_CHILD then
-- At the time of device_added, there is an error that the corresponding capability cannot be found
-- because the profile has not been changed from the generic profile of fingerprint to the Aqara Cube.
reset_thread(device)
device:emit_event(cubeFace.cubeFace("face1Up"))
device:set_field(DEFERRED_CONFIGURE, nil)
-- In the current test framework, the values of device.profile.id and args.old_st_store.profile.id are always the same,
-- so the test coverage cannot be increased. This is why I added DOING_CONFIGURE flag.
device:set_field(DOING_CONFIGURE, true)
end
if device:get_field(DOING_CONFIGURE) then
-- profile has changed, and we deferred setting up our buttons, so do that now
configure_buttons(device)
device:set_field(DOING_CONFIGURE, nil)
end
end
-- used in unit testing, since device.profile.id and args.old_st_store.profile.id are always the same
local TEST_CONFIGURE = "__test_configure"
...
local function info_changed(driver, device, event, args)
if (device.profile.id ~= args.old_st_store.profile.id or device:get_field(TEST_CONFIGURE))
and device:get_field(DEFERRED_CONFIGURE)
and device.network_type ~= device_lib.NETWORK_TYPE_CHILD then
-- At the time of device_added, there is an error that the corresponding capability cannot be found
-- because the profile has not been changed from the generic profile of fingerprint to the Aqara Cube.
reset_thread(device)
device:emit_event(cubeFace.cubeFace("face1Up"))
-- profile has changed, and we deferred setting up our buttons, so do that now
configure_buttons(device)
device:set_field(DEFERRED_CONFIGURE, nil)
end
end

This may add a little bit more clarity to what is going on, without the need for as much explanation.

@hcarter-775
Copy link
Contributor

Just a reminder @DongHoon-Ryu, please squash your commits before merging into main. Thank you very much.

@HunsupJung HunsupJung merged commit 9093cf9 into SmartThingsCommunity:main Oct 2, 2024
12 checks passed
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