Skip to content

Commit

Permalink
Adjust QuantityType calculations for temperatures (#3792)
Browse files Browse the repository at this point in the history 
* fix quantity calculations
* remove temperature offsetProfile workaround

Signed-off-by: Mark Herwege <[email protected]>
  • Loading branch information
@mherwege
mherwege authored Sep 8, 2023
1 parent 4364e3b commit 2b84752
Show file tree
Hide file tree
Showing 3 changed files with 173 additions and 33 deletions.
25 changes: 1 addition & 24 deletions ...ore.thing/src/main/java/org/openhab/core/thing/internal/profiles/SystemOffsetProfile.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,8 @@
import java.math.BigDecimal;

import javax.measure.UnconvertibleException;
import javax.measure.quantity.Temperature;

import org.eclipse.jdt.annotation.NonNullByDefault;
import org.eclipse.jdt.annotation.Nullable;
import org.openhab.core.library.types.DecimalType;
import org.openhab.core.library.types.QuantityType;
import org.openhab.core.library.unit.Units;
Expand Down Expand Up @@ -115,15 +113,7 @@ private Type applyOffset(Type state, boolean towardsItem) {
state, offset);
finalOffset = new QuantityType<>(finalOffset.toBigDecimal(), qtState.getUnit());
}
// take care of temperatures because they start at offset -273°C = 0K
if (Units.KELVIN.equals(qtState.getUnit().getSystemUnit())) {
QuantityType<Temperature> tmp = handleTemperature(qtState, finalOffset);
if (tmp != null) {
result = tmp;
}
} else {
result = qtState.add(finalOffset);
}
result = qtState.add(finalOffset);
} catch (UnconvertibleException e) {
logger.warn("Cannot apply offset '{}' to state '{}' because types do not match.", finalOffset, qtState);
}
Expand All @@ -137,17 +127,4 @@ private Type applyOffset(Type state, boolean towardsItem) {
}
return result;
}

