-
Notifications
You must be signed in to change notification settings - Fork 546
Matter Ikea Scroll: Aggregate multiple responses into one event, add global bounds checking, remove InitialPress handling during scroll #3014
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -6,55 +6,71 @@ local capabilities = require "st.capabilities" | |
| local switch_utils = require "switch_utils.utils" | ||
| local generic_event_handlers = require "switch_handlers.event_handlers" | ||
| local scroll_fields = require "sub_drivers.ikea_scroll.scroll_utils.fields" | ||
| local event_utils = require "sub_drivers.ikea_scroll.scroll_utils.event_utils" | ||
|
|
||
| local IkeaScrollEventHandlers = {} | ||
|
|
||
| local function rotate_amount_event_helper(device, endpoint_id, num_presses_to_handle) | ||
| if num_presses_to_handle <= 0 then return end | ||
|
|
||
| -- to cut down on checks, we can assume that if the endpoint is not in ENDPOINTS_UP_SCROLL, it is in ENDPOINTS_DOWN_SCROLL | ||
| local scroll_direction = switch_utils.tbl_contains(scroll_fields.ENDPOINTS_UP_SCROLL, endpoint_id) and 1 or -1 | ||
| local scroll_amount = st_utils.clamp_value(scroll_direction * scroll_fields.PER_SCROLL_EVENT_ROTATION * num_presses_to_handle, -100, 100) | ||
| device:emit_event_for_endpoint(endpoint_id, capabilities.knob.rotateAmount(scroll_amount, {state_change = true})) | ||
|
|
||
| if event_utils.is_valid_scroll_amount(device, scroll_amount) then | ||
| device:emit_event_for_endpoint(endpoint_id, capabilities.knob.rotateAmount(scroll_amount, {state_change = true})) | ||
| end | ||
| end | ||
|
|
||
| -- Used by ENDPOINTS_UP_SCROLL and ENDPOINTS_DOWN_SCROLL, not ENDPOINTS_PUSH | ||
| function IkeaScrollEventHandlers.multi_press_ongoing_handler(driver, device, ib, response) | ||
| if switch_utils.tbl_contains(scroll_fields.ENDPOINTS_PUSH, ib.endpoint_id) then | ||
| -- Ignore MultiPressOngoing events from push endpoints. | ||
| device.log.debug("Received MultiPressOngoing event from push endpoint, ignoring.") | ||
| else | ||
| local cur_num_presses_counted = ib.data and ib.data.elements and ib.data.elements.current_number_of_presses_counted.value or 0 | ||
| local num_presses_to_handle = cur_num_presses_counted - (device:get_field(scroll_fields.LATEST_NUMBER_OF_PRESSES_COUNTED) or 0) | ||
| if num_presses_to_handle > 0 then | ||
| device:set_field(scroll_fields.LATEST_NUMBER_OF_PRESSES_COUNTED, cur_num_presses_counted) | ||
| rotate_amount_event_helper(device, ib.endpoint_id, num_presses_to_handle) | ||
| local cur_num_presses_counted = ib.data.elements and ib.data.elements.current_number_of_presses_counted.value or 0 | ||
| local cur_multi_press_count = cur_num_presses_counted | ||
| if #response.info_blocks > 1 then | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is there something that prevents adding unit tests for this?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, the current test request block logic only allows one info block per request. Thus, multi-event response blocks cannot be explicitly tested via unit test today without semi-significant Integration Test reworks for Matter. |
||
| if event_utils.is_last_valid_info_block(ib.event_id, cur_num_presses_counted, response.info_blocks) then | ||
| local aggregated_presses = event_utils.aggregate_scroll_amount_for_info_blocks(device, response.info_blocks) or {} | ||
| cur_num_presses_counted = aggregated_presses.total_presses or 0 | ||
| cur_multi_press_count = aggregated_presses.presses_in_current_chain or 0 | ||
| else | ||
| device.log.debug("Received MultiPressOngoing event that is not the last valid info block, ignoring.") | ||
| return | ||
| end | ||
| end | ||
| local num_presses_to_handle = cur_num_presses_counted - (device:get_field(scroll_fields.LATEST_NUMBER_OF_PRESSES_HANDLED) or 0) | ||
| device:set_field(scroll_fields.LATEST_NUMBER_OF_PRESSES_HANDLED, cur_multi_press_count) | ||
| rotate_amount_event_helper(device, ib.endpoint_id, num_presses_to_handle) | ||
| 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 | ||
| local total_num_presses_counted = ib.data and ib.data.elements and ib.data.elements.total_number_of_presses_counted.value or 0 | ||
| local num_presses_to_handle = total_num_presses_counted - (device:get_field(scroll_fields.LATEST_NUMBER_OF_PRESSES_COUNTED) or 0) | ||
| if num_presses_to_handle > 0 then | ||
| rotate_amount_event_helper(device, ib.endpoint_id, num_presses_to_handle) | ||
| local total_num_presses_counted = ib.data.elements and ib.data.elements.total_number_of_presses_counted.value or 0 | ||
| if #response.info_blocks > 1 then | ||
| if event_utils.is_last_valid_info_block(ib.event_id, total_num_presses_counted, response.info_blocks) then | ||
| local aggregated_presses = event_utils.aggregate_scroll_amount_for_info_blocks(device, response.info_blocks) or {} | ||
| total_num_presses_counted = aggregated_presses.total_presses or 0 | ||
| else | ||
| device.log.debug("Received MultiPressComplete event that is not the last valid info block, ignoring.") | ||
| return | ||
| end | ||
| end | ||
| -- reset the LATEST_NUMBER_OF_PRESSES_COUNTED to nil at the end of a MultiPress chain. | ||
| device:set_field(scroll_fields.LATEST_NUMBER_OF_PRESSES_COUNTED, nil) | ||
| local num_presses_to_handle = total_num_presses_counted - (device:get_field(scroll_fields.LATEST_NUMBER_OF_PRESSES_HANDLED) or 0) | ||
| rotate_amount_event_helper(device, ib.endpoint_id, num_presses_to_handle) | ||
| -- always reset the LATEST_NUMBER_OF_PRESSES_HANDLED to nil at the end of a handled MultiPress chain. | ||
| device:set_field(scroll_fields.LATEST_NUMBER_OF_PRESSES_HANDLED, nil) | ||
| end | ||
| end | ||
|
|
||
| function IkeaScrollEventHandlers.initial_press_handler(driver, device, ib, response) | ||
| if switch_utils.tbl_contains(scroll_fields.ENDPOINTS_PUSH, ib.endpoint_id) then | ||
| generic_event_handlers.initial_press_handler(driver, device, ib, response) | ||
| else | ||
| -- the magic number "1" occurs in this handler since the InitialPress event represents the first press. | ||
| local latest_presses_counted = device:get_field(scroll_fields.LATEST_NUMBER_OF_PRESSES_COUNTED) or 0 | ||
| if latest_presses_counted == 0 then | ||
| device:set_field(scroll_fields.LATEST_NUMBER_OF_PRESSES_COUNTED, 1) | ||
| rotate_amount_event_helper(device, ib.endpoint_id, 1) | ||
| end | ||
| device.log.debug("Received InitialPress event from scroll endpoint, ignoring.") | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you either break this change into its own PR or update the title and description to include this? It's pretty significant and I think it's just not sufficiently called out currently.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. added to title/desc! |
||
| end | ||
| end | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,76 @@ | ||
| -- Copyright © 2026 SmartThings, Inc. | ||
| -- Licensed under the Apache License, Version 2.0 | ||
|
|
||
| local st_utils = require "st.utils" | ||
| local clusters = require "st.matter.clusters" | ||
| local scroll_fields = require "sub_drivers.ikea_scroll.scroll_utils.fields" | ||
|
|
||
| local IkeaScrollEventUtils = {} | ||
|
|
||
|
|
||
| function IkeaScrollEventUtils.requeue_clear_scroll_state(device) | ||
| -- cancel any previously queued clear state actions to prevent unintended clears | ||
| if device:get_field(scroll_fields.CLEAR_STATE_TIMER) then | ||
| device.thread:cancel_timer(device:get_field(scroll_fields.CLEAR_STATE_TIMER)) | ||
| end | ||
| local new_timer = device.thread:call_with_delay(scroll_fields.CLEAR_STATE_DELAY_S, function() | ||
| device:set_field(scroll_fields.GLOBAL_ROTATE_AMOUNT_STATE, 0) | ||
| end) | ||
| device:set_field(scroll_fields.CLEAR_STATE_TIMER, new_timer) | ||
| end | ||
|
|
||
| function IkeaScrollEventUtils.is_valid_scroll_amount(device, scroll_amount) | ||
| local global_rotate_amount_state = device:get_field(scroll_fields.GLOBAL_ROTATE_AMOUNT_STATE) or 0 | ||
| local is_rotate_amount_state_at_bounds = (scroll_amount < 0 and global_rotate_amount_state <= -100) or (scroll_amount > 0 and global_rotate_amount_state >= 100) | ||
| if is_rotate_amount_state_at_bounds then | ||
| return false | ||
| end | ||
|
|
||
| device:set_field(scroll_fields.GLOBAL_ROTATE_AMOUNT_STATE, st_utils.clamp_value(global_rotate_amount_state + scroll_amount, -100, 100)) | ||
| IkeaScrollEventUtils.requeue_clear_scroll_state(device) | ||
| return true | ||
| end | ||
|
|
||
| -- inspect all info blocks to find the last one that is not an InitialPress event. We will | ||
| -- only try to emit a rotateAmount event if the current info block being handled is that last one. | ||
| function IkeaScrollEventUtils.is_last_valid_info_block(cur_info_block_event_id, cur_info_block_value, info_blocks) | ||
| local last_valid_emission_idx = #info_blocks | ||
| -- Ignore all InitialPress events in a multi-response block | ||
| while (last_valid_emission_idx > 1) and (info_blocks[last_valid_emission_idx].info_block.event_id == clusters.Switch.events.InitialPress.ID) do | ||
| last_valid_emission_idx = last_valid_emission_idx - 1 | ||
| end | ||
|
|
||
| -- Because the info block does not include the unique_key Matter defined event number, this | ||
| -- logic is a best guess at matching the current info block to the last valid info block. | ||
| local emission_ib = info_blocks[last_valid_emission_idx].info_block | ||
| if emission_ib.event_id ~= cur_info_block_event_id then | ||
| return false | ||
| elseif emission_ib.event_id == clusters.Switch.events.MultiPressComplete.ID then | ||
| local last_valid_ib_value = emission_ib.data.elements and emission_ib.data.elements.total_number_of_presses_counted.value or 0 | ||
| return last_valid_ib_value == cur_info_block_value | ||
| elseif emission_ib.event_id == clusters.Switch.events.MultiPressOngoing.ID then | ||
| local last_valid_ib_value = emission_ib.data.elements and emission_ib.data.elements.current_number_of_presses_counted.value or 0 | ||
| return last_valid_ib_value == cur_info_block_value | ||
| elseif last_valid_emission_idx == 1 then -- aka, all ib's are InitialPress | ||
| return true | ||
| end | ||
|
|
||
| return false | ||
| end | ||
|
|
||
| function IkeaScrollEventUtils.aggregate_scroll_amount_for_info_blocks(device, info_blocks) | ||
| local total_presses = 0 | ||
| local presses_in_current_chain = 0 | ||
| for _, ib in ipairs(info_blocks) do | ||
| if ib.info_block.event_id == clusters.Switch.events.MultiPressOngoing.ID then | ||
| presses_in_current_chain = presses_in_current_chain + (ib.info_block.data.elements and ib.info_block.data.elements.current_number_of_presses_counted.value or 0) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If the message contains multiple MultiPressOngoing events I think we want to take the
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ah, yeah that's very true. Will make that update! |
||
| elseif ib.info_block.event_id == clusters.Switch.events.MultiPressComplete.ID then | ||
| total_presses = total_presses + (ib.info_block.data.elements and ib.info_block.data.elements.total_number_of_presses_counted.value or 0) | ||
| presses_in_current_chain = 0 | ||
| end | ||
| end | ||
| total_presses = total_presses + presses_in_current_chain -- aggregate any presses to the total from the current chain | ||
| return { total_presses = total_presses, presses_in_current_chain = presses_in_current_chain } | ||
| end | ||
|
|
||
| return IkeaScrollEventUtils | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a reason to remove the
ib.data andfrom this line?Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The existence of
ib.datais ensured at the table creation level in the lua libs, something I wasn't thinking about when I first wrote this code.