[KNX] DPT 232.600 converted from RGB to HSB without decimal places

[Gehetzter]

Describe the bug Openhab works with HSB color values. To use the DPT 232.600 the Knx RGB values are converted to HSB. In this conversion decimal places are lost. When converting back to RGB the result is not the same that was sent over Knx.

Expected behavior I expect the RGB values being converted to HSB with decimal places.

Actual result The HSB is truncated to integers.

Additional context Guess the problem is here, somewere between Line 600 and 620: KNXCoreTypeMapper.java

[holgerfriedrich]

We have improved the rounding behavior during the color conversions. This decreases the difference between RGB values being received and sent out again.

Our primary color representation is HSB, and the current implementation rounds to integer values during all conversions. This is the expected behavior. This implies that converting from any representation to HSB and back is not a lossless transformation.

@J-N-K @andrewfg @kaikreuzer For me, the handling of color conversions should be kept as it is (i.e. rounding to integer values). Could you give your opinion on this please?

So for me, this issue should be closed as "won't fix".

[Gehetzter]

Thanks for reviewing my issue, but I´m a bit shocked of your suggestion. Why would you restrict it like that? Don't know about the "current" implementation, I´m on 3.4.2 but there I can easily have decimals in HSB. e.g. when I do It seems to me that only the knx binding is the limit.

If anyone wants to have it rounded to Integer it can be easily done e.g. in the state Description with %.0f On the other hand, if you do this in the knx binding already there is no chance to get the exact value on application side...

[J-N-K]

I'm not sure. What speaks against using decimals internally? Of course the 0-255 part should be int, that is because they represent the values of one byte and therefore decimals don't make sense. DecimalType and PercentType both support decimals.

[andrewfg]

handling of color conversions should be kept as it is

I am not sure how the following relates to this particular issue, but..

In the Philips Hue CLIP 2 API (see this PR) all colours are represented in DTOs as XY values with Gamut. So the recent changes in 'ColourUtil' with Gamut support and improved conversion precision are very welcome when READING an OH HSBType channel state from the binding (reference to code below).

https://github.com/openhab/openhab-addons/blob/fa0f4733b3b9208ba44d2e1536c3f52fd0dcf916/bundles/org.openhab.binding.hue/src/main/java/org/openhab/binding/hue/internal/dto/clip2/Resource.java#L259

Nevertheless when WRITING an OH HSBType thing channel state to the binding we still lose precision. Or to be more precise we still lose precision on round trips XY -> HSB -> XY'.

https://github.com/openhab/openhab-addons/blob/fa0f4733b3b9208ba44d2e1536c3f52fd0dcf916/bundles/org.openhab.binding.hue/src/main/java/org/openhab/binding/hue/internal/dto/clip2/Resource.java#L539

---

EDIT just to point out that the above applies not only to the Color XY parts, but the Brightness parts: the Philips Hue CLIP 2 API produces and consumes JSON floats for both types of DTO elements..

"dimming": {
    "brightness": 93.0,
    "min_dim_level": 2.0
},
"color_temperature": {
    "mirek": null,
    "mirek_valid": false,
    "mirek_schema": {
        "mirek_minimum": 153,
        "mirek_maximum": 500
    }
},
"color": {
    "xy": {
        "x": 0.6367,
        "y": 0.3503
    },
    "gamut": {
        "red": {
            "x": 0.675,
            "y": 0.322
        },
        "green": {
            "x": 0.409,
            "y": 0.518
        },
        "blue": {
            "x": 0.167,
            "y": 0.04
        }
    },
    "gamut_type": "B"
},

[andrewfg]

they represent the values of one byte and therefore decimals don't make sense

@J-N-K I hear what you say but nevertheless all HSBType fields are stored as BigDecimal :)

[andrewfg]

@Gehetzter given what I said in the above post, I wonder if you tried the following? Obviously these are not directly accessible via Channels or in the UI, but you could perhaps access them via a rule?

double h = hsb.getHue().doubleValue()
double s = hsb.getSaturation().doubleValue();
double b = hsb.getBrightness().doubleValue();

[andrewfg]

^ And (BTW) the following should both produce an HSB with internal floating point parameters..

HSBType hsb = new HSBType(new DecimalType(11.111), new PercentType(BigDecimal.valueOf(22.222)), new PercentType(BigDecimal.valueOf(33.333)));

HSBType hsb = new HSBType("11.111,22.222,33.333");

[J-N-K]

@andrewfg Maybe you misunderstood me. What I wanted to say is: We could use the "exact" (i.e. not truncated) value when converting from RGB to HSB (or back), because PercentType and DecimalType (hue in HSB) support that. These values are 0-360° or 0-100% and having 5.5% or 37.5° makes sense. When we convert RGB to 0-255 decimals don't make sense, because when we use a range of 0-255 we usually want to store the value in a byte (or three bytes), so we should use int there.

[holgerfriedrich]

^ And (BTW) the following should both produce an HSB with internal floating point parameters..

HSBType hsb = new HSBType(new DecimalType(11.111), new PercentType(BigDecimal.valueOf(22.222)), new PercentType(BigDecimal.valueOf(33.333)));

HSBType hsb = new HSBType("11.111,22.222,33.333");

Yes, this works.... Just give me some time to upload the PR..... I am checking if addon tests are affected.

[Gehetzter]

@Gehetzter given what I said in the above post, I wonder if you tried the following? Obviously these are not directly accessible via Channels or in the UI, but you could perhaps access them via a rule?

double h = hsb.getHue().doubleValue()
double s = hsb.getSaturation().doubleValue();
double b = hsb.getBrightness().doubleValue();

Thanks, but no, this doesn`t work with a knx written item. The values hsb results are truncated in the binding during the conversion from rgb to hsb. Thats my whole point ;-)

[andrewfg]

When we convert RGB to 0-255 decimals don't make sense

@J-N-K yes, I hear you. My issue is that ColorUtil.xyToHsb() and ColorUtil.hsbToXY() should both be converting real XY inner values to/from real HSB inner values. However the problem is that both those conversions currently make a transition via an unnecessary integer rounded RGB 255 byte value. (And if there is a round trip XY -> HSB -> XY this requires two unnecessary integer rounded transitions). => We need to eliminate these artificial unnecessary transitions.

[holgerfriedrich]

Changes to core (ColorUtil) were introduced openhab/openhab-core#3540 / openhab/openhab-core#3542. The binding now converts RGB to HSB with decimals. Tests were added in #14875 to proof lossless conversion.

This is done for OH 4.x and is not intended to be back-ported to 3.4.x series. Current version of KNX plugin cannot be compiled for OH3 anymore due to the use of now features (Java 17 and core).

I am closing the ticket for now, @Gehetzter please reopen if you feel something is still missing.