Skip to content

[RFC/DRAFT/WIP] ZBT-2 coordinator LED support#715

Draft
TheJulianJES wants to merge 5 commits into
zigpy:devfrom
TheJulianJES:tjj/zbt2_led
Draft

[RFC/DRAFT/WIP] ZBT-2 coordinator LED support#715
TheJulianJES wants to merge 5 commits into
zigpy:devfrom
TheJulianJES:tjj/zbt2_led

Conversation

@TheJulianJES
Copy link
Copy Markdown
Contributor

@TheJulianJES TheJulianJES commented Mar 21, 2026

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:

  • This PR directly accesses bellows internals for the firmware extensions(!)
    • We could either making this more generic using zigpy as an intermediate layer, similar to how "network country code" firmware extensions are handled?
    • Or, we could have bellows expose a "more public API" for directly using firmware extensions, like this.
  • When we reset ZBT-2 firmware whilst connecting, the LED color is turned off, as the ZBT-2 does not persist LED state.
    • So, ZHA now creates a task to restore the last LED state in 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).
    • It might be worth looking into making this persistent within ZBT-2 firmware, though also consider allowing for ways to turn it off, e.g. when switching to Z2M – at least holding down the reset button should also clear the LED state.
  • When the light is turned on for the first time by the user, we send a default DEFAULT_COORDINATOR_LED_XY_COLOR color.
    • Currently, this should match the LED_COLOR_WHITE_DIM color from firmware (RGB8(75, 75, 75))
    • This shows up as "white" with 29% brightness in HA when first turning on the light.
    • The router firmware uses another blue color when searching for a network. Should we also use that here?
  • There is currently no command for properly turning the light off and handing control back to the firmware.
    • led_manager_clear_pattern(LED_PRIORITY_MANUAL) is not supported via firmware extensions, so ZHA turns the light "black" to turn it off.
    • This might be something to expose in firmware and bellows, though I don't think it realistically matters with current coordinator firmware behavior. E.g. if we wipe network settings, we reset firmware anyway, so it should start blinking again(?)
  • It is somewhat confusing that the coordinator device page in HA also shows ZHA light groups.

Additional information

Requires bellows PR:

Copy link
Copy Markdown
Collaborator

@zigpy-review-bot zigpy-review-bot left a comment

Choose a reason for hiding this comment

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

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.py imports bellows.ezsp.xncp.FirmwareFeatures and dereferences app_controller._ezsp._xncp_features directly.
  • CoordinatorLED._async_set_led_state does getattr(..., "_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_rgb reimplements the XY→sRGB transform from scratch. homeassistant.util.color.color_xy_brightness_to_RGB exists 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 from homeassistant.util, but worth a moment.
  • async_turn_on(xy_color: tuple[int, int] | None = None) keeps the type-hint inherited from BaseLight.async_turn_on, but the PR's own conversion (float(xy_color[0])) and tests ((0.64, 0.33)) treat it as tuple[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_color rewrites self._xy_color/self._brightness as 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. Mention requires zigpy/bellows#710 in 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).

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants