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.

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

Add support for rms_current_ph_b/c #324

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

Add support for rms_current_ph_b/c #324

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
2e2e336
Add support for rms_current_ph_b/c
abmantis Dec 9, 2024
abfd574
Make max attribute name overrideable; Add tests
abmantis Dec 10, 2024
a924e3b
Use attribute for conditional create_platform_entity
abmantis Dec 10, 2024
30bfcbc
Fix cluster handler config test
abmantis Dec 10, 2024
7fab2e3
Merge branch 'dev' into current_3ph
abmantis Dec 23, 2024
a0296f5
Use ph B as base for C to reduce duplicated attrs
abmantis Dec 24, 2024
e3ea5ca
Merge branch 'dev' into current_3ph
abmantis Jan 4, 2025
4447b6f
Merge branch 'dev' of github.com:zigpy/zha into current_3ph
abmantis Jan 6, 2025
6976e49
Revert "Use ph B as base for C to reduce duplicated attrs"
abmantis Jan 6, 2025
7c3627f
Address review suggestions
abmantis Jan 11, 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
6 changes: 6 additions & 0 deletions tests/test_cluster_handlers.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -285,11 +285,17 @@ async def poll_control_device_mock(zha_gateway: Gateway) -> Device:
zigpy.zcl.clusters.homeautomation.ElectricalMeasurement.cluster_id,
1,
{
"ac_frequency",
"ac_frequency_max",
"active_power",
"active_power_max",
"apparent_power",
"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_max",
},
Expand Down
46 changes: 39 additions & 7 deletions tests/test_sensor.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import asyncio
from collections.abc import Awaitable, Callable
from datetime import UTC, datetime
from functools import partial
import math
from typing import Any, Optional
from unittest.mock import AsyncMock, MagicMock
Expand Down Expand Up @@ -298,22 +299,27 @@ async def async_test_em_power_factor(


async def async_test_em_rms_current(
zha_gateway: Gateway, cluster: Cluster, entity: PlatformEntity
current_attrid: int,
current_max_attrid: int,
current_max_attr_name: str,
zha_gateway: Gateway,
cluster: Cluster,
entity: PlatformEntity,
) -> None:
"""Test electrical measurement RMS Current sensor."""

await send_attributes_report(zha_gateway, cluster, {0: 1, 0x0508: 1234})
await send_attributes_report(zha_gateway, cluster, {0: 1, current_attrid: 1234})
assert_state(entity, 1.2, "A")

await send_attributes_report(zha_gateway, cluster, {"ac_current_divisor": 10})
await send_attributes_report(zha_gateway, cluster, {0: 1, 0x0508: 236})
await send_attributes_report(zha_gateway, cluster, {0: 1, current_attrid: 236})
assert_state(entity, 23.6, "A")

await send_attributes_report(zha_gateway, cluster, {0: 1, 0x0508: 1236})
await send_attributes_report(zha_gateway, cluster, {0: 1, current_attrid: 1236})
assert_state(entity, 124, "A")

await send_attributes_report(zha_gateway, cluster, {0: 1, 0x050A: 88})
assert entity.state["rms_current_max"] == 8.8
await send_attributes_report(zha_gateway, cluster, {0: 1, current_max_attrid: 88})
assert entity.state[current_max_attr_name] == 8.8


async def async_test_em_rms_voltage(
Expand Down Expand Up @@ -515,10 +521,32 @@ async def async_test_change_source_timestamp(
(
homeautomation.ElectricalMeasurement.cluster_id,
sensor.ElectricalMeasurementRMSCurrent,
async_test_em_rms_current,
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"},
),
(
homeautomation.ElectricalMeasurement.cluster_id,
sensor.ElectricalMeasurementRMSCurrentPhB,
partial(async_test_em_rms_current, 0x0908, 0x090A, "rms_current_max_ph_b"),
{
"ac_current_divisor": 1000,
"ac_current_multiplier": 1,
"rms_current_ph_b": 0,
},
{"active_power", "apparent_power", "rms_voltage"},
),
(
homeautomation.ElectricalMeasurement.cluster_id,
sensor.ElectricalMeasurementRMSCurrentPhC,
partial(async_test_em_rms_current, 0x0A08, 0x0A0A, "rms_current_max_ph_c"),
{
"ac_current_divisor": 1000,
"ac_current_multiplier": 1,
"rms_current_ph_c": 0,
},
{"active_power", "apparent_power", "rms_voltage"},
),
(
homeautomation.ElectricalMeasurement.cluster_id,
sensor.ElectricalMeasurementRMSVoltage,
Expand Down Expand Up @@ -1122,7 +1150,11 @@ async def test_elec_measurement_skip_unsupported_attribute(
"active_power_max",
"apparent_power",
"rms_current",
"rms_current_ph_b",
"rms_current_ph_c",
"rms_current_max",
"rms_current_max_ph_b",
"rms_current_max_ph_c",
"rms_voltage",
"rms_voltage_max",
"power_factor",
Expand Down
72 changes: 46 additions & 26 deletions zha/application/platforms/sensor/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,7 @@ class Sensor(PlatformEntity):
_attr_native_unit_of_measurement: str | None = None
_attr_device_class: SensorDeviceClass | None = None
_attr_state_class: SensorStateClass | None = None
_skip_creation_if_no_attr_cache: bool = False

@classmethod
def create_platform_entity(
Expand All @@ -183,6 +184,12 @@ def create_platform_entity(
)
return None

if (
cls._skip_creation_if_no_attr_cache
and cluster_handlers[0].cluster.get(cls._attribute_name) is None
):
return None

return cls(unique_id, cluster_handlers, endpoint, device, **kwargs)

def __init__(
Expand Down Expand Up @@ -622,6 +629,7 @@ class ElectricalMeasurement(PollableSensor):
_attr_device_class: SensorDeviceClass = SensorDeviceClass.POWER
_attr_state_class: SensorStateClass = SensorStateClass.MEASUREMENT
_attr_native_unit_of_measurement: str = UnitOfPower.WATT
_attr_max_attribute_name: str = None
_div_mul_prefix: str | None = "ac_power"

def __init__(
Expand All @@ -636,17 +644,22 @@ def __init__(
super().__init__(unique_id, cluster_handlers, endpoint, device, **kwargs)
self._attr_extra_state_attribute_names: set[str] = {
"measurement_type",
f"{self._attribute_name}_max",
self._max_attribute_name,
}

@property
def _max_attribute_name(self) -> str:
"""Return the max attribute name."""
return self._attr_max_attribute_name or f"{self._attribute_name}_max"

@property
def state(self) -> dict[str, Any]:
"""Return the state for this sensor."""
response = super().state
if self._cluster_handler.measurement_type is not None:
response["measurement_type"] = self._cluster_handler.measurement_type

max_attr_name = f"{self._attribute_name}_max"
max_attr_name = self._max_attribute_name
if not hasattr(self._cluster_handler.cluster.AttributeDefs, max_attr_name):
return response

Expand Down Expand Up @@ -705,6 +718,28 @@ class ElectricalMeasurementRMSCurrent(PolledElectricalMeasurement):
_div_mul_prefix = "ac_current"


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

_attribute_name = "rms_current_ph_b"
_unique_id_suffix = "rms_current_ph_b"
_attr_translation_key: str = "rms_current_ph_b"
_skip_creation_if_no_attr_cache = True
_attr_max_attribute_name: str = "rms_current_max_ph_b"


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

_attribute_name: str = "rms_current_ph_c"
_unique_id_suffix: str = "rms_current_ph_c"
_attr_translation_key: str = "rms_current_ph_c"
_skip_creation_if_no_attr_cache = True
_attr_max_attribute_name: str = "rms_current_max_ph_c"


@MULTI_MATCH(cluster_handler_names=CLUSTER_HANDLER_ELECTRICAL_MEASUREMENT)
class ElectricalMeasurementRMSVoltage(PolledElectricalMeasurement):
"""RMS Voltage measurement."""
Expand Down Expand Up @@ -1115,30 +1150,15 @@ class SmartEnergySummationReceived(PolledSmartEnergySummation):
_attribute_name = "current_summ_received"
_unique_id_suffix = "summation_received"
_attr_translation_key: str = "summation_received"

@classmethod
def create_platform_entity(
cls: type[Self],
unique_id: str,
cluster_handlers: list[ClusterHandler],
endpoint: Endpoint,
device: Device,
**kwargs: Any,
) -> Self | None:
"""Entity Factory.

This attribute only started to be initialized in HA 2024.2.0,
so the entity would be created on the first HA start after the
upgrade for existing devices, as the initialization to see if
an attribute is unsupported happens later in the background.
To avoid creating unnecessary entities for existing devices,
wait until the attribute was properly initialized once for now.
"""
if cluster_handlers[0].cluster.get(cls._attribute_name) is None:
return None
return super().create_platform_entity(
unique_id, cluster_handlers, endpoint, device, **kwargs
)
"""
This attribute only started to be initialized in HA 2024.2.0,
so the entity would be created on the first HA start after the
upgrade for existing devices, as the initialization to see if
an attribute is unsupported happens later in the background.
To avoid creating unnecessary entities for existing devices,
wait until the attribute was properly initialized once for now.
"""
_skip_creation_if_no_attr_cache = True


@MULTI_MATCH(cluster_handler_names=CLUSTER_HANDLER_PRESSURE)
Expand Down
16 changes: 16 additions & 0 deletions zha/zigbee/cluster_handlers/homeautomation.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,10 +77,26 @@ class MeasurementType(enum.IntFlag):
attr=ElectricalMeasurement.AttributeDefs.rms_current.name,
config=REPORT_CONFIG_OP,
),
AttrReportConfig(
attr=ElectricalMeasurement.AttributeDefs.rms_current_ph_b.name,
config=REPORT_CONFIG_OP,
),
AttrReportConfig(
attr=ElectricalMeasurement.AttributeDefs.rms_current_ph_c.name,
config=REPORT_CONFIG_OP,
),
AttrReportConfig(
attr=ElectricalMeasurement.AttributeDefs.rms_current_max.name,
config=REPORT_CONFIG_DEFAULT,
),
AttrReportConfig(
attr=ElectricalMeasurement.AttributeDefs.rms_current_max_ph_b.name,
config=REPORT_CONFIG_DEFAULT,
),
AttrReportConfig(
attr=ElectricalMeasurement.AttributeDefs.rms_current_max_ph_c.name,
config=REPORT_CONFIG_DEFAULT,
),
Comment on lines +80 to +99
Copy link
Contributor

Choose a reason for hiding this comment

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

Similar to the comment here, this will also cause four additional attributes to be read at every startup per "smart plug", even if the device doesn't support the attributes.

I guess we can't really avoid this for now and maybe it doesn't really matter overall, but it might be worth looking into only polling attributes if they're not marked as unsupported on startup in the future? Since a couple of versions, the startup attribute polling can at least be disabled completely from the ZHA config UI.
Or expand "quirk id / exposes features" to also cover "rare" devices (even if they conform to ZCL spec) for phase b and c devices?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since there 3 phase devices are not super common, maybe the exposes features quirk would be fine for this.
We could also make it so that for devices that do not have the exposes features flag we only setup attr reporting after we get at least one value (similar to what we have with the _skip_creation_if_none). But I am not sure if a device will send anything before attr reporting, so that may just not work at all.

AttrReportConfig(
attr=ElectricalMeasurement.AttributeDefs.rms_voltage.name,
config=REPORT_CONFIG_OP,
Expand Down
Loading