Skip to content

Add support for DC Electrical Measurement #453

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

Open
wants to merge 5 commits into
base: dev
Choose a base branch
from

Conversation

P-R-O-C-H-Y
Copy link
Contributor

This PR adds support for DC measurements of the ElectricalMeasurement cluster. Only AC is supported now.

I have tested using a local home assistant with the changes implemented, all works fine. Values are in correct resolution.
Screenshot 2025-05-19 at 15 20 44

However, I am receiving warnings in the log:

2025-05-19 15:15:13.160 WARNING (MainThread) [homeassistant.components.zha.sensor] Unexpected extra state attributes found for sensor <zha.application.platforms.sensor.ElectricalMeasurementDCVoltage object at 0x1343196a0>: {'dc_voltage_max'}
2025-05-19 15:15:13.160 WARNING (MainThread) [homeassistant.components.zha.sensor] Unexpected extra state attributes found for sensor <zha.application.platforms.sensor.ElectricalMeasurementDCCurrent object at 0x1343197f0>: {'dc_current_max'}
2025-05-19 15:15:13.160 WARNING (MainThread) [homeassistant.components.zha.sensor] Unexpected extra state attributes found for sensor <zha.application.platforms.sensor.ElectricalMeasurementDCPower object at 0x134319940>: {'dc_power_max'}

@puddly Can you please take a look?

Diagnostics file:
zha-01JTN6EVKS6CN88NJXBJPG000P-Espressif ZigbeeElectricalMeasurementDC-c4599a43a5e2d4ab879b05282130e645.json

@TheJulianJES TheJulianJES added the entities Issue or PR about (custom) entities label May 19, 2025
@TheJulianJES
Copy link
Contributor

BaseElectricalMeasurement sets _attr_extra_state_attribute_names. Since HA Core generally doesn't want us to add new extra state attributes, we're expecting existing ones here: https://github.com/home-assistant/core/blob/760f2d1959880abc436c4d8a5e0b0903f06fbace/homeassistant/components/zha/sensor.py#L33-L77

The DC attributes aren't included, so that's why you're seeing the warnings about them being unexpected.
Tbh, I think we should just remove these _max extra state attributes for all EM sensors.
For now, maybe we can add an attribute to BaseElectricalMeasurement that allows us to turn off the extra state attributes per sensor entity class. For now, we would only do that for the new DC entities then.

@puddly
Copy link
Contributor

puddly commented May 19, 2025

Tbh, I think we should just remove these _max extra state attributes for all EM sensors.

Agreed. I think pretty much all current uses of extra state attributes should be new entities. Or removed entirely.

@P-R-O-C-H-Y
Copy link
Contributor Author

Thanks for taking a look. Should I work on that or if you know exactly what you want to do can you take over? :)

@P-R-O-C-H-Y
Copy link
Contributor Author

@puddly Anything I can do here to move on with this PR?

@lboue
Copy link

lboue commented Jun 5, 2025

@puddly Anything I can do here to move on with this PR?

You should add tests here to fix CI tests

zha/tests/test_sensor.py

Lines 570 to 576 in a2de928

(
homeautomation.ElectricalMeasurement.cluster_id,
sensor.ElectricalMeasurementRMSCurrent,
partial(async_test_em_rms_current, 0x0508, 0x050A, "rms_current_max"),
{"ac_current_divisor": 1000, "ac_current_multiplier": 1},
{"active_power", "apparent_power", "rms_voltage"},
),

{
"ac_frequency",
"ac_voltage_divisor",
"ac_current_divisor",
"ac_power_divisor",
"ac_voltage_multiplier",
"ac_power_multiplier",
"power_divisor",
"power_multiplier",
"ac_current_multiplier",
"ac_frequency",
"active_power",
"active_power_ph_b",
"active_power_ph_c",
"apparent_power",
"rms_current",
"rms_current_ph_b",
"rms_current_ph_c",
"rms_voltage",
"rms_voltage_ph_b",
"rms_voltage_ph_c",
},

@P-R-O-C-H-Y
Copy link
Contributor Author

@TheJulianJES @puddly @lboue Can you please help with resolving the failures in the tests? I tried locally without any positive results.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
entities Issue or PR about (custom) entities
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants