Matter Switch Subdriver: Add Bilresa Support #2579
Conversation
hcarter-775
requested review from
ctowns,
gharveymn,
greens,
nickolas-deboom and
tpmanley
|
Duplicate profile check: Passed - no duplicate profiles detected. |
|
Channel deleted. |
Test Results 71 files 477 suites 0s ⏱️ Results for commit a5bc31b. ♻️ This comment has been updated with latest results. |
|
Minimum allowed coverage is Generated by 🐒 cobertura-action against a5bc31b |
tpmanley
left a comment
tpmanley
left a comment
There was a problem hiding this comment.
What's here looks good to me. I do have a question about the capabilities on the main component which we can discuss offline.
| - id: group1 | ||
| label: Group 1 | ||
| capabilities: | ||
| - id: button |
There was a problem hiding this comment.
For consistency with other multi-button Matter devices lets put the first group button on the main component
There was a problem hiding this comment.
Also, if main only has the battery it displays as an active tile with the battery percentage instead of the typical "Standby" state.
I'm going to publish a custom driver to use the wheel and add some features but I'd like to use the same IDs of stock drivers so people don't have to re-create automations and can switch back and forth at will.
drivers/SmartThings/matter-switch/src/sub_drivers/ikea_scroll/scroll_utils/fields.lua
Show resolved
Hide resolved
drivers/SmartThings/matter-switch/src/sub_drivers/ikea_scroll/scroll_utils/fields.lua
Outdated
Show resolved
Hide resolved
| [0x117C] = { -- IKEA_MANUFACTURER_ID | ||
| [0x8000] = { is_ikea_scroll = true } |
There was a problem hiding this comment.
I like this approach for can_handle, maybe we should implement this for other subdrivers eventually
There was a problem hiding this comment.
yeah that's the idea!
There was a problem hiding this comment.
Generally, my thought is to put all device overrides, of any type, into this table in some form or another. I think that would be nice for keeping everything organized.
drivers/SmartThings/matter-switch/src/test/test_ikea_scroll.lua
Outdated
Show resolved
Hide resolved
nickolas-deboom
left a comment
nickolas-deboom
left a comment
There was a problem hiding this comment.
This looks good from my POV. Btw is there only one version / pid of this device? Do we want to add a fingerprint for it?
drivers/SmartThings/matter-switch/src/sub_drivers/ikea_scroll/scroll_utils/fields.lua
Outdated
Show resolved
Hide resolved
ctowns
left a comment
ctowns
left a comment
There was a problem hiding this comment.
LGTM! It's very well organized, nice job Harrison!
|
hcarter-775
force-pushed
the
release/scroll-Q4
branch
from
84e1616 to
94f8838
Compare
hcarter-775
force-pushed
the
release/scroll-Q4
branch
from
94f8838 to
a5bc31b
Compare
Description of Change
Add initial subdriver support for the Bilresa device.
Summary of Completed Tests
Unit test added to ensure proper onboarding. Tested on-device.