Skip to content

Matter Switch: Simplify component to endpoint mapping logic #2560

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
hcarter-775 merged 3 commits into from
Nov 24, 2025
+17 −49
Merged

Matter Switch: Simplify component to endpoint mapping logic #2560

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8afa645
simplify the endpoint to component mapping
hcarter-775 Nov 17, 2025
a8e942f
improve comments, remove unused capabilities from mapping logic
hcarter-775 Nov 18, 2025
574fd58
update param name from opts to ep_info
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
5 changes: 1 addition & 4 deletions ...s/SmartThings/matter-switch/src/sub_drivers/camera/camera_handlers/attribute_handlers.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -396,15 +396,13 @@ end
function CameraAttributeHandlers.camera_av_stream_management_attribute_list_handler(driver, device, ib, response)
if not ib.data.elements then return end
local status_light_enabled_present, status_light_brightness_present = false, false
local attribute_ids, capability_ids = {}, {}
local attribute_ids = {}
for _, attr in ipairs(ib.data.elements) do
if attr.value == clusters.CameraAvStreamManagement.attributes.StatusLightEnabled.ID then
status_light_enabled_present = true
table.insert(capability_ids, capabilities.switch.ID)
table.insert(attribute_ids, clusters.CameraAvStreamManagement.attributes.StatusLightEnabled.ID)
elseif attr.value == clusters.CameraAvStreamManagement.attributes.StatusLightBrightness.ID then
status_light_brightness_present = true
table.insert(capability_ids, capabilities.mode.ID)
table.insert(attribute_ids, clusters.CameraAvStreamManagement.attributes.StatusLightBrightness.ID)
end
end
Expand All @@ -413,7 +411,6 @@ function CameraAttributeHandlers.camera_av_stream_management_attribute_list_hand
endpoint_id = ib.endpoint_id,
cluster_id = ib.cluster_id,
attribute_ids = attribute_ids,
capability_ids = capability_ids
}
device:set_field(fields.COMPONENT_TO_ENDPOINT_MAP, component_map, {persist=true})
camera_cfg.match_profile(device, status_light_enabled_present, status_light_brightness_present)
Expand Down
8 changes: 0 additions & 8 deletions drivers/SmartThings/matter-switch/src/sub_drivers/camera/camera_utils/utils.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,10 +35,6 @@ function CameraUtils.update_camera_component_map(device)
clusters.CameraAvStreamManagement.attributes.MicrophoneMaxLevel.ID,
clusters.CameraAvStreamManagement.attributes.MicrophoneMinLevel.ID,
},
capability_ids = {
capabilities.audioMute.ID,
capabilities.audioVolume.ID,
}
}
end
if CameraUtils.feature_supported(device, clusters.CameraAvStreamManagement.ID, clusters.CameraAvStreamManagement.types.Feature.VIDEO) then
Expand All @@ -51,10 +47,6 @@ function CameraUtils.update_camera_component_map(device)
clusters.CameraAvStreamManagement.attributes.SpeakerMaxLevel.ID,
clusters.CameraAvStreamManagement.attributes.SpeakerMinLevel.ID,
},
capability_ids = {
capabilities.audioMute.ID,
capabilities.audioVolume.ID,
}
}
end
device:set_field(fields.COMPONENT_TO_ENDPOINT_MAP, component_map, {persist = true})
Expand Down
53 changes: 16 additions & 37 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 @@ -168,60 +168,40 @@ function utils.component_to_endpoint(device, component)
return utils.find_default_endpoint(device)
end

--- An extension of the library function endpoint_to_component, to support a mapping scheme
--- that includes cluster and attribute id's so that we can use multiple components for a
--- single endpoint.
--- An extension of the library function endpoint_to_component, used to support a mapping scheme
--- that optionally includes cluster and attribute ids so that multiple components can be mapped
--- to a single endpoint.
---
--- @param device any a Matter device object
--- @param opts number|table either is an ep_id or a table { endpoint_id, capability_id }
--- @param ep_info number|table either an ep_id or a table { endpoint_id, optional(cluster_id), optional(attribute_id) }
Copy link
Contributor

@ctowns ctowns Nov 24, 2025

Choose a reason for hiding this comment

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

Can we choose a single parameter type and make the the caller provide the additional info? I know lua doesn't have strong parameter typing but I think this is a practice we should try to avoid. If you need to handle additional optional values, they can be passed as optional parameters:

