Skip to content

Support greater profiling for parent/child switch devices #2153

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

Closed
wants to merge 8 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
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
140 changes: 87 additions & 53 deletions drivers/SmartThings/matter-switch/src/init.lua
Original file line number Diff line number Diff line change
Expand Up @@ -321,6 +321,22 @@ local function create_multi_press_values_list(size, supportsHeld)
return list
end

-- check if device type is found on the device. Optionally specify the endpoint to search on
local function contains_device_type(device, device_type_id, endpoint_id)
for _, ep in ipairs(device.endpoints) do
for _, dt in ipairs(ep.device_types) do
if dt.device_type_id == device_type_id then
if endpoint_id and ep.endpoint_id == endpoint_id then
return true
elseif endpoint_id == nil then
return true
end
end
end
end
return false
end

local function tbl_contains(array, value)
for _, element in ipairs(array) do
if element == value then
Expand Down Expand Up @@ -385,20 +401,14 @@ end
--- whether the device type for an endpoint is currently supported by a profile for
--- combination button/switch devices.
local function device_type_supports_button_switch_combination(device, endpoint_id)
for _, ep in ipairs(device.endpoints) do
if ep.endpoint_id == endpoint_id then
for _, dt in ipairs(ep.device_types) do
if dt.device_type_id == DIMMABLE_LIGHT_DEVICE_TYPE_ID then
for _, fingerprint in ipairs(child_device_profile_overrides_per_vendor_id[0x115F]) do
if device.manufacturer_info.product_id == fingerprint.product_id then
return false -- For Aqara Dimmer Switch with Button.
end
end
return true
end
end
for _, fingerprint in ipairs(child_device_profile_overrides_per_vendor_id[AQARA_MANUFACTURER_ID]) do
if device.manufacturer_info.product_id == fingerprint.product_id then
return false -- For Aqara Dimmer Switch with Button.
end
end
if contains_device_type(device, DIMMABLE_LIGHT_DEVICE_TYPE_ID, endpoint_id) then
return true
end
return false
end

Expand Down Expand Up @@ -480,43 +490,78 @@ local function check_field_name_updates(device)
end
end

local function assign_child_profile(device, child_ep)
local function assign_switch_profile(device, switch_ep, is_child_device)
local profile

local electrical_tags = ""
for _, ep in ipairs(device.endpoints) do
if ep.endpoint_id == child_ep then
if ep.endpoint_id == switch_ep then
-- Some devices report multiple device types which are a subset of
-- a superset device type (For example, Dimmable Light is a superset of
-- On/Off light). This mostly applies to the four light types, so we will want
-- to match the profile for the superset device type. This can be done by
-- matching to the device type with the highest ID
-- Note: Electrical Sensor does not follow the above logic, so it's ignored
local id = 0
for _, dt in ipairs(ep.device_types) do
id = math.max(id, dt.device_type_id)
if dt.device_type_id ~= ELECTRICAL_SENSOR_ID then
id = math.max(id, dt.device_type_id)
end
end
profile = device_type_profile_map[id]
local power_tags, energy_tags = "", ""
for _, cluster in ipairs(ep.clusters) do
if cluster.cluster_id == clusters.ElectricalPowerMeasurement.ID then
power_tags = "-power"
elseif cluster.cluster_id == clusters.ElectricalEnergyMeasurement.ID then
energy_tags = "-energy-powerConsumption"
end
end
electrical_tags = power_tags .. energy_tags
if electrical_tags ~= "" and (profile == "plug-binary" or profile == "plug-level") then
-- remove the "-binary" in the plug-binary profile name
profile = string.gsub(profile, "-binary", "") .. electrical_tags
end
break
end
end
-- Add electrical support to the switch ep if Electical Sensor is handled on a unique ep
if electrical_tags == "" and contains_device_type(device, ELECTRICAL_SENSOR_ID) then
local electrical_energy_eps = device:get_endpoints(clusters.ElectricalEnergyMeasurement.ID)
local electrical_power_eps = device:get_endpoints(clusters.ElectricalPowerMeasurement.ID)
local switch_eps = device:get_endpoints(clusters.OnOff.ID)
table.sort(electrical_energy_eps)
table.sort(electrical_power_eps)
table.sort(switch_eps)
for i, ep in ipairs(switch_eps) do
Copy link
Contributor

