Matter Switch: Include Ikea subdriver support for knob capability #2678
Conversation
hcarter-775
requested review from
Kwang-Hui,
ctowns,
greens,
nickolas-deboom and
tpmanley
|
Duplicate profile check: Passed - no duplicate profiles detected. |
|
Invitation URL: |
Test Results 71 files 482 suites 0s ⏱️ Results for commit d3b8761. ♻️ This comment has been updated with latest results. |
|
Minimum allowed coverage is Generated by 🐒 cobertura-action against d3b8761 |
nickolas-deboom
left a comment
nickolas-deboom
left a comment
There was a problem hiding this comment.
I left a comment about some confusion I had, but the rest all looks good from my perspective. As long as this has been testing on device and everything's working, I don't see any reason to hold up any of these changes. Nice work!
hcarter-775
force-pushed
the
feature/knob
branch
from
05b47bd to
94a1e0e
Compare
| if switch_utils.tbl_contains(scroll_fields.ENDPOINTS_PUSH, ib.endpoint_id) then | ||
| generic_event_handlers.initial_press_handler(driver, device, ib, response) | ||
| else | ||
| device:set_field(scroll_fields.LATEST_NUMBER_OF_PRESSES_COUNTED, 1) |
There was a problem hiding this comment.
I think the "ticks" calculations are incorrect for the events sent by Bilresa.
For example, when fast scrolling the wheel for 14 "ticks", the messages sent by the Bilresa are as follows:
- InitialPress(1)
- InitialPress(1)
- MultiPressOngoing(1, 11)
- InitialPress(1)
- MultiPressOngoing(1, 14)
which means that instead of 14 "ticks", this event handler will count 28 "ticks" (1 + 1 + 11 + 1 + 14) because LATEST_NUMBER_OF_PRESSES_COUNTED is reset to 1 after each InitialPress event.
I think it would be better to use the MultiPressComplete event to "reset" the ticks counter, while also adding a time-based safeguard in case the MultiPressComplete is lost.
There was a problem hiding this comment.
Hi @perexis, thanks for the investigation on your part. That is interesting that the MultiPressComplete event is not always being sent during fast scrolling. I will look into this further, though on first glance it seems unclear whether InitialPress or MultiPressComplete should be considered the "final say" for how these scroll events are interpreted.
There was a problem hiding this comment.
Hi @hcarter-775
I'm not saying that MultiPressComplete is not generated when fast scrolling. It is not generated because the driver does not subscribe to this event as it was not needed.
I am saying that it is not possible to correctly handle the rotate sequence without subscribing to this event. The fast scrolling was just an example to show the huge error in the calculations.
When the driver subscribes to the MultiPressComplete, the sequence changes to:
- InitialPress(1)
- InitialPress(1)
- MultiPressOngoing(1, 11)
- InitialPress(1)
- MultiPressOngoing(1, 14)
- MultiPressComplete(1, 14)
I modified a little your code and successfully tested it - the full 360 degrees rotation generates knob event with rotateAmount=100 in total. Please take a look at my suggestion and feel free to integrate it into your code it if you think it's worth applying.
Subscribe also to MultiPressComplete:
IkeaScrollFields.switch_scroll_subscribed_events = {
clusters.Switch.events.InitialPress.ID,
clusters.Switch.events.MultiPressOngoing.ID,
clusters.Switch.events.MultiPressComplete.ID,
}
Attach handler for the MultiPressComplete event:
matter_handlers = {
event = {
[clusters.Switch.ID] = {
[clusters.Switch.events.InitialPress.ID] = event_handlers.initial_press_handler,
[clusters.Switch.events.MultiPressOngoing.ID] = event_handlers.multi_press_ongoing_handler,
[clusters.Switch.events.MultiPressComplete.ID] = event_handlers.multi_press_complete_handler,
}
}
},
Handle the MultiPressComplete event:
IkeaScrollFields.SCROLL_STARTED = "__scroll_started"
function IkeaScrollEventHandlers.initial_press_handler(driver, device, ib, response)
-- use the generic handler logic for the push endpoints. Else, use custom logic.
if switch_utils.tbl_contains(scroll_fields.ENDPOINTS_PUSH, ib.endpoint_id) then
generic_event_handlers.initial_press_handler(driver, device, ib, response)
else
-- prevent multiple executions in the same scroll sequence
if not device:get_field(scroll_fields.SCROLL_STARTED) then
device:set_field(scroll_fields.LATEST_NUMBER_OF_PRESSES_COUNTED, 1)
rotate_amount_event_helper(device, ib.endpoint_id, 1)
device:set_field(scroll_fields.SCROLL_STARTED, 1)
end
end
end
function IkeaScrollEventHandlers.multi_press_complete_handler(driver, device, ib, response)
if switch_utils.tbl_contains(scroll_fields.ENDPOINTS_PUSH, ib.endpoint_id) then
generic_event_handlers.multi_press_complete_handler(driver, device, ib, response)
else
-- end the scroll sequence
device:set_field(scroll_fields.SCROLL_STARTED, nil)
end
end
There was a problem hiding this comment.
Hi @perexis, thank you for reporting this. I did some investigation and saw the same behavior. In the current implementation, I have chosen to ignore the InitialPress notification altogether in favor of MultiPressComplete, and in the interest of simplicity, to otherwise keep the logic the same as before.
There was a problem hiding this comment.
Hi @hcarter-775
I have reviewed your changes and also tested it with real hardware. The number of "ticks" is calculated correctly now which means that the full 360-degree rotation generated the knob event with rotateAmount=100.
However, I would not ignore the InitialPress notifications because they significantly improve the time for generating the first event, which is noticeable while using it to dim the light. I did some investigation on the timing here and I found that ignoring the InitialPress increases the first response time to about 500ms. With the InitialPress handling, the time for generating the first event is reduced to about 7ms.
Without InitialPress handling:
1 "tick" initial time=495ms
2026-01-31T14:23:13.945322146Z InitialPress, data: Structure: {Uint8: \x01}
2026-01-31T14:23:14.431281187Z MultiPressComplete, data: Structure: {Uint8: \x01, Uint8: \x01}
2026-01-31T14:23:14.440784312Z {"attribute_id":"rotateAmount","capability_id":"knob","component_id":"group3","state":{"value":6},"state_change":true}
2 "ticks" initial time=444ms
2026-01-31T14:25:37.122185538Z InitialPress, data: Structure: {Uint8: \x01}
2026-01-31T14:25:37.424407621Z InitialPress, data: Structure: {Uint8: \x01}
2026-01-31T14:25:37.558036454Z MultiPressOngoing, data: Structure: {Uint8: \x01, Uint8: \x02}
2026-01-31T14:25:37.566086871Z {"attribute_id":"rotateAmount","capability_id":"knob","component_id":"group3","state":{"value":12},"state_change":true}
2026-01-31T14:25:37.803455121Z MultiPressComplete, data: Structure: {Uint8: \x02, Uint8: \x01}
19 "ticks" initial time=628ms
2026-01-31T14:28:40.097829851Z InitialPress, data: Structure: {Uint8: \x01}
2026-01-31T14:28:40.572669810Z InitialPress, data: Structure: {Uint8: \x01}
2026-01-31T14:28:40.716398185Z MultiPressOngoing, data: Structure: {Uint8: \x01, Uint8: \x11}
2026-01-31T14:28:40.725500726Z {"attribute_id":"rotateAmount","capability_id":"knob","component_id":"group3","state":{"value":100},"state_change":true}
2026-01-31T14:28:41.065468143Z InitialPress, data: Structure: {Uint8: \x01}
2026-01-31T14:28:41.213728643Z MultiPressOngoing, data: Structure: {Uint8: \x01, Uint8: \x12}
2026-01-31T14:28:41.221585101Z {"attribute_id":"rotateAmount","capability_id":"knob","component_id":"group3","state":{"value":6},"state_change":true}
2026-01-31T14:28:41.566226810Z MultiPressComplete, data: Structure: {Uint8: \x12, Uint8: \x01}
With InitialPress handling:
1 "tick" initial time=7ms
2026-01-31T14:59:30.927681180Z InitialPress, data: Structure: {Uint8: \x01}
2026-01-31T14:59:30.934650538Z {"attribute_id":"rotateAmount","capability_id":"knob","component_id":"group3","state":{"value":6},"state_change":true}
2026-01-31T14:59:31.477424206Z MultiPressComplete, data: Structure: {Uint8: \x01, Uint8: \x01}
2 "ticks" initial time=7ms
2026-01-31T15:01:28.079565030Z InitialPress, data: Structure: {Uint8: \x01}
2026-01-31T15:01:28.086676209Z {"attribute_id":"rotateAmount","capability_id":"knob","component_id":"group3","state":{"value":6},"state_change":true}
2026-01-31T15:01:28.556849761Z InitialPress, data: Structure: {Uint8: \x01}
2026-01-31T15:01:29.696025398Z MultiPressOngoing, data: Structure: {Uint8: \x01, Uint8: \x02}
2026-01-31T15:01:29.747370975Z {"attribute_id":"rotateAmount","capability_id":"knob","component_id":"group3","state":{"value":6},"state_change":true}
2026-01-31T15:01:30.144046547Z MultiPressComplete, data: Structure: {Uint8: \x02, Uint8: \x01}
Description of Change
Update scroll profile to support knob capability. Include unique event handlers for knob and initial press.
This breaks apart the work found in #2669 so that the knob capability support can be reviewed as an isolated unit.
Summary of Completed Tests
Unit test included. Tested on-device.