Hue: fix issue with small color temp increments#2771
Hue: fix issue with small color temp increments#2771nickolas-deboom wants to merge 2 commits intomainfrom
Conversation
|
Invitation URL: |
Test Results 72 files 502 suites 0s ⏱️ Results for commit 4506632. ♻️ This comment has been updated with latest results. |
|
Minimum allowed coverage is Generated by 🐒 cobertura-action against 4506632 |
nickolas-deboom
changed the title
| 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.
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.
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.
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.
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.
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.
at least I am a consistent bad speller 🥲
|
we conducted the test in our Philips Hue test environment (Philips Hue + color temperature light), but the issue still exist. Additionally, every time I changed the Kelvin value from the ST app, I printed the related log:
|
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.
nickolas-deboom
force-pushed
the
fix/hue-small-color-temp-increments-leads-to-command-timeout
branch
from
38e8c2f to
4506632
Compare
|
Hi @pInksenberg , I retested this today, and found that the channel link in this PR has not been updated for a few months. I'm not really sure what's going on there, but I needed to manually package the driver and install it on my hub. Would you be able to retest after manually installing the driver? Here are some logs I gathered that show that this change is working as expected: |
3 participants
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