Matter Switch: Add StatelessStep capability support for SwitchLevel and ColorTemperature #2677
Conversation
hcarter-775
requested review from
Kwang-Hui,
ctowns,
greens,
nickolas-deboom and
tpmanley
|
Duplicate profile check: Warning - duplicate profiles detected. |
|
Invitation URL: |
Test Results 71 files 481 suites 0s ⏱️ For more details on these errors, see this check. Results for commit 17f06e2. ♻️ This comment has been updated with latest results. |
|
Minimum allowed coverage is Generated by 🐒 cobertura-action against 17f06e2 |
drivers/SmartThings/matter-switch/src/switch_handlers/capability_handlers.lua
Outdated
Show resolved
Hide resolved
drivers/SmartThings/matter-switch/src/switch_handlers/capability_handlers.lua
Outdated
Show resolved
Hide resolved
drivers/SmartThings/matter-switch/capabilities/statelessColorTemperatureStep.yml
Outdated
Show resolved
Hide resolved
| local step_mode = step_percent_change > 0 and (clusters.ColorControl.types.StepModeEnum and clusters.ColorControl.types.StepModeEnum.DOWN or 3) or (clusters.ColorControl.types.StepModeEnum and clusters.ColorControl.types.StepModeEnum.UP or 1) | ||
| local min_mireds = switch_utils.get_field_for_endpoint(device, fields.COLOR_TEMP_BOUND_RECEIVED_MIRED..fields.COLOR_TEMP_MIN, endpoint_id) or fields.COLOR_TEMPERATURE_MIRED_MIN -- default min mireds | ||
| local max_mireds = switch_utils.get_field_for_endpoint(device, fields.COLOR_TEMP_BOUND_RECEIVED_MIRED..fields.COLOR_TEMP_MAX, endpoint_id) or fields.COLOR_TEMPERATURE_MIRED_MAX -- default max mireds | ||
| local step_size_in_mireds = (max_mireds - min_mireds) * st_utils.round((math.abs(step_percent_change)/100)) |
There was a problem hiding this comment.
I think we wouldn't want to round this value right? Since abs(step_percent_change) is between 0 and 100 so this expression would round to 0 or 1
There was a problem hiding this comment.
Oh shoot, when making some changes last week I dropped the "*100". Will add that back in
drivers/SmartThings/matter-switch/src/switch_handlers/capability_handlers.lua
Show resolved
Hide resolved
hcarter-775
force-pushed
the
feature/statelessStep
branch
from
80a5c0a to
00f1bf4
Compare
nickolas-deboom
left a comment
nickolas-deboom
left a comment
There was a problem hiding this comment.
I approved earlier but noticed there are some print statements from debugging I assume
|
@nickolas-deboom thanks for the catch, got those removed |
nickolas-deboom
left a comment
nickolas-deboom
left a comment
There was a problem hiding this comment.
LGTM! Looks like there is are test failures but I ran the tests locally and they pass, so they might need to be re-run in CI
| version: 1 | ||
| - id: switchLevel | ||
| version: 1 | ||
| - id: statelessSwitchLevelStep |
There was a problem hiding this comment.
This should be after the config here and in some of the other profiles
| @@ -0,0 +1,163 @@ | |||
| -- Copyright © 2022 SmartThings, Inc. | |||
There was a problem hiding this comment.
| -- Copyright © 2022 SmartThings, Inc. | |
| -- Copyright © 2026 SmartThings, Inc. |
| local COLOR_TEMPERATURE_KELVIN_MIN = 1000 | ||
| SwitchFields.COLOR_TEMPERATURE_MIRED_MAX = SwitchFields.MIRED_KELVIN_CONVERSION_CONSTANT/COLOR_TEMPERATURE_KELVIN_MIN | ||
| SwitchFields.COLOR_TEMPERATURE_MIRED_MIN = SwitchFields.MIRED_KELVIN_CONVERSION_CONSTANT/COLOR_TEMPERATURE_KELVIN_MAX | ||
| SwitchFields.COLOR_TEMPERATURE_MIRED_MAX = math.floor(SwitchFields.MIRED_KELVIN_CONVERSION_CONSTANT/COLOR_TEMPERATURE_KELVIN_MIN) |
There was a problem hiding this comment.
If these default values got used (which normally they would not be), COLOR_TEMPERATURE_MIRED_MAX specifically would be a decimal value while commands are expecting ints, which a unit test was complaining about. I should also note that the min/max kelvin values here were arbitrarily chosen, and I mistakenly assumed these conversions would be a clean integer division
There was a problem hiding this comment.
the step function requires that these are numbers, not floats. I could have done the floor-ing in the handler, but since they are just arbitrary constants, I figured why not do it in the base def
Description of Change
Update all profiles to support statelessStep capabilities as needed. Include handlers for these step capabilities using the Matter-provided step commands.
This breaks apart the work found in #2669 so that the stateless capability support can be reviewed as an isolated unit.
Summary of Completed Tests
Unit tests added. Tested on-device.