search

peter-murray / node-hue-api

Node.js Library for interacting with the Philips Hue Bridge and Lights
Apache License 2.0
1.19k stars 145 forks source link

Enable .rgb() support in .setGroupLightState() #32

Closed ardouglass closed 9 years ago

ardouglass commented 9 years ago

I don't know if this was left out intentionally, but it works on my bulbs with this fix.

peter-murray commented 9 years ago

Hi,

RGB is not really part of the lights API, it is an addition that I added. The code that you have copied to convert the RGB values does not generate correct colours in extreme ranges, i.e. Black and white.

I recently rewrote the conversion code, but it needs to apply it to each light as it needs to understand the bounds of the color space. Seeing that a group can include any light bulbs or lamps, and they may not all be the same, it is not trivial to perform this conversion to a group, although entirely doable.

I have been working on releasing the 1.0.0 version of the library that fixes a number of issues with the LightState object, as well as supporting scenes and some other new features. I was aware of what I would need to do around applying RGB colours to a group, but it may not make it into the initial 1.0.0 release and instead be added to a patch or minor release soon after.

Due to the fact the code will not generate the correct colours, I will not apply this pull request, but will open an issue to cater for group RGB colors in the 1.x releases.

ardouglass commented 9 years ago

I didn't think about the multiple types of bulbs. That makes sense. What would be the best way to apply color to a group until you add RGB?

peter-murray commented 9 years ago

It's a bit of a mess until I release 1.x as the LightState object had a number of bugs around properties and functions overlapping, which caused all kind of issues. Also the deletion of RGB values was a problem, that has since been remediated.

The solution would be to chain a couple of calls, which will be what I do in the final implementation. Effectively you need to get the types of bulbs in the group and put then in sub groups based on type, then perform the RGB transformation for each sub group, and then unfortunately queue multiple calls to each sub group. It is less than optimal, but about the best implementation I can think of for now.

The down and dirty option could be to only allow RGB to be used if the lights are all the same type, and error if that is not the case, or possibly just do one conversion and pass those Love you heaps and miss you lots x y values, as the lights will adjust the values to something within range.

The use of scenes may be an appropriate option too, as in a scene you can set different light settings when you recall the scene. The API currently only has this functionality on the development branch, and I have not fully tested that.

ardouglass commented 9 years ago

I like the idea of subgroups and think that could work pretty well. I have groups in my house including "downstairs", "living room", and "living room ceiling fan." I know all of them in the ceiling fan will be the same, but all of them downstairs or in a single room won't necessarily be the same.

On Sat, Jan 24, 2015 at 2:20 PM, Peter Murray notifications@github.com wrote:

It's a bit of a mess until I release 1.x as the LightState object had a number of bugs around properties and functions overlapping, which caused all kind of issues. Also the deletion of RGB values was a problem, that has since been remediated. The solution would be to chain a couple of calls, which will be what I do in the final implementation. Effectively you need to get the types of bulbs in the group and put then in sub groups based on type, then perform the RGB transformation for each sub group, and then unfortunately queue multiple calls to each sub group. It is less than optimal, but about the best implementation I can think of for now. The down and dirty option could be to only allow RGB to be used if the lights are all the same type, and error if that is not the case, or possibly just do one conversion and pass those Love you heaps and miss you lots x y values, as the lights will adjust the values to something within range.

The use of scenes may be an appropriate option too, as in a scene you can set different light settings when you recall the scene. The API currently only has this functionality on the development branch, and I have not fully tested that.

Reply to this email directly or view it on GitHub: https://github.com/peter-murray/node-hue-api/pull/32#issuecomment-71335481

peter-murray commented 9 years ago

I have just performed a refactoring to support scene definitions and modifications that has resulted in the code for the light state being extracted into a common function.

That should make it relatively easy for me to support RGB on a sub group within a group. I may be able to put something together for the 1.0.x release later this week to cater for your requirement.