Update matter window covering edge driver by hongye-samsung · Pull Request #3052 · SmartThingsCommunity/SmartThingsEdgeDrivers

hongye-samsung · 2026-06-26T02:04:54Z

Check all that apply

Type of Change

WWST Certification Request
- If this is your first time contributing code:
  - I have reviewed the README.md file
  - I have reviewed the CODE_OF_CONDUCT.md file
  - I have signed the CLA
- I plan on entering a WWST Certification Request or have entered a request through the WWST Certification console at developer.smartthings.com
Bug fix
New feature
Refactor

Checklist

I have performed a self-review of my code
I have commented my code in hard-to-understand areas
I have verified my changes by testing with a device or have communicated a plan for testing
I am adding new behavior, such as adding a sub-driver, and have added and run new unit tests to cover the new behavior

Description of Change

Summary of Completed Tests

Co-Authored-By: Cline SR (50%)
CGAI & AI Review

vangoran · 2026-06-29T05:03:21Z

Edge driver development for R2 new feature (https://smartthings.atlassian.net/browse/CHAD-18661)

hongye-samsung · 2026-06-30T08:48:15Z

@HunsupJung @hcarter-775 @z-michel

hcarter-775

Seems like the new fields may be unnecessary, and may be adding some unnecessary complexity

hcarter-775 · 2026-07-06T19:35:45Z

+    -- Step control logic
+    local target_level_field = device:get_field(LATEST_TARGET_LEVEL)
+    if target_level_field and attribute == capabilities.windowShadeLevel.shadeLevel then
+      -- Allow ±1 degree tolerance for reaching target
+      if math.abs(position - target_level_field) <= TARGET_REACH_TOLERANCE then
+        -- Device reached target position, clear target marker and timeout timer
+        device:set_field(LATEST_TARGET_LEVEL, nil)
+        local timer = device:get_field(TARGET_LEVEL_TIME_OUT)
+        if timer ~= nil then
+          device.thread:cancel_timer(timer)
+          device:set_field(TARGET_LEVEL_TIME_OUT, nil)
+        end
+      end
+    end


I don't think I understand why this is useful. The command will send the device here anyway? This just removes the field without changing anything as far as I can see

If the clearing operation is not performed, if the user manually pulls the curtain or sends the setShadeLevel command to change the curtain position within 30 seconds before the timer triggers, the baseline value (in line 194) will be an expired target value when the step command is sent subsequently, instead of the actual position of the curtain, causing the curtain to move to the wrong position.

I think the issue here is rather that the timeout is too long. We should probably just make the timeout shorter, since all that the timer is doing is affecting the step-level functionality.

I propose we make the timer ~5 seconds or so, and then remove this logic altogether

We believe it's better to retain this part, as it makes the logic more rigorous and avoids some extreme cases.

I propose we make the timeout shorter, to more like 10 or 15 seconds.

Okay, we'll change it to 15 seconds.

github-actions · 2026-07-07T16:41:53Z

Duplicate profile check: Warning - duplicate profiles detected.
window-covering-battery.yml == window-covering-profile.yml

github-actions · 2026-07-07T16:42:41Z

Invitation URL:
https://bestow-regional.api.smartthings.com/invite/eGM65K67oejA

github-actions · 2026-07-07T16:43:35Z

Test Results

73 files 536 suites 0s ⏱️
3 207 tests 3 136 ✅ 0 💤 0 ❌ 71 🔥
5 228 runs 5 125 ✅ 0 💤 0 ❌ 103 🔥

For more details on these errors, see this check.

Results for commit e53ee91.

♻️ This comment has been updated with latest results.

github-actions · 2026-07-07T16:43:37Z

File	Coverage
All files	`30%`	❌
/home/runner/work/SmartThingsEdgeDrivers/SmartThingsEdgeDrivers/drivers/SmartThings/matter-window-covering/src/init.lua	`30%`	❌

Minimum allowed coverage is 90%

Generated by 🐒 cobertura-action against e53ee91

hcarter-775 · 2026-07-07T16:44:36Z

Hi, I just had the workflows run and there are some warnings showing up. Can you fix these warnings as well? Thanks!

accidental approval, unit tests and comments must still be addressed

zhangshen001 · 2026-07-16T06:34:12Z

The statelessWindowShadeLevelStep capability is newly introduced in version 62 (dev), so the driver tests on versions 59 to 61 have failed.

hcarter-775 · 2026-07-16T16:38:32Z

+  local lift_percentage_value
+  if device:get_field(REVERSE_POLARITY) then
+    lift_percentage_value = target_level
+  else
+    lift_percentage_value = 100 - target_level
+  end
+  local hundredths_lift_percentage = lift_percentage_value * 100
+
+  local endpoint_id = device:component_to_endpoint(cmd.component)
+  local req = clusters.WindowCovering.server.commands.GoToLiftPercentage(
+    device, endpoint_id, hundredths_lift_percentage
+  )
+  device:send(req)


Suggested change

local lift_percentage_value

if device:get_field(REVERSE_POLARITY) then

lift_percentage_value = target_level

else

lift_percentage_value = 100 - target_level

end

local hundredths_lift_percentage = lift_percentage_value * 100

local endpoint_id = device:component_to_endpoint(cmd.component)

local req = clusters.WindowCovering.server.commands.GoToLiftPercentage(

device, endpoint_id, hundredths_lift_percentage

)

device:send(req)

driver:inject_capability_command(device, {

capability = capabilities.windowShadeLevel.ID,

component = command.component,

command = capabilities.windowShadeLevel.commands.setShadeLevel.NAME,

named_args = { shadeLevel = target_level }

})

This will allow the handler to support the closure device type as well. This also makes me realize that the current setShadeLevel handlers (here and in the closure subdriver) do not support "reverse". We must add this with this implementation. Thanks

We've added reverse polarity support to handle_shade_level.

This PR also includes reverse polarity support for handling_shade_level, so some tests will focus on this aspect.

hcarter-775 · 2026-07-18T17:27:47Z

  {test_init = test_init_door}
 )

+-- Test: handle_level command for door type


Hi, I think a lot of unrelated unit tests have been added to this file, related to various utilities of the closures subdriver. I think these are outside the scope of this PR and should be removed for now.

I also do not see any unit tests directly related to the stateless step handling, which would be the tests within scope that we'd need.

Okay, tests irrelevant to this PR change have been removed.

Hi @partner-hub-chenxuelong , perhaps I'm not seeing something, but that does not appear to be true? Would you mind checking this again?

This PR also includes reverse polarity support for handling_shade_level, so some tests will focus on this aspect.

We re-examined these two files (test_matter_closure.lua, test_matter_window_covering.lua), removing unnecessary tests and keeping only those related to stepShadeLevel and reverse polarity.

partner-hub-chenxuelong · 2026-07-20T10:36:30Z

TC has been updated. Please recalculate the coverage

hcarter-775 · 2026-07-20T17:22:05Z

+  }
+)
+
+-- Test power_source_attribute_list_handler - BatChargeLevel present


we have way too many tests added in this file as well. Please prune these to only the relevant files to this PR

We re-examined these two files (test_matter_closure.lua, test_matter_window_covering.lua), removing unnecessary tests and keeping only those related to stepShadeLevel and reverse polarity.

Signed-off-by: Hong Ye/MDE Lab /SRC-Nanjing/Staff Engineer/Samsung Electronics <hong.ye@samsung.com>

hcarter-775 · 2026-07-21T15:19:13Z

-  "Test mains powered device does not switch to battery profile",
-  function()
-  end,
-  {


there is no need to remove this test

hcarter-775 · 2026-07-21T15:20:26Z

+    test.socket.device_lifecycle():__queue_receive(mock_device:generate_info_changed({
+      old_st_store = { profile = { name = "window-covering-tilt-battery" }, preferences = { reverse = false } },
+      preferences = { reverse = "true" }
+    }))


I am not sure why a profile update is required to occur here?

hcarter-775 · 2026-07-21T15:20:44Z

+    test.socket.device_lifecycle():__queue_receive(mock_device:generate_info_changed({
+      old_st_store = { profile = { name = "window-covering-tilt-battery" }, preferences = { reverse = false } },
+      preferences = { reverse = "true" }
+    }))


I am not sure why a profile update needs to happen here?

hcarter-775 · 2026-07-21T19:51:30Z

+)
+
+test.register_coroutine_test(
+  "WindowShade stepShadeLevel - other commands clear target marker", function()


remove this test, since this no longer occurs.

hcarter-775 · 2026-07-21T19:51:47Z

+
+-- Test handle_close with reverse polarity
+test.register_coroutine_test(
+  "WindowShade close cmd with reverse polarity true",


remove this test, it's not in the scope of this PR and is tested elsewhere

hcarter-775 · 2026-07-21T19:53:47Z

+
+-- Test current_pos_handler - reverse polarity state handling
+test.register_coroutine_test(
+  "CurrentPosition - reverse polarity affects state interpretation",


remove this test, it's not in the scope of this PR and is tested elsewhere

hcarter-775 · 2026-07-21T19:54:08Z

+
+-- Test current_status_handler - stopped clears target level
+test.register_coroutine_test(
+  "OperationalStatus stopped clears target level field",


remove this test, since this no longer occurs.

hcarter-775 · 2026-07-21T19:56:20Z

+-- ---------------------------------------------------------------------------
+-- Reverse polarity tests
+-- ---------------------------------------------------------------------------
+
+test.register_coroutine_test(
+  "windowShade close command with reverse polarity sends MOVE_TO_FULLY_OPEN", function()
+    update_profile()
+    test.wait_for_events()
+    -- Set REVERSE_POLARITY field directly
+    local fields = require "sub_drivers.closure.closure_utils.fields"
+    mock_device:set_field(fields.REVERSE_POLARITY, true, {persist = true})
+    test.socket.capability:__queue_receive({
+      mock_device.id,
+      {capability = "windowShade", component = "main", command = "close", args = {}},
+    })
+    -- With reverse=true, close should send MOVE_TO_FULLY_OPEN
+    test.socket.matter:__expect_send({
+      mock_device.id,
+      clusters.ClosureControl.server.commands.MoveTo(
+        mock_device, 10, clusters.ClosureControl.types.TargetPositionEnum.MOVE_TO_FULLY_OPEN
+      )
+    })
+  end
+)
+
+test.register_coroutine_test(
+  "windowShade open command with reverse polarity sends MOVE_TO_FULLY_CLOSED", function()
+    update_profile()
+    test.wait_for_events()
+    -- Set REVERSE_POLARITY field directly
+    local fields = require "sub_drivers.closure.closure_utils.fields"
+    mock_device:set_field(fields.REVERSE_POLARITY, true, {persist = true})
+    test.socket.capability:__queue_receive({
+      mock_device.id,
+      {capability = "windowShade", component = "main", command = "open", args = {}},
+    })
+    -- With reverse=true, open should send MOVE_TO_FULLY_CLOSED
+    test.socket.matter:__expect_send({
+      mock_device.id,
+      clusters.ClosureControl.server.commands.MoveTo(
+        mock_device, 10, clusters.ClosureControl.types.TargetPositionEnum.MOVE_TO_FULLY_CLOSED
+      )
+    })
+  end
+)


Remove these tests, they are outside the scope of this PR

hcarter-775 · 2026-07-21T19:56:54Z

+test.register_coroutine_test(
+  "windowShade setShadeLevel with reverse polarity inverts level", function()
+    update_profile()
+    test.wait_for_events()
+    -- Set REVERSE_POLARITY field directly
+    local fields = require "sub_drivers.closure.closure_utils.fields"
+    mock_device:set_field(fields.REVERSE_POLARITY, true, {persist = true})
+    test.socket.capability:__queue_receive({
+      mock_device.id,
+      {capability = "windowShadeLevel", component = "windowShade1", command = "setShadeLevel", args = {75}},
+    })
+    -- With reverse=true, 75 should be inverted to 25
+    test.socket.matter:__expect_send({
+      mock_device.id,
+      clusters.ClosureDimension.server.commands.SetTarget(mock_device, 11, 25 * 100)
+    })
+  end
+)


Let's add a test for this without reverse polarity.

hcarter-775 · 2026-07-21T20:13:52Z

+  local lift_percentage_value
+  if device:get_field(REVERSE_POLARITY) then
+    lift_percentage_value = cmd.args.shadeLevel
+  else
+    lift_percentage_value = 100 - cmd.args.shadeLevel
+  end


Hi all, I know I suggested this originally, but I have looked into it further and I am no longer comfortable making this change to the "reverse" handling here. I believe we should leave this as it was before for the time being, and if we decide that the current behavior is not good handling, we should add this stuff back later.

My primary issue is that I do not want this PR to possibly screw up the current reverse handling without understanding deeply why it was done like this in the first place. I suggest we remove the reverse handling in the closures subdriver as well. Thanks for understanding

hongye-samsung marked this pull request as draft June 26, 2026 02:08

hongye-samsung force-pushed the matter_blind branch 3 times, most recently from 571f3d8 to efdf5b3 Compare June 26, 2026 08:35

hongye-samsung marked this pull request as ready for review June 29, 2026 04:56

hongye-samsung force-pushed the matter_blind branch 2 times, most recently from c659987 to f5b95e2 Compare June 29, 2026 10:45

hcarter-775 reviewed Jul 6, 2026

View reviewed changes

tpmanley reviewed Jul 6, 2026

View reviewed changes

Comment thread drivers/SmartThings/matter-window-covering/profiles/window-covering-tilt-only-battery.yml Outdated

hcarter-775 reviewed Jul 6, 2026

View reviewed changes

Comment thread drivers/SmartThings/matter-window-covering/profiles/window-covering-tilt-battery.yml

tpmanley requested a review from HunsupJung July 8, 2026 18:25

hongye-samsung force-pushed the matter_blind branch from f5b95e2 to ccbd180 Compare July 9, 2026 09:36

hcarter-775 previously approved these changes Jul 10, 2026

View reviewed changes

hcarter-775 self-requested a review July 10, 2026 22:43

hcarter-775 reviewed Jul 10, 2026

View reviewed changes

Comment thread drivers/SmartThings/matter-window-covering/src/init.lua Outdated

partner-hub-chenxuelong referenced this pull request Jul 14, 2026

Handle Zigbee Window Stateless Step Commands

04af865

hongye-samsung force-pushed the matter_blind branch from ccbd180 to 734a936 Compare July 14, 2026 11:58

hongye-samsung force-pushed the matter_blind branch from 734a936 to d62b349 Compare July 16, 2026 09:31

hcarter-775 reviewed Jul 16, 2026

View reviewed changes

hongye-samsung force-pushed the matter_blind branch from d62b349 to f1bfc46 Compare July 17, 2026 08:13

hcarter-775 reviewed Jul 18, 2026

View reviewed changes

hcarter-775 reviewed Jul 20, 2026

View reviewed changes

Update matter blind edge driver

e53ee91

Signed-off-by: Hong Ye/MDE Lab /SRC-Nanjing/Staff Engineer/Samsung Electronics <hong.ye@samsung.com>

hongye-samsung force-pushed the matter_blind branch from f1bfc46 to e53ee91 Compare July 21, 2026 03:22

hcarter-775 requested changes Jul 21, 2026

View reviewed changes

Uh oh!

Conversation

hongye-samsung commented Jun 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Type of Change

Checklist

Description of Change

Summary of Completed Tests

Uh oh!

vangoran commented Jun 29, 2026

Uh oh!

hongye-samsung commented Jun 30, 2026

Uh oh!

hcarter-775 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions Bot commented Jul 7, 2026

Uh oh!

github-actions Bot commented Jul 7, 2026

Uh oh!

github-actions Bot commented Jul 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Test Results

Uh oh!

github-actions Bot commented Jul 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hcarter-775 commented Jul 7, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

zhangshen001 commented Jul 16, 2026

Uh oh!

hcarter-775 Jul 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

hongye-samsung commented Jun 26, 2026 •

edited

Loading

github-actions Bot commented Jul 7, 2026 •

edited

Loading

github-actions Bot commented Jul 7, 2026 •

edited

Loading

hcarter-775 Jul 16, 2026 •

edited

Loading

hcarter-775 Jul 21, 2026 •

edited

Loading