Matter Switch: Add subdriver for Third Reality MK1 #2109
Conversation
|
Duplicate profile check: Passed - no duplicate profiles detected. |
|
Invitation URL: |
Test Results 66 files ±0 426 suites +1 0s ⏱️ ±0s Results for commit 02b2f8a. ± Comparison against base commit ea07e1b. This pull request removes 8 and adds 1 tests. Note that renamed tests count towards both.♻️ This comment has been updated with latest results. |
|
Minimum allowed coverage is Generated by 🐒 cobertura-action against 02b2f8a |
nickolas-deboom
changed the title
nickolas-deboom
changed the title
|
Is there a reason why the profiles for 9, 10 and 11 buttons are intentionally omitted? A device like the 3rd Reality MK1 Keyboard would work out of the box with these profiles. The device and the platform are Matter certified after all... |
This PR is specifically meant for supporting the 3rd Reality MK1. |
nickolas-deboom
changed the title
nickolas-deboom
force-pushed
the
add-new-button-profile
branch
3 times, most recently
from
7a822d1 to
ccbdbbd
Compare
nickolas-deboom
force-pushed
the
add-new-button-profile
branch
2 times, most recently
from
52e5174 to
bc937cc
Compare
nickolas-deboom
changed the base branch from
main
to
matter-switch-update-field-names
drivers/SmartThings/matter-switch/src/third-reality-mk1/init.lua
Outdated
Show resolved
Hide resolved
| end | ||
|
|
||
| local function device_added(driver, device) | ||
| device_init(driver, device) |
There was a problem hiding this comment.
maybe let's just subscribe or even return here rather than re-call the whole init? I understand we're mostly doing this call to overwrite the main driver, so I think we could also make a comment of that.
There was a problem hiding this comment.
The main driver also calls device_init within device_added, which was done to ensure init is called in the case of device caching: ed8fe8f
So I think it might be needed here still?
There was a problem hiding this comment.
This was done to ensure that subscribe was called for all devices, because in some cases there is a race condition that causes the init event to not fire for some devices. Therefore, calling init again in added was meant to ensure that the processing in init was occuring even if the init event was not queued due to the race condition.
Parent-Child devices were primarily affected, so I wouldn't expect it to be an issue here.
There is a detail explanation here: https://smartthings.atlassian.net/browse/CHAD-13449
| local function match_profile(driver, device) | ||
| device:try_update_metadata({profile = "12-button-keyboard"}) | ||
| build_button_component_map(device) | ||
| configure_buttons(device) |
There was a problem hiding this comment.
would we want to use the new buttons.configure call here as well?
There was a problem hiding this comment.
Yes but that will need to wait until PR 1754 gets merged (or if this one gets merged first, then that other PR will need to do this change)
| local function info_changed(driver, device, event, args) | ||
| if device.profile.id ~= args.old_st_store.profile.id then | ||
| configure_buttons(device) | ||
| subscribe(device) |
There was a problem hiding this comment.
You could potentially just override the device:subscribe function directly by calling:
device:extend_device("subscribe", subscribe)
which will override the library subscribe function directly and point the device to the subscribe function you have defined here. I can't think of any cases where this would be needed over the solution you currently have, but if a library function call somewhere calls device:subscribe, it might be nice to just have the default subscribe itself overridden, since that is what the extend_device api is meant to be used for.
nickolas-deboom
force-pushed
the
matter-switch-update-field-names
branch
from
3d2d422 to
49ba419
Compare
The Aqara Cube and Eve Energy subdrivers need doConfigure and driverSwitched lifecycle event handler overrides to prevent the handlers from running in the main driver, which would result in match_profile running. Also fix nil value error in is_aqara_cube function.
nickolas-deboom
force-pushed
the
matter-switch-update-field-names
branch
from
49ba419 to
ea07e1b
Compare
Add new profiles to support 13-button devices.
This commit makes this change specific to the Third Reality MK1 keyboard
rather than just adding support for a 12 button device. This allows
custom component names to be used ("F1", "F2", ...) rather than the
default names (i.e. "button3").
Button and switch init will eventually be moved to doConfigure in the main driver, so moving it there in the subdriver.
Override additional functions in the subdriver to resolve issues found during testing.
A handler for driverSwitched was added to the main driver, so add an override to the subdriver to prevent the main driver version from running.
This driver does not support creating child devices, and any child device would be prevented from using this driver due to the logic in is_third_reality_mk1, so the additional gates in other functions can be removed.
nickolas-deboom
force-pushed
the
add-new-button-profile
branch
from
ac5d0a0 to
02b2f8a
Compare
nickolas-deboom
force-pushed
the
matter-switch-update-field-names
branch
from
ea07e1b to
653030b
Compare
Type of Change
Description of Change
Add new profile and a subdriver to support the Third Reality keyboard, which contains 12 matter-enabled buttons.
Summary of Testing
Tested on device and with new unit tests.