Open m4rcu5 opened 1 month ago
This issue has been mentioned on openHAB Community. There might be relevant details there:
https://community.openhab.org/t/dimmer-and-color-item-to-same-channel/157241/15
@m4rcu5 I am thinking of writing a PR to address this. What I think you are asking for is that when someone changes the color temperature (CT) channel, then the color channel should be set also to the color that matches the CT on the CT locus in the color XY space. That would be doable. However I am concerned that if such CT channel command would be sent to the lamp and thus force it to switch from CT mode to color mode. And that obviously is not desirable at all. So perhaps you can clarify what you need?
Hi @andrewfg, I think that setting the Color channel to a representative of the ColorTemperature would be really nice (more akin how Google Home handles it), but would be over achieving my goal.
For me, I would be happy if the Color channel would return 0,0,<brightness>
instead of UNDEF when the CT value is changed, or when a poll happens. This would be really close to its current behavior.
To be clear, that 0 Hue and 0 Saturation is not send to the device, but seems to be currently returned by the Color channel when setting the brightness while in HSB mode.
So currently; If you set CT, the Color is set to UNDEF, setting a brightness afterwards, makes it return 0,0,B. And after the channel is polled it plops back to UNDEF. If we can get rid of UNDEF, and just consistently return that 0,0,B value when bulb is in CT mode, that would be great. If instead of 0H 0S, you could map that to a valid color that looks like the colortemp, that would be more awesome, but without knowing the mapping of % (on the CT channel) to kelvin (on the actual lamp), I think that is impossible.
Ok I think reading (polling) may return 4 different state combinations..
I think writing (commands) may have following combinations..
I am concerned about the last bullet point, since that risks to flip the lamp from CT mode to color mode. => WDYT?
Ok I think reading (polling) may return 4 different state combinations..
* Color is defined, CT not => OH color channel = actual HSB; CT channel = UNDEF
:heavy_check_mark: Sound correct
* Color is undefined, CT defined => color channel = HSB of locus of CT on XY chart; CT channel = actual CT
I am a bit concerned how this HS/XY value would be calculated, as the CT channel itself is a dimmer value, and the temperature in kelvin at 0% and 100% are only known to the hardware itself (or if you set them in the google home metadata)
* both defined => both channels = actual value
I do not think this is a valid combination. The bulb is either in Color or ColorTemperature mode. Digging in the current code, it seems
clusterColorControl
is what defines its state.* both undefined => both channels UNDEF
:heavy_check_mark: sounds logical.
I think writing (commands) may have following combinations..
* On/off command to color channel => send switch command to lamp
Yes, currently it seems to use a ClusterOnOff command.
private Future<CommandResult> changeOnOff(OnOffType onoff) throws InterruptedException, ExecutionException {
boolean on = onoff == OnOffType.ON;
if (clusterOnOff == null) {
if (clusterLevelControl == null) {
logger.warn("{}: ignoring on/off command", endpoint.getIeeeAddress());
return null;
} else {
return changeBrightness(on ? PercentType.HUNDRED : PercentType.ZERO);
}
}
ZclOnOffCommand command;
if (on) {
command = new OnCommand();
} else {
command = new OffCommand();
}
return clusterOnOff.sendCommand(command);
}
* Dimming command to color channel, and CT is UNDEF => send prior HSB with different B
Then dimming, I think it sends a ClusterLevelControl command, and not the full HSB values
private Future<CommandResult> changeBrightness(PercentType brightness)
throws InterruptedException, ExecutionException {
if (clusterLevelControl == null) {
if (clusterOnOff == null) {
logger.warn("{}: ignoring brightness command", endpoint.getIeeeAddress());
return null;
} else {
return changeOnOff(brightness.intValue() == 0 ? OnOffType.OFF : OnOffType.ON);
}
}
int level = percentToLevel(brightness);
ZclLevelControlCommand command;
if (clusterOnOff != null) {
if (brightness.equals(PercentType.ZERO)) {
return clusterOnOff.sendCommand(new OffCommand());
} else {
command = new MoveToLevelWithOnOffCommand(level, configLevelControl.getDefaultTransitionTime());
}
} else {
command = new MoveToLevelCommand(level, configLevelControl.getDefaultTransitionTime());
}
return clusterLevelControl.sendCommand(command);
}
* Dimming command to color channel, and CT is NOT UNDEF => send 0,0,B ?? .. or send HSB value from CT point in XY space ??
So the same here. If you would send the HSB command, it will go back to color mode. But I think the issue is with the channel updates and not the commands send to the device itself.
In my debugging, the commands to the device are OK, but the representation on the openHAB side is wrong. I'm trying to follow the code and see where updates to the channel are send.
I am concerned about the last bullet point, since that risks to flip the lamp from CT mode to color mode. => WDYT?
the CT channel itself is a dimmer value, and the temperature in kelvin at 0% and 100% are only known to the hardware itself
In Philips Hue there is a default of 500 Mirek = 0% and 153 Mirek = 100% .. so I would use that as basis. And we could eventually add a config param for Min/Max Mirek to cover more exotic lamps.
I am a bit concerned how this HS/XY value would be calculated, as the CT channel itself is a dimmer value, and the temperature in kelvin at 0% and 100% are only known to the hardware itself
That's not correct - the binding knows the min/max - otherwise it wouldn't be able to set the colour temperature which is in mired. The binding uses the min/max values to convert the percentage to mired.
In Philips Hue there is a default of 500 Mirek = 0% and 153 Mirek = 100% .. so I would use that as basis.
You shouldn't just guess the min/max - just read the min/max mired from the device. Note that if the device doesn't provide the min/max mired, then the binding has some internal defaults, and those should be used - not another set. Otherwise things will be inconsistent.
if the device doesn't provide the min/max mired, then the binding has some internal defaults, and those should be used
In Philips Hue there is a default of 500 Mirek = 0% and 153 Mirek = 100%
@cdjackson just for the avoidance of doubt the Zigbee binding default CT range is 2000 K -- 6500 K .. which is .. (tada) .. 500 Mirek -- 153 Mirek .. so there would not have been any risk of using something other than that :)
Ok, that's fine - it happens to be the same in this instance (which is good :) ), but my point is we should use a single set of constants to avoid any confusion.
@cdjackson in any case the code that I would add does not need to check or process the min/max mired capabilities of the lamp. Reason is that my added code would respond to the CT attributeUpdated()
event which contains a mired value from the lamp (so obviously anything in that event must be within range of the respective lamp.
HOWEVER on a related note: I see that your ZigBeeConverterColorTemperature class currently converts UI dimmer percentages => mired (which is correct) .. but it actually does the conversion percent => kelvin => mired -- which I think is incorrect. Kelvin is too non linear to be useful for things like UI sliders, so the inverse mired was invented so that a 0%..100% control (in mired) gives a more linear change of perceived CT. Therefore I suggest another PR needs to be made so that there is a linear relationship percent <=> mired (i.e. in fact to ignore the Kelvin code entirely). => WDYT?
Outline
Most of my Zigbee bulbs have a ColorTemperature and Color channel. These lights have a separate CCT led and RGB leds. So it makes sense that if you set a ColorTemperature, the Color channel will be reset to UNDEF (as no RGB LEDs are in use). Which I believe is also working-as-expected looking at PR #336. However the brightness still has to be controlled by the Color channel.
Initially this works somewhat, you can send a brightness value to the Color channel. Now the Color channel switches to
0,0,B-value
and the bulb lights. However after the polling interval has passed, the Color channel bounces back to UNDEF.Leaving the lights with a ColorTemperature set, also makes them bounce on every poll. The channels go to a value and back to UNDEF in the same poll.
I've done some more in depth digging into the behavior on the forum (https://community.openhab.org/t/dimmer-and-color-item-to-same-channel/157241/11). I'll add the summary below.
Configuration
Logs
Color channel UNDEF even when brightness is set
Starting at the off (
0,0,0
) settingSetting a normal color (including a brightness)
Setting a color temperature (which will turn the Color channel UNDEF, even though the light is still on)
Setting a new brightness, so we get our Color channel back
Bouncing issue
Send a OFF command to the Color channel when a ColorTemperature is set, and send a couple of REFRESH commands.
A single REFRESH when the light is ON (ColorTemperature set, followed by a brightness value to Color), will cause the _Color channel to go back to UNDEF as well.
Demo, this time the Hue bulb for reference, but the Milight behaves the same.
Note: In most examples you will see a
_Dimmer
item follow along. This is just a Dimmer item linked to the Color channel for easy sending of brightness values. I have tried the examples without this link as well.