function utils.endpoint_to_component(device, ep, cluster_id, attribute_id)

Copy link
Contributor Author

@hcarter-775 hcarter-775 Nov 24, 2025

Choose a reason for hiding this comment

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

We cannot. Since we want to have this be compatible with the lua libs, we cannot add extra arguments. This was a compromise I figured was relatively clear given the limitation.

Copy link
Contributor

@ctowns ctowns Nov 24, 2025

Choose a reason for hiding this comment

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

I don't understand the reasoning for why we cannot add extra arguments - why is backwards compatibility a concern if we are creating a new function that overrides the lua libs? I think we should only use this override when needed, and it shouldn't be a blanket override for all devices in the switch driver, unless they specifically require this advanced mapping, right?

If this is a function override, I assume we can make it whatever we want. Even using extend_device allows us to override the function pointer from the lua libs and just point to this new function, even though I don't neccessarily think using extend_device in this case is the way to go.

Copy link
Contributor

@ctowns ctowns Nov 24, 2025

Choose a reason for hiding this comment

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

Also just a note that this discussion isn't a blocker if this needs to go in urgently, but I think it's worth having more discussion around how we should be designing these APIs for this complex mapping.

--- where cluster_id is required for an attribute_id to be handled.
--- @return string component
function utils.endpoint_to_component(device, opts)
local ep_info = {}
if type(opts) == "number" then
ep_info.endpoint_id = opts
elseif type(opts) == "table" then
if opts.endpoint_info then
ep_info = opts.endpoint_info
else
ep_info = {
endpoint_id = opts.endpoint_id,
cluster_id = opts.cluster_id,
attribute_id = opts.attribute_id
}
end
function utils.endpoint_to_component(device, ep_info)
if type(ep_info) == "number" then
ep_info = { endpoint_id = ep_info }
end
for component, map_info in pairs(device:get_field(fields.COMPONENT_TO_ENDPOINT_MAP) or {}) do
if type(map_info) == "number" and map_info == ep_info.endpoint_id then
return component
elseif type(map_info) == "table" and map_info.endpoint_id == ep_info.endpoint_id then
if (not map_info.cluster_id or (map_info.cluster_id == ep_info.cluster_id
and utils.tbl_contains(map_info.attribute_ids, ep_info.attribute_id)))
and (not opts.capability_id or utils.tbl_contains(map_info.capability_ids, opts.capability_id)) then
elseif type(map_info) == "table" and map_info.endpoint_id == ep_info.endpoint_id
and (not map_info.cluster_id or (map_info.cluster_id == ep_info.cluster_id
and (not map_info.attribute_ids or utils.tbl_contains(map_info.attribute_ids, ep_info.attribute_id)))) then
return component
end
end
end
return "main"
end

--- An extension of the library function emit_event_for_endpoint, to support devices with
--- multiple components defined for the same endpoint, since they can't be easily
--- differentiated based on a simple endpoint id to component mapping, but we can extend
--- this mapping to include the cluster and attribute id's so that we know which component
--- to route events to.
--- An extension of the library function emit_event_for_endpoint, used to support devices with
--- multiple components mapped to the same endpoint. This is handled by extending the parameters to optionally
--- include a cluster id and attribute id for more specific routing
---
--- @param device any a Matter device object
--- @param ep_info number|table endpoint_id or ib (includes endpoint_id, cluster_id, attribute_id)
--- @param ep_info number|table endpoint_id or an ib (the ib data includes endpoint_id, cluster_id, and attribute_id fields)
--- @param event any a capability event object
function utils.emit_event_for_endpoint(device, ep_info, event)
if type(ep_info) == "number" then
ep_info = { endpoint_id = ep_info }
elseif type(ep_info) == "table" then
ep_info = {
endpoint_id = ep_info.endpoint_id,
cluster_id = ep_info.cluster_id,
attribute_id = ep_info.attribute_id
}
end
if device:get_field(fields.IS_PARENT_CHILD_DEVICE) then
local child = utils.find_child(device, ep_info.endpoint_id)
Expand All @@ -230,8 +210,7 @@ function utils.emit_event_for_endpoint(device, ep_info, event)
return
end
end
local opts = { endpoint_info = ep_info, capability_id = event.capability.ID }
local comp_id = utils.endpoint_to_component(device, opts)
local comp_id = utils.endpoint_to_component(device, ep_info)
local comp = device.profile.components[comp_id]
device:emit_component_event(comp, event)
end
Expand Down
Loading