Skip to content

Commit

Permalink
Fix QuantityType.toInvertibleUnit() to use system unit (#4561)
Browse files Browse the repository at this point in the history
* QuantityType toInvertibleUnit fix
* Improve unit tests
* Extend JavaDoc and tests

Signed-off-by: Andrew Fiddian-Green <software@whitebear.ch>
  • Loading branch information
andrewfg authored Feb 15, 2025
1 parent 5b28e6f commit 2d29ff5
Show file tree
Hide file tree
Showing 3 changed files with 227 additions and 7 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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.
* <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.
Expand All @@ -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);
}
Expand All @@ -337,7 +350,6 @@ && getUnit().inverse().isCompatible(targetUnit)) {
if (unit != null) {
return toInvertibleUnit(unit);
}

return null;
}

Expand Down Expand Up @@ -581,6 +593,17 @@ public String toFullString() {

/**
* 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.
Expand All @@ -603,6 +626,18 @@ public QuantityType<T> negate() {
/**
* 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.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -649,13 +649,14 @@ public void testDataTransferRate() {

@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
Expand All @@ -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<Temperature> temp1 = new QuantityType<>("293.15 K");
Expand Down
Original file line number Diff line number Diff line change
@@ -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);
}
}

0 comments on commit 2d29ff5

Please sign in to comment.