From 2d29ff5545e2744b649ff1f3cbb4383cd2c1a9cb Mon Sep 17 00:00:00 2001 From: Andrew Fiddian-Green Date: Sat, 15 Feb 2025 17:08:05 +0000 Subject: [PATCH] Fix QuantityType.toInvertibleUnit() to use system unit (#4561) * QuantityType toInvertibleUnit fix * Improve unit tests * Extend JavaDoc and tests Signed-off-by: Andrew Fiddian-Green --- .../core/library/types/QuantityType.java | 43 ++++- .../core/library/types/QuantityTypeTest.java | 29 +++- .../QuantityTypeToInvertibleUnitTest.java | 162 ++++++++++++++++++ 3 files changed, 227 insertions(+), 7 deletions(-) create mode 100644 bundles/org.openhab.core/src/test/java/org/openhab/core/library/types/QuantityTypeToInvertibleUnitTest.java diff --git a/bundles/org.openhab.core/src/main/java/org/openhab/core/library/types/QuantityType.java b/bundles/org.openhab.core/src/main/java/org/openhab/core/library/types/QuantityType.java index 572f677c78d..004ab670085 100644 --- a/bundles/org.openhab.core/src/main/java/org/openhab/core/library/types/QuantityType.java +++ b/bundles/org.openhab.core/src/main/java/org/openhab/core/library/types/QuantityType.java @@ -317,8 +317,20 @@ public Dimension getDimension() { /** * 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. + *

+ * 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}. + *

+ * Notes on units not yet implemented in openHAB: + *

  • 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.
    • + *
  • 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.
    • + *
  • If you discover other units similar to {@code Kaiser} above: => Please inform openHAB maintainers.
    • + *

    * * @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 Dimension getDimension() { // 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 @@ && getUnit().inverse().isCompatible(targetUnit)) { if (unit != null) { return toInvertibleUnit(unit); } - return null; } @@ -581,6 +593,17 @@ public String toFullString() { /** * Returns the sum of the given {@link QuantityType} with this QuantityType. + *

    + * 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: + *

  • Expression '{@code new QuantityType("20 °C").add(new QuantityType("30 °C")}' gives '{@code 50 °C}'
    • + *
  • Expression '{@code new QuantityType("20 °C").add(new QuantityType("30 K")}' gives '{@code 50 °C}'
    • + *
  • Expression '{@code new QuantityType("20 °C").add(new QuantityType("54 °F")}' gives '{@code 50 °C}'
    • + *
  • Expression '{@code new QuantityType("20 K").add(new QuantityType("30 °C")}' gives '{@code 50 K}'
    • + *
  • Expression '{@code new QuantityType("20 K").add(new QuantityType("30 K")}' gives '{@code 50 K}'
    • + *
  • Expression '{@code new QuantityType("20 K").add(new QuantityType("54 °F")}' gives '{@code 50 K}'
    • + *

    * * @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 QuantityType negate() { /** * Subtract the given {@link QuantityType} from this QuantityType. * + *

    + * 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: + *

  • Expression '{@code new QuantityType("50 °C").subtract(new QuantityType("30 °C")}' gives '{@code 20 °C}'
    • + *
  • Expression '{@code new QuantityType("50 °C").subtract(new QuantityType("30 K")}' gives '{@code 20 °C}'
    • + *
  • Expression '{@code new QuantityType("50 °C").subtract(new QuantityType("54 °F")}' gives '{@code 20 °C}'
    • + *
  • Expression '{@code new QuantityType("50 K").subtract(new QuantityType("30 °C")}' gives '{@code 20 K}'
    • + *
  • Expression '{@code new QuantityType("50 K").subtract(new QuantityType("30 K")}' gives '{@code 20 K}'
    • + *
  • Expression '{@code new QuantityType("50 K").subtract(new QuantityType("54 °F")}' gives '{@code 20 K}'
    • + *

    + * * @param state the {@link QuantityType} to subtract from this QuantityType. * @return the difference by subtracting the given {@link QuantityType} from this QuantityType. */ diff --git a/bundles/org.openhab.core/src/test/java/org/openhab/core/library/types/QuantityTypeTest.java b/bundles/org.openhab.core/src/test/java/org/openhab/core/library/types/QuantityTypeTest.java index a3fec4b0a1a..cb390239eb7 100644 --- a/bundles/org.openhab.core/src/test/java/org/openhab/core/library/types/QuantityTypeTest.java +++ b/bundles/org.openhab.core/src/test/java/org/openhab/core/library/types/QuantityTypeTest.java @@ -649,13 +649,14 @@ public void testDataTransferRate() { @Test public void testMireds() { - QuantityType colorTemp = new QuantityType<>("2700 K"); + // test value is selected to prevent any round-trip rounding errors + QuantityType 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 void testRelativeConversion() { 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 temp1 = new QuantityType<>("293.15 K"); diff --git a/bundles/org.openhab.core/src/test/java/org/openhab/core/library/types/QuantityTypeToInvertibleUnitTest.java b/bundles/org.openhab.core/src/test/java/org/openhab/core/library/types/QuantityTypeToInvertibleUnitTest.java new file mode 100644 index 00000000000..3f60bec4833 --- /dev/null +++ b/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); + } +}