Add support for rms_current_ph_b/c

[abmantis]

Add support for 3 phase current attributes from the ElectricalMeasurement cluster.
To avoid adding the 2 extra entities to all devices with an ElectricalMeasurement cluster, this only adds them if the attributes have a value. The downside is that this only adds them after a restart/reload. With #311 this could be improved to match on a 3phase feature instead.

Related PR on HA Core: home-assistant/core#132871

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 96.51%. Comparing base (91cc37b) to head (e3ea5ca).

Additional details and impacted files

@@           Coverage Diff           @@
##              dev     #324   +/-   ##
=======================================
  Coverage   96.50%   96.51%           
=======================================
  Files          61       61           
  Lines        9439     9459   +20     
=======================================
+ Hits         9109     9129   +20     
  Misses        330      330           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[puddly]

Does this one also need _skip_creation_if_none = True?

Just so I understand correctly:

1. For existing devices with these attributes you will have to click the "re-configure" button (and probably reload ZHA) to get them to show up.
2. If you don't do that, there won't be any disabled entities.
3. For new joins, they just work properly?

[abmantis]

Does this one also need _skip_creation_if_none = True?

ElectricalMeasurementRMSCurrentPhC inherits from ElectricalMeasurementRMSCurrentPhB, so no need to repeat the attribute.

Just so I understand correctly:

1. For existing devices with these attributes you will have to click the "re-configure" button (and probably reload ZHA) to get them to show up.

2. If you don't do that, there won't be any disabled entities.

3. For new joins, they just work properly?

It works a bit differently: it adds the entities when the integration gets loaded if there is a value for the attribute already. So, this means that:

1. If the user already had a 3 phase device, after this update the new entities would just show up (because the ph_b and ph_c attributes already have values on the database).
2. For new joins the two new entities will not show until the user reboots HA or reloads ZHA. This is because when the device is added the attributes have no values.

We already used this approach for a few other entities like this .

Ideally, ZHA would add entities as soon as the attribute gets a value, but I don't think we support that ATM.
Also we could use exposes_feaures to add the entities for devices that expose a 3_phase feature, instead of waiting for a value on the attr.

I am planning to eventually work on the exposes_feaures feature, but IMO it is better to have support for the 3ph devices now, even if it is not perfect, than delaying it even more.

[puddly]

Looks good to me. @dmulcahey @TheJulianJES what do you think?

Only nitpick is the inheritance of phase C from phase B. I think it would be clearer to be explicit with _skip_creation_if_none = True.

[abmantis]

Only nitpick is the inheritance of phase C from phase B. I think it would be clearer to be explicit with _skip_creation_if_none = True.

That's what I had initially but changed it to reduce the number of duplicated attributes. I am fine either way so I'll revert back.

[TheJulianJES]

Added some small comments.

I think we can do this generally, with the only real caveat being that we will poll four additional attributes on startup for each device with an EM cluster, even if most devices never support ph_b and ph_c.
.. It should be fine though (for now)? Users will large networks can disable the "initialize attributes" on startup option, but that's also not a great solution.
We might be able to just never poll unsupported attributes on startup again. (If we want to do this, it's for a future PR and I don't consider that to be blocking this.)

[TheJulianJES]

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?

[TheJulianJES]

Is this docstring properly formatted? Can't add a susggestion here, but this might be better:

    """
    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_none = True

[TheJulianJES]

I guess the name is fine, but to make the "if None" part a bit clearer, how about _skip_creation_if_no_attr_cache or (inverted) _require_attr_cache_for_creation?
They're both a bit long though, so your call on what you like best out of the three.

[TheJulianJES]

Only minor too, but since we override the property with a static string twice, we could have a _attr_max_attribute_name attribute that's None by default in the base Sensor class.

Then have the property in Sensor look like this:

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

That would allow us to only define _attr_max_attribute_name: str = "rms_current_max_ph_b" (or ph_c) in the other classes and we don't have to override the whole property every time and have the attribute defined in a way similar to others. We only do this twice at the moment, so eh, do this if you like it.