Hue color bulb hue/sat values are all messed up

[Zoooook]

What happened?

Setting hue and saturation values for a Hue color bulb and then reading those same values back are not consistent. Using the _step and _move functions introduces even more inconsistency.

Publishing an event to the /set topic with saturation: 50, and then publishing an event to the /get endpoint, I get 2 events back, the first one shows saturation at 50, the second one shows it at 79. Any further /get events show a saturation of 79. Testing this for all integer inputs from 0 until error, the following graph is obtained:

Input values of up to 107 are accepted, and then at 108 I get the error: The value of "value" is out of range. It must be >= 0 and <= 255. Received 256

If a negative number is input for saturation, it is converted to a positive number, and the hue is flipped, although not necessarily exactly 180, as described below. Negative inputs are allowed down to at least -1 billion, probably -infinity, and the following graph is generated:

Sending saturation_step: 1 takes about 256 increments to go from 0 to 100. Sending saturation_move: 1 takes about 256 seconds to go from 0 to 100.

Sending hue: 30, I again get 2 events, the first one shows a hue of 30, and the second and all subsequent reads show a hue of 12.8. Graphing this from 0 to 120:

The pattern appears to repeat from 120 to 240 and from 240 to 360. Values above 360 are accepted and mapped back to values from 0 to 360, however if a negative value is sent for the hue, then hue and saturation are both set to 0.

Sending hue_step: 1 takes 256 increments to cycle from red to red, and sending hue_move: 1 takes 256 seconds to cycle from red to red.

What did you expect to happen?

I expect to see a consistent interface. Values I send in should be returned back out unchanged. _step and _move functions should use the same scale as the set function. Negative values for saturation and values above 100 should be rejected or mapped to the min and max value. Negative values for hue should be mapped onto the 0-360 range the same way that large values are.

Documentation should reflect the correct behavior, and show the accepted ranges.

Any conversions to a different scale should be linear transformations. I have specific color and saturation values that I used on the Hue bridge that I want to use with zigbee2mqtt, and I would really prefer not to have to write deconversion functions to undo the nonlinear transformations that are currently happening.

Ideally these values would just be passthroughs that give me access to the raw values on the Hue bulb. I'm not sure why brightness acts as a passthrough on a 0-255 scale but saturation is mapped to 0-100.

How to reproduce it (minimal and precise)

No response

Zigbee2MQTT version

1.30.4

Adapter firmware version

20210708

Adapter

SONOFF Zigbee 3.0 USB Dongle Plus ZBDongle-P

Debug log

No response

[sjorge]

It's not just Hue branded bulbs but all of them, if you can definitely optimistic publishing and color_sync options for devices and enable reporting for currentHue and currentSaturation if you can.

It should at least get rid of the publishes when calling set, and then update the state when the bulb returns it's new value. (And stop color sync from trying to get all attributes 'in-sync' )

debug 2023-05-28 20:43:20: Received MQTT message on 'zigbee2mqtt/light/office/mood/bulb/set/color' with data '{"hue": 30, "saturation": 50}'
debug 2023-05-28 20:43:20: Publishing 'set' 'color' to 'light/office/mood/bulb'
debug 2023-05-28 20:43:21: Received Zigbee message from 'light/office/mood/bulb', type 'attributeReport', cluster 'lightingColorCtrl', data '{"colorTemperature":500,"currentHue":16,"currentSaturation":200,"currentX":36476,"currentY":24543}' from endpoint 11 with groupID 0
debug 2023-05-28 20:43:21: Reading 'colorTemperature, currentY, currentX, currentSaturation, currentHue, colorMode' from 'light/office/mood/bulb' (0x001788010bb1ed75)
debug 2023-05-28 20:43:21: Received Zigbee message from 'light/office/mood/bulb', type 'readResponse', cluster 'lightingColorCtrl', data '{"colorMode":0,"colorTemperature":500,"currentHue":16,"currentSaturation":200,"currentX":36476,"currentY":24543}' from endpoint 11 with groupID 0
info  2023-05-28 20:43:21: MQTT publish: topic 'zigbee2mqtt/light/office/mood/bulb', payload '{"brightness":254,"color":{"hue":23,"saturation":79,"x":0.4689,"y":0.3839},"color_mode":"hs","color_options":{"execute_if_off":true},"color_temp":425,"linkquality":80,"power_on_behavior":"off","state":"ON","update":{"installed_version":16785162,"latest_version":16785162,"state":"idle"}}'

But yes, values are nowhere near what is 'set', you can see the raw pre any conversion values the bulbs returns in the attributeReport or readResponse lines in the debug log, you probably won't get the later, It's only there because the hue bulb I used does not support reporting of the colorMode attribute and color_sync needs to to make the 'correct' choice as to what to convert from.

