-
Notifications
You must be signed in to change notification settings - Fork 524
Hue: fix issue with small color temp increments #2771
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Hue: fix issue with small color temp increments #2771
Conversation
Incrementing or decrementing the color temp by 1 can lead to the driver sending the Hue bridge the same value in mireds as the device is already set to, and the bridge not responding, which in turn leads to a spinning wheel in the app and eventually the value being reset to its previous value. This change emits the capability with the updated value without waiting for a response from the bridge.
|
Invitation URL: |
Test Results 72 files 485 suites 0s ⏱️ Results for commit a493b40. |
|
Minimum allowed coverage is Generated by 🐒 cobertura-action against a493b40 |
| if current_mirek == mirek then | ||
| log.debug(string.format("Color temp change from %dK to %dK results in same mirek value (%d), emitting event directly", current_color_temp, clamped_kelvin, mirek)) | ||
| device:emit_event(capabilities.colorTemperature.colorTemperature(clamped_kelvin)) | ||
| return |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should still send the command to the bridge and not return early here. It might be possible that the user changed the temp in the hue app and we never emitted an event for the change so we might actually be changing it here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah good point!
| for _, device in ipairs(group.devices) do | ||
| device:emit_event(capabilities.colorTemperature.colorTemperature(clamped_kelvin)) | ||
| end | ||
| return |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Similar reasoning as above for not returning early here.
| end | ||
| end | ||
|
|
||
| if all_same_mirek and #group.devices > 0 then |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rather then checking if they are all the same mirek, I wonder if we want to track all the devices that have the same mirek and then emit the event for all of those devices. I'm not sure how this interaction works with the grouped lighting but it seems like if only one device had the same mirek then the bridge might not send anything for that specific device.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah good call. When I was testing it I had all my bulbs at the same color temp value and neglected to think of the case where they are at different values initially.
| ---@param bridge_device HueBridgeDevice | ||
| ---@param group table | ||
| ---@param args table | ||
| ---@param aux table auxilary data needed for the command that the devices all had in common |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
at least I am a consistent bad speller 🥲
Type of Change
Checklist
Description of Change
Incrementing or decrementing the color temp by a small amount can lead to the driver sending the Hue bridge the same value in mireds as the device is already set to, and the bridge not responding, which in turn leads to a spinning wheel in the app and eventually the value being reset to its previous value. This change emits the capability with the updated value without waiting for a response from the bridge if the new mireds value would be the same as the previous.
Summary of Completed Tests