-
-
Notifications
You must be signed in to change notification settings - Fork 113
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Recognize color mode of color bulb #336
Recognize color mode of color bulb #336
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One question. This PR sets the color temperature channel to UNDEF if the color mode is not in color temp. Why doesn't it set the color channel to undef if the mode IS color temp? If we're saying they are exclusive, then shouldn't both channels be consistent?
I've implemented it to behave like the Hue binding. Here you can see that there only the color temperature is set to UNDEF but never the color. |
Ok, but why shouldn’t it be the other way around as well? Just because one binding does something, doesn’t (necessarily) mean that it’s right? Maybe there’s a good reason that it’s only done on the color temperature. Maybe it’s considered that color is still valid even when in color temp mode? Or maybe it was just an oversight when the Hue change was made and it should also be done for color?
… On 16 Dec 2018, at 08:31, Tommaso Travaglino ***@***.***> wrote:
One question. This PR sets the color temperature channel to UNDEF if the color mode is not in color temp. Why doesn't it set the color channel to undef if the mode IS color temp? If we're saying they are exclusive, then shouldn't both channels be consistent?
I've implemented it to behave like the Hue binding. Here <https://github.com/eclipse/smarthome/blob/303ba1c1c22f4cd8bdb9b068735373c29c43de06/extensions/binding/org.eclipse.smarthome.binding.hue/src/main/java/org/eclipse/smarthome/binding/hue/internal/handler/HueLightHandler.java#L420> you can see that there only the color temperature is set to UNDEF but never the color.
—
You are receiving this because your review was requested.
Reply to this email directly, view it on GitHub <#336 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AA_kQ5OUX0cLxNKmQuU9UqtnmASIVy1kks5u5gTagaJpZM4ZRx2J>.
|
@kaikreuzer and @triller-telekom did the review of the Hue PR. Maybe they know why in the Hue binding the color temperature is set to UNDEF when the color mode is not in color temp but not the other way around. |
That's exactly the reason behind it: See eclipse-archived/smarthome#5762 (comment) where Kai says:
So the way @TomTravaglino implemented it is correct. Does that make sense to you @cdjackson ? |
To be honest, it doesn’t.
With at least one of my bulbs here, I can read back a color that is in no way related to color temperature. So while it might be correct that you _could_ display color temperature in a color box, it does not follow that the color you are displaying will in any way be related to the color temperature if the bulb reports something different when it is in a different color mode. Based on what I see here in at least one bulb I just tested, this will in fact be the case.
Maybe in the Hue bridge this is accounted for to align color and color temperature, but that is not necessarily correct when reading the bulbs directly.
… On 2 Jan 2019, at 12:31, Stefan Triller ***@***.***> wrote:
Maybe it’s considered that color is still valid even when in color temp mode?
That's exactly the reason behind it: See eclipse-archived/smarthome#5762 (comment) <eclipse-archived/smarthome#5762 (comment)> where Kai says:
I don't think that this [setting the color_channel to NULL] makes sense, because the color channel can always have a valid state and you always want to be able to display it in a color picker (e.g. if the user switches from slider to color picker).
So the way @TomTravaglino <https://github.com/TomTravaglino> implemented it is correct.
Does that make sense to you @cdjackson <https://github.com/cdjackson> ?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub <#336 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AA_kQ7sfZ6Tym3f9A5iED8dcPJrA13ndks5u_Ka4gaJpZM4ZRx2J>.
|
I had to read your answer quite a few times, so let me sum up what I understand from it: You are saying that if you set a colourtemperature and keep the colour (i.e. HSB or x/y) value, then the zigbee bulb that you have reports two colours: One in a colourtemperature (the newly set one) and the colour one (old value which was previously set). Then you have the problem that you have to update the OH channel and don't know which value to pick. Is that correct? In this case I would agree with you that in the zigbee binding we should set the colour value to |
Sorry for the bad explanation. Yes - basically you're right with your understanding. The color temperature is not handled as a color - it's a different mode. The color and colortemperature are independent (at least in this bulb - I've not tried others yet) - and the mode is what differentiates them. It's the same (or similar) with the way that Level control works (at least in some devices) - a device can report OFF, but report the Level to be 50% - that doesn't mean it is on - just that if you change the On/Off mode to On, it will come on at 50%. I hope that makes sense? I also had a look through the ZCL docs, and there is nothing in there to link color and color-temperature. The two seem to be treated as separate modes. The docs talk about switching modes when a color temperature command is received, but it doesn't say that is should update the current X/Y or Hue attributes, so I would assume they should remain set as they were. |
I changed ZigBeeConverterColorColor to set the channel state to UNDEF when the color mode turns to color temp. Is this an expected behavior? I tested with a Osram Smart+ Classic E27 Multicolor and a Osram Lightify CLA60 RGBW. |
I've not looked at the log, but I suspect what you are seeing is what I eluded to above? That is that attribute reports will be sent to report color, even when the bulb is in the CT mode, and unless you use the state you will update the color when the device is in CT mode? Is that what you see? I will take a look at the log later, but if I understand your question, then yes, this is expected. |
And report happens although nothing changed.
I update the state of the color channel although the device is in CT mode. |
Yes, that is correct. In the reporting, we have to set a minimum and maximum report time. So if there are changes, the reports will come in at the minimum report time, and if there is no change the reports will come in at the maximum report time.
I would suggest that you need to not update color if it is in CT mode - ie you need to remember the color mode, and only update color if CM is in color mode, and only update CT if CM is in CT mode. Otherwise the system is going to propagate incorrect state to the user. |
- Enable reporting of color mode attribute in color temperature converter - Handle update of color mode attribute and set color temperature to UnDefType.NULL when color mode is not 'color temperature' Signed-off-by: Tommaso Travaglino <[email protected]>
from beeing updated by periodical attribute reporting Signed-off-by: Tommaso Travaglino <[email protected]>
b614e1d
to
ba41e65
Compare
…or mode check - This change was made to make unit testing easier Signed-off-by: Tommaso Travaglino <[email protected]>
@cdjackson I updated the PR and had to merge master into my branch. Unfortunately the checks fail with this message
Maybe a temporary problem. Is there a way to restart the checks? |
The checks have passed now. Apparently it was a temporary problem. |
Gentle reminder 😉 |
Sorry - I will look at this once CI is working again. |
I am watching this with interest, as I had a similar request some time ago. I have solved this privately by programming a dummy CT channel as a rule, and using that to convert CT commands to HSB commands, thus always keeping my bulbs in HSB mode whilst being able to give color temperature commands. I kind of made the same conclusions as you all did above, that the color channel is always valid and useful, that is why I am translating CT into HSB. This way the state of the bulb is always refelected correctly in the HSB channel. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM - thanks @TomTravaglino
This pull request has been mentioned on openHAB Community. There might be relevant details there: |
1 similar comment
This pull request has been mentioned on openHAB Community. There might be relevant details there: |
This pull request has been mentioned on openHAB Community. There might be relevant details there: |
Joining the conversation here vs the one on community.openhab - Anyway Chris your last post mentioned you guys might implement a dimmer channel back again so we have CT/Dimmer/Color?? That does seem to make a lot of sense to me as that would then allow us to always see a brightness state. One other thing I just thought of regarding this current UNDEF is that if we are in CT mode and want to go back to color mode keeping the previous state we currently have no way to do that as the color state would have been switch to UNDEF. It would almost be best to have a switch that shows color mode on/off and then leave the CT and Color items alone... that way you can send the command to the color mode to switch it. All in that would then leave us with Color HSB , Brightness Dimmer, CT Dimmer and a color mode switch. I know the brightness is redundant when using HSB but obviously we want to keep color as an HSB type. I think that channel layout would allow for the most robust management and operation of the bulbs. What do you guys think? |
I got loads of 0210 Bulbs from Philips and i am using the deconz bridge. Using the latest Snapshot build. Atm, you can read the Brightness value by just assigning a Dimmer to the color channel, but as stated you have the problem, that if you just change one channel, the others are not updated. If you then touch another channel there can be a huge jump in bulb visibility, since this channel has some random old value... I was about to program my OH2 functionality with rules as following : 1 Switch between ColorTemperature and Color Mode (so i know which mode is active) When you adjust any of the 3 Channels 2-4, the other 2 Channels are updated with the respective values or the ones that are closest to the changed Channel. |
This binding doesn't support the deconz bridge so I suspect you're using the deconz binding? |
Yep, true, but i am having the exact same trouble with the hue binding emulated by deconz atm. have to take a look there what this is doing, but it seems to me the functionality is approx the same |
If you want to get something implemented, then it will need to be in the other binding… However, I think this might raise cross binding questions on how to handle this sort of thing, so feel free to comment ;)
… On 27 Feb 2019, at 20:47, SuperSchelli ***@***.***> wrote:
Yep, true, but i am having the exact same trouble with the hue binding emulated by deconz atm. have to take a look there what this is doing.
sorry for the screwup, but the discussion is interesting nonetheless
i have ordered a cc2531 USB Stick and wanted to try around with this one as well, that should use this binding then if i am not mistaken.
—
You are receiving this because you modified the open/close state.
Reply to this email directly, view it on GitHub <#336 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AA_kQ6IrR_bp5vvfmFYHQDm3958NbBQLks5vRu7ggaJpZM4ZRx2J>.
|
Well. I wanted to add, that there are also bulbs, that can only handle a color channel and no CT channel. Now I have rooms which have a mixture of bulbs in them, but now I just want it darker, I don't want to adjust all bulbs on their own. so u want 1 brightness slider for the whole room, but depending on which bulb I control this command needs To be translated into the right channel for each bulb, depending on its capabilities. |
Resolves #326
This is an approach to recognize if the color has been set via color scheme or via color temperature scheme. It corresponds to the solution realized in eclipse-archived/smarthome#5762 which sets the color temperature to UnDefType.NULL when the color mode is not "color temperature".
Your feedback is welcome.