Choose a reason for hiding this comment

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

This loop makes an assumption that the nth electrical ep will correspond to the nth switch ep, but what if only some eps support electrical power/energy, and some don't? For example, a 6 plug device with 3 outlets that support power and 1 that supports power and energy, one that supports only energy, and one that supports none. I would assume this is not a likely scenario, but a simpler combination of these factors could happen in a real device.

Overall it seems like it should not be on us as the client to try to make assumptions about which electrical endpoint corresponds to which switch endpoint. Shouldn't the electrical device type that is relevant to a given switch/plug be combined on the same endpoint? I might need to refresh myself on the spec for clarity, but it seems ambiguous if this convention is not used.

if ep == switch_ep then
if electrical_power_eps[i] then
electrical_tags = electrical_tags .. "-power"
end
if electrical_energy_eps[i] then
electrical_tags = electrical_tags .. "-energy-powerConsumption"
end
break
end
end
if electrical_tags ~= "" and (profile == "plug-binary" or profile == "plug-level" or profile == "light-binary") then
profile = string.gsub(profile, "-binary", "") .. electrical_tags
end
end

-- Check if device has an overridden child profile that differs from the profile that would match
-- the child's device type for the following two cases:
-- 1. To add Electrical Sensor only to the first EDGE_CHILD (light-power-energy-powerConsumption)
-- for the Aqara Light Switch H2. The profile of the second EDGE_CHILD for this device is
-- determined in the "for" loop above (e.g., light-binary)
-- 2. The selected profile for the child device matches the initial profile defined in
-- child_device_profile_overrides
for id, vendor in pairs(child_device_profile_overrides_per_vendor_id) do
for _, fingerprint in ipairs(vendor) do
if device.manufacturer_info.product_id == fingerprint.product_id and
((device.manufacturer_info.vendor_id == AQARA_MANUFACTURER_ID and child_ep == 1) or profile == fingerprint.initial_profile) then
return fingerprint.target_profile
if is_child_device then
-- Check if child device has an overridden child profile that differs from the child's generic device type profile
for _, vendor in pairs(child_device_profile_overrides_per_vendor_id) do
for _, fingerprint in ipairs(vendor) do
if device.manufacturer_info.product_id == fingerprint.product_id and profile == fingerprint.initial_profile then
return fingerprint.target_profile
end
end
end
-- default to "switch-binary" if no child profile is found
return profile or "switch-binary"
end

-- default to "switch-binary" if no profile is found
return profile or "switch-binary"
return profile
end

local function configure_buttons(device)
Expand Down Expand Up @@ -598,7 +643,7 @@ local function build_child_switch_profiles(driver, device, main_endpoint)
num_switch_server_eps = num_switch_server_eps + 1
if ep ~= main_endpoint then -- don't create a child device that maps to the main endpoint
local name = string.format("%s %d", device.label, num_switch_server_eps)
local child_profile = assign_child_profile(device, ep)
local child_profile = assign_switch_profile(device, ep, true)
driver:try_create_device(
{
type = "EDGE_CHILD",
Expand Down Expand Up @@ -675,14 +720,7 @@ local function initialize_buttons_and_switches(driver, device, main_endpoint)
end

local function detect_bridge(device)
for _, ep in ipairs(device.endpoints) do
for _, dt in ipairs(ep.device_types) do
if dt.device_type_id == AGGREGATOR_DEVICE_TYPE_ID then
return true
end
end
end
return false
contains_device_type(device, AGGREGATOR_DEVICE_TYPE_ID)
end

local function device_init(driver, device)
Expand Down Expand Up @@ -728,22 +766,9 @@ local function match_profile(driver, device)
end

local fan_eps = device:get_endpoints(clusters.FanControl.ID)
local level_eps = device:get_endpoints(clusters.LevelControl.ID)
local energy_eps = embedded_cluster_utils.get_endpoints(device, clusters.ElectricalEnergyMeasurement.ID)
local power_eps = embedded_cluster_utils.get_endpoints(device, clusters.ElectricalPowerMeasurement.ID)
local valve_eps = embedded_cluster_utils.get_endpoints(device, clusters.ValveConfigurationAndControl.ID)
local profile_name = nil
local level_support = ""
if #level_eps > 0 then
level_support = "-level"
end
if #energy_eps > 0 and #power_eps > 0 then
profile_name = "plug" .. level_support .. "-power-energy-powerConsumption"
elseif #energy_eps > 0 then
profile_name = "plug" .. level_support .. "-energy-powerConsumption"
elseif #power_eps > 0 then
profile_name = "plug" .. level_support .. "-power"
elseif #valve_eps > 0 then
if #valve_eps > 0 then
profile_name = "water-valve"
if #embedded_cluster_utils.get_endpoints(device, clusters.ValveConfigurationAndControl.ID,
{feature_bitmap = clusters.ValveConfigurationAndControl.types.Feature.LEVEL}) > 0 then
Expand All @@ -754,6 +779,15 @@ local function match_profile(driver, device)
end
if profile_name then
device:try_update_metadata({ profile = profile_name })
return
end

-- after doing all previous profiling steps, attempt to re-profile main/parent switch/plug device
profile_name = assign_switch_profile(device, main_endpoint, false)
-- ignore attempts to dynamically profile light-level-colorTemperature devices for now, since
-- these may lose fingerprinted Kelvin ranges when dynamically profiled.
if profile_name and profile_name ~= "light-level-colorTemperature" and profile_name ~= "light-color-level" then
device:try_update_metadata({profile = profile_name})
end
end

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,10 +51,29 @@ local mock_device = test.mock_device.build_test_matter_device({
endpoint_id = 2,
clusters = {
{ cluster_id = clusters.OnOff.ID, cluster_type = "SERVER", cluster_revision = 1, feature_map = 0, },
{cluster_id = clusters.LevelControl.ID, cluster_type = "SERVER", feature_map = 2}
{ cluster_id = clusters.LevelControl.ID, cluster_type = "SERVER", feature_map = 2},
},
device_types = {
{ device_type_id = 0x010A, device_type_revision = 1 } -- OnOff Plug
{ device_type_id = 0x010B, device_type_revision = 1 }, -- OnOff Dimmable Plug
}
},
{
endpoint_id = 3,
clusters = {
{ cluster_id = clusters.ElectricalEnergyMeasurement.ID, cluster_type = "SERVER", feature_map = 14, },
},
device_types = {
{ device_type_id = 0x0510, device_type_revision = 1 }, -- Electrical Sensor
}
},
{
endpoint_id = 4,
clusters = {
{ cluster_id = clusters.OnOff.ID, cluster_type = "SERVER", cluster_revision = 1, feature_map = 0, },
{ cluster_id = clusters.LevelControl.ID, cluster_type = "SERVER", feature_map = 2},
},
device_types = {
{ device_type_id = 0x010B, device_type_revision = 1 }, -- OnOff Dimmable Plug
}
},
},
Expand All @@ -80,16 +99,18 @@ local mock_device_periodic = test.mock_device.build_test_matter_device({
{
endpoint_id = 1,
clusters = {
{ cluster_id = clusters.OnOff.ID, cluster_type = "SERVER", cluster_revision = 1, feature_map = 0, },
{ cluster_id = clusters.ElectricalEnergyMeasurement.ID, cluster_type = "SERVER", feature_map = 10, },
},
device_types = {
{ device_type_id = 0x0510, device_type_revision = 1 } -- Electrical Sensor
{ device_type_id = 0x010A, device_type_revision = 1 }, -- OnOff Plug
}
},
},
})

local subscribed_attributes_periodic = {
clusters.OnOff.attributes.OnOff,
clusters.ElectricalEnergyMeasurement.attributes.PeriodicEnergyImported,
clusters.ElectricalEnergyMeasurement.attributes.CumulativeEnergyImported,
}
Expand Down Expand Up @@ -645,6 +666,13 @@ test.register_coroutine_test(
function()
test.socket.device_lifecycle:__queue_receive({ mock_device.id, "doConfigure" })
mock_device:expect_metadata_update({ profile = "plug-level-power-energy-powerConsumption" })
mock_device:expect_device_create({
type = "EDGE_CHILD",
label = "nil 2",
profile = "plug-level-energy-powerConsumption",
parent_device_id = mock_device.id,
parent_assigned_child_key = string.format("%d", 4)
})
mock_device:expect_metadata_update({ provisioning_state = "PROVISIONED" })
end,
{ test_init = test_init }
Expand Down Expand Up @@ -692,7 +720,7 @@ test.register_message_test(
direction = "receive",
message = {
mock_device.id,
clusters.LevelControl.server.commands.MoveToLevelWithOnOff:build_test_command_response(mock_device, 2)
clusters.LevelControl.server.commands.MoveToLevelWithOnOff:build_test_command_response(mock_device, 1)
}
},
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -427,6 +427,7 @@ local function test_init_mounted_on_off_control()
end
test.socket.matter:__expect_send({mock_device_mounted_on_off_control.id, subscribe_request})
test.socket.device_lifecycle:__queue_receive({ mock_device_mounted_on_off_control.id, "doConfigure" })
mock_device_mounted_on_off_control:expect_metadata_update({ profile = "switch-binary" })
mock_device_mounted_on_off_control:expect_metadata_update({ provisioning_state = "PROVISIONED" })
test.mock_device.add_test_device(mock_device_mounted_on_off_control)
end
Expand All @@ -443,6 +444,7 @@ local function test_init_mounted_dimmable_load_control()
end
test.socket.matter:__expect_send({mock_device_mounted_dimmable_load_control.id, subscribe_request})
test.socket.device_lifecycle:__queue_receive({ mock_device_mounted_dimmable_load_control.id, "doConfigure" })
mock_device_mounted_dimmable_load_control:expect_metadata_update({ profile = "switch-level" })
mock_device_mounted_dimmable_load_control:expect_metadata_update({ provisioning_state = "PROVISIONED" })
test.mock_device.add_test_device(mock_device_mounted_dimmable_load_control)
end
Expand Down Expand Up @@ -477,6 +479,7 @@ local function test_init_parent_child_different_types()
test.socket.matter:__expect_send({mock_device_parent_child_different_types.id, subscribe_request})

test.socket.device_lifecycle:__queue_receive({ mock_device_parent_child_different_types.id, "doConfigure" })
mock_device_parent_child_different_types:expect_metadata_update({ profile = "switch-binary" })
mock_device_parent_child_different_types:expect_metadata_update({ provisioning_state = "PROVISIONED" })

test.mock_device.add_test_device(mock_device_parent_child_different_types)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,7 @@ local function test_init()
test.socket.matter:__expect_send({mock_device.id, subscribe_request})

test.socket.device_lifecycle:__queue_receive({ mock_device.id, "doConfigure" })
mock_device:expect_metadata_update({ profile = "light-binary" })
mock_device:expect_metadata_update({ provisioning_state = "PROVISIONED" })

test.mock_device.add_test_device(mock_device)
Expand Down Expand Up @@ -247,6 +248,7 @@ local function test_init_parent_child_endpoints_non_sequential()
test.socket.matter:__expect_send({mock_device_parent_child_endpoints_non_sequential.id, subscribe_request})

test.socket.device_lifecycle:__queue_receive({ mock_device_parent_child_endpoints_non_sequential.id, "doConfigure" })
mock_device_parent_child_endpoints_non_sequential:expect_metadata_update({ profile = "light-binary" })
mock_device_parent_child_endpoints_non_sequential:expect_metadata_update({ provisioning_state = "PROVISIONED" })

test.mock_device.add_test_device(mock_device_parent_child_endpoints_non_sequential)
Expand Down
Loading
Loading