From 2909e1c4fe0bcb8bd4449e2ef52078bfaabee830 Mon Sep 17 00:00:00 2001 From: Jack <46714706+jeverley@users.noreply.github.com> Date: Tue, 30 Jan 2024 22:11:18 +0000 Subject: [PATCH] Fix ZHA handling of power factor ElectricalMeasurement attribute sensor (#107641) * Correct handling of power_factor ElectricalMeasurement attribute The Zigbee Cluster Library defines PowerFactor as an int8 with value supported from -100 to 100. Currently the zha sensor handler attempts to apply the ac_power_divisor and ac_power_multiplier formatters against the attribute value, the spec outlines that this should not be the case. The impact of the current code is that quirks not using the default values of 1 are multiplying/dividing power and power factor values prior to updating the cluster attribute. This results in either a non-conformant power_factor e.g. the value was multiplied by 10 so that an ac_power_divider of 10 could be used, or the power readings sacrificing a point of measurement for lower readings. Two quirks currently use this workaround: * ts0601_din_power.py * ts0601_rcbo.py * Update ZHA Metering formatter to perform None check on _div_mul_prefix To address feedback: https://github.com/home-assistant/core/pull/107641#discussion_r1447547054 * _div_mul_prefix needs self reference * Simplify None check for _div_mul_prefix Co-authored-by: Joakim Plate * Updates to formatting and CI test typing fix * Use ' | ' in place of Union * Add tests for power_factor sensor --------- Co-authored-by: Joakim Plate --- homeassistant/components/zha/sensor.py | 17 +++++++++++------ tests/components/zha/test_sensor.py | 26 ++++++++++++++++++++++++++ 2 files changed, 37 insertions(+), 6 deletions(-) diff --git a/homeassistant/components/zha/sensor.py b/homeassistant/components/zha/sensor.py index bb62494396a..4986742c63d 100644 --- a/homeassistant/components/zha/sensor.py +++ b/homeassistant/components/zha/sensor.py @@ -319,7 +319,7 @@ class ElectricalMeasurement(PollableSensor): _attr_device_class: SensorDeviceClass = SensorDeviceClass.POWER _attr_state_class: SensorStateClass = SensorStateClass.MEASUREMENT _attr_native_unit_of_measurement: str = UnitOfPower.WATT - _div_mul_prefix = "ac_power" + _div_mul_prefix: str | None = "ac_power" @property def extra_state_attributes(self) -> dict[str, Any]: @@ -342,10 +342,14 @@ class ElectricalMeasurement(PollableSensor): def formatter(self, value: int) -> int | float: """Return 'normalized' value.""" - multiplier = getattr( - self._cluster_handler, f"{self._div_mul_prefix}_multiplier" - ) - divisor = getattr(self._cluster_handler, f"{self._div_mul_prefix}_divisor") + if self._div_mul_prefix: + multiplier = getattr( + self._cluster_handler, f"{self._div_mul_prefix}_multiplier" + ) + divisor = getattr(self._cluster_handler, f"{self._div_mul_prefix}_divisor") + else: + multiplier = self._multiplier + divisor = self._divisor value = float(value * multiplier) / divisor if value < 100 and divisor > 1: return round(value, self._decimals) @@ -419,13 +423,14 @@ class ElectricalMeasurementFrequency(PolledElectricalMeasurement): @MULTI_MATCH(cluster_handler_names=CLUSTER_HANDLER_ELECTRICAL_MEASUREMENT) # pylint: disable-next=hass-invalid-inheritance # needs fixing class ElectricalMeasurementPowerFactor(PolledElectricalMeasurement): - """Frequency measurement.""" + """Power Factor measurement.""" _attribute_name = "power_factor" _unique_id_suffix = "power_factor" _use_custom_polling = False # Poll indirectly by ElectricalMeasurementSensor _attr_device_class: SensorDeviceClass = SensorDeviceClass.POWER_FACTOR _attr_native_unit_of_measurement = PERCENTAGE + _div_mul_prefix = None @MULTI_MATCH( diff --git a/tests/components/zha/test_sensor.py b/tests/components/zha/test_sensor.py index 48651df082d..e25430a293b 100644 --- a/tests/components/zha/test_sensor.py +++ b/tests/components/zha/test_sensor.py @@ -259,6 +259,24 @@ async def async_test_em_apparent_power(hass: HomeAssistant, cluster, entity_id): assert_state(hass, entity_id, "9.9", UnitOfApparentPower.VOLT_AMPERE) +async def async_test_em_power_factor(hass: HomeAssistant, cluster, entity_id): + """Test electrical measurement Power Factor sensor.""" + # update divisor cached value + await send_attributes_report(hass, cluster, {"ac_power_divisor": 1}) + await send_attributes_report(hass, cluster, {0: 1, 0x0510: 100, 10: 1000}) + assert_state(hass, entity_id, "100", PERCENTAGE) + + await send_attributes_report(hass, cluster, {0: 1, 0x0510: 99, 10: 1000}) + assert_state(hass, entity_id, "99", PERCENTAGE) + + await send_attributes_report(hass, cluster, {"ac_power_divisor": 10}) + await send_attributes_report(hass, cluster, {0: 1, 0x0510: 100, 10: 5000}) + assert_state(hass, entity_id, "100", PERCENTAGE) + + await send_attributes_report(hass, cluster, {0: 1, 0x0510: 99, 10: 5000}) + assert_state(hass, entity_id, "99", PERCENTAGE) + + async def async_test_em_rms_current(hass: HomeAssistant, cluster, entity_id): """Test electrical measurement RMS Current sensor.""" @@ -428,6 +446,14 @@ async def async_test_device_temperature(hass: HomeAssistant, cluster, entity_id) {"ac_power_divisor": 1000, "ac_power_multiplier": 1}, {"active_power", "rms_current", "rms_voltage"}, ), + ( + homeautomation.ElectricalMeasurement.cluster_id, + "power_factor", + async_test_em_power_factor, + 7, + {"ac_power_divisor": 1000, "ac_power_multiplier": 1}, + {"active_power", "apparent_power", "rms_current", "rms_voltage"}, + ), ( homeautomation.ElectricalMeasurement.cluster_id, "current",