Matter Switch: Add greater Energy profiling logic for Switches #2199
Conversation
|
Duplicate profile check: Passed - no duplicate profiles detected. |
|
Channel deleted. |
Test Results 68 files 446 suites 0s ⏱️ Results for commit b0b7b50. ♻️ This comment has been updated with latest results. |
|
Minimum allowed coverage is Generated by 🐒 cobertura-action against b0b7b50 |
drivers/SmartThings/matter-switch/src/test/test_aqara_light_switch_h2.lua
Outdated
Show resolved
Hide resolved
|
These changes look great! I think it might be a good idea to test on some additional device types since some of the init code is being changed. |
hcarter-775
force-pushed
the
add/power-topology-handling
branch
from
aeafdff to
ca5b1d0
Compare
|
I'm trying to remember but can't quite recall, I think you mentioned that these changes would result in re-profiling for certain devices that would currently not be re-profiled and just rely on fingerprints, is that the case? |
@nickolas-deboom yes, that's true. See the |
|
Taking another look at this, "update powerConsumptionReport handling to only send reports to one single capability, even when there are multiple child devices getting energy reports." - wondering where this change is coming from? Was there a request or something? |
| @@ -141,6 +141,7 @@ local function test_init() | |||
| end | |||
|
|
|||
| test.socket.matter:__expect_send({aqara_mock_device.id, subscribe_request}) | |||
| aqara_mock_device:set_field("__ELECTRICAL_TOPOLOGY", {topology = false, tags_on_ep = {}}, {persist = false}) -- since we're assuming this would have happened during device_added in this case. | |||
There was a problem hiding this comment.
The Aqara subdriver overrides device_added so this field would not be set in the current implementation and so the subdriver may need to be updated to set it. The other subdrivers may also need to account for this
There was a problem hiding this comment.
this test doesn't handle a device that's using a subdriver.
There was a problem hiding this comment.
Oops, my bad, I saw Aqara and thought this was the Aqara cube test file
| @@ -277,7 +280,7 @@ test.register_coroutine_test( | |||
| { | |||
| -- don't use "aqara_mock_children[aqara_child1_ep].id," | |||
| -- because energy management is at the root endpoint. | |||
There was a problem hiding this comment.
This comment can be removed now 😅
There was a problem hiding this comment.
Actually, is this the correct behavior? If I remember right this device has it's energy management cluster on the root endpoint, so why is it now testing aqara_child1_ep?
There was a problem hiding this comment.
It is, this update actually makes the whole aqara child system work better now we use emit_component_event(), side stepping the original issues we faced having the energy management cluster on the root endpoint.
ST Energy will only take one report per device at the moment, so there's no reason to be sending multiple reports. Before this PR, multiple reports just weren't supported at all, so this isn't really a "change". |
hcarter-775
force-pushed
the
add/power-topology-handling
branch
from
f2e532c to
b0b7b50
Compare
Description of Change
Update the Matter Switch driver to support the Power Topology cluster. Specifically, include support for the Node and Set features of the cluster. Update unit tests to handle this update.
Further, update
powerConsumptionReporthandling to only send reports to one single capability, even when there are multiple child devices getting energy reports.Last, update energy and power handlers from emit_event_for_endpoint() calls to emit_component_event() calls to simplify handling by avoiding the weaknesses in our current
endpoint_to_component()functionality.Summary of Completed Tests
Tests updated and continue to pass.
Tested on multi-plug energy device.
Tested on single plug energy device.
Tested for backwards compatibility with other light devices.