Skip to content

Comments

Buffer brightness attribute reports during light transitions#671

Merged
TheJulianJES merged 6 commits intozigpy:devfrom
TheJulianJES:tjj/light_buffer_last_brightness_report
Feb 25, 2026
Merged

Buffer brightness attribute reports during light transitions#671
TheJulianJES merged 6 commits intozigpy:devfrom
TheJulianJES:tjj/light_buffer_last_brightness_report

Conversation

@TheJulianJES
Copy link
Contributor

@TheJulianJES TheJulianJES commented Feb 22, 2026

Proposed change

Instead of ignoring brightness attribute reports during a light transition, we buffer them and apply the last received value when the transition completes.

Additional information

Currently, we ignore all brightness attribute reports during a light transition. If a user were to use a remote directly bound to the light during a transition, we'd currently have an incorrect state at the end.

Also, there are some lights (or dimmers to be exact) which acknowledge an on command during an already ongoing transition to off-state, but then immediately send an attribute report from the OnOff cluster that they're off. We expect them to be on, as that's what the user last sent.

This seems to be some kind of firmware bug unique to these dimmers, but we should still handle this better in ZHA.
But it seems like the device does support move_to_level_with_on_off commands during an off-transition to turn back on, just not pure on commands.....

Side-note/future issue

There's likely also an issue where a HA restart mode automation cancels the ongoing turn_on task during the Zigbee on call, so we end up with a flag that is never reset, as the timer has not started.
Maybe we can just catch CancelledError and reset the flag (+ initiate a poll and/or emit updated state, as the light can be in some kind of in-between state)? Or actually, we can just use finally: to complete the timer?

AI summary

Problem

  • When turning on a Zigbee light, many bulbs apply a default 1-second transition. During this time the bulb sends attribute reports with a slowly increasing brightness level. Previously, these intermediate reports were completely ignored while is_transitioning was True, and we showed the optimistically-set target brightness, until another attribute report came in AFTER the transition.
    • This worked well for well-behaved lights, but if a light refuses to turn on while still returning Status.SUCCESS for the turn-on command, we'd always show the (never-reached) target brightness, not updating it to the actual brightness afterwards, even if it sent a "correct" on_off=false attribute report during the supposed transition.
    • A related issue occurs when turning a light on while it is mid-way through an off transition: the device may refuse the on command (returning SUCCESS anyway) and report on_off=false to correctly reflect that it is still off. Previously this report was silently dropped, causing HA to show the light as on indefinitely.
    • If a user used a remote directly bound to a light during a ZHA-initiated transition, we'd also end up with wrong state in HA.
  • A secondary issue was that DEFAULT_EXTRA_TRANSITION_DELAY_SHORT (previously 0.25 s) was short enough that a late-arriving attribute report could slip through after the flag cleared and be applied directly to state, briefly showing a stale intermediate brightness.
    • Note: This wasn't a real issue until now, as we just ignored all attribute reports during a transition, so if the final one arrived after (rare, but happened), it would be the same as the target brightness we had already set. Now, the brightness would assume the last

Solution

Instead of discarding attribute reports during a transition, the last received value is now buffered in _transition_brightness_buffer. When async_transition_complete fires at the end of the transition window, the buffer is applied before emitting state:

  • A non-zero buffered brightness is used as the final _brightness (reflecting what the device actually reached).
  • A buffered value of 0 — written when an on_off=false report arrives — overrides _state to False, correctly reflecting a device that refused to turn on.
  • on_off=true reports during a transition are still skipped (the optimistic state is already correct).

Reports during transition are still not shown in Home Assistant (no intermediate state flicker). The buffer is cleared when a new transition starts so stale values can never bleed through.

DEFAULT_EXTRA_TRANSITION_DELAY_SHORT is increased from 0.25 s to 0.5 s to ensure any report that arrives slightly after the physical transition ends is still captured in the buffer rather than applied to state directly.

Changes

  • zha/application/platforms/light/const.pyDEFAULT_EXTRA_TRANSITION_DELAY_SHORT: 0.25 → 0.5
  • zha/application/platforms/light/__init__.py
    • Add _transition_brightness_buffer field to BaseClusterHandlerLight
    • Clear buffer at the start of each transition (async_transition_set_flag)
    • Buffer the last brightness report in handle_cluster_handler_set_level instead of silently ignoring it
    • Buffer on_off=false as brightness 0 in handle_cluster_handler_attribute_updated
    • Apply the buffer in async_transition_complete: non-zero → update _brightness; zero → set _state = False
  • tests/test_light.py
    • test_transition_brightness_buffering: verifies intermediate reports are buffered and the last value is applied on completion; also verifies that when no reports arrive the optimistic target is preserved
    • test_turn_on_during_off_transition: verifies that a device which refuses to turn on during an off-transition correctly ends up shown as off in HA once the transition window closes

@codecov
Copy link

codecov bot commented Feb 22, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 97.48%. Comparing base (aa03919) to head (22bb3ec).
⚠️ Report is 7 commits behind head on dev.

Additional details and impacted files
@@            Coverage Diff             @@
##              dev     #671      +/-   ##
==========================================
+ Coverage   97.44%   97.48%   +0.03%     
==========================================
  Files          62       62              
  Lines       10731    10742      +11     
==========================================
+ Hits        10457    10472      +15     
+ Misses        274      270       -4     

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@TheJulianJES
Copy link
Contributor Author

I think this is also fine to merge. Depending on #672, it or this may need to rebased after merging.

(The light class could also be cleaned up in general. If we could just get rid of the non-success status guards mentioned in the other PR (that we never hit?), that would already be good. Test coverage could also be improved somewhat.)

@TheJulianJES TheJulianJES marked this pull request as ready for review February 25, 2026 00:33
Copy link
Contributor

@puddly puddly left a comment

Choose a reason for hiding this comment

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

This looks good to me. Just for clarity, the increase of the transition timer delay by 250ms won't result in the entity state change emitting "slower", right?

@TheJulianJES
Copy link
Contributor Author

Yes.

There's no extra delay to state at all. It will still be updated directly after an on/off command is sent and received by the light. No change there. It just lengthens the timer a bit until "unexpected" attribute reports immediately impact state again.

(This is because we sometimes had the "final attribute report" after a transition be received outside of the timer range. If that were to happen now, we'd restore the previous one, where the brightness doesn't yet match the target brightness (we already set state to for the successful command), so we would run the risk of briefly bouncing the light to (e.g.) 90% at the end of the transition, before the final report arrives and bumps it back to 100%.)

@TheJulianJES TheJulianJES merged commit fb79a96 into zigpy:dev Feb 25, 2026
9 checks passed
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