@SuppressWarnings("null")
private @Nullable QuantityType<Temperature> handleTemperature(QuantityType<Temperature> qtState,
QuantityType<Temperature> offset) {
// do the math in Kelvin and afterwards convert it back to the unit of the state
final QuantityType<Temperature> kelvinState = qtState.toUnit(Units.KELVIN);
final QuantityType<Temperature> kelvinOffset = offset.toUnitRelative(Units.KELVIN);
if (kelvinState == null || kelvinOffset == null) {
return null;
}

return kelvinState.add(kelvinOffset).toUnit(qtState.getUnit());
}
}
48 changes: 40 additions & 8 deletions bundles/org.openhab.core/src/main/java/org/openhab/core/library/types/QuantityType.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,7 @@ public QuantityType(Number value, Unit<T> unit) {
* @param quantity the {@link Quantity} for the new {@link QuantityType}.
*/
private QuantityType(Quantity<T> quantity) {
this.quantity = quantity;
this.quantity = (Quantity<T>) Quantities.getQuantity(quantity.getValue(), quantity.getUnit(), Scale.RELATIVE);
}

/**
Expand Down Expand Up @@ -475,7 +475,9 @@ public String toFullString() {
* @return the sum of the given {@link QuantityType} with this QuantityType.
*/
public QuantityType<T> add(QuantityType<T> state) {
return new QuantityType<>(this.quantity.add(state.quantity));
Quantity<T> quantity = Quantities.getQuantity(this.quantity.getValue(), this.quantity.getUnit(),
Scale.ABSOLUTE);
return new QuantityType<>(quantity.add(state.quantity));
}

/**
Expand All @@ -494,7 +496,9 @@ public QuantityType<T> negate() {
* @return the difference by subtracting the given {@link QuantityType} from this QuantityType.
*/
public QuantityType<T> subtract(QuantityType<T> state) {
return new QuantityType<>(this.quantity.subtract(state.quantity));
Quantity<T> quantity = Quantities.getQuantity(this.quantity.getValue(), this.quantity.getUnit(),
Scale.ABSOLUTE);
return new QuantityType<>(quantity.subtract(state.quantity));
}

/**
Expand All @@ -504,7 +508,9 @@ public QuantityType<T> subtract(QuantityType<T> state) {
* @return the product of the given value with this {@link QuantityType}.
*/
public QuantityType<?> multiply(BigDecimal value) {
return new QuantityType<>(this.quantity.multiply(value));
Quantity<T> quantity = Quantities.getQuantity(this.quantity.getValue(), this.quantity.getUnit(),
Scale.ABSOLUTE);
return new QuantityType<>(quantity.multiply(value));
}

/**
Expand All @@ -514,7 +520,17 @@ public QuantityType<?> multiply(BigDecimal value) {
* @return the product of the given {@link QuantityType} and this QuantityType.
*/
public QuantityType<?> multiply(QuantityType<?> state) {
return new QuantityType<>(this.quantity.multiply(state.quantity));
Quantity<T> quantity = Quantities.getQuantity(this.quantity.getValue(), this.quantity.getUnit(),
Scale.ABSOLUTE);
Quantity<?> stateQuantity = Quantities.getQuantity(state.quantity.getValue(), state.quantity.getUnit(),
Scale.ABSOLUTE);
QuantityType<?> result = new QuantityType<>(quantity.multiply(stateQuantity));
// If dimension did not change from dimension of one of the arguments, reapply the unit so add associativity is
// guaranteed
Unit<?> unit = result.getUnit();
QuantityType<?> convertedResult = getUnit().isCompatible(unit) ? result.toUnit(getUnit())
: state.getUnit().isCompatible(unit) ? result.toUnit(state.getUnit()) : result;
return convertedResult == null ? result : convertedResult;
}

/**
Expand All @@ -524,7 +540,9 @@ public QuantityType<?> multiply(QuantityType<?> state) {
* @return the quotient from this QuantityType and the given value.
*/
public QuantityType<?> divide(BigDecimal value) {
return new QuantityType<>(this.quantity.divide(value));
Quantity<T> quantity = Quantities.getQuantity(this.quantity.getValue(), this.quantity.getUnit(),
Scale.ABSOLUTE);
return new QuantityType<>(quantity.divide(value));
}

/**
Expand All @@ -534,7 +552,17 @@ public QuantityType<?> divide(BigDecimal value) {
* @return the quotient from this QuantityType and the given {@link QuantityType}.
*/
public QuantityType<?> divide(QuantityType<?> state) {
return new QuantityType<>(this.quantity.divide(state.quantity));
Quantity<T> quantity = Quantities.getQuantity(this.quantity.getValue(), this.quantity.getUnit(),
Scale.ABSOLUTE);
Quantity<?> stateQuantity = Quantities.getQuantity(state.quantity.getValue(), state.quantity.getUnit(),
Scale.ABSOLUTE);
QuantityType<?> result = new QuantityType<>(quantity.divide(stateQuantity));
// If dimension did not change from dimension of one of the arguments, reapply the unit so add associativity is
// guaranteed
Unit<?> unit = result.getUnit();
QuantityType<?> convertedResult = getUnit().isCompatible(unit) ? result.toUnit(getUnit())
: state.getUnit().isCompatible(unit) ? result.toUnit(state.getUnit()) : result;
return convertedResult == null ? result : convertedResult;
}

/**
Expand All @@ -544,6 +572,8 @@ public QuantityType<?> divide(QuantityType<?> state) {
* @return changed QuantityType by offset
*/
public QuantityType<T> offset(QuantityType<T> offset, Unit<T> unit) {
Quantity<T> quantity = Quantities.getQuantity(this.quantity.getValue(), this.quantity.getUnit(),
Scale.ABSOLUTE);
final Quantity<T> sum = Arrays.asList(quantity, offset.quantity).stream().reduce(QuantityFunctions.sum(unit))
.get();
return new QuantityType<>(sum);
Expand All @@ -555,6 +585,8 @@ public QuantityType<T> offset(QuantityType<T> offset, Unit<T> unit) {
* @return a QuantityType with both the value and unit reciprocated
*/
public QuantityType<?> inverse() {
return new QuantityType<>(this.quantity.inverse());
Quantity<T> quantity = Quantities.getQuantity(this.quantity.getValue(), this.quantity.getUnit(),
Scale.ABSOLUTE);
return new QuantityType<>(quantity.inverse());
}
}
133 changes: 132 additions & 1 deletion bundles/org.openhab.core/src/test/java/org/openhab/core/library/types/QuantityTypeTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@

import static org.hamcrest.CoreMatchers.is;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.number.IsCloseTo.closeTo;
import static org.junit.jupiter.api.Assertions.*;
import static org.openhab.core.library.unit.MetricPrefix.CENTI;

Expand Down Expand Up @@ -340,6 +341,26 @@ public void testAdd(Locale locale) {

QuantityType<?> result = new QuantityType<>("20 m").add(new QuantityType<>("20cm"));
assertThat(result, is(new QuantityType<>("20.20 m")));

assertThat(new QuantityType<>("65 °F").add(new QuantityType<>("1 °F")), is(new QuantityType<>("66 °F")));
assertThat(new QuantityType<>("65 °F").add(new QuantityType<>("2 °F")), is(new QuantityType<>("67 °F")));
assertThat(new QuantityType<>("1 °F").add(new QuantityType<>("65 °F")), is(new QuantityType<>("66 °F")));
assertThat(new QuantityType<>("2 °F").add(new QuantityType<>("65 °F")), is(new QuantityType<>("67 °F")));

result = new QuantityType<>("65 °F").add(new QuantityType<>("5 °C")).toUnit("°F");
assertThat(result.doubleValue(), is(closeTo(74d, 0.0000000000000001d)));
assertEquals(ImperialUnits.FAHRENHEIT, result.getUnit());

// test associativity of add
QuantityType<Temperature> tempResult = new QuantityType<Temperature>("1 °F")
.add(new QuantityType<Temperature>("2 °F")).add(new QuantityType<Temperature>("3 °F"));
assertThat(tempResult, is(new QuantityType<Temperature>("1 °F")
.add(new QuantityType<Temperature>("2 °F").add(new QuantityType<Temperature>("3 °F")))));
assertThat(tempResult, is(new QuantityType<Temperature>("6 °F")));

assertThat(new QuantityType<>("65 kWh").add(new QuantityType<>("1 kWh")), is(new QuantityType<>("66 kWh")));
assertThat(new QuantityType<>("65 kJ").add(new QuantityType<>("1 kJ")), is(new QuantityType<>("66 kJ")));
assertThat(new QuantityType<>("65 kWh").add(new QuantityType<>("1 kJ")), is(new QuantityType<>("234001 kJ")));
}

@Test
Expand All @@ -354,16 +375,73 @@ public void testSubtract(Locale locale) {

QuantityType<?> result = new QuantityType<>("20 m").subtract(new QuantityType<>("20cm"));
assertThat(result, is(new QuantityType<>("19.80 m")));

assertThat(new QuantityType<>("65 °F").subtract(new QuantityType<>("1 °F")), is(new QuantityType<>("64 °F")));
assertThat(new QuantityType<>("65 °F").subtract(new QuantityType<>("2 °F")), is(new QuantityType<>("63 °F")));
assertThat(new QuantityType<>("1 °F").subtract(new QuantityType<>("65 °F")), is(new QuantityType<>("-64 °F")));
assertThat(new QuantityType<>("2 °F").subtract(new QuantityType<>("65 °F")), is(new QuantityType<>("-63 °F")));

assertThat(new QuantityType<>("65 kWh").subtract(new QuantityType<>("1 kWh")),
is(new QuantityType<>("64 kWh")));
assertThat(new QuantityType<>("65 kJ").subtract(new QuantityType<>("1 kJ")), is(new QuantityType<>("64 kJ")));
assertThat(new QuantityType<>("65 kWh").subtract(new QuantityType<>("1 kJ")),
is(new QuantityType<>("233999 kJ")));
}

@Test
public void testMultiplyNumber() {
assertThat(new QuantityType<>("2 m").multiply(BigDecimal.valueOf(2)), is(new QuantityType<>("4 m")));

assertThat(new QuantityType<>("65 °F").multiply(BigDecimal.valueOf(1)).toUnit("°F").doubleValue(),
is(closeTo(65d, 0.0000000000000001d)));
assertThat(new QuantityType<>("65 °F").multiply(BigDecimal.valueOf(2)).toUnit("°F").doubleValue(),
is(closeTo(589.67d, 0.0000000000000001d)));
}

@Test
public void testMultiplyQuantityType() {
QuantityType<?> result;

assertThat(new QuantityType<>("2 m").multiply(new QuantityType<>("4 cm")), is(new QuantityType<>("8 m·cm")));

// Make sure the original unit is preserved when multiplying with dimensionless, so add associativity is
// guaranteed
result = new QuantityType<>("65 °F").multiply(QuantityType.valueOf(1, Units.ONE));
assertThat(result.doubleValue(), is(closeTo(65d, 0.0000000000000001d)));
assertEquals(ImperialUnits.FAHRENHEIT, result.getUnit());
result = new QuantityType<>("65 °F").multiply(QuantityType.valueOf(2, Units.ONE));
assertThat(result.doubleValue(), is(closeTo(589.67d, 0.0000000000000001d)));
assertEquals(ImperialUnits.FAHRENHEIT, result.getUnit());
result = QuantityType.valueOf(1, Units.ONE).multiply(new QuantityType<>("65 °F"));
assertThat(result.doubleValue(), is(closeTo(65d, 0.0000000000000001d)));
assertEquals(ImperialUnits.FAHRENHEIT, result.getUnit());
result = QuantityType.valueOf(2, Units.ONE).multiply(new QuantityType<>("65 °F"));
assertThat(result.doubleValue(), is(closeTo(589.67d, 0.0000000000000001d)));
assertEquals(ImperialUnits.FAHRENHEIT, result.getUnit());

result = new QuantityType<>("65 °F").multiply(new QuantityType<>("1 °F"));
assertThat(result.doubleValue(), is(closeTo(74598.68175925925925925925925925927d, 0.0000000000000001d)));
assertEquals(Units.KELVIN.multiply(Units.KELVIN), result.getUnit());
result = new QuantityType<>("65 °F").multiply(new QuantityType<>("2 °F"));
assertThat(result.doubleValue(), is(closeTo(74760.6169444444444444444444444444d, 0.0000000000000001d)));
assertEquals(Units.KELVIN.multiply(Units.KELVIN), result.getUnit());
result = new QuantityType<>("1 °F").multiply(new QuantityType<>("65 °F"));
assertThat(result.doubleValue(), is(closeTo(74598.68175925925925925925925925927d, 0.0000000000000001d)));
assertEquals(Units.KELVIN.multiply(Units.KELVIN), result.getUnit());
result = new QuantityType<>("2 °F").multiply(new QuantityType<>("65 °F"));
assertThat(result.doubleValue(), is(closeTo(74760.6169444444444444444444444444d, 0.0000000000000001d)));
assertEquals(Units.KELVIN.multiply(Units.KELVIN), result.getUnit());

assertThat(new QuantityType<>("65 kWh").multiply(QuantityType.valueOf(1, Units.ONE)),
is(new QuantityType<>("65 kWh")));
assertThat(new QuantityType<>("65 kJ").multiply(QuantityType.valueOf(1, Units.ONE)),
is(new QuantityType<>("65 kJ")));
assertThat(new QuantityType<>("65 kWh").multiply(new QuantityType<>("1 kWh")),
is(new QuantityType<>(65, Units.KILOWATT_HOUR.multiply(Units.KILOWATT_HOUR))));
assertThat(new QuantityType<>("65 kJ").multiply(new QuantityType<>("1 kJ")),
is(new QuantityType<>(65, MetricPrefix.KILO(Units.JOULE).multiply(MetricPrefix.KILO(Units.JOULE)))));
assertThat(new QuantityType<>("65 kWh").multiply(new QuantityType<>("1 kJ")),
is(new QuantityType<>(65, Units.KILOWATT_HOUR.multiply(MetricPrefix.KILO(Units.JOULE)))));
}

@ParameterizedTest
Expand All @@ -372,14 +450,67 @@ public void testDivideNumber(Locale locale) {
Locale.setDefault(locale);

assertThat(new QuantityType<>("4 m").divide(BigDecimal.valueOf(2)), is(new QuantityType<>("2 m")));

assertThat(new QuantityType<>("65 °F").divide(BigDecimal.valueOf(1)).toUnit("°F").doubleValue(),
is(closeTo(65d, 0.0000000000000001d)));
assertThat(new QuantityType<>("65 °F").divide(BigDecimal.valueOf(2)).toUnit("°F").doubleValue(),
is(closeTo(-197.335d, 0.0000000000000001d)));
}

@ParameterizedTest
@MethodSource("locales")
public void testDivideQuantityType(Locale locale) {
Locale.setDefault(locale);

QuantityType<?> result;

assertThat(new QuantityType<>("4 m").divide(new QuantityType<>("2 cm")), is(new QuantityType<>("2 m/cm")));

// Make sure the original unit is preserved when dividing with dimensionless, so add associativity is guaranteed
result = new QuantityType<>("65 °F").divide(QuantityType.valueOf(1, Units.ONE));
assertThat(result.doubleValue(), is(closeTo(65d, 0.0000000000000001d)));
assertEquals(ImperialUnits.FAHRENHEIT, result.getUnit());
result = new QuantityType<>("65 °F").divide(QuantityType.valueOf(2, Units.ONE));
assertThat(result.doubleValue(), is(closeTo(-197.335d, 0.0000000000000001d)));
assertEquals(ImperialUnits.FAHRENHEIT, result.getUnit());
result = QuantityType.valueOf(1, Units.ONE).divide(new QuantityType<>("65 °F"));
assertThat(result.doubleValue(), is(closeTo(0.003430727886099834181485505174681228d, 0.0000000000000001d)));
assertEquals(Units.KELVIN.inverse(), result.getUnit());
result = QuantityType.valueOf(2, Units.ONE).divide(new QuantityType<>("65 °F"));
assertThat(result.doubleValue(), is(closeTo(0.006861455772199668362971010349362456d, 0.0000000000000001d)));
assertEquals(Units.KELVIN.inverse(), result.getUnit());

result = new QuantityType<>("65 °F").divide(new QuantityType<>("1 °F"));
assertThat(result.doubleValue(),
is(closeTo(1.138928083009529598193934920876115122114246640762367855514793670089202480d,
0.0000000000000001d)));
assertEquals(Units.ONE, result.getUnit());
result = new QuantityType<>("65 °F").divide(new QuantityType<>("2 °F"));
assertThat(result.doubleValue(),
is(closeTo(1.136461108584053544739749171486126553533555353390950245846600385556783676d,
0.0000000000000001d)));
assertEquals(Units.ONE, result.getUnit());
result = new QuantityType<>("1 °F").divide(new QuantityType<>("65 °F"));
assertThat(result.doubleValue(),
is(closeTo(0.878018564049783673547182038233556349552596235093804994885674169795613456d,
0.0000000000000001d)));
assertEquals(Units.ONE, result.getUnit());
result = new QuantityType<>("2 °F").divide(new QuantityType<>("65 °F"));
assertThat(result.doubleValue(),
is(closeTo(0.879924523986505803648007318886157031927295252253797625173918844225890256d,
0.0000000000000001d)));
assertEquals(Units.ONE, result.getUnit());

assertThat(new QuantityType<>("65 kWh").divide(QuantityType.valueOf(1, Units.ONE)),
is(new QuantityType<>("65 kWh")));
assertThat(new QuantityType<>("65 kJ").divide(QuantityType.valueOf(1, Units.ONE)),
is(new QuantityType<>("65 kJ")));
assertThat(new QuantityType<>("65 kWh").divide(new QuantityType<>("1 kWh")),
is(new QuantityType<>(65, Units.ONE)));
assertThat(new QuantityType<>("65 kJ").divide(new QuantityType<>("1 kJ")),
is(new QuantityType<>(65, Units.ONE)));
assertThat(new QuantityType<>("65 kWh").divide(new QuantityType<>("1 kJ")).toUnit(Units.ONE),
is(new QuantityType<>(234000, Units.ONE)));
}

@ParameterizedTest
Expand Down Expand Up @@ -500,7 +631,7 @@ public void testMireds() {

@Test
public void testRelativeConversion() {
QuantityType<Temperature> c = new QuantityType("1 °C");
QuantityType<Temperature> c = new QuantityType<>("1 °C");
QuantityType<Temperature> f = c.toUnitRelative(ImperialUnits.FAHRENHEIT);
assertEquals(1.8, f.doubleValue());
}
Expand Down

0 comments on commit 2b84752

Please sign in to comment.