Skip to content

Commit de951f5

Browse files
authored
[basicprofiles] Fix regular comparison of Percent Quantity interpreted as $DELTA_PERCENT check (openhab#18121)
* [basicprofiles] Fix regular comparison of Percent Quantity interpreted as $DELTA_PERCENT check Signed-off-by: Jimmy Tanagra <[email protected]>
1 parent 087a511 commit de951f5

File tree

3 files changed

+69
-21
lines changed

3 files changed

+69
-21
lines changed

bundles/org.openhab.transform.basicprofiles/README.md

+19-3
Original file line numberDiff line numberDiff line change
@@ -229,8 +229,9 @@ The `LHS_OPERAND` and the `RHS_OPERAND` can be either one of these:
229229
For example, with an initial data of `10`, a new data of `12` or `8` would both result in a $DELTA of `2`.
230230
- `$DELTA_PERCENT` to represent the difference in percentage.
231231
It is calculated as `($DELTA / current_data) * 100`.
232-
Note that this can also be done by omitting the `LHS_OPERAND` and using a number followed with a percent sign `%` as the `RHS_OPERAND`.
233-
See the example below.
232+
Note in most cases, this check can be written without `$DELTA_PERCENT`, e.g. `> 5%`. It is equivalent to `$DELTA_PERCENT > 5`.
233+
However, when the incoming value from the binding is a Percent Quantity Type, for example a Humidity data in %, the `$DELTA_PERCENT` must be explicitly written in order to perform a delta percent check.
234+
See the examples below.
234235
- `$AVERAGE`, or `$AVG` to represent the average of the previous unfiltered incoming values.
235236
- `$STDDEV` to represent the _population_ standard deviation of the previous unfiltered incoming values.
236237
- `$MEDIAN` to represent the median value of the previous unfiltered incoming values.
@@ -296,12 +297,27 @@ Number:Temperature BoilerTemperature {
296297
channel="mybinding:mything:mychannel" [ profile="basic-profiles:state-filter", conditions="$DELTA_PERCENT > 10" ]
297298
}
298299

299-
// Or more succinctly:
300+
// Or more succinctly, the $DELTA_PERCENT is inferred here
300301
Number:Temperature BoilerTemperature {
301302
channel="mybinding:mything:mychannel" [ profile="basic-profiles:state-filter", conditions="> 10%" ]
302303
}
303304
```
304305

306+
When the incoming value from the binding is a Percent Quantity Type:
307+
308+
```java
309+
// This performs a value comparison, not a delta percent comparison.
310+
// Because the incoming value is a Percent Quantity, it isn't inferred as a $DELTA_PERCENT check.
311+
Number:Dimensionless Humidity {
312+
channel="mybinding:mything:humidity" [ profile="basic-profiles:state-filter", conditions="> 0%, <= 100%" ]
313+
}
314+
315+
// To actually perform a $DELTA_PERCENT check against a Percent Quantity data, specify it explicitly
316+
Number:Dimensionless Humidity {
317+
channel="mybinding:mything:humidity" [ profile="basic-profiles:state-filter", conditions="$DELTA_PERCENT > 5%" ]
318+
}
319+
```
320+
305321
The incoming state can be compared against other items:
306322

307323
```java

bundles/org.openhab.transform.basicprofiles/src/main/java/org/openhab/transform/basicprofiles/internal/profiles/StateFilterProfile.java

+23-15
Original file line numberDiff line numberDiff line change
@@ -278,9 +278,7 @@ FunctionType parseFunction(String functionDefinition) {
278278
String functionName = matcher.group(1).toUpperCase(Locale.ROOT);
279279
try {
280280
FunctionType.Function type = FunctionType.Function.valueOf(functionName);
281-
282-
Optional<Integer> windowSize = Optional.empty();
283-
windowSize = Optional.ofNullable(matcher.group(2)).map(Integer::parseInt);
281+
Optional<Integer> windowSize = Optional.ofNullable(matcher.group(2)).map(Integer::parseInt);
284282
return new FunctionType(type, windowSize);
285283
} catch (IllegalArgumentException e) {
286284
logger.warn("Invalid function name: '{}'. Expected one of: {}", functionName,
@@ -306,16 +304,8 @@ class StateCondition {
306304

307305
public StateCondition(String lhs, ComparisonType comparisonType, String rhs) {
308306
this.comparisonType = comparisonType;
309-
310-
if (lhs.isEmpty() && rhs.endsWith("%")) {
311-
// Allow comparing percentages without a left hand side,
312-
// e.g. `> 50%` -> translate this to `$DELTA_PERCENT > 50`
313-
lhsString = "$DELTA_PERCENT";
314-
rhsString = rhs.substring(0, rhs.length() - 1).trim();
315-
} else {
316-
lhsString = lhs;
317-
rhsString = rhs;
318-
}
307+
lhsString = lhs;
308+
rhsString = rhs;
319309
// Convert quoted strings to StringType, and UnDefTypes to UnDefType
320310
// UnDefType gets special treatment because we don't want `UNDEF` to be parsed as a string
321311
// Anything else, defer parsing until we're checking the condition
@@ -357,7 +347,23 @@ public boolean check(State input) {
357347

358348
if (lhsString.isEmpty()) {
359349
lhsItem = getLinkedItem();
360-
lhsState = input;
350+
// special handling for `> 50%` condition
351+
// we need to calculate the delta percent between the input and the accepted state
352+
// but if the input is an actual Percent Quantity, perform a direct comparison between input and rhs
353+
if (rhsString.endsWith("%")) {
354+
// late-parsing because now we have the input state and can determine its type
355+
if (input instanceof QuantityType qty && "%".equals(qty.getUnit().getSymbol())) {
356+
lhsState = input;
357+
} else {
358+
lhsString = "$DELTA_PERCENT";
359+
// Override rhsString and this.lhsState to avoid re-parsing them later
360+
rhsString = rhsString.substring(0, rhsString.length() - 1).trim();
361+
this.lhsState = new FunctionType(FunctionType.Function.DELTA_PERCENT, Optional.empty());
362+
lhsState = this.lhsState;
363+
}
364+
} else {
365+
lhsState = input;
366+
}
361367
} else if (lhsState == null) {
362368
lhsItem = getItemOrNull(lhsString);
363369
lhsState = itemStateOrParseState(lhsItem, lhsString, rhsItem);
@@ -368,7 +374,9 @@ public boolean check(State input) {
368374
lhsString, rhsString);
369375
return false;
370376
}
371-
} else if (lhsState instanceof FunctionType lhsFunction) {
377+
}
378+
379+
if (lhsState instanceof FunctionType lhsFunction) {
372380
if (acceptedState == UnDefType.UNDEF && (lhsFunction.getType() == FunctionType.Function.DELTA
373381
|| lhsFunction.getType() == FunctionType.Function.DELTA_PERCENT)) {
374382
return true;

bundles/org.openhab.transform.basicprofiles/src/test/java/org/openhab/transform/basicprofiles/internal/profiles/StateFilterProfileTest.java

+27-3
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,9 @@
2828
import java.util.Map;
2929
import java.util.stream.Stream;
3030

31+
import javax.measure.quantity.Dimensionless;
32+
import javax.measure.quantity.Power;
33+
3134
import org.eclipse.jdt.annotation.NonNullByDefault;
3235
import org.junit.jupiter.api.BeforeEach;
3336
import org.junit.jupiter.api.Test;
@@ -670,10 +673,13 @@ public void testComparingInputStateWithItem(GenericItem linkedItem, State inputS
670673

671674
public static Stream<Arguments> testFunctions() {
672675
NumberItem powerItem = new NumberItem("Number:Power", "powerItem", UNIT_PROVIDER);
676+
NumberItem percentItem = new NumberItem("Number:Dimensionless", "percentItem", UNIT_PROVIDER);
673677
NumberItem decimalItem = new NumberItem("decimalItem");
674678
List<Number> numbers = List.of(1, 2, 3, 4, 5);
675679
List<Number> negatives = List.of(-1, -2, -3, -4, -5);
676-
List<QuantityType> quantities = numbers.stream().map(n -> new QuantityType(n, Units.WATT)).toList();
680+
List<QuantityType<Power>> quantities = numbers.stream().map(n -> new QuantityType<>(n, Units.WATT)).toList();
681+
List<QuantityType<Dimensionless>> percentQuantities = numbers.stream()
682+
.map(n -> new QuantityType<>(n, Units.PERCENT)).toList();
677683
List<DecimalType> decimals = numbers.stream().map(DecimalType::new).toList();
678684
List<DecimalType> negativeDecimals = negatives.stream().map(DecimalType::new).toList();
679685

@@ -715,14 +721,32 @@ public static Stream<Arguments> testFunctions() {
715721
Arguments.of(decimalItem, "$DELTA_PERCENT < 10", decimals, DecimalType.valueOf("0.91"), true), //
716722
Arguments.of(decimalItem, "$DELTA_PERCENT < 10", decimals, DecimalType.valueOf("0.89"), false), //
717723

718-
Arguments.of(decimalItem, "$DELTA_PERCENT < 10", negativeDecimals, DecimalType.valueOf("0"), false), //
719-
Arguments.of(decimalItem, "10 > $DELTA_PERCENT", negativeDecimals, DecimalType.valueOf("0"), false), //
724+
Arguments.of(decimalItem, "$DELTA_PERCENT < 10", negativeDecimals, DecimalType.valueOf("0"), false),
725+
//
726+
Arguments.of(decimalItem, "10 > $DELTA_PERCENT", negativeDecimals, DecimalType.valueOf("0"), false),
727+
//
720728

721729
Arguments.of(decimalItem, "< 10%", decimals, DecimalType.valueOf("1.09"), true), //
722730
Arguments.of(decimalItem, "< 10%", decimals, DecimalType.valueOf("1.11"), false), //
723731
Arguments.of(decimalItem, "< 10%", decimals, DecimalType.valueOf("0.91"), true), //
724732
Arguments.of(decimalItem, "< 10%", decimals, DecimalType.valueOf("0.89"), false), //
725733

734+
// Contrast a simple comparison against a Percent QuantityType vs delta percent check
735+
Arguments.of(percentItem, "> 5%", percentQuantities, QuantityType.valueOf("5.1 %"), true), //
736+
Arguments.of(percentItem, "$DELTA_PERCENT > 5", percentQuantities, QuantityType.valueOf("5.1 %"),
737+
false), //
738+
739+
Arguments.of(percentItem, "> 5%", percentQuantities, QuantityType.valueOf("-10 %"), false), //
740+
Arguments.of(percentItem, "$DELTA_PERCENT > 5", percentQuantities, QuantityType.valueOf("-10 %"), true), //
741+
742+
Arguments.of(percentItem, "< 200%", percentQuantities, QuantityType.valueOf("100 %"), true), //
743+
Arguments.of(percentItem, "$DELTA_PERCENT < 200", percentQuantities, QuantityType.valueOf("100 %"),
744+
false), //
745+
746+
Arguments.of(percentItem, "< 200%", percentQuantities, QuantityType.valueOf("-100 %"), true), //
747+
Arguments.of(percentItem, "$DELTA_PERCENT < 200", percentQuantities, QuantityType.valueOf("-100 %"),
748+
false), //
749+
726750
Arguments.of(decimalItem, "1 == $MIN", decimals, DecimalType.valueOf("20"), true), //
727751
Arguments.of(decimalItem, "0 < $MIN", decimals, DecimalType.valueOf("20"), true), //
728752
Arguments.of(decimalItem, "$MIN > 0", decimals, DecimalType.valueOf("20"), true), //

0 commit comments

Comments
 (0)