Zigbee Switch: Add support/handling for Stateless Step Capabilities#2818
Zigbee Switch: Add support/handling for Stateless Step Capabilities#2818
Conversation
|
Invitation URL: |
Test Results 72 files 494 suites 0s ⏱️ Results for commit 0d7e0a1. ♻️ This comment has been updated with latest results. |
|
Minimum allowed coverage is Generated by 🐒 cobertura-action against 0d7e0a1 |
535962c to
cf1fc47
Compare
|
Duplicate profile check: Warning - duplicate profiles detected. |
3d5baa4 to
8855768
Compare
c0a48c5 to
f4a7af9
Compare
drivers/SmartThings/zigbee-switch/src/stateless_handlers/init.lua
Outdated
Show resolved
Hide resolved
f10a277 to
1bbafc5
Compare
drivers/SmartThings/zigbee-switch/src/stateless_handlers/init.lua
Outdated
Show resolved
Hide resolved
1bbafc5 to
21d5850
Compare
drivers/SmartThings/zigbee-switch/src/stateless_handlers/init.lua
Outdated
Show resolved
Hide resolved
drivers/SmartThings/zigbee-switch/src/stateless_handlers/init.lua
Outdated
Show resolved
Hide resolved
4ad4ca3 to
75ef0dc
Compare
drivers/SmartThings/zigbee-switch/src/stateless_handlers/init.lua
Outdated
Show resolved
Hide resolved
drivers/SmartThings/zigbee-switch/src/stateless_handlers/can_handle.lua
Outdated
Show resolved
Hide resolved
671071f to
2023b46
Compare
2023b46 to
2a1d09d
Compare
| Level.server.commands.Step(mock_device, Level.types.MoveStepMode.UP, 254, 3, 0x01, 0x00) | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Should probably add the min_api_version to these tests like in the other tests
There was a problem hiding this comment.
@cjswedes @aleclorimer Can you remind me what these checks are for? These capabilities should not be gated by the api version 19 I don't think. Is that what min_api_version is saying?
There was a problem hiding this comment.
Min api version specifies the minimum lua lib api version that we will run against. So I think this is needed, since the stateless step caps are newer, and are not present in old lua libs artifacts.
There was a problem hiding this comment.
added to all new tests
There was a problem hiding this comment.
Talked to @aleclorimer about this and I'm not sure we should be gating what tests can be run based on the capabilities in old lua libs artifacts, since that isn't an accurate representation of actual hub characteristics.
I am going to leave these for now since it matches the current behavior across drivers, but in a vacuum I don't think this test should be gated by api 19.
There was a problem hiding this comment.
There's certainly going to be a weird grey area/time where tests would run on earlier versions of lua-libs than what is set by the min_api_version. If you would like to test this against earlier versions of lua_libs then update the min_api_version to the versions that you tested against, you could do that. You could also run the tools/update_min_tags.py --filter <driver> with the driver and it will update those values automatically (with a bit of lua-libs modifications).
There was a problem hiding this comment.
I'm not sure that should be the case, as this blocks all automatic pre-60 testing from matter switch, for example? And since new capabilities (with commands) are always defined in the top-level init file today, this means that anytime we create a new one and implement it, we can never automatically run tests against lower lua libs. This seems like a strange, artificial requirement to me.
For example, if we had older fw tests for switch, the fix implemented here would have been caught in CI
drivers/SmartThings/zigbee-switch/src/stateless_handlers/init.lua
Outdated
Show resolved
Hide resolved
| local kelvin_min = device:get_field(switch_utils.KELVIN_MIN); | ||
| local kelvin_max = device:get_field(switch_utils.KELVIN_MAX); |
There was a problem hiding this comment.
These fields are set with the endpoint appended to the field name, but are being read here without the endpoint.
There was a problem hiding this comment.
Also noticed that the 3 lines above here have semicolons at the end that should be removed
There was a problem hiding this comment.
done to both. This was a bit of a weird issue- in matter we have the component_to_endpoint logic to get endpoints from a component, but in zigbee we don't. For now, I removed the endpoint logic altogether, so we cannot support different bounds across parent/child zigbee bulbs. I think this is an ok trade-off for the work it might take to add new infrastructure to handle this.
| -- Third tier: use defaults | ||
| min_mireds = switch_utils.COLOR_TEMPERATURE_MIRED_MIN | ||
| max_mireds = switch_utils.COLOR_TEMPERATURE_MIRED_MAX | ||
| end |
There was a problem hiding this comment.
It would probably be worth adding test cases to cover tiers 1 & 2
There was a problem hiding this comment.
I removed the tier 1 case... since I now have us persist the max/min fields there is never a case where the tier 1 exists and tier 2 doesn't.
That said, I added a test for the tier 2 (now tier 1) case
74f650c to
555e4e3
Compare
| local COLOR_TEMPERATURE_MIRED_MAX = 1000 -- 15000 Kelvin | ||
| local COLOR_TEMPERATURE_MIRED_MIN = 67 -- 1000 Kelvin |
There was a problem hiding this comment.
A note to reviewers- while looking at this all, I started noticing that while we are saving these fields in Kelvin, almost all functionality involving these occurs in Mireds. Therefore, I have altered the flow to more directly use mireds, only converting into kelvin at the specific colorTemperatureRange step.
I believe this should simplify the constant conversion back and forth.
| -- Copyright 2026 SmartThings, Inc. | ||
| -- Licensed under the Apache License, Version 2.0 | ||
|
|
||
| local capabilities = require "st.capabilities" |
There was a problem hiding this comment.
nit: Move this inside the function call for slight memory savings
There was a problem hiding this comment.
I'm not sure that will cause savings in this case, since st.capabilities is already required at the top of the main init.lua file.
6a091be to
0d7e0a1
Compare
Check all that apply
Type of Change
Checklist
Description of Change
Summary of Completed Tests