[RFC/DRAFT/WIP] ZBT-2 coordinator LED support#715
Conversation
zigpy-review-bot
left a comment
There was a problem hiding this comment.
RFC engagement — leaving as COMMENT, no asks to block on while the design is still being shaped.
Direction
The overall shape (entity yielded from discover_coordinator_device_entities, modelled as a BaseLight subclass like DeviceCounterSensor is a BaseEntity, gated on FirmwareFeatures.LED_CONTROL) reads naturally next to the existing coordinator-entity path. EntityCategory.CONFIG + _attr_primary = False is the right framing — this is plumbing, not a primary device control. Tests cover the on/off/restore happy paths and the no-feature gate; the testing seam (zigpy_app_controller._ezsp = SimpleNamespace(...)) is workable but is itself a tell about the bellows-internal coupling discussed below.
Open questions worth nailing down before promoting from draft
1. Bellows-internal coupling — pick a venue. Both call sites reach across the API boundary:
discovery.pyimportsbellows.ezsp.xncp.FirmwareFeaturesand dereferencesapp_controller._ezsp._xncp_featuresdirectly.CoordinatorLED._async_set_led_statedoesgetattr(..., "_ezsp", None)+hasattr(ezsp, "xncp_set_led_state").
This works, but every other coordinator-feature check inside zha right now goes through application_controller public surface, not _ezsp. The PR body already lists the two reasonable resolutions (zigpy-as-intermediary like the country-code path, or bellows exposing a public XNCP-features API). My weak preference is the zigpy intermediary: it generalises naturally to non-EZSP coordinators (zigpy-znp, future hardware) without zha needing per-radio branches, and the RadioType.ezsp guard in discovery.py falls out for free. Either path needs the upstream change first; landing this PR without it locks in a coupling that's awkward to refactor through.
2. Restore semantics on first paint vs. firmware default. on_add calls _schedule_restore_turn_on, which only fires if _restore_on_startup was set — i.e. only after restore_external_state_attributes ran. On an install with no prior persisted state, the LED stays whatever the firmware boot pattern produced (off, by the PR body's description). That's defensible, but it's also the path every freshly-paired user is on, so it's worth being explicit: should there be a one-shot "first ever turn on after pairing" that surfaces something? The PR body's note about router firmware using a blue colour while searching for a network points at the same UX question from the other direction.
3. "Turn off" semantics. The PR body already flags this: xncp_set_led_state(0,0,0) isn't "give control back to firmware," it's "force black." If a user disables the LED entity in HA and later the firmware wants to indicate something (joining state, error), it stays dark. Worth confirming with the firmware folks whether led_manager_clear_pattern(LED_PRIORITY_MANUAL) (or equivalent) can be added to the XNCP surface so the off path can hand control back cleanly. Doesn't need to block this PR, but the answer shapes whether async_turn_off stays as-is or grows a mode argument.
4. HA-core wiring. Linked PR home-assistant/core#162307 is about coordinator-device grouping, separate concern. The actual HA-side wiring needed for this entity — coordinator device showing a light entity in the right place, not under "Zigbee group" — is the missing piece. The current behaviour will work (entity discovery yields a BaseLight, HA renders it), but visual placement on the device page is the bit the PR body flags as confusing. Cross-link the HA-core PR when you have one so this PR doesn't merge in isolation.
Smaller things worth a glance
_xy_brightness_to_rgbreimplements the XY→sRGB transform from scratch.homeassistant.util.color.color_xy_brightness_to_RGBexists and matches what most HA-side colour pickers feed in — would also avoid the manual gamma table here. Not a strong preference if you want zha to stay decoupled fromhomeassistant.util, but worth a moment.async_turn_on(xy_color: tuple[int, int] | None = None)keeps the type-hint inherited fromBaseLight.async_turn_on, but the PR's own conversion (float(xy_color[0])) and tests ((0.64, 0.33)) treat it astuple[float, float]. The base-class hint is wrong, but worth being consistent: either fix the override here, or leave a TODO pointing at the base class._get_rgb_colorrewritesself._xy_color/self._brightnessas a side-effect of computing the RGB tuple. Splitting state-update from value-derivation makes the restore path easier to reason about, but it's idiomatic-enough as a private helper if you'd rather leave it.- CI is failing because the bellows PR isn't released yet (
AttributeError: type object 'FirmwareFeatures' has no attribute 'LED_CONTROL') — expected for the cross-repo dependency, not a real failure. Mentionrequires zigpy/bellows#710in the merge plan and gate on a bellows release.
No mypy regression against dev (72 errors in PR vs ~76 baseline — diff is noise from a few unrelated touched-file shifts).
DRAFT. Just playing around for now.
Proposed change
This adds experimental support for directly controlling the ZBT-2 coordinator LED using firmware extensions.
The changes work (with the linked bellows PR), but this is very experimental and rough. Nothing that should be merged.
Notes:
on_add. That way, you'll only see the ZBT-2 LED turn off for a few seconds when ZHA is reloaded (or HA restarted).DEFAULT_COORDINATOR_LED_XY_COLORcolor.LED_COLOR_WHITE_DIMcolor from firmware (RGB8(75, 75, 75))led_manager_clear_pattern(LED_PRIORITY_MANUAL)is not supported via firmware extensions, so ZHA turns the light "black" to turn it off.Additional information
Requires bellows PR: