Skip to content
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

[deconz] Hue/Saturation Conversions incorrect. #11031

Closed
BClark09 opened this issue Jul 20, 2021 · 6 comments · Fixed by #14622
Closed

[deconz] Hue/Saturation Conversions incorrect. #11031

BClark09 opened this issue Jul 20, 2021 · 6 comments · Fixed by #14622
Labels
bug An unexpected problem or unintended behavior of an add-on

Comments

@BClark09
Copy link
Member

BClark09 commented Jul 20, 2021

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

  • Version used: openHAB 3.2.0-SNAPSHOT (build #2453) (current as of 19/07/21)
  • Java Environment: OpenJDK Runtime Environment (build 11.0.11+9-Ubuntu-0ubuntu2)
  • deCONZ environment: Version 2.12.03, Firmware 0x266F0700
  • Operating System and version (desktop or mobile, Windows 10, Raspbian Buster, ...): Ubuntu 21.04 arm64 on a Raspberry Pi 4
@BClark09 BClark09 added the bug An unexpected problem or unintended behavior of an add-on label Jul 20, 2021
@J-N-K
Copy link
Member

J-N-K commented Jul 20, 2021

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
Copy link
Contributor

cweitkamp commented Jul 20, 2021

The observed behavior might be caused by a similar reason like described in #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 #10608).

@BClark09
Copy link
Member Author

BClark09 commented Jul 20, 2021

Thanks @J-N-K and @cweitkamp,

From

if ("xy".equals(colorMode)) {
PercentType[] xy = hsbCommand.toXY();
if (xy.length < 2) {
logger.warn("Failed to convert {} to xy-values", command);
}
newLightState.xy = new double[] { xy[0].doubleValue() / 100.0, xy[1].doubleValue() / 100.0 };
newLightState.bri = Util.fromPercentType(hsbCommand.getBrightness());
} else {
// default is colormode "hs" (used when colormode "hs" is set or colormode is unknown)
newLightState.bri = Util.fromPercentType(hsbCommand.getBrightness());
newLightState.hue = (int) (hsbCommand.getHue().doubleValue() * HUE_FACTOR);
newLightState.sat = Util.fromPercentType(hsbCommand.getSaturation());
}
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
Copy link
Contributor

cweitkamp commented Jul 20, 2021

should be changed here too?

Yes, I think so. I proposed #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
Copy link
Member

J-N-K commented Jul 20, 2021

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

@BClark09
Copy link
Member Author

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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug An unexpected problem or unintended behavior of an add-on
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants