More accurately convert colortemp to xy and hs #146
Conversation
| @@ -299,7 +300,7 @@ def _calc_rgb(self): | |||
| return color_temperature_to_rgb(self._color_temperature()) | |||
|
|
|||
| def _calc_xy(self): | |||
| return color_RGB_to_xy(*self._calc_rgb()) | |||
| return list(colour.temperature.CCT_to_xy_CIE_D(self._color_temperature())) | |||
There was a problem hiding this comment.
This is something that's bothered me too but adding additional package requirements has always caused problems in the past for users that have non "standard" installations. If the colour-science package fails to install then the entire CL integration will break. I think a better solution would be to wrap this in a try and if an exception occurs to use the current built-in method. I can make that change when I get a chance or feel free to handle it if you're so inclined.
There was a problem hiding this comment.
Hey, thanks for the feedback. Yep, agree with your point, also because currently the packaged version of colour-science has a problem when running on raspberry-pi (this was fixed ~8 months ago on their repo, but it is not yet on the release version). I'll add this check by the end of the week :)
There was a problem hiding this comment.
If the only method used is color.temperature.CCT_to_xy_CIE_D() another option would be to just extract the code for this method (https://github.com/colour-science/colour/blob/a412b670448534987fc25d3a1a09cedc2a3ea5f2/colour/temperature/cie_d.py#L127-L183) from the package and omit the rest of it.
This would completely ditch the dependency while still providing the same functionality.
| @@ -352,7 +353,8 @@ async def _adjust_lights(self, lights, transition): | |||
| if not is_on(self.hass, light): | |||
| continue | |||
|
|
|||
| service_data = {ATTR_ENTITY_ID: light, ATTR_TRANSITION: transition} | |||
| service_data = {ATTR_ENTITY_ID: light} | |||
| service_data[ATTR_TRANSITION] = transition | |||
There was a problem hiding this comment.
Is there an advantage to breaking this into two lines? Have you run into issues where service_data was not set because of an issue with light or transition?
There was a problem hiding this comment.
Ah well spotted! I actually didn't intend to commit this, but I forgot to remove it after I tested it on my system. The problem is that IKEA light bulbs don't handle transition and color changes commands issued at the same time very well. Particularly, they don't change the color when this happens, so I just commented out the transition part on my system.
The current color conversion from color temperature to rgb distorts colors. For instance during the day whiter colors get shifted slightly towards magenta, while in the evening warmer color temperatures shift more towards red. This is a problem when using led bulbs that don't directly support color temperature but can for instance be set in xy or hs. This PR uses a more accurate conversion for color temperature by using the functions provided by the colour-science package.
This photo shows the effect of this PR. The light bulbs in the three rooms in the picture (left, middle, and right) have all been set with the hass-circardian_lighting component.
The room on the right has a light that directly accepts color temperatures as a setting. The color of the middle room comes from setting a RGB light with the current version of this repo (without this PR). On the room on the left we have again a RGB light, but set in xy with this PR applied. As it can be seen the color generated with this PR (room on the left) more accurately resembles the light generated by the native color temperature light bulb (room on the right).