Skip to content
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

Matter Window Covering: Support the tilt feature #1817

Merged
merged 2 commits into from
Feb 7, 2025
Merged

Matter Window Covering: Support the tilt feature #1817

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
735b1d6
Support the tilt feature
nickolas-deboom Dec 10, 2024
57ffd97
Merge branch 'main' into matter-window-covering-support-tilt
nickolas-deboom Feb 7, 2025
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
23 changes: 23 additions & 0 deletions drivers/SmartThings/matter-window-covering/profiles/window-covering-tilt-battery.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
name: window-covering-tilt-battery
components:
- id: main
capabilities:
- id: windowShade
version: 1
- id: windowShadePreset
version: 1
- id: windowShadeLevel
version: 1
- id: windowShadeTiltLevel
version: 1
- id: battery
version: 1
- id: firmwareUpdate
version: 1
- id: refresh
version: 1
categories:
- name: Blind
preferences:
- preferenceId: presetPosition
explicit: true
16 changes: 16 additions & 0 deletions drivers/SmartThings/matter-window-covering/profiles/window-covering-tilt-only-battery.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
name: window-covering-tilt-only-battery
components:
- id: main
capabilities:
- id: windowShade
version: 1
- id: windowShadeTiltLevel
version: 1
- id: battery
version: 1
- id: firmwareUpdate
version: 1
- id: refresh
version: 1
categories:
- name: Blind
14 changes: 14 additions & 0 deletions drivers/SmartThings/matter-window-covering/profiles/window-covering-tilt-only.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
name: window-covering-tilt-only
components:
- id: main
capabilities:
- id: windowShade
version: 1
- id: windowShadeTiltLevel
version: 1
- id: firmwareUpdate
version: 1
- id: refresh
version: 1
categories:
- name: Blind
21 changes: 21 additions & 0 deletions drivers/SmartThings/matter-window-covering/profiles/window-covering-tilt.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
name: window-covering-tilt
components:
- id: main
capabilities:
- id: windowShade
version: 1
- id: windowShadePreset
version: 1
- id: windowShadeLevel
version: 1
- id: windowShadeTiltLevel
version: 1
- id: firmwareUpdate
version: 1
- id: refresh
version: 1
categories:
- name: Blind
preferences:
- preferenceId: presetPosition
explicit: true
131 changes: 104 additions & 27 deletions drivers/SmartThings/matter-window-covering/src/init.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ local log = require "log"
local clusters = require "st.matter.clusters"
local MatterDriver = require "st.matter.driver"

