Skip to content

Matter Switch: remove ENERGY_MANAGEMENT_ENDPOINT field #2436

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

Closed
wants to merge 2 commits into from
Closed

Matter Switch: remove ENERGY_MANAGEMENT_ENDPOINT field #2436

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
da6c04c
remove ENERGY_MANAGEMENT_ENDPOINT field
hcarter-775 Sep 29, 2025
d52d763
remove unrelated null powerMeter report logic
hcarter-775 Oct 1, 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
28 changes: 11 additions & 17 deletions drivers/SmartThings/matter-switch/src/generic_handlers/attribute_handlers.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ local capabilities = require "st.capabilities"
local clusters = require "st.matter.clusters"
local version = require "version"
local im = require "st.matter.interaction_model"
local device_lib = require "st.device"

local st_utils = require "st.utils"
local fields = require "utils.switch_fields"
Expand Down Expand Up @@ -245,17 +246,13 @@ end
-- [[ ELECTRICAL POWER MEASUREMENT CLUSTER ATTRIBUTES ]] --

function AttributeHandlers.active_power_handler(driver, device, ib, response)
local component = device.profile.components["main"]
Copy link
Contributor

@ctowns ctowns Oct 1, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

I don't think we should be hard coding in use of the main endpoint, because then this will break on a multi-component device. Do we have any multi-component power devices as far as we know?

Why does ENERGY_MANAGEMENT_ENDPOINT need to be saved in the first place when we already know it is on endpoint 0 given the if ib.endpoint_id ~= 0 then statement? nevermind, I see we need to emit on the first switch endpoint

Copy link
Contributor

Choose a reason for hiding this comment

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

If we are going to hard code a component, why not just hardcode the endpoint for certain device types? If this is limited to a couple Aqara devices (or only 1?) I might perfer just adding special handling for them.

Is having the energy measurement cluster on the root endpoint typical, or is this the only device?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We don't have any profiles where energy isn't on the main component. As I noted in the description, even if that comes up, I still think this is a better handling than the current.

Copy link
Contributor Author

@hcarter-775 hcarter-775 Oct 1, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

At the moment, this only shows up for Aqara because it is the only device that has energy on a child device. However, this will change, and we know these devices will often have the Electrical Sensor device type on multiple EPs. This would normally be where endpoint_to_component would come in. In this case, I've just circumvented that function and defined the relationship explicitly (since we already know it will be like this in all cases for the foreseeable future since plugs and switches are locked into single component profiles).

Perhaps endpoint_to_component could be a better option though, but tbh that is just punting this same logic to a helper function.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think this is all a matter of opinion, but to me we should avoid hard coding components in our function as a rule since we created the component_to_endpoint and endpoint_to_component apis explicitly for this purpose. I believe the cleanest solution is to handle this mapping in our available APIs, and leave component and endpoint mapping outside of attribute handlers as much as we can. If I am not mistaken, can we not just add the root node to the component to endpoint map and have it map to the main component, and then continue to leverage that function as expected?

Copy link
Contributor

Choose a reason for hiding this comment

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

If we are going to hard code a component, why not just hardcode the endpoint for certain device types? If this is limited to a couple Aqara devices (or only 1?) I might perfer just adding special handling for them.

Is having the energy measurement cluster on the root endpoint typical, or is this the only device?

I might prefer specific handling for this device, because according to the spec it isn't supported to have clusters with the Application role implemented on the root node, which would include the Electrical Energy+Power Measurement clusters. So it's unlikely we would see this on another device

Copy link
Contributor

Choose a reason for hiding this comment

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

But then again, it's already been implemented generically for a while, so I don't necessarily think we need to change it back to the original handling

Copy link
Contributor Author

@hcarter-775 hcarter-775 Oct 2, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

The issue is more generic than the fact that it's happening on the root node. It will occur for other devices if

  1. the device is a child device
  2. it supports Energy on an endpoint that is different from the endpoint OnOff is on.

From initial testing, I have seen this case show up twice (not counting aqara). It generically can and will occur for generic endpoints, not just root.

Copy link
Contributor

Choose a reason for hiding this comment

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

For that find_child problem you mentioned where child device only use one endpoint, we can also update the find_child function specifically for our purposes here so that is could allow for multiple endpoints to map to the child device using a new device key that can have multiple endpoints. You could even update it so that we have a specific override of this function that is only set for devices like this where we need multiple endpoints mapped to a child, and other devices can continue to use the generic one.The find child function is device-scoped and not driver-scoped, so you can make a custom one specifically for devices like this and not affect others in the 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.

per this discussion, I have updated the logic to leave a single emit_event_for_endpoint call in each of these handlers in this PR: #2444

It cannot be easily broken out of the above PR anymore, since the new logic is inherently tied to the new PowerTopology handling.

if ib.data.value then
local watt_value = ib.data.value / fields.CONVERSION_CONST_MILLIWATT_TO_WATT
if ib.endpoint_id ~= 0 then
device:emit_event_for_endpoint(ib.endpoint_id, capabilities.powerMeter.power({ value = watt_value, unit = "W"}))
else
-- when energy management is defined in the root endpoint(0), replace it with the first switch endpoint and process it.
device:emit_event_for_endpoint(device:get_field(fields.ENERGY_MANAGEMENT_ENDPOINT), capabilities.powerMeter.power({ value = watt_value, unit = "W"}))
end
if type(device.register_native_capability_attr_handler) == "function" then
device:register_native_capability_attr_handler("powerMeter","power")
end
device:emit_component_event(component, capabilities.powerMeter.power({ value = watt_value, unit = "W"}))
end
if type(device.register_native_capability_attr_handler) == "function" then
device:register_native_capability_attr_handler("powerMeter","power")
end
end

Expand All @@ -281,25 +278,22 @@ end

function AttributeHandlers.cumul_energy_imported_handler(driver, device, ib, response)
if ib.data.elements.energy then
local energy_component = device.profile.components["main"]
local watt_hour_value = ib.data.elements.energy.value / fields.CONVERSION_CONST_MILLIWATT_TO_WATT
device:set_field(fields.TOTAL_IMPORTED_ENERGY, watt_hour_value, {persist = true})
if ib.endpoint_id ~= 0 then
device:emit_event_for_endpoint(ib.endpoint_id, capabilities.energyMeter.energy({ value = watt_hour_value, unit = "Wh" }))
else
-- when energy management is defined in the root endpoint(0), replace it with the first switch endpoint and process it.
device:emit_event_for_endpoint(device:get_field(fields.ENERGY_MANAGEMENT_ENDPOINT), capabilities.energyMeter.energy({ value = watt_hour_value, unit = "Wh" }))
end
device:emit_component_event(energy_component, capabilities.energyMeter.energy({ value = watt_hour_value, unit = "Wh" }))
switch_utils.report_power_consumption_to_st_energy(device, device:get_field(fields.TOTAL_IMPORTED_ENERGY))
end
end

function AttributeHandlers.per_energy_imported_handler(driver, device, ib, response)
if ib.data.elements.energy then
local energy_component = device.profile.components["main"]
local watt_hour_value = ib.data.elements.energy.value / fields.CONVERSION_CONST_MILLIWATT_TO_WATT
local latest_energy_report = device:get_field(fields.TOTAL_IMPORTED_ENERGY) or 0
local summed_energy_report = latest_energy_report + watt_hour_value
device:set_field(fields.TOTAL_IMPORTED_ENERGY, summed_energy_report, {persist = true})
device:emit_event(capabilities.energyMeter.energy({ value = summed_energy_report, unit = "Wh" }))
device:emit_component_event(energy_component, capabilities.energyMeter.energy({ value = summed_energy_report, unit = "Wh" }))
switch_utils.report_power_consumption_to_st_energy(device, device:get_field(fields.TOTAL_IMPORTED_ENERGY))
end
end
Expand All @@ -309,7 +303,7 @@ function AttributeHandlers.energy_imported_factory(is_cumulative_report)
-- workaround: ignore devices supporting Eve's private energy cluster AND the ElectricalEnergyMeasurement cluster
local EVE_MANUFACTURER_ID, EVE_PRIVATE_CLUSTER_ID = 0x130A, 0x130AFC01
local eve_private_energy_eps = device:get_endpoints(EVE_PRIVATE_CLUSTER_ID)
if device.manufacturer_info.vendor_id == EVE_MANUFACTURER_ID and #eve_private_energy_eps > 0 then
if device.network_type == device_lib.NETWORK_TYPE_MATTER and device.manufacturer_info.vendor_id == EVE_MANUFACTURER_ID and #eve_private_energy_eps > 0 then
Copy link
Contributor Author

@hcarter-775 hcarter-775 Sep 29, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

This check may be required if a child device attempts to call this function for two reasons:

  1. a child device may not have manufacturer_info filled, which would cause an error.
  2. The Eve subdriver only accepts non-child devices, so technically this is a more accurate check since we're trying to only gate devices that would be using the subdriver.

Both of these are kinda theoretical, but a unit test caught the issue and rather than updating the unit test, I figured the above reasons justified the inclusion anyway.

Copy link
Contributor

@ctowns ctowns Oct 1, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Is this directly related to removing the ENERGY_MANAGEMENT_ENDPOINT? If not, I would make this a separate PR. If yes, I'm not sure I understand the connection, could you explain?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's semi-related. A child device may not have manufacturer info set, which would cause an issue. So in a unit test, that wasn't set. We also don't set the manufacturer info for child devices in our current logic.

return
end

Expand Down
10 changes: 1 addition & 9 deletions drivers/SmartThings/matter-switch/src/init.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ local fields = require "utils.switch_fields"
local switch_utils = require "utils.switch_utils"
local cfg = require "utils.device_configuration"
local device_cfg = cfg.DeviceCfg
local switch_cfg = cfg.SwitchCfg
local button_cfg = cfg.ButtonCfg

local attribute_handlers = require "generic_handlers.attribute_handlers"
Expand Down Expand Up @@ -87,15 +86,8 @@ function SwitchLifecycleHandlers.device_init(driver, device)
end
local main_endpoint = switch_utils.find_default_endpoint(device)
-- ensure subscription to all endpoint attributes- including those mapped to child devices
for idx, ep in ipairs(device.endpoints) do
for _, ep in ipairs(device.endpoints) do
if ep.endpoint_id ~= main_endpoint then
if device:supports_server_cluster(clusters.OnOff.ID, ep) then
local child_profile = switch_cfg.assign_child_profile(device, ep)
if idx == 1 and string.find(child_profile, "energy") then
-- when energy management is defined in the root endpoint(0), replace it with the first switch endpoint and process it.
device:set_field(fields.ENERGY_MANAGEMENT_ENDPOINT, ep, {persist = true})
end
end
local id = 0
for _, dt in ipairs(ep.device_types) do
id = math.max(id, dt.device_type_id)
Expand Down
22 changes: 11 additions & 11 deletions drivers/SmartThings/matter-switch/src/test/test_aqara_light_switch_h2.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -271,10 +271,8 @@ test.register_coroutine_test(
function()
test.socket.matter:__queue_receive(
{
-- don't use "aqara_mock_children[aqara_child1_ep].id,"
-- because energy management is at the root endpoint.
aqara_mock_device.id,
clusters.ElectricalPowerMeasurement.attributes.ActivePower:build_test_report_data(aqara_mock_device, 1, 17000)
aqara_mock_children[aqara_child1_ep].id,
clusters.ElectricalPowerMeasurement.attributes.ActivePower:build_test_report_data(aqara_mock_children[aqara_child1_ep], 1, 17000)
}
)

Expand All @@ -283,12 +281,14 @@ test.register_coroutine_test(
aqara_mock_children[aqara_child1_ep]:generate_test_message("main", capabilities.powerMeter.power({value = 17.0, unit="W"}))
)

aqara_mock_children[aqara_child1_ep]:expect_native_attr_handler_registration("powerMeter", "power")

test.mock_time.advance_time(901) -- move time 15 minutes past 0 (this can be assumed to be true in practice in all cases)

test.socket.matter:__queue_receive(
{
aqara_mock_device.id,
clusters.ElectricalEnergyMeasurement.attributes.CumulativeEnergyImported:build_test_report_data(aqara_mock_device, 1, cumulative_report_val_19)
aqara_mock_children[aqara_child1_ep].id,
clusters.ElectricalEnergyMeasurement.attributes.CumulativeEnergyImported:build_test_report_data(aqara_mock_children[aqara_child1_ep], 1, cumulative_report_val_19)
}
)

Expand All @@ -307,8 +307,8 @@ test.register_coroutine_test(

test.socket.matter:__queue_receive(
{
aqara_mock_device.id,
clusters.ElectricalEnergyMeasurement.attributes.CumulativeEnergyImported:build_test_report_data(aqara_mock_device, 1, cumulative_report_val_29)
aqara_mock_children[aqara_child1_ep].id,
clusters.ElectricalEnergyMeasurement.attributes.CumulativeEnergyImported:build_test_report_data(aqara_mock_children[aqara_child1_ep], 1, cumulative_report_val_29)
}
)

Expand All @@ -323,9 +323,9 @@ test.register_coroutine_test(

test.socket.matter:__queue_receive(
{
aqara_mock_device.id,
aqara_mock_children[aqara_child1_ep].id,
clusters.ElectricalEnergyMeasurement.attributes.CumulativeEnergyImported:build_test_report_data(
aqara_mock_device, 1, cumulative_report_val_39
aqara_mock_children[aqara_child1_ep], 1, cumulative_report_val_39
)
}
)
Expand All @@ -338,7 +338,7 @@ test.register_coroutine_test(
aqara_mock_children[aqara_child1_ep]:generate_test_message("main", capabilities.powerConsumptionReport.powerConsumption({
start = "1970-01-01T00:15:01Z",
["end"] = "1970-01-01T00:40:00Z",
deltaEnergy = 0.0,
deltaEnergy = 20.0,
energy = 39.0
}))
)
Expand Down
8 changes: 2 additions & 6 deletions drivers/SmartThings/matter-switch/src/utils/device_configuration.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ function SwitchDeviceConfiguration.assign_child_profile(device, child_ep)
-- child_device_profile_overrides
for id, vendor in pairs(fields.child_device_profile_overrides_per_vendor_id) do
for _, fingerprint in ipairs(vendor) do
if device.manufacturer_info.product_id == fingerprint.product_id and
if device.manufacturer_info and device.manufacturer_info.product_id == fingerprint.product_id and
((device.manufacturer_info.vendor_id == fields.AQARA_MANUFACTURER_ID and child_ep == 1) or profile == fingerprint.initial_profile) then
return fingerprint.target_profile
end
Expand All @@ -75,7 +75,7 @@ function SwitchDeviceConfiguration.create_child_switch_devices(driver, device, m
local parent_child_device = false
local switch_eps = device:get_endpoints(clusters.OnOff.ID)
table.sort(switch_eps)
for idx, ep in ipairs(switch_eps) do
for _, ep in ipairs(switch_eps) do
if device:supports_server_cluster(clusters.OnOff.ID, ep) then
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
Expand All @@ -92,10 +92,6 @@ function SwitchDeviceConfiguration.create_child_switch_devices(driver, device, m
}
)
parent_child_device = true
if idx == 1 and string.find(child_profile, "energy") then
-- when energy management is defined in the root endpoint(0), replace it with the first switch endpoint and process it.
device:set_field(fields.ENERGY_MANAGEMENT_ENDPOINT, ep, {persist = true})
end
end
end
end
Expand Down
4 changes: 2 additions & 2 deletions drivers/SmartThings/matter-switch/src/utils/switch_fields.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,6 @@ SwitchFields.CONVERSION_CONST_MILLIWATT_TO_WATT = 1000 -- A milliwatt is 1/1000t
-- table for devices that joined prior to this transition, and is also used for
-- button devices that require component mapping.
SwitchFields.COMPONENT_TO_ENDPOINT_MAP = "__component_to_endpoint_map"
SwitchFields.ENERGY_MANAGEMENT_ENDPOINT = "__energy_management_endpoint"
SwitchFields.IS_PARENT_CHILD_DEVICE = "__is_parent_child_device"
SwitchFields.COLOR_TEMP_BOUND_RECEIVED_KELVIN = "__colorTemp_bound_received_kelvin"
SwitchFields.COLOR_TEMP_BOUND_RECEIVED_MIRED = "__colorTemp_bound_received_mired"
Expand All @@ -99,7 +98,8 @@ SwitchFields.COLOR_MODE = "__color_mode"

SwitchFields.updated_fields = {
{ current_field_name = "__component_to_endpoint_map_button", updated_field_name = SwitchFields.COMPONENT_TO_ENDPOINT_MAP },
{ current_field_name = "__switch_intialized", updated_field_name = nil }
{ current_field_name = "__switch_intialized", updated_field_name = nil },
{ current_field_name = "__energy_management_endpoint", updated_field_name = nil }
}

SwitchFields.HUE_SAT_COLOR_MODE = clusters.ColorControl.types.ColorMode.CURRENT_HUE_AND_CURRENT_SATURATION
Expand Down
29 changes: 11 additions & 18 deletions drivers/SmartThings/matter-switch/src/utils/switch_utils.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ function utils.device_type_supports_button_switch_combination(device, endpoint_i
for _, dt in ipairs(ep.device_types) do
if dt.device_type_id == fields.DIMMABLE_LIGHT_DEVICE_TYPE_ID then
for _, fingerprint in ipairs(fields.child_device_profile_overrides_per_vendor_id[0x115F]) do
if device.manufacturer_info.product_id == fingerprint.product_id then
if device.manufacturer_info and device.manufacturer_info.product_id == fingerprint.product_id then
return false -- For Aqara Dimmer Switch with Button.
end
end
Expand All @@ -98,8 +98,9 @@ end
--- find_default_endpoint is a helper function to handle situations where
--- device does not have endpoint ids in sequential order from 1
function utils.find_default_endpoint(device)
if device.manufacturer_info.vendor_id == fields.AQARA_MANUFACTURER_ID and
device.manufacturer_info.product_id == fields.AQARA_CLIMATE_SENSOR_W100_ID then
if device.manufacturer_info and
device.manufacturer_info.vendor_id == fields.AQARA_MANUFACTURER_ID and
device.manufacturer_info.product_id == fields.AQARA_CLIMATE_SENSOR_W100_ID then
-- In case of Aqara Climate Sensor W100, in order to sequentially set the button name to button 1, 2, 3
return device.MATTER_DEFAULT_ENDPOINT
end
Expand Down Expand Up @@ -224,21 +225,13 @@ function utils.report_power_consumption_to_st_energy(device, latest_total_import
local epoch_to_iso8601 = function(time) return os.date("!%Y-%m-%dT%H:%M:%SZ", time) end -- Return an ISO-8061 timestamp from UTC

-- Report the energy consumed during the time interval. The unit of these values should be 'Wh'
if not device:get_field(fields.ENERGY_MANAGEMENT_ENDPOINT) then
device:emit_event(capabilities.powerConsumptionReport.powerConsumption({
start = epoch_to_iso8601(last_time),
["end"] = epoch_to_iso8601(current_time - 1),
deltaEnergy = energy_delta_wh,
energy = latest_total_imported_energy_wh
}))
else
device:emit_event_for_endpoint(device:get_field(fields.ENERGY_MANAGEMENT_ENDPOINT),capabilities.powerConsumptionReport.powerConsumption({
start = epoch_to_iso8601(last_time),
["end"] = epoch_to_iso8601(current_time - 1),
deltaEnergy = energy_delta_wh,
energy = latest_total_imported_energy_wh
}))
end
local power_consumption_component = device.profile.components["main"]
device:emit_component_event(power_consumption_component, capabilities.powerConsumptionReport.powerConsumption({
start = epoch_to_iso8601(last_time),
["end"] = epoch_to_iso8601(current_time - 1),
deltaEnergy = energy_delta_wh,
energy = latest_total_imported_energy_wh
}))
end

return utils
Loading