Skip to content

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

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

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

Conversation

@TheJulianJES

@TheJulianJES TheJulianJES commented Mar 21, 2026 •

Copy link
Copy Markdown
Contributor

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.

Changes

  • New CoordinatorLED light entity (BaseLight + PlatformEntity, XY color mode, EntityCategory.CONFIG, not primary), yielded by CoordinatorDevice.discover_entities when the EZSP coordinator reports FirmwareFeatures.LED_CONTROL.
  • The entity converts XY + brightness to RGB and sends it via xncp_set_led_state; turning it off sends black.
  • The last LED state is restored through the state-restore path and replayed to the adapter once the entity is added.
  • Tests in tests/test_coordinator_light.py cover discovery gating (no EZSP adapter, and an adapter without the LED feature bit), on/off, the default dim-white first turn-on, brightness-only updates keeping the XY color, a failed adapter command leaving the stored state untouched, and restore replay.
  • The LED feature bit is looked up with getattr, so bellows releases without FirmwareFeatures.LED_CONTROL just don't get the entity (coordinator discovery keeps working).
  • The branch was merged with dev after the device/entity rework, so discovery now lives in CoordinatorDevice and the entity is a PlatformEntity bound to the coordinator device.

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.

Testing

  • tests/test_coordinator_light.py passes (7 tests) and the full suite passes (1788 tests), run against the branch of the bellows PR below.
  • That bellows branch is based on bellows 0.49.0 and conflicts with bellows dev, so it needs a rebase onto current bellows before this can be re-tested against a real release.
  • Without the bellows change (e.g. with the pinned bellows 1.2.1), FirmwareFeatures.LED_CONTROL does not exist: the entity is simply not created, and five of the seven tests (the ones using the ZBT-2 fixture with the LED feature bit) fail.

Open items

  • The bellows pin has to be bumped to a release containing the LED support before this can merge.

Additional information

Requires bellows PR (not merged or released yet; bellows 1.2.1 does not have LED_CONTROL):

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

Copy link
Copy Markdown
Collaborator

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).

@TheJulianJES

Copy link
Copy Markdown
Contributor Author

@zigpy-review-bot Update this (code + PR body/title, ...), do review rounds, possibly fix stuff. Anything missing then?

…ucture

- CoordinatorLED is now a BaseLight + PlatformEntity (like Light) bound to the coordinator device, so it inherits device_ieee/available/state handling
- Discovery moved from discovery.py into CoordinatorDevice.discover_entities
- Tests use the dataclass entity state
…uccessful send

- Look up FirmwareFeatures.LED_CONTROL with getattr, so released bellows without it no longer raises in coordinator discovery (which also dropped the counter sensors)
- Resolve brightness/XY color without mutating state; store them only after xncp_set_led_state succeeds
- Add tests for a failed send and for an adapter without the LED feature bit
@zigpy-review-bot zigpy-review-bot changed the title [RFC/DRAFT/WIP] ZBT-2 coordinator LED support Add ZBT-2 coordinator LED support [RFC/DRAFT/WIP] Nov 12, 2026
@zigpy-review-bot

Copy link
Copy Markdown
Collaborator

Pushed c775574 to the branch (fast-forward) and updated the PR title and body.

What changed

  • Merged dev (the PR was conflicting) and ported the entity to the new structure: CoordinatorLED is now a BaseLight + PlatformEntity bound to the coordinator device, and discovery moved into CoordinatorDevice.discover_entities. State and availability now come from the base classes, and the xy_color hint is tuple[float, float].
  • Review rounds (Opus sub-agent + Copilot, two rounds) found two real issues, both fixed:
    • With the pinned bellows 1.2.1, FirmwareFeatures.LED_CONTROL does not exist, so discovery raised for every EZSP coordinator and also dropped the counter sensors. It is now looked up with getattr, so older bellows just gets no LED entity.
    • A failed xncp_set_led_state left the stored brightness and color changed. They are now committed only after a successful send.
  • Added tests for both (7 in total). The full suite passes (1788) against the bellows#710 branch, and ruff and mypy are clean.

Still missing

  • Add XNCP coordinator LED support bellows#710 is unreleased, and its branch is based on bellows 0.49.0 and conflicts with bellows dev, so it needs a rebase before this can be tested against a real release. The bellows pin here must be bumped once it ships.
  • The design questions from the earlier review are still open: the bellows-internal access (_ezsp._xncp_features), "off" meaning black rather than handing control back to the firmware, and the HA-side placement of the entity.
  • The default XY color (0.3127, 0.3290) at brightness 75 converts to RGB (72, 75, 75), not the firmware's (75, 75, 75). I left it unchanged.
  • Restore always comes up off and then replays "on", so HA history shows a brief off→on at every start. If the LED was off at shutdown, HA stores no color or brightness, so the next turn-on uses the dim-white default. The on_add replay hook is effectively dead, because restore runs after on_add; the replay is triggered from the restore call.
  • The PR is still a draft, and I did not touch the Notes section of the body.

@TheJulianJES

This comment was marked as resolved.

@zigpy-review-bot

This comment was marked as resolved.

@TheJulianJES

This comment was marked as resolved.

@zigpy-review-bot

This comment was marked as resolved.

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