Skip to content

Compute entity capabilities centrally, and keep existing entities' capabilities current on re-initialization #940

Open

Description

@zigpy-review-bot

Follow-up to #917, including the question raised there of whether entities should call recompute_capabilities() in __init__. Two related points: a refactor prototyped on a branch, and a low-impact bug whose straightforward fix collides with how Light uses recompute_capabilities(). Neither is urgent: #917 fixed the only class that crashed, and zigpy/zigpy#1902 contains the damage of a similar bug.

1. Compute capabilities centrally right after construction

Today: entities subscribe to cluster events in on_add() and can be asked for their state from then on, but the only central recompute_capabilities() call during initialization is in Device._add_pending_entities(), after the initialization reads. So every entity class has to make itself state-ready on its own. Ten classes call self.recompute_capabilities() in __init__ (BinarySensor, WriteAttributeButton, Thermostat, Cover, Shade, Fan, Light, LightGroup, Sensor, and AnalogOutputNumber since #917); the others rely on class-level defaults. Before #917, AnalogOutputNumber had neither, which is the bug #917 fixed.

Branch: zigpy-bot/central-entity-capabilities computes capabilities once in Device._discover_new_entities(), right after each entity is constructed, and once in discover_group_entities() for group entities. It removes nine of the ten __init__ calls. A failure in the new call is logged and skips only that entity.

Two constraints came up while doing this, and any central call has to respect both:

  • Quirk predicates see construction-time capabilities. The function= predicates of prevent_default_entity_creation() and change_entity_metadata() run before on_add() and can match on computed attributes. The Develco smart siren and intelligent keypad quirks match entity.translation_key == "ias_zone", which IASZone.recompute_capabilities() sets. A first version that recomputed just before on_add(), after the quirk hooks, let those IASZone entities through on four Frient snapshot devices. The branch therefore recomputes before the quirk hooks, where the __init__ calls ran.
  • Cover.__init__ uses its own capabilities. Its __init__ continues after the call and ends with _determine_cover_state(refresh=True), which needs the supported features. Without the call, the Aqara curtain snapshots lost their open/closed state. The branch keeps that one call, with a comment, rather than also restructuring Cover.__init__.

With that, the full suite passes and all 925 device snapshots are unchanged. With the analog output number code from before #917 (no __init__ call, no class-level defaults), this branch alone gives the right range, step and name on an empty-cache join, while dev reproduces the #917 bug. Only the unit still needs #917's other change, from cached_property to a plain property. So the guarantee no longer depends on every entity class remembering it.

The alternative is to keep the per-class calls as they are, with the guarantee staying implicit.

2. Existing entities keep stale capabilities after re-initialization

Bug (low impact): Device.async_initialize() reads the entities' configuration attributes and then calls _add_pending_entities(), which recomputes only newly discovered entities. Entities that already exist keep capabilities computed from the older cache. This happens when a live device is initialized again, e.g. by the startup poll of mains-powered devices, which runs after the from-cache initialization. If one of those reads fills in an attribute that was missing before (the device didn't answer it, or a newer ZHA release reads a new attribute), the entity keeps the stale value until the next Home Assistant restart creates fresh entities. Example: an analog input sensor whose cache lacked resolution keeps suggested_display_precision at None after the startup poll caches resolution = 0.1. #917 already covers the analog output number on its own, by recomputing on its configuration attribute events, and Cover does the same for window_covering_type.

This only affects attributes that change an existing entity. AnalogInputSensor, for example, is only created once description and its units are known, so when the poll reads those, the next discovery creates the entity with complete data.

Why there is no branch for this: the straightforward fix is to recompute the existing entities in async_initialize() right after initialize_cluster_configs(), guarded per entity; the end of _add_pending_entities() already emits the resulting changes. That fixes the example above, but Light.recompute_capabilities() also sets live state from the cache (_brightness, _color_temp, _xy_color, _effect, _color_mode), not only capabilities. During a transition, Light keeps the target brightness and buffers intermediate current_level updates (_transition_brightness_buffer, applied in async_transition_complete()), and a recompute bypasses that buffer. In a test with a light transitioning to 200 while the poll reads an intermediate level of 50, dev keeps 200 and the prototype drops it to 50. Among device entities, it is the only recompute_capabilities() that sets live state. (recompute_entities() has the same property, but only tests call it.)

Options:

  • Move Light's state seeding out of recompute_capabilities(), then recompute existing entities generically as above. The color part overlaps with Lights don't get their color state updated on reports #855, since color state is not updated from reports today.
  • Keep per-class refreshes on attribute events, like Fix analog output number losing its range, unit and name on the first read #917 and Cover, for the attributes where staleness matters.
  • Recompute existing entities generically but skip lights that are transitioning. That covers the case above, but not the state Light sets right after a successful command (e.g. _brightness in async_turn_on()), before the device reports it and the cache catches up, so a recompute in that window would still revert it.

Related

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    code qualityImprovement to code qualityentitiesIssue or PR about (custom) entities

    Type

    Fields

    Priority

    None yet

    Size

    None yet

    Area

    None yet

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions