RGB lights support #85
Conversation
|
@tobre6, thanks for your PR! By analyzing the history of the files in this pull request, we identified @maddox, @shmuelzon and @rcloran to be potential reviewers. |
accessories/light.js
Outdated
| }, | ||
| rgbToHsv: function(r, g, b) { | ||
| if (arguments.length === 1) { | ||
| g = r.g, b = r.b, r = r.r; |
There was a problem hiding this comment.
Expected an assignment or function call and instead saw an expression.
accessories/light.js
Outdated
| case 2: r = p, g = v, b = t; break; | ||
| case 3: r = p, g = q, b = v; break; | ||
| case 4: r = t, g = p, b = v; break; | ||
| case 5: r = v, g = p, b = q; break; |
There was a problem hiding this comment.
Expected an assignment or function call and instead saw an expression.
accessories/light.js
Outdated
| case 1: r = q, g = v, b = p; break; | ||
| case 2: r = p, g = v, b = t; break; | ||
| case 3: r = p, g = q, b = v; break; | ||
| case 4: r = t, g = p, b = v; break; |
There was a problem hiding this comment.
Expected an assignment or function call and instead saw an expression.
accessories/light.js
Outdated
| case 0: r = v, g = t, b = p; break; | ||
| case 1: r = q, g = v, b = p; break; | ||
| case 2: r = p, g = v, b = t; break; | ||
| case 3: r = p, g = q, b = v; break; |
There was a problem hiding this comment.
Expected an assignment or function call and instead saw an expression.
accessories/light.js
Outdated
| switch (i % 6) { | ||
| case 0: r = v, g = t, b = p; break; | ||
| case 1: r = q, g = v, b = p; break; | ||
| case 2: r = p, g = v, b = t; break; |
There was a problem hiding this comment.
Expected an assignment or function call and instead saw an expression.
accessories/light.js
Outdated
| t = v * (1 - (1 - f) * s); | ||
| switch (i % 6) { | ||
| case 0: r = v, g = t, b = p; break; | ||
| case 1: r = q, g = v, b = p; break; |
There was a problem hiding this comment.
Expected an assignment or function call and instead saw an expression.
accessories/light.js
Outdated
| q = v * (1 - f * s); | ||
| t = v * (1 - (1 - f) * s); | ||
| switch (i % 6) { | ||
| case 0: r = v, g = t, b = p; break; |
There was a problem hiding this comment.
Expected an assignment or function call and instead saw an expression.
accessories/light.js
Outdated
| hsvToRgb: function(h, s, v) { | ||
| var r, g, b, i, f, p, q, t; | ||
| if (arguments.length === 1) { | ||
| s = h.s, v = h.v, h = h.h; |
There was a problem hiding this comment.
Expected an assignment or function call and instead saw an expression.
accessories/light.js
Outdated
| this.client.callService(this.domain, 'turn_on', service_data, function(data) { | ||
| if (data) { | ||
| that.log("Successfully set rgb on the '"+that.name+"' to " + service_data.rgb_color); | ||
| callback() |
There was a problem hiding this comment.
Eww. Why are we demanding semis on this repo.
There was a problem hiding this comment.
Semicolons are considered as best practice. I already fixed them before but there were some previous code where they didn't exist. I fixed them now.
accessories/light.js
Outdated
| } else { | ||
| callback(communicationError) | ||
| } | ||
| }.bind(this)) |
accessories/light.js
Outdated
| that.log("Successfully set rgb on the '" + that.name + "' to " + service_data.rgb_color); | ||
| callback() | ||
| } else { | ||
| callback(communicationError) |
accessories/light.js
Outdated
| this.client.callService(this.domain, 'turn_on', service_data, function (data) { | ||
| if (data) { | ||
| that.log("Successfully set rgb on the '" + that.name + "' to " + service_data.rgb_color); | ||
| callback() |
accessories/light.js
Outdated
| } else { | ||
| callback(communicationError) | ||
| } | ||
| }.bind(this)) |
accessories/light.js
Outdated
| if (data) { | ||
| that.log("Successfully set brightness on the '"+that.name+"' to " + level); | ||
| callback() | ||
| }else{ | ||
| } else { | ||
| callback(communicationError) |
accessories/light.js
Outdated
| } else { | ||
| callback(communicationError) | ||
| } | ||
| }.bind(this)) |
accessories/light.js
Outdated
|
|
||
| callback(null, hue) | ||
| } else { | ||
| callback(communicationError) |
accessories/light.js
Outdated
| } else { | ||
| callback(communicationError); | ||
| } | ||
| }.bind(this)) |
accessories/light.js
Outdated
| } else { | ||
| callback(communicationError); | ||
| } | ||
| }.bind(this)) |
accessories/light.js
Outdated
| } else { | ||
| callback(communicationError); | ||
| } | ||
| }.bind(this)) |
accessories/light.js
Outdated
| } else { | ||
| callback(communicationError); | ||
| } | ||
| }.bind(this)) |
accessories/light.js
Outdated
| @@ -38,53 +38,109 @@ HomeAssistantLight.prototype = { | |||
| is_supported: function(feature) { | |||
| // If the supported_features attribute doesn't exist, assume supported | |||
| return this.data.attributes.supported_features === undefined || | |||
| ((this.data.attributes.supported_features & feature) > 0) | |||
| ((this.data.attributes.supported_features & feature) > 0) | |||
|
@robbiet480 Now it works on my Hue. Please check. :) Hopefully we can merge and if there are problems with some other lights we can fix them later. |
|
@tobre6 Apologies for not getting back to you, I've been pretty busy with work. I set up your fork and could set colours accurately on LiFX but I was getting some odd behaviour where it would set a colour and it would instantly snap back to the previous colour. cheers |
|
@matt-cahill I have experienced this issue as well, but this is not always so. So I think this is something related with with timout in Home app, because it takes a little bit time to send the request to HA and respond back. Can't do nothing about it. But still, it works. |
|
@robbiet480 Have you had chance to try with hue? |
|
I am right now! |
|
This appears to be working on all my Hue devices! I think this will be good to merge, just going to take another look through before I do. |
accessories/light.js
Outdated
| if ((this.data.attributes.supported_features & feature) > 0) { | ||
| //workaround because homeassistant includes RGB_COLOR feature in zwave lights even if they haven't RGB | ||
| //https://github.com/home-assistant/home-assistant/issues/5333 | ||
| if (feature == this.features.RGB_COLOR && this.data.attributes.rgb_color === undefined) { |
There was a problem hiding this comment.
This caused an issue with one of my (non-Hue) lights (a flux_led platform light). The light wasn't detected by Homebridge because it was off when I started Homebridge. After stopping Homebridge, turning the light on, and starting Homebridge again, it showed up. Is there any way around this?
There was a problem hiding this comment.
Oh hey, just noticed the referenced issue has been fixed since the last release, so this code should be entirely removable now right?
|
Just one small comment then this is good to go! |
|
Yes, it should be ok, but I'm running homeassistant in docker container in rpi. Official image is not up2date, so I can't test it easily yet. If you can confirm that it is working, let's remove. |
|
The code is removable yeah. |
|
@robbiet480 I removed workaround. Now it should be OK for merge I hope. |
|
@tobre6 Will take care of this later today, after the release of 0.38 |
|
This is a MASSIVE PR. Looking forward to getting this in and hopefully dropping my homebridge-lifx dependence completely. Thank you very very much for your effort, @tobre6 |
|
@cmsimike Thanks for the bump on this. Merging now, going to leave in GitHub for a couple days for people to test it out before going to npm. |
|
I updated to the latest version, but my RGB light using the 'flux' component in HASS still only has brightness controls. Am I doing something wrong here? Is there something else that I need to add to the config to get this working? |
|
Same, updated to 2.0.6. I can still only control Brightness with my Yeelight and the light.component. |
|
What supported_features attributes say? |
|
In Home-assistant it shows "supported_features: 59" In HA I can control brightness, color and color Temperature. Is there anything else I have to change in the homebridge config.jcon? |
|
No, there is nothing to configure. It should work out of the box. Supported feature 59 contains RGB color feature and should show Color button on the bottom left corner of the device page in Home app. |
|
Ok now it works. I have to readd the light to homebridge. I did this by temporary hide the device (homebridge_hidden: true) in HA. |
|
Does anyone have this working with the Ikea Tradfri bulbs? In HASS i can set the colortemperature but HOME only offers me brightness. |
|
Does this work with Xiaomi LightStrip? |
|
hmm, when setting RGB color for a ledstrip, it goes dark no matter what color i try to set, but when i change it from HA it works properly on the log it says: |
There have been some PRs and issues about RGB support for lights, but even there is an interest nobody haven't resolved the problem yet. So I'm giving to try.
This is the initial working version of the RGB lights.
Different RGB PRs and issues have been quite useful and descriptive of the problem. For example PR #11
The main problem of the topic isn't just different standards. HA uses RGB as well as the led stripes, but HomeKit uses HSV (HSB), where V (B) is value or brightness. So you have to convert from one to another, but extra dimension is added from HA as well. In HA UI there is RGB color picker and brightness at the same time. Converting from RGB to HSV you can calculate brightness or giving brightness, it is possible to calculate respective RGB value.
Useful calculators:
http://www.rapidtables.com/convert/color/hsv-to-rgb.htm
http://www.rapidtables.com/convert/color/rgb-to-hsv.htm
So my approach from HA perspective is when I choose darker colors, brightness is adjusted to lower and when brighter colors, brightness will be higher.
When changing hue and saturation from HomeKit, I'm calculating respective RGB value and sending this to HA and when changing brightness value I'm also calculating the RGB value and sending this to HA.
From hardware side I'm using RGB led stripe and MQTT lights.
This logic works good and intuitive from HA and HomeKit. So I think you should give it a try at least.
I'm using docker to run homebridge on Raspberry.
You can test to run my PR when using development tag: https://hub.docker.com/r/tobre/rpi-homebridge-homeassistant/
It uses code from: https://github.com/tobre6/homebridge-homeassistant
Open question for now is how should RGBW converted? Or even RGBWW where there are warm and cold whites used separately? But I think this is matter of another PR. :)
Let me know if you have questions or ideas.