Buffer brightness attribute reports during light transitions#671
Draft
TheJulianJES wants to merge 6 commits intozigpy:devfrom
Draft
Buffer brightness attribute reports during light transitions#671TheJulianJES wants to merge 6 commits intozigpy:devfrom
TheJulianJES wants to merge 6 commits intozigpy:devfrom
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. 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. 🚀 New features to boost your workflow:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
DRAFT.
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
oncommand during an already ongoing transition to off-state, but then immediately send an attribute report from theOnOffcluster 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_offcommands during an off-transition to turn back on, just not pureoncommands.....Side-note/future issue
There's likely also an issue where a HA
restartmode automation cancels the ongoingturn_ontask during the Zigbeeoncall, so we end up with a flag that is never reset, as the timer has not started.Maybe we can just catch
CancelledErrorand 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 usefinally:to complete the timer?AI summary
Problem
is_transitioningwasTrue, and we showed the optimistically-set target brightness, until another attribute report came in AFTER the transition.Status.SUCCESSfor 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=falseattribute report during the supposed transition.on_off=falseto correctly reflect that it is still off. Previously this report was silently dropped, causing HA to show the light as on indefinitely.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.Solution
Instead of discarding attribute reports during a transition, the last received value is now buffered in
_transition_brightness_buffer. Whenasync_transition_completefires at the end of the transition window, the buffer is applied before emitting state:_brightness(reflecting what the device actually reached).0— written when anon_off=falsereport arrives — overrides_statetoFalse, correctly reflecting a device that refused to turn on.on_off=truereports 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_SHORTis 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.py—DEFAULT_EXTRA_TRANSITION_DELAY_SHORT: 0.25 → 0.5zha/application/platforms/light/__init__.py_transition_brightness_bufferfield toBaseClusterHandlerLightasync_transition_set_flag)handle_cluster_handler_set_levelinstead of silently ignoring iton_off=falseas brightness0inhandle_cluster_handler_attribute_updatedasync_transition_complete: non-zero → update_brightness; zero → set_state = Falsetests/test_light.pytest_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 preservedtest_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