Conversation
| command['sat'] = int(kwargs[ATTR_HS_COLOR][1] / 100 * 255) | ||
| if self.is_osram: | ||
| command['hue'] = int(kwargs[ATTR_HS_COLOR][0] / 360 * 65535) | ||
| command['sat'] = int(kwargs[ATTR_HS_COLOR][1] / 100 * 254) |
| command['hue'] = int(kwargs[ATTR_HS_COLOR][0] / 360 * 65535) | ||
| command['sat'] = int(kwargs[ATTR_HS_COLOR][1] / 100 * 254) | ||
| else: | ||
| # Philips bulbs seem don't seem to respond correctly to hue/sat |
There was a problem hiding this comment.
🤦♂️ Thanks for the catches!
|
It seems unfair to label Philips as being incorrect. I think it is more a matter of the Home Assistant Hue platform historically treating RGB (and by extension HS) in an unusual way by not using the bulb gamut. We are currently treating HS in the normal way and this PR will then go back to the unusual way. However, I am not against this change as it maintains compatibility with previous versions of Home Assistant. |
|
That's totally fair. My main intent is to ensure there's a good explanation in the comment to save it from being removed in a refactor. What if the comment read like this:
|
|
Hue/sat does support all possible bulb colors but they are shifted from xy and we don't know by how much because it differes for each light model. Maybe a reasonable comment would be:
However, I checked our xy conversion functions against Hue docs and I found a difference: diff --git a/homeassistant/util/color.py b/homeassistant/util/color.py
--- a/homeassistant/util/color.py
+++ b/homeassistant/util/color.py
@@ -203,7 +203,7 @@ def color_RGB_to_xy_brightness(
# Wide RGB D65 conversion formula
X = R * 0.664511 + G * 0.154324 + B * 0.162028
- Y = R * 0.313881 + G * 0.668433 + B * 0.047685
+ Y = R * 0.283881 + G * 0.668433 + B * 0.047685
Z = R * 0.000088 + G * 0.072310 + B * 0.986039
# Convert XYZ to xyI think we should apply that diff to ensure that xy -> HS -> xy (as we end up with after this PR) actually does pass the original xy value to the bulb. That should then fix #13686 as well. |
|
Is this staying in dev for the next release cycle or will there be another MR for a hotfix? Not saying it's urgent as I'm just sitting back on a 0.65 version - just curious how these things fit into the updated release cycle. |
|
Updated to the beta which should include this but the color in the UI is now worse. Changing the colors on the picker now change the bulb colors but they don't change the icon in the UI. The color is now "stuck" on the green color even when the bulbs are changing. |
|
@NaffanDroo Another hotfix is unlikely. The idea of the updated release cycle is that people like you try out the beta (it's out now) and report issues so the .0 release will have less bugs. @omriasta you should report new bugs in new issues, though that one sounds like #13589 |
Description:
It seems that Philips bulbs don't display the correct colors when given a hue/sat command. This PR restores the previous behavior of sending an XY color command for non-osram bulbs.
Original behavior for reference:
https://github.com/home-assistant/home-assistant/pull/11288/files#diff-09fd3b3fbdd20cedcad9fcf474b4aeb7L275
Related issue (if applicable): fixes #13632 fixes #13614
Checklist:
tox. Your PR cannot be merged unless tests pass