Fix Color Temperature conversion of values represented in Celsius

[andrewfg]

Affects the following PRs

• [amazonechocontrol] Support QuantityType Color Temperature command #17919
• [deconz] Support QuantityType Color Temperature command #17942
• [lifx] Fix QuantityType Color Temperature command #17943
• [tapocontrol] Support QuantityType Color Temperature command #17944
• [tplinksmarthome] Support QuantityType Color Temperature command #17946
• [shelly] Support QuantityType Color Temperature command #17947
• [hue] Improve QuantityType Color Temperature command #17948

The above-mentioned PRs added code to convert color temperature UoM values from Kelvin to Mirek. However this overlooked a potential problem when the user would have a color temperature item that (wrongly) represents its color temperature in Celsius (or probably also Fahrenheit).

Whereas the conversion Kelvin => Mirek works fine, the conversion Celsius => Mirek throws an exception. The solution is to convert such values in two steps as follows Celsius => Kelvin => Mirek.

[maniac103]

Whereas the conversion Kelvin => Mirek works fine, the conversion Celsius => Mirek throws an exception. The solution is to convert such values in two steps as follows Celsius => Kelvin => Mirek.

Wouldn't it be a better solution to make it stop throwing that exception instead? ;-)
If the 3-step solution works fine, I don't see why the 2-step equivalent shouldn't. Or is the exception thrown in some code we do not control?

[andrewfg]

a better solution to make it stop throwing that exception instead?
is the exception thrown in some code we do not control?

Your suggestion is quite correct. And the issue is definitely in our own code (link below) where in the case of Celsius or Fahrenheit the if statement evaluation on lines 325 and 326 does pass, yet the code throws an exception in the toUnit() conversion on line 327.

=> So this seems indeed to be a bug in OH core!! @maniac103 good catch!

https://github.com/openhab/openhab-core/blob/15eb5cccd3133c746fe773f5b29401d0fa60259b/bundles/org.openhab.core/src/main/java/org/openhab/core/library/types/QuantityType.java#L323

However I am not sure the proper way forward. It would be easy enough just to plug in a hard coded fix for colour temperature. However I think probably the code should really fix the issue for all units. And/or there may be some maintainers who prefer for this case to actually continue to throw an exception here??

---

PS test cases are here..

    @Test
    void testKelvinToMirekOk() {
        assertNotNull(QuantityType.valueOf(2000, Units.KELVIN).toInvertibleUnit(Units.MIRED));
    }

    @Test
    void testMirekToMirekOk() {
        assertNotNull(QuantityType.valueOf(500, Units.MIRED).toInvertibleUnit(Units.MIRED));
    }

    @Test
    void testNonTemperatureReturnsNullOk() {
        assertNull(QuantityType.valueOf(500, SIUnits.METRE).toInvertibleUnit(Units.MIRED));
    }

    @Test
    void testCelsiusToMirekShouldBeOkButActuallyFails() {
        assertNotNull(QuantityType.valueOf(2000, SIUnits.CELSIUS).toInvertibleUnit(Units.MIRED));
    }

    @Test
    void testFahrenheitToMirekShouldBeOkButActuallyFails() {
        assertNotNull(QuantityType.valueOf(3000, ImperialUnits.FAHRENHEIT).toInvertibleUnit(Units.MIRED));
    }

[andrewfg]

By the way -- for the avoidance of doubt -- the REVERSE conversions do NOT throw an exception for Celsius or Fahrenheit!

    @Test
    void testMirekToCelsius() {
        assertNotNull(QuantityType.valueOf(500, Units.MIRED).toInvertibleUnit(SIUnits.CELSIUS));
    }

    @Test
    void testMirekToFahrenheit() {
        assertNotNull(QuantityType.valueOf(500, Units.MIRED).toInvertibleUnit(ImperialUnits.FAHRENHEIT));
    }

[openhab-bot]

This issue has been mentioned on openHAB Community. There might be relevant details there:

https://community.openhab.org/t/openhab-4-3-milestone-discussion/158139/123

[maniac103]

For completenesss, the exception thrown is

java.lang.UnsupportedOperationException: °C is non-linear, cannot convert
	at tech.units.indriya.unit.ProductUnit.getSystemConverter(ProductUnit.java:321) ~[bundleFile:2.2]
	at tech.units.indriya.AbstractUnit.getConverterToAny(AbstractUnit.java:327) ~[bundleFile:2.2]
	at org.openhab.core.library.types.QuantityType.toUnit(QuantityType.java:290) ~[bundleFile:?]
	at org.openhab.core.library.types.QuantityType.toInvertibleUnit(QuantityType.java:325) ~[bundleFile:?]

The non-linearity happens because Celsius is defined as Kelvin + offset and thus the inverse of it becomes non-linear.

However, since (judging from Javadoc toInvertibleUnit is made for conversions to and from Mirek, I don't see an issue in adding a special case there.
I wonder how it should look like, though. Maybe check for getUnit().isCompatible(Units.KELVIN) && !getUnit().equals(Units.KELVIN) and do the intermediate conversion in that case?

[boehan]

I'm by no means an expert on color temperature and corresponding units, but IMO core code is correct here.
While mired and K are invertible as mired = 1 / K, this is not the case with °C (mired =! 1 / °C) or °F. This is also stated in the comment in L324 that toInvertibleUnit only inverts if inverse units are compatible. To achieve invertibility with mired, °C and °F have to be converted to K first.

[mherwege]

First question, is Color Temperature expressed in Celcius or Fahrenheit a real thing (does it make sense, is it used anywhere outside of OH), or is this because the unit is not properly set in the users' configuration in openHAB?
To my knowledge, all unit conversions are handled in the libraries we include, except these invertible unit conversions. So we must establish if the conversion makes sense in the first place.

It is very tricky for temperatures in general, as the reference point is not the same for the different temperature units. I spent quite a bit of time to come up with some reasonble logic when doing temperature calculations in OH (openhab/openhab-core#3792), which illustrates the difficulty. I was trying to preserve commutativity and distributivity of math operations on temperatures with different unit (while losing distributivity in some cases along the way). And obviously none of this is preserved when you bring mireds in the mix.

I believe that only a conversion from Kelvin to Mired makes sense. Which means if we need to support it for Celcius, we may have to take care of always converting to Kelvin before doing this. But I would actually opt to catch the error and write a warning in the log that the item does not have the right unit in the first place.

And I hate to say this, and I understand why invertible units have been created to be in the same dimension, but this is yet another example where the assumption that an inverted unit is of the same dimension is a stretch. It breaks normal math logic on values in the dimension.

[andrewfg]

You guys may want to have a look at the proposed PR to fix this in OH Core

openhab/openhab-core#4561