search

openhab / openhab-addons

Add-ons for openHAB
https://www.openhab.org/
Eclipse Public License 2.0
1.88k stars 3.59k forks source link

[deconz] Hue/Saturation Conversions incorrect. #11031

Closed BClark09 closed 1 year ago

BClark09 commented 3 years ago

Expected Behavior

Updating a new Hue light (with the latest firmware) with the following command:

events.sendCommand("LivingRoom_TVBacklight_Colour", "0,100,33");

Should turn the Hue colorbulbs to red with full saturation at 33% brightness and the deconz rest API should return:

"state": {
    "alert": "none",
    "bri": 83,
    "colormode": "xy",
    "ct": 153,
    "effect": "none",
    "hue": 0,
    "on": true,
    "reachable": true,
    "sat": 254,
    "xy": [
      0.6915,
      0.3083
    ]
  }

Current Behavior

The openHAB logs acknowledge the correct value being set in openHAB which is percent type saturation and brightness:

2021-07-20 01:03:06.113 [INFO ] [openhab.event.ItemStateChangedEvent ] - Item 'LivingRoom_TVBacklight_Colour' changed from 0,100,36 to 0,100,33

However the bulb is visibly less saturated and at a different hue than it should be. The deCONZ API reports:

"state": {
    "alert": "none",
    "bri": 83,
    "colormode": "xy",
    "ct": 153,
    "effect": "none",
    "hue": 1032,
    "on": true,
    "reachable": true,
    "sat": 228,
    "xy": [
      0.6402,
      0.3299
    ]
  }

Note that state.hue, state.sat and state.xy are quite a way out. If the colour is changed via Hue bluetooth or through the deconz app, the blub shows the correct colour and openHAB reports the same three values for HSB (i.e. 0,100,33)

Possible Solution

It looks like openHAB isn't converting between HSB and the values required by the deconz API. Hue should convert from 0-360 to 0-65535 and saturation should covert from 0-100 to 0-255

Steps to Reproduce (for Bugs)

  1. Use the rule above to change the light to deep red
  2. Read the state of the bulb using the deconz REST API - (/api/<API_KEY>/lights/<light_ID>)
  3. Change the bulb colour using any method other than openHAB
  4. Read the state of the bulb again

Context

This happens with all my colour lights, all of which are Extended Colour Phillips Hue of different types (strip lights or standard bulbs) but with the latest firmware.

Your Environment

J-N-K commented 3 years ago

The binding used the openhab-core methods for converting hsb to xy (as does the hue binding).

I'm not an expert on these color things, but it seems there are different colorspaces which need different conversions: https://community.openhab.org/t/solved-convert-hsbtype-to-cie-xy-needed-for-ikea-tradfri-control-through-deconz-rest/48825/12

cweitkamp commented 3 years ago

The observed behavior might be caused by a similar reason like described in https://github.com/openhab/openhab-addons/issues/10159 for IKEA CWS bulbs controlled by hue binding. Currently if a bulb is in CT color mode the binding will send hue/sat/bri values which are not applicable for IKEA bulbs. This happens because we only send xy/bri values if the current color mode is XY. Thus we changed the default behavior of the hue binding to preferr XY values over HSB unless a Bild is in HS color mode (see https://github.com/openhab/openhab-addons/pull/10608).

BClark09 commented 3 years ago

Thanks @J-N-K and @cweitkamp,

From https://github.com/openhab/openhab-addons/blob/487dc0e49fb83282dc77d823869945d706ca1d19/bundles/org.openhab.binding.deconz/src/main/java/org/openhab/binding/deconz/internal/handler/LightThingHandler.java#L213-L225 it looks like the default should be changed here too?

I'm not confident that it will solve this particular problem, in the test case above state.colormode is xy from the deCONZ api before I try to send a command with openHAB.

As @J-N-K said, the actual conversion happens in openhab-core for a specific gamma "compression", but wouldn't this mean that I would experience the same behavior if I were to use the Hue bridge and binding, then use the same rule to set the light red? I can try this if necessary.

A Hue application note for XY Color conversion uses different matrix coeficients than: openhab-core's implementation. Which uses the same values as found in https://en.wikipedia.org/wiki/SRGB#The_reverse_transformation_(sRGB_to_CIE_XYZ).

cweitkamp commented 3 years ago

should be changed here too?

Yes, I think so. I proposed https://github.com/openhab/openhab-addons/pull/11036

I agree on that this PR will not solve your issue. Reading the linked pages I came to the following thoughts. First we need a way to use different matrix coefficients to convert RGB values to XY. This can be done in OHC by extending the RawType method. Second we need to change bindings like Hue or deCONZ to consider different color gamut for different lights. According to https://developers.meethue.com/develop/hue-api/supported-devices/ Hue lights support three different color gamuts.

Gamut A Color x y
Red 0.704 0.296
Green 0.2151 0.7106
Blue 0.138 0.08

Gamut B

Color x y
Red 0.675 0.322
Green 0.409 0.518
Blue 0.167 0.04
Gamut C Color x y
Red 0.692 0.308
Green 0.17 0.7
Blue 0.153 0.048
  1. Calculate the closest point on the color gamut triangle and use that as xy value The closest value is calculated by making a perpendicular line to one of the lines the triangle consists of and when it is then still not inside the triangle, we choose the closest corner point of the triangle.

A quick check on my Hue API shows me that we get the supported color gamut type and the color gamut specification itself in the response. Something like this:

...
                "colorgamuttype": "C",
                "colorgamut": [
                    [
                        0.6915,
                        0.3083
                    ],
                    [
                        0.1700,
                        0.7000
                    ],
                    [
                        0.1532,
                        0.0475
                    ]
                ],
...

Unfortunately those are not available on deCONZ REST API.

J-N-K commented 3 years ago

I'll check with the deconz Team if the API can be extended.

BClark09 commented 3 years ago

Thanks again @cweitkamp and @J-N-K,

First we need a way to use different matrix coefficients to convert RGB values to XY. This can be done in OHC by extending the RawType method. Second we need to change bindings like Hue or deCONZ to consider different color gamut for different lights

Sounds good to me, being able to select the appropriate matrix manually in the thing's configuration would be a good first implementation, shall I create an issue on OHC?