Fix QuantityType.toInvertibleUnit() to use system unit (#4561)

* QuantityType toInvertibleUnit fix
* Improve unit tests
* Extend JavaDoc and tests

Signed-off-by: Andrew Fiddian-Green <software@whitebear.ch>

bundles/org.openhab.core/src/main/java/org/openhab/core/library/types/QuantityType.java

@@ -317,8 +317,20 @@ public class QuantityType<T extends Quantity<T>> extends Number
     /**
      * Convert this QuantityType to a new {@link QuantityType} using the given target unit.
      *
-     * Implicit conversions using inverse units are allowed (i.e. {@code mired <=> Kelvin}). This may
-     * change the dimension.
+     * Implicit conversions using inverse units are allowed (i.e. {@code mired <=> Kelvin} / {@code Hertz <=> Second} /
+     * {@code Ohm <=> Siemens}). This may change the dimension.
+     * <p>
+     * This method converts the quantity from its actual unit to its respective system unit before converting to the
+     * inverse unit. This enables it to support not only conversions {@code mired <=> Kelvin} but also conversions
+     * {@code mired <=> Fahrenheit} and {@code mired <=> Celsius}.
+     * <p>
+     * Notes on units not yet implemented in openHAB:
+     * <li>The optics unit {@code Dioptre} ({@code dpt} / {@code D}) is the inverse of length ({@code m-1}); if it were
+     * added it would give correct results.</li>
+     * <li>The optics unit {@code Kaiser} for wave number is also the inverse of length ({@code cm-1}); it is old and
+     * not commonly used, but if it were added it would NOT give correct results.</li>
+     * <li>If you discover other units similar to {@code Kaiser} above: => Please inform openHAB maintainers.</li>
+     * <p>
      *
      * @param targetUnit the unit to which this {@link QuantityType} will be converted to.
      * @return the new {@link QuantityType} in the given {@link Unit} or {@code null} in case of an error.
@@ -327,7 +339,8 @@ public class QuantityType<T extends Quantity<T>> extends Number
         // only invert if unit is not equal and inverse is compatible and targetUnit is not ONE
         if (!targetUnit.equals(getUnit()) && !targetUnit.isCompatible(AbstractUnit.ONE)
                 && getUnit().inverse().isCompatible(targetUnit)) {
-            return inverse().toUnit(targetUnit);
+            QuantityType<?> systemQuantity = toUnit(getUnit().getSystemUnit());
+            return systemQuantity == null ? null : systemQuantity.inverse().toUnit(targetUnit);
         }
         return toUnit(targetUnit);
     }
@@ -337,7 +350,6 @@ public class QuantityType<T extends Quantity<T>> extends Number
         if (unit != null) {
             return toInvertibleUnit(unit);
         }
-
         return null;
     }
 
@@ -581,6 +593,17 @@ public class QuantityType<T extends Quantity<T>> extends Number
 
     /**
      * Returns the sum of the given {@link QuantityType} with this QuantityType.
+     * <p>
+     * The result is an incremental addition where the operand is interpreted as an amount to be added based on the
+     * difference between its value converted to the unit of this instance and the zero point of the unit of this
+     * instance. So for example:
+     * <li>Expression '{@code new QuantityType("20 °C").add(new QuantityType("30 °C")}' gives '{@code 50 °C}'</li>
+     * <li>Expression '{@code new QuantityType("20 °C").add(new QuantityType("30 K")}' gives '{@code 50 °C}'</li>
+     * <li>Expression '{@code new QuantityType("20 °C").add(new QuantityType("54 °F")}' gives '{@code 50 °C}'</li>
+     * <li>Expression '{@code new QuantityType("20 K").add(new QuantityType("30 °C")}' gives '{@code 50 K}'</li>
+     * <li>Expression '{@code new QuantityType("20 K").add(new QuantityType("30 K")}' gives '{@code 50 K}'</li>
+     * <li>Expression '{@code new QuantityType("20 K").add(new QuantityType("54 °F")}' gives '{@code 50 K}'</li>
+     * <p>
      *
      * @param state the {@link QuantityType} to add to this QuantityType.
      * @return the sum of the given {@link QuantityType} with this QuantityType.
@@ -603,6 +626,18 @@ public class QuantityType<T extends Quantity<T>> extends Number
     /**
      * Subtract the given {@link QuantityType} from this QuantityType.
      *
+     * <p>
+     * The result is an incremental subtraction where the operand is interpreted as an amount to be subtracted based on
+     * the difference between its value converted to the unit of this instance and the zero point of the unit of this
+     * instance. So for example:
+     * <li>Expression '{@code new QuantityType("50 °C").subtract(new QuantityType("30 °C")}' gives '{@code 20 °C}'</li>
+     * <li>Expression '{@code new QuantityType("50 °C").subtract(new QuantityType("30 K")}' gives '{@code 20 °C}'</li>
+     * <li>Expression '{@code new QuantityType("50 °C").subtract(new QuantityType("54 °F")}' gives '{@code 20 °C}'</li>
+     * <li>Expression '{@code new QuantityType("50 K").subtract(new QuantityType("30 °C")}' gives '{@code 20 K}'</li>
+     * <li>Expression '{@code new QuantityType("50 K").subtract(new QuantityType("30 K")}' gives '{@code 20 K}'</li>
+     * <li>Expression '{@code new QuantityType("50 K").subtract(new QuantityType("54 °F")}' gives '{@code 20 K}'</li>
+     * <p>
+     *
      * @param state the {@link QuantityType} to subtract from this QuantityType.
      * @return the difference by subtracting the given {@link QuantityType} from this QuantityType.
      */

bundles/org.openhab.core/src/test/java/org/openhab/core/library/types/QuantityTypeTest.java

@@ -649,13 +649,14 @@ public class QuantityTypeTest {
 
     @Test
     public void testMireds() {
-        QuantityType<Temperature> colorTemp = new QuantityType<>("2700 K");
+        // test value is selected to prevent any round-trip rounding errors
+        QuantityType<Temperature> colorTemp = new QuantityType<>("2000 K");
         QuantityType<?> mireds = colorTemp.toInvertibleUnit(Units.MIRED);
-        assertEquals(370, mireds.intValue());
+        assertEquals(500, mireds.intValue());
         assertThat(colorTemp.equals(mireds), is(true));
         assertThat(mireds.equals(colorTemp), is(true));
         QuantityType<?> andBack = mireds.toInvertibleUnit(Units.KELVIN);
-        assertEquals(2700, andBack.intValue());
+        assertEquals(2000, andBack.intValue());
     }
 
     @Test
@@ -665,6 +666,28 @@ public class QuantityTypeTest {
         assertEquals(1.8, f.doubleValue());
     }
 
+    @Test
+    @SuppressWarnings({ "rawtypes", "unchecked" })
+    public void testIncrementalAdd() {
+        assertEquals(new QuantityType("50 °C"), new QuantityType("20 °C").add(new QuantityType("30 °C")));
+        assertEquals(new QuantityType("50 °C"), new QuantityType("20 °C").add(new QuantityType("30 K")));
+        assertEquals(new QuantityType("50 °C"), new QuantityType("20 °C").add(new QuantityType("54 °F")));
+        assertEquals(new QuantityType("50 K"), new QuantityType("20 K").add(new QuantityType("30 °C")));
+        assertEquals(new QuantityType("50 K"), new QuantityType("20 K").add(new QuantityType("30 K")));
+        assertEquals(new QuantityType("50 K"), new QuantityType("20 K").add(new QuantityType("54 °F")));
+    }
+
+    @Test
+    @SuppressWarnings({ "rawtypes", "unchecked" })
+    public void testIncrementalSubtract() {
+        assertEquals(new QuantityType("20 °C"), new QuantityType("50 °C").subtract(new QuantityType("30 °C")));
+        assertEquals(new QuantityType("20 °C"), new QuantityType("50 °C").subtract(new QuantityType("30 K")));
+        assertEquals(new QuantityType("20 °C"), new QuantityType("50 °C").subtract(new QuantityType("54 °F")));
+        assertEquals(new QuantityType("20 K"), new QuantityType("50 K").subtract(new QuantityType("30 °C")));
+        assertEquals(new QuantityType("20 K"), new QuantityType("50 K").subtract(new QuantityType("30 K")));
+        assertEquals(new QuantityType("20 K"), new QuantityType("50 K").subtract(new QuantityType("54 °F")));
+    }
+
     @Test
     public void testEquals() {
         QuantityType<Temperature> temp1 = new QuantityType<>("293.15 K");

bundles/org.openhab.core/src/test/java/org/openhab/core/library/types/QuantityTypeToInvertibleUnitTest.java

@@ -0,0 +1,162 @@
+/*
+ * Copyright (c) 2010-2025 Contributors to the openHAB project
+ *
+ * See the NOTICE file(s) distributed with this work for additional
+ * information.
+ *
+ * This program and the accompanying materials are made available under the
+ * terms of the Eclipse Public License 2.0 which is available at
+ * http://www.eclipse.org/legal/epl-2.0
+ *
+ * SPDX-License-Identifier: EPL-2.0
+ */
+package org.openhab.core.library.types;
+
+import static org.junit.jupiter.api.Assertions.*;
+
+import org.eclipse.jdt.annotation.NonNullByDefault;
+import org.junit.jupiter.api.Test;
+import org.openhab.core.library.unit.ImperialUnits;
+import org.openhab.core.library.unit.SIUnits;
+import org.openhab.core.library.unit.Units;
+
+/**
+ * Tests for {@link QuantityType#toInvertibleUnit}.
+ *
+ * @author Andrew Fiddian-Green - Initial contribution
+ */
+@NonNullByDefault
+class QuantityTypeToInvertibleUnitTest {
+
+    /**
+     * Test conversion some other UoM values to Mirek
+     */
+    @Test
+    void testConversionsToMirek() {
+        // pass cases
+        QuantityType<?> v1 = QuantityType.valueOf(2000, Units.KELVIN).toInvertibleUnit(Units.MIRED);
+        QuantityType<?> v2 = QuantityType.valueOf(1726.85, SIUnits.CELSIUS).toInvertibleUnit(Units.MIRED);
+        QuantityType<?> v3 = QuantityType.valueOf(3140.33, ImperialUnits.FAHRENHEIT).toInvertibleUnit(Units.MIRED);
+
+        assertNotNull(v1);
+        assertNotNull(v2);
+        assertNotNull(v3);
+
+        assertEquals(500, v1.doubleValue(), 0.01);
+        assertEquals(500, v2.doubleValue(), 0.01);
+        assertEquals(500, v3.doubleValue(), 0.01);
+
+        // fail case
+        assertNull(QuantityType.valueOf(500, SIUnits.METRE).toInvertibleUnit(Units.MIRED));
+    }
+
+    /**
+     * Test conversions from Mirek to other UoM values
+     */
+    @Test
+    void testConversionsFromMirek() {
+        // pass cases
+        QuantityType<?> m = QuantityType.valueOf(500, Units.MIRED);
+
+        QuantityType<?> v1 = m.toInvertibleUnit(Units.KELVIN);
+        QuantityType<?> v2 = m.toInvertibleUnit(SIUnits.CELSIUS);
+        QuantityType<?> v3 = m.toInvertibleUnit(ImperialUnits.FAHRENHEIT);
+
+        assertNotNull(v1);
+        assertNotNull(v2);
+        assertNotNull(v3);
+
+        assertEquals(2000, v1.doubleValue(), 0.01);
+        assertEquals(1726.85, v2.doubleValue(), 0.01);
+        assertEquals(3140.33, v3.doubleValue(), 0.01);
+
+        // fail case
+        assertNull(m.toInvertibleUnit(SIUnits.METRE));
+    }
+
+    /**
+     * Test conversions from UoM values to themselves
+     */
+    @Test
+    void testConversionsToSelf() {
+        // pass cases
+        QuantityType<?> v1 = QuantityType.valueOf(500, Units.MIRED).toInvertibleUnit(Units.MIRED);
+        QuantityType<?> v2 = QuantityType.valueOf(2000, Units.KELVIN).toInvertibleUnit(Units.KELVIN);
+
+        assertNotNull(v1);
+        assertNotNull(v2);
+
+        assertEquals(500, v1.doubleValue(), 0.01);
+        assertEquals(2000, v2.doubleValue(), 0.01);
+    }
+
+    /**
+     * Test conversion Ohm to/from Siemens
+     */
+    @Test
+    void testOhmAndSiemensConversions() {
+        // pass cases
+        QuantityType<?> v1 = QuantityType.valueOf(10, Units.OHM).toInvertibleUnit(Units.SIEMENS);
+        QuantityType<?> v2 = QuantityType.valueOf(0.1, Units.SIEMENS).toInvertibleUnit(Units.OHM);
+
+        assertNotNull(v1);
+        assertNotNull(v2);
+
+        assertEquals(0.1, v1.doubleValue(), 0.01);
+        assertEquals(10, v2.doubleValue(), 0.01);
+
+        // fail cases
+        assertNull(v1.toInvertibleUnit(SIUnits.METRE));
+        assertNull(v2.toInvertibleUnit(SIUnits.METRE));
+        assertNull(QuantityType.valueOf(5, Units.OHM).toInvertibleUnit(SIUnits.METRE));
+    }
+
+    /**
+     * Test time and frequency conversion
+     */
+    @Test
+    void testHertzSecondConversions() {
+        // pass cases
+        QuantityType<?> v1 = QuantityType.valueOf(10, Units.HERTZ).toInvertibleUnit(Units.SECOND);
+        QuantityType<?> v2 = QuantityType.valueOf(0.1, Units.SECOND).toInvertibleUnit(Units.HERTZ);
+
+        assertNotNull(v1);
+        assertNotNull(v2);
+
+        assertEquals(0.1, v1.doubleValue(), 0.01);
+        assertEquals(10, v2.doubleValue(), 0.01);
+
+        // fail cases
+        assertNull(v1.toInvertibleUnit(SIUnits.METRE));
+        assertNull(v2.toInvertibleUnit(SIUnits.METRE));
+        assertNull(QuantityType.valueOf(5, Units.HERTZ).toInvertibleUnit(SIUnits.METRE));
+    }
+
+    /**
+     * Test dimensionless conversion
+     */
+    @Test
+    void testDimensionlessConversion() {
+        assertNotNull(QuantityType.valueOf(100, Units.ONE).toInvertibleUnit(Units.ONE));
+    }
+
+    /**
+     * Some addons mistakenly call {@link QuantityType#toInvertibleUnit} instead of {@link QuantityType#toUnit}. The
+     * good news is that when the target unit is not an inversion then the former method falls through to the latter.
+     * However for good hygiene we should test that such calls do indeed also return the proper results.
+     */
+    @Test
+    void testNonInvertingConversions() {
+        QuantityType<?> v1 = QuantityType.valueOf(600, Units.SECOND).toInvertibleUnit(Units.MINUTE);
+        QuantityType<?> v2 = QuantityType.valueOf(100, ImperialUnits.FAHRENHEIT).toInvertibleUnit(SIUnits.CELSIUS);
+        QuantityType<?> v3 = QuantityType.valueOf(100, ImperialUnits.FOOT).toInvertibleUnit(SIUnits.METRE);
+
+        assertNotNull(v1);
+        assertNotNull(v2);
+        assertNotNull(v3);
+
+        assertEquals(10, v1.doubleValue(), 0.01);
+        assertEquals(37.78, v2.doubleValue(), 0.01);
+        assertEquals(30.48, v3.doubleValue(), 0.01);
+    }
+}