There is lots of room for improvement here, I mostly gave up on using hue/set and just go with x/y as that seems to be what is 'closest' to the input you send.

All the color conversion stuff using by color_sync is here https://github.com/Koenkk/zigbee-herdsman-converters/blob/master/src/lib/color.js and the code that handles both hue/sat/x/y setting is here https://github.com/Koenkk/zigbee-herdsman-converters/blob/7d98618204f2288fc4cbb07d091305f8a5921fe7/src/converters/toZigbee.js#L1070-L1169

There are also some workarounds for different firmware bugs e.g. applyRedFix and handling enhancedHue vs currentHue support.

I think the deviation specifically for Philips Hue bulbs that we use enhancedHue for those. A while ago I was looking into this, but gave up but disabling enhancedHue for in the meta for the bulb I was testing with gave a correcter mapping between what was set and returned by a get (or attributeReporting)

[Zoooook]

Well that gives me somewhere to start looking, thank you for the info.

[sjorge]

While I was looking around just now I already spotted some issues :(

Per the readme:

• enhancedHue: see toZigbee.light_color (default: true)
• supportsHueAndSaturation: see toZigbee.light_color (default: false)

Issues I spotted just now:

1. Some devices, notably the hue extends in src/lib/extend.js for philipsHue still use the older supportHS naming!
2. readColorAttributes defaults supportsHueAndSaturation to true but it should be false as per the readme.
3. enhancedHue defaults to true, which seems a bit weird but this might be due to historical reasons that we used to always use enhancedHue, this should probably be renamed to supportsEnhancedHue to keep in line with the others.

I have tomorrow off work so I might try and at least fix the things mentioned above, doubt it will fix your issue. But it's something that needs fixing anyway.

Ping @Koenkk is there some history with supportHS vs supportsHueAndSaturation I'm missing? Or was supportHS just renamed with one of the refactors that moved a lot of the logic out of the convertor to src/lib/color.js and some older references were not renamed?

[Zoooook]

Ok, the culprit is the gamma correction: https://github.com/Koenkk/zigbee-herdsman-converters/blob/master/src/lib/color.js#L470, tested at https://jsfiddle.net/k5vbc7q6/

I don't want it doing this correction at all. I can hack it locally to not do that, but what would you suggest as a path forward?

I can live with the mismatch in scales in the _step and _move functions, that's all linear and I can compensate.

[sjorge]

Ok, the culprit is the gamma correction: https://github.com/Koenkk/zigbee-herdsman-converters/blob/master/src/lib/color.js#L470

I don't want it doing this correction at all. I can hack it locally to not do that, but what would you suggest as a path forward?

I suggest introducing a device_options parameter that allows disabling gamma correction. This way, the function can check whether it needs to perform any action or simply act as a passthrough based on the provided option.

Or if it's doing more harm than good, perhaps you can convince Koen to just never do the correction.

[Koenkk]

As far as I can see now, gamma correction is only applied when setting an RGB value, not hue/sat: https://github.com/Koenkk/zigbee-herdsman-converters/blob/0ef8e51a8b64d9256a37c59f24c9ecbd3b92e6ed/src/converters/toZigbee.js#L1082

[Zoooook]

It happens at https://github.com/Koenkk/zigbee-herdsman-converters/blob/0ef8e51a8b64d9256a37c59f24c9ecbd3b92e6ed/src/converters/toZigbee.js#L1101

[Koenkk]

This has been introduced in Koenkk/zigbee-herdsman-converters#1183 . Since HS are natively supported by Zigbee bulbs, I'm wondering wether this was a good decision (currently I think we should set the values as is). @jasperro could you comment on this?

[Zoooook]

Well, I'm trying to figure out how to access the device options in either color.js or toZigbee.js, if I can get that working I'll have pull requests for you shortly to disable it via an option, but if you want to just kill it with no option then I'll stop working on that.

[sjorge]

+1 for not modifying the hue/sat values.

Personally I would expect it device to get the hue/sat value I pass it, same for XY for that matter. (with the exception of applyRedFix).

It does make sense to do the corrections when setting it as RGB though, which I think was the intend of all the corrections ?

[Zoooook]

As far as I can tell the change is this simple: Koenkk/zigbee-herdsman-converters#5820

I don't see any tests checking that gamma correction is performed for HSV colors.

[github-actions]

This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 7 days

[Soukyuu]

Is this in any way related to the fact that I'm unable to set the philips hue E27 bulb to cyan (comes out as desaturated blue), while the E14 one works?

[github-actions]

This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 7 days