Skip to content

Matter Switch: Add more reliability to device type checking #2564

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

Merged
merged 4 commits into from
Nov 19, 2025
Merged

Matter Switch: Add more reliability to device type checking #2564

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
7f6eb1c
add more reliability to device type checking
hcarter-775 Nov 18, 2025
b11bf3f
update superset comment
hcarter-775 Nov 18, 2025
1e4fcde
update comment about superset device types again
hcarter-775 Nov 18, 2025
98d0d61
update comment
hcarter-775 Nov 19, 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
2 changes: 1 addition & 1 deletion drivers/SmartThings/matter-switch/src/switch_utils/device_configuration.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ function SwitchDeviceConfiguration.assign_profile_for_onoff_ep(device, server_on
-- per spec, the Switch device types support OnOff as CLIENT, though some vendors break spec and support it as SERVER.
local primary_dt_id = switch_utils.find_max_subset_device_type(ep_info, fields.DEVICE_TYPE_ID.LIGHT)
or switch_utils.find_max_subset_device_type(ep_info, fields.DEVICE_TYPE_ID.SWITCH)
or ep_info.device_types[1] and ep_info.device_types[1].device_type_id
or switch_utils.find_primary_device_type(ep_info)

local generic_profile = fields.device_type_profile_map[primary_dt_id]

Expand Down
1 change: 1 addition & 0 deletions drivers/SmartThings/matter-switch/src/switch_utils/fields.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ SwitchFields.CURRENT_HUESAT_ATTR_MAX = 254

SwitchFields.DEVICE_TYPE_ID = {
AGGREGATOR = 0x000E,
BRIDGED_NODE = 0x0013,
CAMERA = 0x0142,
CHIME = 0x0146,
DIMMABLE_PLUG_IN_UNIT = 0x010B,
Expand Down
36 changes: 23 additions & 13 deletions drivers/SmartThings/matter-switch/src/switch_utils/utils.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,23 +93,33 @@ function utils.device_type_supports_button_switch_combination(device, endpoint_i
return utils.tbl_contains(dimmable_eps, endpoint_id)
end

-- Some devices report multiple device types which are a subset of
-- a superset device type (Ex. Dimmable Light is a superset of On/Off Light).
-- We should map to the largest superset device type supported.
-- This can be done by matching to the device type with the highest ID
--- Some devices report multiple device types which are a subset of a superset
--- device type (Ex. Dimmable Light is a superset of On/Off Light). We should map
--- to the largest superset device type supported.
--- This can be done by matching to the device type with the highest ID
Copy link
Contributor

@gharveymn gharveymn Nov 18, 2025

Choose a reason for hiding this comment

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

Can we cite this?

Copy link
Contributor Author

@hcarter-775 hcarter-775 Nov 18, 2025

Choose a reason for hiding this comment

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

done b11bf3f, how's this look?

Copy link
Contributor

@gharveymn gharveymn Nov 18, 2025

Choose a reason for hiding this comment

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

Sorry, I wasn't being clear. I was actually referring to the assertion that the superset IDs are always greater than the IDs of subsets. I think what you had before for the rest is actually more clear than after the change.

Copy link
Contributor

@gharveymn gharveymn Nov 18, 2025

Choose a reason for hiding this comment

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

If it's not explicitly specified, and it isn't feasible to switch to an alternative algorithm, we should explicitly point out that this is just a heuristic, and could break in the future.

Copy link
Contributor Author

@hcarter-775 hcarter-775 Nov 18, 2025

Choose a reason for hiding this comment

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

revrerted, and added a note

--- note: that superset device types have a higher ID than those of their subset
--- is heuristic and could therefore break in the future, were the spec expanded
function utils.find_max_subset_device_type(ep, device_type_set)
if ep.endpoint_id == 0 then return end -- EP-scoped device types not permitted on Root Node
local primary_dt_id = ep.device_types[1] and ep.device_types[1].device_type_id
if utils.tbl_contains(device_type_set, primary_dt_id) then
for _, dt in ipairs(ep.device_types) do
-- only device types in the subset should be considered.
if utils.tbl_contains(device_type_set, dt.device_type_id) then
primary_dt_id = math.max(primary_dt_id, dt.device_type_id)
end
local primary_dt_id = -1
for _, dt in ipairs(ep.device_types) do
-- only device types in the subset should be considered.
if utils.tbl_contains(device_type_set, dt.device_type_id) then
primary_dt_id = math.max(primary_dt_id, dt.device_type_id)
end
end
return (primary_dt_id > 0) and primary_dt_id or nil
end

--- Lights and Switches are Device Types that have Superset-style functionality
--- For all other device types, this function should be used to identify the primary device type
function utils.find_primary_device_type(ep_info)
for _, dt in ipairs(ep_info.device_types) do
Copy link
Contributor

@nickolas-deboom nickolas-deboom Nov 18, 2025

Choose a reason for hiding this comment

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

Sort this list first maybe?

Copy link
Contributor Author

@hcarter-775 hcarter-775 Nov 18, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

what would it be sorting? Further, the device type ids aren't necessarily "ordered" in a meaningful way, adding to my question of what "sorting" would be wanted per se?

Copy link
Contributor

@nickolas-deboom nickolas-deboom Nov 18, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Is there anything special about the 'first' device type in ep_info.device_types that would be returned by this function in that case?

Copy link
Contributor Author

@hcarter-775 hcarter-775 Nov 18, 2025

Choose a reason for hiding this comment

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

well, the idea is we need to return something. Generally, there should only be one. And before, it was just the first one. Now it's the first one that isn't bridged node. It's kinda jank cause it handles a spec-breaking system.

Copy link
Contributor

@gharveymn gharveymn Nov 18, 2025

Choose a reason for hiding this comment

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

the device type ids aren't necessarily "ordered" in a meaningful way

does that fact add additional context to this discussion, or is this referring to something else?

Copy link
Contributor Author

@hcarter-775 hcarter-775 Nov 18, 2025

Choose a reason for hiding this comment

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

@gharveymn kinda, but not necessarily. It does speak to the fact that for "superset" scoped device types, the fact that the ids increment is heuristic and may not always be true (though at present it is). On the other hand, this comment also speaks to the fact that the ordering of device types on an endpoint is arbitrary as well

if dt.device_type_id ~= fields.DEVICE_TYPE_ID.BRIDGED_NODE then
-- if this is not a bridged node, return the first device type seen
return dt.device_type_id
end
return primary_dt_id
end
return nil
end

--- find_default_endpoint is a helper function to handle situations where
Expand Down
Loading