Skip to content
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

Add remaining 3 phase electrical attributes #339

Draft
wants to merge 11 commits into
base: dev
Choose a base branch
from
9 changes: 9 additions & 0 deletions tests/test_cluster_handlers.py
Original file line number Diff line number Diff line change
@@ -288,16 +288,25 @@ async def poll_control_device_mock(zha_gateway: Gateway) -> Device:
"ac_frequency",
"ac_frequency_max",
"active_power",
"active_power_ph_b",
"active_power_ph_c",
"active_power_max",
"apparent_power",
"power_factor",
"power_factor_ph_b",
"power_factor_ph_c",
"rms_current",
"rms_current_ph_b",
"rms_current_ph_c",
"rms_current_max",
"rms_current_max_b",
"rms_current_max_c",
"rms_voltage",
"rms_voltage_ph_b",
"rms_voltage_ph_c",
"rms_voltage_max",
"rms_voltage_max_ph_b",
"rms_voltage_max_ph_c",
},
),
],
10 changes: 10 additions & 0 deletions tests/test_sensor.py
Original file line number Diff line number Diff line change
@@ -1147,7 +1147,11 @@ async def test_elec_measurement_skip_unsupported_attribute(

all_attrs = {
"active_power",
"active_power_ph_b",
"active_power_ph_c",
"active_power_max",
"active_power_max_ph_b",
"active_power_max_ph_c",
"apparent_power",
"rms_current",
"rms_current_ph_b",
@@ -1156,8 +1160,14 @@ async def test_elec_measurement_skip_unsupported_attribute(
"rms_current_max_ph_b",
"rms_current_max_ph_c",
"rms_voltage",
"rms_voltage_ph_b",
"rms_voltage_ph_c",
"rms_voltage_max",
"rms_voltage_max_ph_b",
"rms_voltage_max_ph_c",
"power_factor",
"power_factor_ph_b",
"power_factor_ph_c",
"ac_frequency",
"ac_frequency_max",
}
72 changes: 70 additions & 2 deletions zha/application/platforms/sensor/__init__.py
Original file line number Diff line number Diff line change
@@ -694,6 +694,30 @@ class PolledElectricalMeasurement(ElectricalMeasurement):
_use_custom_polling: bool = True


@MULTI_MATCH(cluster_handler_names=CLUSTER_HANDLER_ELECTRICAL_MEASUREMENT)
class ElectricalMeasurementRMSActivePowerPhB(PolledElectricalMeasurement):
"""RMS active power phase B measurement."""

_attribute_name = "active_power_ph_b"
_unique_id_suffix = "active_power_ph_b"
_attr_translation_key: str = "active_power_ph_b"
_use_custom_polling = False # Poll indirectly by ElectricalMeasurementSensor
_skip_creation_if_no_attr_cache = True
_attr_max_attribute_name = "active_power_max_ph_b"


@MULTI_MATCH(cluster_handler_names=CLUSTER_HANDLER_ELECTRICAL_MEASUREMENT)
class ElectricalMeasurementRMSActivePowerPhC(PolledElectricalMeasurement):
"""RMS active power phase C measurement."""

_attribute_name = "active_power_ph_c"
_unique_id_suffix = "active_power_ph_c"
_attr_translation_key: str = "active_power_ph_c"
_use_custom_polling = False # Poll indirectly by ElectricalMeasurementSensor
_skip_creation_if_no_attr_cache = True
_attr_max_attribute_name = "active_power_max_ph_c"


@MULTI_MATCH(cluster_handler_names=CLUSTER_HANDLER_ELECTRICAL_MEASUREMENT)
class ElectricalMeasurementApparentPower(PolledElectricalMeasurement):
"""Apparent power measurement."""
@@ -720,7 +744,7 @@ class ElectricalMeasurementRMSCurrent(PolledElectricalMeasurement):

@MULTI_MATCH(cluster_handler_names=CLUSTER_HANDLER_ELECTRICAL_MEASUREMENT)
class ElectricalMeasurementRMSCurrentPhB(ElectricalMeasurementRMSCurrent):
"""RMS current measurement."""
"""RMS current phase B measurement."""

_attribute_name = "rms_current_ph_b"
_unique_id_suffix = "rms_current_ph_b"
@@ -731,7 +755,7 @@ class ElectricalMeasurementRMSCurrentPhB(ElectricalMeasurementRMSCurrent):

@MULTI_MATCH(cluster_handler_names=CLUSTER_HANDLER_ELECTRICAL_MEASUREMENT)
class ElectricalMeasurementRMSCurrentPhC(ElectricalMeasurementRMSCurrent):
"""RMS current measurement."""
"""RMS current phase C measurement."""

_attribute_name: str = "rms_current_ph_c"
_unique_id_suffix: str = "rms_current_ph_c"
@@ -752,6 +776,28 @@ class ElectricalMeasurementRMSVoltage(PolledElectricalMeasurement):
_div_mul_prefix = "ac_voltage"


@MULTI_MATCH(cluster_handler_names=CLUSTER_HANDLER_ELECTRICAL_MEASUREMENT)
class ElectricalMeasurementRMSVoltagePhB(ElectricalMeasurementRMSVoltage):
"""RMS voltage phase B measurement."""

_attribute_name = "rms_voltage_ph_b"
_unique_id_suffix = "rms_voltage_ph_b"
_attr_translation_key: str = "rms_voltage_ph_b"
_skip_creation_if_no_attr_cache = True
_attr_max_attribute_name = "rms_voltage_max_ph_b"


@MULTI_MATCH(cluster_handler_names=CLUSTER_HANDLER_ELECTRICAL_MEASUREMENT)
class ElectricalMeasurementRMSVoltagePhC(ElectricalMeasurementRMSVoltage):
"""RMS voltage phase C measurement."""

_attribute_name = "rms_voltage_ph_c"
_unique_id_suffix = "rms_voltage_ph_c"
_attr_translation_key: str = "rms_voltage_ph_c"
_skip_creation_if_no_attr_cache = True
_attr_max_attribute_name = "rms_voltage_max_ph_c"


@MULTI_MATCH(cluster_handler_names=CLUSTER_HANDLER_ELECTRICAL_MEASUREMENT)
class ElectricalMeasurementFrequency(PolledElectricalMeasurement):
"""Frequency measurement."""
@@ -777,6 +823,28 @@ class ElectricalMeasurementPowerFactor(PolledElectricalMeasurement):
_div_mul_prefix = None


@MULTI_MATCH(cluster_handler_names=CLUSTER_HANDLER_ELECTRICAL_MEASUREMENT)
class ElectricalMeasurementPowerFactorPhB(ElectricalMeasurementPowerFactor):
"""Power factor phase B measurement."""

_attribute_name = "power_factor_ph_b"
_unique_id_suffix = "power_factor_ph_b"
_attr_translation_key: str = "power_factor_ph_b"
_skip_creation_if_no_attr_cache = True
_attr_max_attribute_name = "power_factor_max_ph_b"


@MULTI_MATCH(cluster_handler_names=CLUSTER_HANDLER_ELECTRICAL_MEASUREMENT)
class ElectricalMeasurementPowerFactorPhC(ElectricalMeasurementPowerFactor):
"""Power factor phase C measurement."""

_attribute_name = "power_factor_ph_c"
_unique_id_suffix = "power_factor_ph_c"
_attr_translation_key: str = "power_factor_ph_c"
_skip_creation_if_no_attr_cache = True
_attr_max_attribute_name = "power_factor_max_ph_c"


@MULTI_MATCH(
generic_ids=CLUSTER_HANDLER_ST_HUMIDITY_CLUSTER,
stop_on_match_group=CLUSTER_HANDLER_HUMIDITY,
45 changes: 44 additions & 1 deletion zha/zigbee/cluster_handlers/homeautomation.py
Original file line number Diff line number Diff line change
@@ -65,10 +65,26 @@ class MeasurementType(enum.IntFlag):
attr=ElectricalMeasurement.AttributeDefs.active_power.name,
config=REPORT_CONFIG_OP,
),
AttrReportConfig(
attr=ElectricalMeasurement.AttributeDefs.active_power_ph_b.name,
config=REPORT_CONFIG_OP,
),
AttrReportConfig(
attr=ElectricalMeasurement.AttributeDefs.active_power_ph_c.name,
config=REPORT_CONFIG_OP,
),
AttrReportConfig(
attr=ElectricalMeasurement.AttributeDefs.active_power_max.name,
config=REPORT_CONFIG_DEFAULT,
),
AttrReportConfig(
attr=ElectricalMeasurement.AttributeDefs.active_power_max_ph_b.name,
config=REPORT_CONFIG_DEFAULT,
),
AttrReportConfig(
attr=ElectricalMeasurement.AttributeDefs.active_power_max_ph_c.name,
config=REPORT_CONFIG_DEFAULT,
),
abmantis marked this conversation as resolved.
Show resolved Hide resolved
AttrReportConfig(
attr=ElectricalMeasurement.AttributeDefs.apparent_power.name,
config=REPORT_CONFIG_OP,
@@ -101,10 +117,26 @@ class MeasurementType(enum.IntFlag):
attr=ElectricalMeasurement.AttributeDefs.rms_voltage.name,
config=REPORT_CONFIG_OP,
),
AttrReportConfig(
attr=ElectricalMeasurement.AttributeDefs.rms_voltage_ph_b.name,
config=REPORT_CONFIG_OP,
),
AttrReportConfig(
attr=ElectricalMeasurement.AttributeDefs.rms_voltage_ph_c.name,
config=REPORT_CONFIG_OP,
),
AttrReportConfig(
attr=ElectricalMeasurement.AttributeDefs.rms_voltage_max.name,
config=REPORT_CONFIG_DEFAULT,
),
AttrReportConfig(
attr=ElectricalMeasurement.AttributeDefs.rms_voltage_max_ph_b.name,
config=REPORT_CONFIG_DEFAULT,
),
AttrReportConfig(
attr=ElectricalMeasurement.AttributeDefs.rms_voltage_max_ph_c.name,
config=REPORT_CONFIG_DEFAULT,
),
abmantis marked this conversation as resolved.
Show resolved Hide resolved
AttrReportConfig(
attr=ElectricalMeasurement.AttributeDefs.ac_frequency.name,
config=REPORT_CONFIG_OP,
@@ -113,6 +145,18 @@ class MeasurementType(enum.IntFlag):
attr=ElectricalMeasurement.AttributeDefs.ac_frequency_max.name,
config=REPORT_CONFIG_DEFAULT,
),
AttrReportConfig(
attr=ElectricalMeasurement.AttributeDefs.power_factor.name,
config=REPORT_CONFIG_OP,
),
AttrReportConfig(
attr=ElectricalMeasurement.AttributeDefs.power_factor_ph_b.name,
config=REPORT_CONFIG_OP,
),
AttrReportConfig(
attr=ElectricalMeasurement.AttributeDefs.power_factor_ph_c.name,
config=REPORT_CONFIG_OP,
),
abmantis marked this conversation as resolved.
Show resolved Hide resolved
)
ZCL_INIT_ATTRS = {
ElectricalMeasurement.AttributeDefs.ac_current_divisor.name: True,
@@ -126,7 +170,6 @@ class MeasurementType(enum.IntFlag):
ElectricalMeasurement.AttributeDefs.measurement_type.name: True,
ElectricalMeasurement.AttributeDefs.power_divisor.name: True,
ElectricalMeasurement.AttributeDefs.power_multiplier.name: True,
ElectricalMeasurement.AttributeDefs.power_factor.name: True,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am not sure why power_factor was here, but I think reporting needs to be configured for it too

Copy link
Contributor

Choose a reason for hiding this comment

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

According to ZCL spec, it is not reportable.

Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm, apparently we also set up attribute reporting for some attributes that don't apparently don't even support it before this PR...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed reporting for all the ones that are unsupported

Copy link
Contributor

@TheJulianJES TheJulianJES Jan 15, 2025

Choose a reason for hiding this comment

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

So the "good thing" is that we already poll all supported attributes at runtime, so that's why power_factor and others work in the first place.

But for the ones that change over time, where attribute reporting can't be set up (according to spec), we should just add them to the ZCL_INIT_ATTRS dict with cache=True, as we don't need to poll them when starting up ZHA (the unsupported_attributes check isn't done there), but we do need to poll them at runtime, which is done through the cluster handler, so there is one big attribute read for multiple attributes.

But we should still move some attributes (where attribute reporting isn't possible) from REPORT_CONFIG to ZCL_INIT_ATTRS.
(EDIT: Wrote this before your comment above 😄)

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 seems that we only poll attributes that for which we set up reporting, because we only poll the ones in REPORT_CONFIG, so if we move them to ZCL_INIT_ATTRS they will not be polled, right?

Also, why do we poll the ones that have reporting set up?

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, you're right. I confused the startup polling doing both and this one only doing the reporting attributes.

We poll the attribute because many smart plugs (mostly Tuya) do not respect/allow setting up attribute reporting on most/all EM cluster attributes.

I do wonder a bit how some of this would work at all then, or at least how it works at the moment (with power_factor and so on), but maybe we've just not had any devices that support it? I wanna check how Z2M deals with some of these devices later. It has a polling configuration per plug/device at least.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Either no one complained about power_factor not getting updated (since there maybe not be many devices with it), or the devices report the value even without reporting config 🤷

I reverted the attributes back to REPORT_CONFIG for now.

}

async def async_update(self):

Unchanged files with check annotations Beta

while chunk:
try:
self.debug("Reading attributes in chunks: %s", chunk)
read, _ = await self.cluster.read_attributes(

Check warning on line 587 in zha/zigbee/cluster_handlers/__init__.py

GitHub Actions / shared-ci / Run tests Python 3.12

coroutine 'Cluster.read_attributes' was never awaited

Check warning on line 587 in zha/zigbee/cluster_handlers/__init__.py

GitHub Actions / shared-ci / Run tests Python 3.12

coroutine 'Cluster.read_attributes' was never awaited

Check warning on line 587 in zha/zigbee/cluster_handlers/__init__.py

GitHub Actions / shared-ci / Run tests Python 3.13

coroutine 'Cluster.read_attributes' was never awaited

Check warning on line 587 in zha/zigbee/cluster_handlers/__init__.py

GitHub Actions / shared-ci / Run tests Python 3.13

coroutine 'Cluster.read_attributes' was never awaited
chunk,
allow_cache=from_cache,
only_cache=only_cache,