Skip to content

Fix on/off-only lights reverting to off after rapid turn_off/turn_on - #767

Merged
TheJulianJES merged 3 commits into
zigpy:devfrom
TheJulianJES:tjj/fix-light-transitioning-flag-on-off-only
Oct 28, 2026
Merged

TheJulianJES merged 3 commits into
zigpy:devfrom
TheJulianJES:tjj/fix-light-transitioning-flag-on-off-only

Conversation

@TheJulianJES

@TheJulianJES TheJulianJES commented May 19, 2026 •

Copy link
Copy Markdown
Contributor

Proposed change

This fixes an issue where on/off-only lights (so ones that do not support brightness) incorrectly revert their state to "off" when quickly turning the light off, and then back on again.

The underlying issue was that we only set the transitioning flag in turn_on for lights supporting brightness, but always in turn_off, which should not have been the case. Thus, the turn_on call didn't cancel/restart the turn_off timer.

To fix this, we just do not set the flag for on/off-only lights. They do not need at all, since they have no brightness/transitions, so we can always parse attribute reports.

Additional information

As a little background: The transitioning flag and timer is needed because Zigbee lights always send their current brightness, even during ongoing transitions.

However, in Home Assistant, we need to show the target brightness instead. Otherwise, you see the brightness slider going back down instantly after moving it up, then slowly moving back up (but only for lights that support attribute reporting). There's also some other issues this fixes (including for group lights that would have very jumpy/weird state otherwise, especially if some lights do not support attribute reporting).

In general, the light class needs to cover a lot(!) of edge-cases, and generally does so successfully. But there are plans to clean up the entire class a bit, e.g. with #734.

AI summary

Useful-ish AI summary (click to expand)

Fixes #766. async_turn_off unconditionally armed the transitioning flag and a ~1.5s timer whenever _zha_config_enable_light_transitioning_flag was set. For on/off-only lights this causes a rapid turn_off → turn_on sequence to revert to off after the timer fires, even though the device is on and never received another command.

Root cause

async_transition_set_flag (light/__init__.py:734) is the only path that resets _transition_brightness_buffer and cancels the pending timer. async_turn_on calls it only when its set_transition_flag predicate is true:

set_transition_flag = (
    brightness_supported or color_temp is not None or xy_color is not None
) and self._zha_config_enable_light_transitioning_flag

For an on/off-only light, brightness_supported is False and there are no color_temp / xy_color runtime params, so async_turn_on never resets the buffer or cancels the timer left over from async_turn_off. The sequence:

  1. turn_off: flag armed, 1.5s timer scheduled, optimistic _state = False
  2. Device's on_off=False report arrives → _transition_brightness_buffer = 0 (because is_transitioning is True)
  3. turn_on: set_transition_flag is False → async_transition_set_flag is not called → buffer and timer survive; the only state change is the optimistic _state = True
  4. Device's on_off=True report arrives → ignored: the buffered-attribute handler only resets the buffer when the value is falsy
  5. Original timer fires → async_transition_complete reads buffer == 0 and overrides _state = False ⇒ revert to off

Fix

Mirror async_turn_on's predicate in async_turn_off: gate the flag set, timer start, and stuck-cleanup on brightness_supported. On/off-only lights have no transition to wait for, so the flag had no useful purpose for them.

Brightness-supporting lights are unaffected: any follow-up turn_on already arms the flag (since brightness_supported is true), which resets the buffer and cancels the previous timer cleanly. That existing path is unchanged.

A narrow theoretical edge case — a color-only light with no Level cluster, where turn_on is called with no color_temp / xy_color — is also covered by gating on brightness_supported. Such hardware is rare in practice.

Regression test

Added test_on_off_only_rapid_toggle_does_not_revert: on an on/off-only light, calls turn_off then turn_on, replays both attribute reports, sleeps past the old 1.5s window, asserts state stays on. Verified the test fails on dev HEAD without the fix and passes with it.

…lights

`async_turn_off` unconditionally set the transitioning flag and started a
~1.5s timer. For on/off-only lights, a follow-up `async_turn_on` does not
call `async_transition_set_flag`, so neither the stale `_transition_brightness_buffer = 0`
(from the device's on_off=False report after the off command) nor the
pending timer get cleared. When the timer fires it overrides the optimistic
on-state back to off.

Mirror the predicate from `async_turn_on`: only set the flag and start the
timer when brightness is actually supported. On/off-only lights have no
intermediate state to buffer, so the flag has no useful purpose for them.

Fixes zigpy#766.
Copilot AI review requested due to automatic review settings May 19, 2026 11:11
@codecov

codecov Bot commented May 19, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 97.58%. Comparing base (a680944) to head (409f969).
⚠️ Report is 1 commits behind head on dev.

Additional details and impacted files
@@           Coverage Diff           @@
##              dev     #767   +/-   ##
=======================================
  Coverage   97.57%   97.58%           
=======================================
  Files          61       61           
  Lines       10862    10870    +8     
=======================================
+ Hits        10599    10607    +8     
  Misses        263      263           

☔ View full report in Codecov by Harness.
📢 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.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Fixes a ZHA light state regression where on/off-only lights could revert back to off after a rapid turn_off → turn_on sequence when enable_light_transitioning_flag is enabled, by avoiding arming the transition buffering/timer for lights that don’t support brightness transitions.

Changes:

  • Gate transition-flag arming/timer scheduling in async_turn_off behind brightness_supported (in addition to the config flag).
  • Ensure cancellation cleanup uses the same gated predicate to avoid “stuck transitioning” only when relevant.
  • Add a regression test covering rapid off→on toggling for an on/off-only light.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
zha/application/platforms/light/__init__.py Prevents transition buffering/timer behavior from being applied to on/off-only lights, eliminating the delayed revert-to-off.
tests/test_light.py Adds a regression test validating that rapid off→on toggling on on/off-only lights does not revert after the prior ~1.5s window.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread tests/test_light.py
Comment on lines +2121 to +2123
# Wait past the old 1.5s timer window. State must remain on.
await asyncio.sleep(2)
await zha_gateway.async_block_till_done()

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

We already have this elsewhere and this isn't a real 2 second wait (due to pytest-looptime). I think this is fine.

Comment thread tests/test_light.py Outdated
# Turn off, then immediately turn back on (within the default ~1.5s window).
await entity.async_turn_off()
await zha_gateway.async_block_till_done()
assert bool(entity.state["on"]) is False

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

nit: is bool(...) needed? The data type of on should be bool.

Comment thread tests/test_light.py
assert not entity.is_transitioning

# The device's on_off=False report from the off command arrives.
await send_attributes_report(

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Not sure if it's useful to have as an explicit test but this also passes (as expected) when the reports both come in after the commands succeed (i.e. if you manage to spam the on/off toggle faster than the light reports).

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

LGTM. Tight, surgical bugfix with a load-bearing regression test.

Why this is correct

The diagnosis matches the code exactly:

  • async_turn_on's set_transition_flag predicate (line 348) requires brightness_supported or color_temp is not None or xy_color is not None, so on an on/off-only light a follow-up turn_on never calls async_transition_set_flag(), which is the only path that nulls _transition_brightness_buffer and cancels the pending _transition_listener.
  • handle_cluster_handler_attribute_updated (line 1018–1024) buffers on_off=False to _transition_brightness_buffer = 0 while transitioning, and ignores on_off=True reports entirely.
  • async_transition_complete (line 794) then overrides _state = False when the buffer is 0.

Mirroring async_turn_on's predicate (sans the color params, which async_turn_off doesn't take) closes the asymmetry. The _async_cleanup_transition_if_stuck(set_transition_flag) change in the finally block is also correct — passing False for on/off-only lights means the stuck-cleanup is a no-op there, which is what we want since we never armed the flag.

Brightness-supporting lights are unaffected because their turn_on arms the flag and resets buffer + timer cleanly via async_transition_set_flag(). Color-only-with-no-Level hardware (where xy_color/color_temp is set on turn_on but brightness_supported is False on turn_off) is essentially nonexistent in practice and would still recover the next time the flag is armed.

Verification

  • Ran the new test on PR HEAD: passes in 1.6s (looptime correctly virtualizes the asyncio.sleep(2)).
  • Reverted the production change locally with the test kept — test fails on the timer-fires assertion, confirming it actually exercises the bug.
  • Full tests/test_light.py (21 tests) passes on PR HEAD.
  • mypy zha/application/platforms/light/__init__.py against the venv: 1 pre-existing error on line 438, same as dev — no regression.
  • All CI green; title and label match recent merged PRs.

Nice fix.

@jasonmx

jasonmx commented Oct 25, 2026

Copy link
Copy Markdown

@TheJulianJES this fixes #766 for on/off-only lights, but dimmable lights hit the same root cause from another direction, and I have a branch that might help get both landed.

The dimmable case. While the transitioning flag is set, an on_off = false report is buffered (as brightness 0) but an on_off = true report is discarded. If something other than the entity turns the light on within the window of a turn_off (a remote bound to the bulb, or a cluster command sent through zha.issue_zigbee_cluster_command), the entity stays off after the window ends. Hue LTA004, zha 2.3.1, HA 2026.10.2:

20:12:03.262 [zha.application.platforms] <ieee>-11: setting transitioning flag to True
20:12:03.263 [zigpy.zcl] [<nwk>:11:0x0006] Sending request: off()
20:12:03.364 [zha.application.platforms] <ieee>-11: starting transitioning timer for 1.5
20:12:03.380 [zigpy.zcl] [<nwk>:11:0x0006] Sending request: on_with_timed_off(on_off_control=<OnOffControl: 0>, on_time=60, off_wait_time=0)
20:12:03.498 [zigpy.zcl] [<nwk>:11:0x0006] Decoded ZCL frame: OnOff:Report_Attributes(attribute_reports=[Attribute(attrid=0x0000, value=TypeValue(type=Bool, value=<Bool.true: 1>))])
20:12:03.499 [zha.application.platforms] <ieee>-11: received onoff Bool.true while transitioning - buffering
20:12:04.486 [zigpy.zcl] [<nwk>:11:0x0008] Decoded ZCL frame: LevelControl:Report_Attributes(attribute_reports=[Attribute(attrid=0x0000, value=TypeValue(type=uint8_t, value=128))])
20:12:04.487 [zha.application.platforms] <ieee>-11: received level change event AttributeReportedEvent(...) while transitioning - buffering

When the timer fires, async_transition_complete applies brightness 128 but leaves _state false, since only a buffered 0 changes the state.

A branch, if it's useful: jasonmx:light-transition-on-off-buffer, two commits on current dev:

  1. This PR's commit, rebased (84 commits behind; no conflicts). Your authorship is kept; the only change is the test reading entity.state.on, since state became a LightState in Merge info_object into state and emit state change diffs #804.
  2. Buffering the on_off value itself through the transition and applying it in async_transition_complete (last report wins), with the brightness buffer holding brightness only. A parametrized test covers the dimmable case above and on-then-off within the window. test_light.py passes in full (including test_turn_on_during_off_transition), the rest of the suite shows no new failures, and ruff and mypy pass.

The bot's review of #734 raised merge order with this PR, so I didn't want to presume: you could pull the second commit into this PR, I could open the branch as a PR that supersedes this one, or leave it if #734 is the intended home for this. Whichever suits you, I'm happy to take review feedback and see it through.

For transparency: I code with Claude, with human oversight and review, and this investigation and branch were done the same way.

@TheJulianJES

Copy link
Copy Markdown
Contributor Author

@zigpy-review-bot Fix test failure

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

The fix itself looks right, but the new test fails at the current head, so CI is red. One must-address item below.

Must address

LightState is no longer subscriptable (#804), so the new test raises TypeError: 'LightState' object is not subscriptable at tests/test_light.py:2092, and the Python 3.12 and 3.14 jobs fail. Use entity.state.on for all four assertions (the four state["on"] lines in the new test). With that change tests/test_light.py passes in full (22 passed) at this head, and the new test fails when zha/ is swapped back to dev, so it does guard the regression.

Notes

  • The source change is correct. async_turn_off now gates the flag, the timer and the stuck-cleanup on the same predicate, so on/off-only lights never arm the transitioning flag, and nothing buffers the on_off=False report that the later timer would apply. Brightness-capable lights are unchanged.
  • The bool(...) wrappers in the test are unnecessary, as puddly noted on the existing thread; they go away with the .on change.
  • jasonmx's comment describes a related case on dimmable lights, where an external on inside the window is buffered and dropped. That is a separate follow-up to this PR, which is scoped to on/off-only lights, and it is compatible with this change.

Comment thread tests/test_light.py Resolved
@zigpy-review-bot

Copy link
Copy Markdown
Collaborator

Pushed 409f969: the failing test test_on_off_only_rapid_toggle_does_not_revert read entity.state["on"], but state became a LightState object in #804, so CI raised TypeError: 'LightState' object is not subscriptable. The four assertions now use entity.state.on. Test-only change, no fix logic touched.

tests/test_light.py passes locally (22 passed), and ruff and the pre-commit hooks pass.

@TheJulianJES
TheJulianJES enabled auto-merge (squash) October 28, 2026 03:59
@TheJulianJES
TheJulianJES merged commit c552c7b into zigpy:dev Oct 28, 2026
10 checks passed
@TheJulianJES

Copy link
Copy Markdown
Contributor Author

@zigpy-review-bot Open PR(s) for what's mentioned above, possibly stacked. Do review rounds. Consider adding user as co-author. If anything is left to do (not part of PR), open issue (bug/task/feature).

@zigpy-review-bot

Copy link
Copy Markdown
Collaborator

Opened #989 with the dimmable-light fix from jasonmx's branch (buffers the on_off value during a transition and applies it when the window ends), with jasonmx as co-author. It's a single PR rather than a stack, since it only builds on the already-merged #767.

Review rounds: a Fable reviewer found nothing, and Copilot raised two edge cases I verified as rare and not regressions (group members' buffers never being applied, which already held for the brightness buffer, and an on report followed by a zero-level report in the same window). Both are tracked in #988 as a task and listed as open items in the PR.

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

Labels

bugfix This PR fixes a bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

light_transitioning_flag causes spurious state-revert on rapid turn_off→turn_on for some on/off-only lights

5 participants