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

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

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

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

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:12:47Z

      [capabilities.windowShadeLevel.commands.setShadeLevel.NAME] = handle_shade_level,
    },
+    [capabilities.statelessWindowShadeLevelStep.ID] = {
+      [capabilities.statelessWindowShadeLevelStep.commands.stepShadeLevel.NAME] = window_shade_step_level_cmd


Suggested change

[capabilities.statelessWindowShadeLevelStep.commands.stepShadeLevel.NAME] = window_shade_step_level_cmd

[capabilities.statelessWindowShadeLevelStep.commands.stepShadeLevel.NAME] = handle_step_shade_level

let's keep the same language consistency

hcarter-775 · 2026-07-06T19:13:00Z


 local matter_driver = MatterDriver("matter-window-covering", matter_driver_template)
-matter_driver:run()
+matter_driver:run()


we shouldn't remove the newline

hcarter-775 · 2026-07-06T19:17:59Z


 -- capability handlers
 local function handle_preset(driver, device, cmd)
+  device:set_field(LATEST_TARGET_LEVEL, nil)


why are we setting this to nil in every handler?

hcarter-775 · 2026-07-06T19:25:50Z

+  local target_level = current_level + step
+  if target_level > 100 then target_level = 100
+  elseif target_level < 0 then target_level = 0
+  end


Suggested change

local target_level = current_level + step

if target_level > 100 then target_level = 100

elseif target_level < 0 then target_level = 0

end

local target_level = st_utils.clamp_value(current_level + step, 0, 100)

and then you can do local st_utils = require "st.utils" at the top of the file.

hcarter-775 · 2026-07-06T19:27:03Z

+  -- Matter uses inverted logic (like IKEA)
+  -- User level: 0=closed, 100=open
+  -- Matter level: 10000=open, 0=closed (in percent100ths)


remove this comment

hcarter-775 · 2026-07-06T19:27:22Z

+  local lift_percentage_value = 100 - target_level
+  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)


we need to handle the case where the device is in the reverse mode

hcarter-775 · 2026-07-06T19:27:55Z

+  end
+
+  -- Update tracking state
+  device:set_field(LATEST_TARGET_LEVEL, target_level)


why do we even need this field? What purpose is it really serving here?

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

hcarter-775 · 2026-07-06T19:36:27Z

+        device.thread:cancel_timer(timer)
+        device:set_field(TARGET_LEVEL_TIME_OUT, nil)
+      end
+    end


I don't see why this is needed. kinda repeating myself here, but I'm not really sure what this logic is gaining for us.

tpmanley · 2026-07-06T20:19:57Z

    version: 1
  - id: windowShadeTiltLevel
    version: 1
+  - id: statelessWindowShadeLevelStep


My initial thought is we'd only add the new capability to profiles that have the windowShadeLevel capability, and so it shouldn't be added to all these profiles that have windowShadeTiltLevel and not windowShadeLevel. In other words, we wouldn't support the stateless step commands on shades that only support tilt. If we want to do that in the future we should add a statelessWindowShadeTiltStep capability, but that isn't in scope for the current project.

hcarter-775 · 2026-07-06T20:22:47Z

+  - id: statelessWindowShadeLevelStep
+    version: 1


I might recommend moving the statelessWindowShadeLevelStep capability just below the windowShadeLevel capability in all cases, for consistency. A few of these profile updates have the new capability in different spots.

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 from d97b8d2 to c659987 Compare June 29, 2026 09:40

Update matter blind edge driver

f5b95e2

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 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

hcarter-775 reviewed Jul 6, 2026

View reviewed changes

	[capabilities.statelessWindowShadeLevelStep.commands.stepShadeLevel.NAME] = window_shade_step_level_cmd
	[capabilities.statelessWindowShadeLevelStep.commands.stepShadeLevel.NAME] = handle_step_shade_level

Uh oh!

Conversation

hongye-samsung commented Jun 26, 2026

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!

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

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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants