Skip to content

Matter Window Covering: Update preset handling #2295

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.

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
wants to merge 4 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,5 @@ components:
categories:
- name: Blind
preferences:
- preferenceId: presetPosition
explicit: true
- preferenceId: reverse
explicit: true
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,5 @@ components:
categories:
- name: Blind
preferences:
- preferenceId: presetPosition
explicit: true
- preferenceId: reverse
explicit: true
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,5 @@ components:
categories:
- name: Blind
preferences:
- preferenceId: presetPosition
explicit: true
- preferenceId: reverse
explicit: true
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,5 @@ components:
categories:
- name: Blind
preferences:
- preferenceId: presetPosition
explicit: true
- preferenceId: reverse
explicit: true
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,5 @@ components:
categories:
- name: Blind
preferences:
- preferenceId: presetPosition
explicit: true
- preferenceId: reverse
explicit: true
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,5 @@ components:
categories:
- name: Blind
preferences:
- preferenceId: presetPosition
explicit: true
- preferenceId: reverse
explicit: true
36 changes: 30 additions & 6 deletions drivers/SmartThings/matter-window-covering/src/init.lua
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@ local battery_support = {
BATTERY_PERCENTAGE = "BATTERY_PERCENTAGE"
}
local REVERSE_POLARITY = "__reverse_polarity"
local PRESET_LEVEL_KEY = "__preset_level_key"
local PRESET_LEVEL = 50

local function find_default_endpoint(device, cluster)
local res = device.MATTER_DEFAULT_ENDPOINT
Expand Down Expand Up @@ -76,6 +78,17 @@ end

local function device_init(driver, device)
device:set_component_to_endpoint_fn(component_to_endpoint)
if device:supports_capability_by_id(capabilities.windowShadePreset.ID) and
device:get_latest_state("main", capabilities.windowShadePreset.ID, capabilities.windowShadePreset.position.NAME) == nil then
-- These should only ever be nil once (and at the same time) for already-installed devices
-- It can be removed after migration is complete
device:emit_event(capabilities.windowShadePreset.supportedCommands({"presetPosition", "setPresetPosition"}, {visibility = {displayed = false}}))
local preset_position = device:get_field(PRESET_LEVEL_KEY) or
(device.preferences ~= nil and device.preferences.presetPosition) or
PRESET_LEVEL
device:emit_event(capabilities.windowShadePreset.position(preset_position, {visibility = {displayed = false}}))
device:set_field(PRESET_LEVEL_KEY, preset_position, {persist = true})
end
device:subscribe()
end

Expand Down Expand Up @@ -115,9 +128,12 @@ local function info_changed(driver, device, event, args)
end

local function device_added(driver, device)
device:emit_event(
capabilities.windowShade.supportedWindowShadeCommands({"open", "close", "pause"}, {visibility = {displayed = false}})
)
device:emit_event(capabilities.windowShade.supportedWindowShadeCommands({"open", "close", "pause"}, {visibility = {displayed = false}}))
device:emit_event(capabilities.windowShadePreset.supportedCommands({"presetPosition", "setPresetPosition"}, {visibility = {displayed = false}}))
if device:supports_capability_by_id(capabilities.windowShadePreset.ID) and
device:get_latest_state("main", capabilities.windowShadePreset.ID, capabilities.windowShadePreset.position.NAME) == nil then
device:emit_event(capabilities.windowShadePreset.position(PRESET_LEVEL, {visibility = {displayed = false}}))
end
Copy link
Contributor

Choose a reason for hiding this comment

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

I ultimately decided to omit the additions in the added handler for now, since these events will be sent by init. After migration is complete we can remove the init code and add the added events.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sounds good, updated in the latest commit!

device:set_field(REVERSE_POLARITY, false, { persist = true })
end

Expand All @@ -129,22 +145,29 @@ local function handle_preset(driver, device, cmd)
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 = reverse_polarity_if_needed(device, device.preferences.presetPosition)
local lift_value = device:get_latest_state(
"main", capabilities.windowShadePreset.ID, capabilities.windowShadePreset.position.NAME
) or PRESET_LEVEL
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, endpoint_id, PRESET_LEVEL * 100
)
device:send(req)
end
end

local function handle_set_preset(driver, device, cmd)
local endpoint_id = device:component_to_endpoint(cmd.component)
device:set_field(PRESET_LEVEL_KEY, cmd.args.position)
device:emit_event_for_endpoint(endpoint_id, capabilities.windowShadePreset.position(cmd.args.position))
end

-- close covering
local function handle_close(driver, device, cmd)
local endpoint_id = device:component_to_endpoint(cmd.component)
Expand Down Expand Up @@ -347,6 +370,7 @@ local matter_driver_template = {
},
[capabilities.windowShadePreset.ID] = {
[capabilities.windowShadePreset.commands.presetPosition.NAME] = handle_preset,
[capabilities.windowShadePreset.commands.setPresetPosition.NAME] = handle_set_preset,
},
[capabilities.windowShade.ID] = {
[capabilities.windowShade.commands.close.NAME] = handle_close,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,21 @@ local CLUSTER_SUBSCRIBE_LIST_NO_BATTERY = {
WindowCovering.server.attributes.OperationalStatus,
}

local function set_preset(device)
test.socket.capability:__expect_send(
device:generate_test_message(
"main", capabilities.windowShadePreset.supportedCommands({"presetPosition", "setPresetPosition"}, {visibility = {displayed = false}})
)
)
test.socket.capability:__expect_send(
device:generate_test_message(
"main", capabilities.windowShadePreset.position(30, {visibility = {displayed = false}})
)
)
end
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd maybe extend this test just to verify that the updated value is used for the next presetPosition command

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good call, I updated the test case called 'Handle preset commands' to verify this behavior in the latest commit


local function test_init()
set_preset(mock_device)
local subscribe_request = CLUSTER_SUBSCRIBE_LIST[1]:subscribe(mock_device)
for i, clus in ipairs(CLUSTER_SUBSCRIBE_LIST) do
if i > 1 then subscribe_request:merge(clus:subscribe(mock_device)) end
Expand All @@ -139,6 +153,7 @@ local function test_init()
end

local function test_init_switch_to_battery()
set_preset(mock_device_switch_to_battery)
local subscribe_request = CLUSTER_SUBSCRIBE_LIST_NO_BATTERY[1]:subscribe(mock_device_switch_to_battery)
for i, clus in ipairs(CLUSTER_SUBSCRIBE_LIST_NO_BATTERY) do
if i > 1 then subscribe_request:merge(clus:subscribe(mock_device_switch_to_battery)) end
Expand All @@ -152,6 +167,7 @@ local function test_init_switch_to_battery()
end

local function test_init_mains_powered()
set_preset(mock_device_mains_powered)
local subscribe_request = CLUSTER_SUBSCRIBE_LIST_NO_BATTERY[1]:subscribe(mock_device_mains_powered)
for i, clus in ipairs(CLUSTER_SUBSCRIBE_LIST_NO_BATTERY) do
if i > 1 then subscribe_request:merge(clus:subscribe(mock_device_mains_powered)) end
Expand Down Expand Up @@ -684,6 +700,11 @@ test.register_coroutine_test(
},
}
)
test.socket.capability:__expect_send(
mock_device:generate_test_message(
"main", capabilities.windowShadePreset.supportedCommands({"presetPosition", "setPresetPosition"}, {visibility = {displayed = false}})
)
)
test.wait_for_events()

test.socket.capability:__queue_receive(
Expand Down Expand Up @@ -774,7 +795,7 @@ test.register_coroutine_test("OperationalStatus report contains current position
)
end)

test.register_coroutine_test("Handle windowcoveringPreset", function()
test.register_coroutine_test("Handle presetPosition command", function()
test.socket.capability:__queue_receive(
{
mock_device.id,
Expand Down
Loading