Hue specific updates with attribute reporting#146
Open
duvholt wants to merge 2 commits intochrivers:masterfrom
Open
Hue specific updates with attribute reporting#146duvholt wants to merge 2 commits intochrivers:masterfrom
duvholt wants to merge 2 commits intochrivers:masterfrom
Conversation
Previously effects didn't send brightness because it wasn't handled and instead ended up sending a separate request for setting brightness immediately. So to avoid similar bugs let's go back to either always sending full Hue specific messages when possible.
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.
The reason I started looking into this is because I found a bug with timed effects:
The following request should result in a sunrise effect which peaks at 10% brightness
{ "dimming": { "brightness": 10 }, "timed_effects": { "effect": "sunrise", "duration": 10000 } }But since we send both standard Zigbee updates and Hue updates (when necessary) this results in the following:
Notice the lack of brightness in the effect.
In theory I think this could lead to a race condition where the light goes to 25% brightness after 0.4s, but it seems like the effect is always ran last.
A quick fix for this would be to add brightness to the Hue specific update under some conditions, but I wanted to see if we could get rid of the workaround (and potentially other bugs like this) by using attribute reporting.
After this PR the previous request results in this:
From my testing this seems to work consistently, but it does lead to double the amount of Zigbee messages when doing light updates. I don't personally notice any issues, but I guess a huge Zigbee network could have problems handling all the read requests. I think this could be handled by doing debouncing or something similar to throttle the flood of read attributes when for example selecting color.