Skip to content

Zigbee window treatment: extend step timeout, use input step component #3319

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.

Sign up for GitHub

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

Open
hcarter-775 wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Zigbee window treatment: extend step timeout, use input step component #3319

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9a70540
Zigbee window treatment: extend step timeout to 120s and fix step com…
hcarter-775 Oct 30, 2026
1373f85
remove not-needed comment
hcarter-775 Nov 2, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
62 changes: 62 additions & 0 deletions ...ings/zigbee-window-treatment/src/test/test_zigbee_window_treatment_level_cluster_step.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -397,4 +397,66 @@ test.register_coroutine_test(
{ min_api_version = 17 }
)

-- Test: a step after the accumulation window expires starts from the reported level again
test.register_coroutine_test(
"statelessWindowShadeLevelStep with Level cluster - step after timeout starts from current shade level",
function()
test.timer.__create_and_queue_test_time_advance_timer(121, "oneshot")
test.socket.zigbee:__set_channel_ordering("relaxed")
test.socket.zigbee:__queue_receive({
axis_device.id,
Level.attributes.CurrentLevel:build_test_attr_report(axis_device, 127)
})
test.socket.capability:__expect_send({
axis_device.id,
{ capability_id = "windowShadeLevel", component_id = "main", attribute_id = "shadeLevel", state = { value = 50 } }
})
test.socket.capability:__expect_send({
axis_device.id,
{ capability_id = "windowShade", component_id = "main", attribute_id = "windowShade", state = { value = "partially open" } }
})
test.wait_for_events()

test.socket.capability:__queue_receive({
axis_device.id,
{ capability = "statelessWindowShadeLevelStep", component = "main", command = "stepShadeLevel", args = { 10 } }
})
test.socket.capability:__expect_send({
axis_device.id,
{ capability_id = "windowShadeLevel", component_id = "main", attribute_id = "shadeLevel", state = { value = 60 } }
})
test.socket.capability:__expect_send({
axis_device.id,
{ capability_id = "windowShade", component_id = "main", attribute_id = "windowShade", state = { value = "opening" } }
})
test.socket.zigbee:__expect_send({
axis_device.id,
Level.server.commands.MoveToLevelWithOnOff(axis_device, 152)
})
test.wait_for_events()

-- The 120 second window has elapsed, so the next step is computed from the current shade level (60)
test.mock_time.advance_time(121)
test.wait_for_events()

test.socket.capability:__queue_receive({
axis_device.id,
{ capability = "statelessWindowShadeLevelStep", component = "main", command = "stepShadeLevel", args = { 10 } }
})
test.socket.capability:__expect_send({
axis_device.id,
{ capability_id = "windowShadeLevel", component_id = "main", attribute_id = "shadeLevel", state = { value = 70 } }
})
test.socket.capability:__expect_send({
axis_device.id,
{ capability_id = "windowShade", component_id = "main", attribute_id = "windowShade", state = { value = "opening" } }
})
test.socket.zigbee:__expect_send({
axis_device.id,
Level.server.commands.MoveToLevelWithOnOff(axis_device, 178)
})
end,
{ min_api_version = 17 }
)

test.run_registered_tests()
7 changes: 4 additions & 3 deletions drivers/SmartThings/zigbee-window-treatment/src/window_shade_utils.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ utils.PRESET_LEVEL_KEY = "_presetLevel"
-- within a short time frame while preventing indefinite accumulation.
local LATEST_TARGET_LEVEL = "_latestTargetLevel"
-- Timeout in seconds for the stateless step accumulation window
local TARGET_LEVEL_TIME_OUT_SECONDS = 15
local TARGET_LEVEL_TIME_OUT_SECONDS = 120

@cjswedes cjswedes Nov 2, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems excessively long. What blinds actually take 2 minutes to move through their full range? Was this from a testing situation?

@hcarter-775 hcarter-775 Nov 2, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It definitely can take over a minute, especially to go 0-100 -- to clarify, if the device reaches the final state, it will stop this timer. So it really is just a backup, catchall. I recall Ikea takes a long time. This is mostly cleanup while investigating improvements while I'm doing Zwave.

-- Field name for the timer that clears LATEST_TARGET_LEVEL after timeout
local TARGET_LEVEL_TIME_OUT = "_targetLevelTimeOut"

Expand Down Expand Up @@ -48,17 +48,18 @@ utils.step_shade_level_handler = function(driver, device, command)

-- Step from the latest target level if it exists, or from the current shade level
local latest_target_level = device:get_field(LATEST_TARGET_LEVEL) or
device:get_latest_state("main", capabilities.windowShadeLevel.ID, capabilities.windowShadeLevel.shadeLevel.NAME, 0)
device:get_latest_state(command.component, capabilities.windowShadeLevel.ID, capabilities.windowShadeLevel.shadeLevel.NAME, 0)
local new_target_level = st_utils.clamp_value(latest_target_level + step, 0, 100)

-- Cancel any previous timer and set a 15 second timeout timer to ensure LATEST_TARGET_LEVEL is cleared
-- Cancel any previous timer and set a 120 second timeout timer to ensure LATEST_TARGET_LEVEL is cleared
-- This is to prevent the stateless step from accumulating indefinitely, while ensuring that a single
-- "scroll" action can accumulate multiple steps within a short time frame
if device:get_field(TARGET_LEVEL_TIME_OUT) then
device.thread:cancel_timer(device:get_field(TARGET_LEVEL_TIME_OUT))
end
local timer = device.thread:call_with_delay(TARGET_LEVEL_TIME_OUT_SECONDS, function(d)
device:set_field(LATEST_TARGET_LEVEL, nil)
device:set_field(TARGET_LEVEL_TIME_OUT, nil)

@cjswedes cjswedes Nov 2, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the only place the target timeout and target level is cleared. You should clear it from the open, close, preset, and setShadeLevel handlers also (which is what matter does). Its needed to avoid a situation where subsequent commands dont have stale data.

Example (all within the 2 min window):

  1. Step +10 from 50-60
  2. close command
  3. report level 0
  4. step +10; expected to move to 10, but moves to 70 instead.

@hcarter-775 hcarter-775 Nov 2, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Matter is much more comprehensive, I agree. I put a lot of work into making that driver tick in tune, haha. However, I do not think adding a bunch of handling like that is particular helpful, as the case you described is fairly unlikely in my opinion. I would prefer to track something like you're describing, a more comprehensive state-tracking system, in some kind of follow up. One big difference of Matter, for example, is that there aren't 10 subdrivers that might override open/close, etc in various ways. This basically is just a cleanup commit, to actually nil out what should be unset anyway, but just wasn't.

end)
device:set_field(TARGET_LEVEL_TIME_OUT, timer)
device:set_field(LATEST_TARGET_LEVEL, new_target_level)
Expand Down
Loading