Add color support to lights #37
Conversation
Previously only had color temperature support
haaska.py
Outdated
| @@ -364,6 +402,15 @@ def set_percentage(self, val): | |||
| brightness = (val / 100.0) * 255.0 | |||
| self._call_service('light/turn_on', {'brightness': brightness}) | |||
|
|
|||
| def set_color(self, hue, saturation, brightness): | |||
| red, green, blue = colorsys.hsv_to_rgb(hue, saturation, brightness) | |||
There was a problem hiding this comment.
colorsys's documentation says
In the YIQ space, the Y coordinate is between 0 and 1, but the I and Q coordinates can be positive or negative. In all other spaces, the coordinates are all between 0 and 1.
but Amazon's documentation for the hue field says:
Valid range is 0.00 to 360.00, inclusive.
So I think some scaling needs to be done here, right?
There was a problem hiding this comment.
We aren't using YIQ, we are using HSV which doesn't seem to require the same scaling...
haaska.py
Outdated
| @@ -364,6 +402,15 @@ def set_percentage(self, val): | |||
| brightness = (val / 100.0) * 255.0 | |||
| self._call_service('light/turn_on', {'brightness': brightness}) | |||
|
|
|||
| def set_color(self, hue, saturation, brightness): | |||
| red, green, blue = colorsys.hsv_to_rgb(hue, saturation, brightness) | |||
| rgb = [int(red), int(green), int(blue)] | |||
There was a problem hiding this comment.
colorsys's documentation indicates that the returned values are between 0.0 and 1.0, but doesn't Home Assistant expect rgb_color to be a 3-tuple of integers between 0 and 255?
|
@auchter Realized that colorsys was using the wrong color system, HSV instead of HSB. Found a function which I confirmed works to replace it with. Should be good to merge once I fix these conflicts. |
haaska.py
Outdated
| @@ -527,7 +574,7 @@ def set_temperature(self, val, mode=None, state=None): | |||
| self._call_service('climate/set_temperature', data) | |||
|
|
|||
|
|
|||
| def mk_entity(ha, entity_id): | |||
| def mk_entity(ha, entity_id, supported_features): | |||
There was a problem hiding this comment.
Perhaps supported_features should be an optional arg if few callers need it, or there should be a wrapper for mk_entity that takes payload directly, to avoid all the duplicated calls to payload_to_entity.
There was a problem hiding this comment.
Not a bad thought.
haaska.py
Outdated
| G = int(round(var_g * 255)) | ||
| B = int(round(var_b * 255)) | ||
|
|
||
| return [R, G, B] |
There was a problem hiding this comment.
Was this function copied wholesale from somewhere, and if so, you know what licence it's under?
There was a problem hiding this comment.
http://stackoverflow.com/a/26182355/486182. No license stated.
There was a problem hiding this comment.
Apparently pre-2016 posts there are CC-BY-SA, so that might be a problem. Every source I can find (including Wikipedia and http://codeitdown.com/hsl-hsb-hsv-color/ ) say HSB is just an alternative name for HSV though, so colorsys might be fine.
There was a problem hiding this comment.
Whenever I used colorsys I got very inconsistent results. I actually wrote a little script to check the outputs of a few different functions I found online as well as colorsys. Here's the script.
There was a problem hiding this comment.
Looks like the functions all expect different input value ranges, so you need to scale it appropriately.
There was a problem hiding this comment.
For SetColorRequest, etc. rgb = [int(round(i * 255)) for i in colorsys.hsv_to_rgb(hue/360.0, saturation, brightness)] since the colorsys range is 0.0-1.0 for both input and output.
There was a problem hiding this comment.
I'm cool with merging this PR once hsb2rgb is replaced with @trisk's suggestion that uses hsv_to_rgb.
haaska.py
Outdated
| G = int(round(var_g * 255)) | ||
| B = int(round(var_b * 255)) | ||
|
|
||
| return [R, G, B] |
There was a problem hiding this comment.
For SetColorRequest, etc. rgb = [int(round(i * 255)) for i in colorsys.hsv_to_rgb(hue/360.0, saturation, brightness)] since the colorsys range is 0.0-1.0 for both input and output.
No description provided.