local CURRENT_LIFT = "__current_lift"
local CURRENT_TILT = "__current_tilt"
local battery_support = {
NO_BATTERY = "NO_BATTERY",
BATTERY_LEVEL = "BATTERY_LEVEL",
Expand All @@ -46,7 +48,16 @@ local function component_to_endpoint(device, component_name)
end

local function match_profile(device, battery_supported)
local lift_eps = device:get_endpoints(clusters.WindowCovering.ID, {feature_bitmap = clusters.WindowCovering.types.Feature.LIFT})
local tilt_eps = device:get_endpoints(clusters.WindowCovering.ID, {feature_bitmap = clusters.WindowCovering.types.Feature.TILT})
local profile_name = "window-covering"
if #tilt_eps > 0 then
profile_name = profile_name .. "-tilt"
if #lift_eps == 0 then
profile_name = profile_name .. "-only"
end
end

if battery_supported == battery_support.BATTERY_PERCENTAGE then
profile_name = profile_name .. "-battery"
elseif battery_supported == battery_support.BATTERY_LEVEL then
Expand Down Expand Up @@ -100,13 +111,23 @@ local function device_removed(driver, device) log.info("device removed") end
-- capability handlers
local function handle_preset(driver, device, cmd)
local endpoint_id = device:component_to_endpoint(cmd.component)
local lift_value = 100 - device.preferences.presetPosition
local hundredths_lift_percent = lift_value * 100
local req = clusters.WindowCovering.server.commands.GoToLiftPercentage(
device, endpoint_id, hundredths_lift_percent
)

device:send(req)
local lift_eps = device:get_endpoints(clusters.WindowCovering.ID, {feature_bitmap = clusters.WindowCovering.types.Feature.LIFT})
local tilt_eps = device:get_endpoints(clusters.WindowCovering.ID, {feature_bitmap = clusters.WindowCovering.types.Feature.TILT})
if #lift_eps > 0 then
local lift_value = 100 - device.preferences.presetPosition
local hundredths_lift_percent = lift_value * 100
local req = clusters.WindowCovering.server.commands.GoToLiftPercentage(
device, endpoint_id, hundredths_lift_percent
)
device:send(req)
end
if #tilt_eps > 0 then
-- Use default preset tilt percentage to 50 until a canonical preference is created for preset tilt position
local req = clusters.WindowCovering.server.commands.GoToTiltPercentage(
device, endpoint_id, 50 * 100
)
device:send(req)
end
end

-- close covering
Expand All @@ -130,8 +151,7 @@ local function handle_pause(driver, device, cmd)
device:send(req)
end

-- move to shade level
-- beteween 0-100
-- move to shade level between 0-100
local function handle_shade_level(driver, device, cmd)
local endpoint_id = device:component_to_endpoint(cmd.component)
local lift_percentage_value = 100 - cmd.args.shadeLevel
Expand All @@ -142,29 +162,78 @@ local function handle_shade_level(driver, device, cmd)
device:send(req)
end

-- current lift percentage, changed to 100ths percent
local function current_pos_handler(driver, device, ib, response)
if ib.data.value == nil then
return
end
local windowShade = capabilities.windowShade.windowShade
local position = 100 - math.floor((ib.data.value / 100))
device:emit_event_for_endpoint(ib.endpoint_id, capabilities.windowShadeLevel.shadeLevel(position))
if position == 0 then
device:emit_event_for_endpoint(ib.endpoint_id, windowShade.closed())
elseif position == 100 then
device:emit_event_for_endpoint(ib.endpoint_id, windowShade.open())
elseif position > 0 and position < 100 then
device:emit_event_for_endpoint(ib.endpoint_id, windowShade.partially_open())
else
device:emit_event_for_endpoint(ib.endpoint_id, windowShade.unknown())
-- move to shade tilt level between 0-100
local function handle_shade_tilt_level(driver, device, cmd)
local endpoint_id = device:component_to_endpoint(cmd.component)
local tilt_percentage_value = 100 - cmd.args.level
local hundredths_tilt_percentage = tilt_percentage_value * 100
local req = clusters.WindowCovering.server.commands.GoToTiltPercentage(
device, endpoint_id, hundredths_tilt_percentage
)
device:send(req)
end

-- current lift/tilt percentage, changed to 100ths percent
local current_pos_handler = function(attribute)
return function(driver, device, ib, response)
if ib.data.value == nil then
return
end
local windowShade = capabilities.windowShade.windowShade
local position = 100 - math.floor((ib.data.value / 100))
Copy link

Choose a reason for hiding this comment

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

@nickolas-deboom I think the position values of both tilt and lift must be considered on every report when deciding the window shade's status as currently the shade's status is based solely on the attribute that was last reported.
For example:

  1. Lift is at 100%. Window Shade says "Open".
  2. Move Tilt to 0%. Window Shade would now be shown as "Closed".

Please let me know if I'm overlooking anything, or if this is intentional.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi @SathvikGatti , I think you are correct, and I made an update to consider both lift and tilt on each position update. The lift/tilt values are captured each time current_pos_handler runs, and new logic is introduced with a few considerations for shades that support both lift and tilt. For example, if the lift is at 100%, the tilt value does not matter, the shade will be considered open. Similarly, if the lift is at 0%, the shade is only considered closed if the tilt value is also 0% (it would be partially open for any other tilt value).

Copy link

Choose a reason for hiding this comment

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

@nickolas-deboom, thanks for the update! There is a scenario where the issue persists when a device with both Tilt and Lift features that is also handled by sub-driver is onboarded.

This issue is seen as the lift attribute handling is done in the sub-driver, while the tilt reports are handled in the base/main driver. Due to this, the window shade status shown is based on the attribute that was last reported.

Currently, only the Virtual Device App is listed in the sub-driver's can_handle. Should this be handled as well?

CC: @HunsupJung

Copy link
Collaborator

Choose a reason for hiding this comment

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

@SathvikGatti
I'm against modifying to sub driver. VDA's role is to test drivers before actual products are released. Virtual window covering is using a sub driver, which is only used to test edge cases. I don't think it's a good direction. VDA should use the main driver used by most real devices. There are two ways to do that.

  1. Remove the VDA from the sub-driver and modify the VDA to operate like a device that uses a main driver.
  2. Create a new virtual window covering using the main driver.

I think we should decide between the two.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@HunsupJung I agree with you and think the VDA should probably be updated to be compatible with the main driver.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree and that way the VDA will continue to work in the meantime!

Copy link

Choose a reason for hiding this comment

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

@nickolas-deboom sometimes there's an issue seen with window shade status observed with VDA window covering having tilt feature and probably could be seen with real devices.
Currently VDA window covering reports tilt and lift values once the movement is complete. I'll try to explain with an example, pardon me for the lengthy explanation :)

Assuming lift and tilt are set to 100, on invoking "Close", tilt and lift move towards 0. Depending on what attribute's report is received (tilt/lift) different window shade status can be emitted twice in a short span of time.
Driver receives tilt's report set to 0 first:
windowShadeTiltLevel="0"
windowShade="\open"

The window shade status set to "Open" is updated as current tilt "0" is compared with previous lift "100". This is because the latest value of lift is not received yet.

Then on receiving lift's report(0) from device "closed" is emitted
shadeLevel="0"
windowShade="\closed"

Since there were two window shade statuses emitted within a short span (arround 200ms) sometimes capability events displayed in ST-App's event history are not in order and the first sent status I.e "Open" is shown.

CC: @HunsupJung

Copy link
Contributor

@hcarter-775 hcarter-775 Jan 17, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

@SathvikGatti so if I'm understanding correctly, there are two orders that these events can be sent in.

The first:

  1. tilt received for 0 - leaving it as "open"
  2. lift received for 0 - correctly showing it as "closed"

The second:

  1. lift received for 0 - showing it as "partially open"
  2. tilt received for 0 - correctly showing it as "closed"

Both of these paths leave the device in the "closed" state in the end, so I am confused what case you are describing.

Edit: I think I see what you are saying now; because the VDA sends reports multiple times while closing, it is possible that it one of its open commands is received after the final closed command.

If this is what you are describing, this should be seen as an error with the VDA device. There is no reason to be sending updates that quickly where issues like this arise, and real devices generally should not be handled like this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think they meant that there is no guaranteed order of the capabilities being processed and so the final state of the capability in the app could appear as either open or closed (for scenario 1) after both capabilities are emitted. This is a good point and I think we may need to make an update with this consideration.

Copy link

Choose a reason for hiding this comment

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

yes, even though the right window shade status is reported at the end, sometimes the capability events are not processed in the right order and the final status shown is not correct.

device:emit_event_for_endpoint(ib.endpoint_id, attribute(position))

if attribute == capabilities.windowShadeLevel.shadeLevel then
device:set_field(CURRENT_LIFT, position)
else
device:set_field(CURRENT_TILT, position)
end

local lift_position = device:get_field(CURRENT_LIFT)
local tilt_position = device:get_field(CURRENT_TILT)

-- Update the window shade status according to the lift and tilt positions.
-- LIFT TILT Window Shade
-- 100 any Open
-- 1-99 any Partially Open
-- 0 1-100 Partially Open
-- 0 0 Closed
-- 0 nil Closed
-- nil 100 Open
-- nil 1-99 Partially Open
-- nil 0 Closed
-- Note that lift or tilt may be nil if either the window shade does not
-- support them or if they haven't been received from a device report yet.

if lift_position == nil then
if tilt_position == 0 then
device:emit_event_for_endpoint(ib.endpoint_id, windowShade.closed())
elseif tilt_position == 100 then
device:emit_event_for_endpoint(ib.endpoint_id, windowShade.open())
else
device:emit_event_for_endpoint(ib.endpoint_id, windowShade.partially_open())
end

elseif lift_position == 100 then
device:emit_event_for_endpoint(ib.endpoint_id, windowShade.open())

elseif lift_position > 0 then
device:emit_event_for_endpoint(ib.endpoint_id, windowShade.partially_open())

elseif lift_position == 0 then
if tilt_position == nil or tilt_position == 0 then
device:emit_event_for_endpoint(ib.endpoint_id, windowShade.closed())
elseif tilt_position > 0 then
device:emit_event_for_endpoint(ib.endpoint_id, windowShade.partially_open())
end
end
end
end

-- checks the current position of the shade
local function current_status_handler(driver, device, ib, response)
local windowShade = capabilities.windowShade.windowShade
local state = ib.data.value & clusters.WindowCovering.types.OperationalStatus.GLOBAL --Could use LIFT instead
local state = ib.data.value & clusters.WindowCovering.types.OperationalStatus.GLOBAL
if state == 1 then -- opening
device:emit_event_for_endpoint(ib.endpoint_id, windowShade.opening())
elseif state == 2 then -- closing
Expand Down Expand Up @@ -228,7 +297,8 @@ local matter_driver_template = {
},
[clusters.WindowCovering.ID] = {
--uses percent100ths more often
[clusters.WindowCovering.attributes.CurrentPositionLiftPercent100ths.ID] = current_pos_handler,
[clusters.WindowCovering.attributes.CurrentPositionLiftPercent100ths.ID] = current_pos_handler(capabilities.windowShadeLevel.shadeLevel),
[clusters.WindowCovering.attributes.CurrentPositionTiltPercent100ths.ID] = current_pos_handler(capabilities.windowShadeTiltLevel.shadeTiltLevel),
[clusters.WindowCovering.attributes.OperationalStatus.ID] = current_status_handler,
},
[clusters.PowerSource.ID] = {
Expand All @@ -246,6 +316,9 @@ local matter_driver_template = {
clusters.LevelControl.attributes.CurrentLevel,
clusters.WindowCovering.attributes.CurrentPositionLiftPercent100ths,
},
[capabilities.windowShadeTiltLevel.ID] = {
clusters.WindowCovering.attributes.CurrentPositionTiltPercent100ths,
},
[capabilities.battery.ID] = {
clusters.PowerSource.attributes.BatPercentRemaining
},
Expand All @@ -268,9 +341,13 @@ local matter_driver_template = {
[capabilities.windowShadeLevel.ID] = {
[capabilities.windowShadeLevel.commands.setShadeLevel.NAME] = handle_shade_level,
},
[capabilities.windowShadeTiltLevel.ID] = {
[capabilities.windowShadeTiltLevel.commands.setShadeTiltLevel.NAME] = handle_shade_tilt_level,
},
},
supported_capabilities = {
capabilities.windowShadeLevel,
capabilities.windowShadeTiltLevel,
capabilities.windowShade,
capabilities.windowShadePreset,
capabilities.battery,
Expand Down
Loading
Loading