Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[basicprofiles] Fix regular comparison of Percent Quantity interpreted as $DELTA_PERCENT check #18121

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

[basicprofiles] Fix regular comparison of Percent Quantity interpreted as $DELTA_PERCENT check #18121

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 19 additions & 3 deletions bundles/org.openhab.transform.basicprofiles/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -229,8 +229,9 @@ The `LHS_OPERAND` and the `RHS_OPERAND` can be either one of these:
For example, with an initial data of `10`, a new data of `12` or `8` would both result in a $DELTA of `2`.
- `$DELTA_PERCENT` to represent the difference in percentage.
It is calculated as `($DELTA / current_data) * 100`.
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`.
See the example below.
Note in most cases, this check can be written without `$DELTA_PERCENT`, e.g. `> 5%`. It is equivalent to `$DELTA_PERCENT > 5`.
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.
See the examples below.
- `$AVERAGE`, or `$AVG` to represent the average of the previous unfiltered incoming values.
- `$STDDEV` to represent the _population_ standard deviation of the previous unfiltered incoming values.
- `$MEDIAN` to represent the median value of the previous unfiltered incoming values.
Expand Down Expand Up @@ -296,12 +297,27 @@ Number:Temperature BoilerTemperature {
channel="mybinding:mything:mychannel" [ profile="basic-profiles:state-filter", conditions="$DELTA_PERCENT > 10" ]
}

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

When the incoming value from the binding is a Percent Quantity Type:

```java
// This performs a value comparison, not a delta percent comparison.
// Because the incoming value is a Percent Quantity, it isn't inferred as a $DELTA_PERCENT check.
Number:Dimensionless Humidity {
channel="mybinding:mything:humidity" [ profile="basic-profiles:state-filter", conditions="> 0%, <= 100%" ]
}

// To actually perform a $DELTA_PERCENT check against a Percent Quantity data, specify it explicitly
Number:Dimensionless Humidity {
channel="mybinding:mything:humidity" [ profile="basic-profiles:state-filter", conditions="$DELTA_PERCENT > 5%" ]
}
```

The incoming state can be compared against other items:

```java
Expand Down
38 changes: 23 additions & 15 deletions ...c/main/java/org/openhab/transform/basicprofiles/internal/profiles/StateFilterProfile.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -277,9 +277,7 @@ FunctionType parseFunction(String functionDefinition) {
String functionName = matcher.group(1).toUpperCase(Locale.ROOT);
try {
FunctionType.Function type = FunctionType.Function.valueOf(functionName);

Optional<Integer> windowSize = Optional.empty();
windowSize = Optional.ofNullable(matcher.group(2)).map(Integer::parseInt);
Optional<Integer> windowSize = Optional.ofNullable(matcher.group(2)).map(Integer::parseInt);
return new FunctionType(type, windowSize);
} catch (IllegalArgumentException e) {
logger.warn("Invalid function name: '{}'. Expected one of: {}", functionName,
Expand All @@ -305,16 +303,8 @@ class StateCondition {

public StateCondition(String lhs, ComparisonType comparisonType, String rhs) {
this.comparisonType = comparisonType;

if (lhs.isEmpty() && rhs.endsWith("%")) {
// Allow comparing percentages without a left hand side,
// e.g. `> 50%` -> translate this to `$DELTA_PERCENT > 50`
lhsString = "$DELTA_PERCENT";
rhsString = rhs.substring(0, rhs.length() - 1).trim();
} else {
lhsString = lhs;
rhsString = rhs;
}
lhsString = lhs;
rhsString = rhs;
// Convert quoted strings to StringType, and UnDefTypes to UnDefType
// UnDefType gets special treatment because we don't want `UNDEF` to be parsed as a string
// Anything else, defer parsing until we're checking the condition
Expand Down Expand Up @@ -357,7 +347,23 @@ public boolean check(State input) {

if (lhsString.isEmpty()) {
lhsItem = getLinkedItem();
lhsState = input;
// special handling for `> 50%` condition
// we need to calculate the delta percent between the input and the accepted state
// but if the input is an actual Percent Quantity, perform a direct comparison between input and rhs
if (rhsString.endsWith("%")) {
// late-parsing because now we have the input state and can determine its type
if (input instanceof QuantityType qty && "%".equals(qty.getUnit().getSymbol())) {
lhsState = input;
} else {
lhsString = "$DELTA_PERCENT";
// Override rhsString and this.lhsState to avoid re-parsing them later
rhsString = rhsString.substring(0, rhsString.length() - 1).trim();
this.lhsState = new FunctionType(FunctionType.Function.DELTA_PERCENT, Optional.empty());
lhsState = this.lhsState;
}
} else {
lhsState = input;
}
} else if (lhsState == null) {
lhsItem = getItemOrNull(lhsString);
lhsState = itemStateOrParseState(lhsItem, lhsString, rhsItem);
Expand All @@ -368,7 +374,9 @@ public boolean check(State input) {
lhsString, rhsString);
return false;
}
} else if (lhsState instanceof FunctionType lhsFunction) {
}

if (lhsState instanceof FunctionType lhsFunction) {
if (acceptedState == UnDefType.UNDEF && (lhsFunction.getType() == FunctionType.Function.DELTA
|| lhsFunction.getType() == FunctionType.Function.DELTA_PERCENT)) {
acceptedState = input;
Expand Down
25 changes: 23 additions & 2 deletions ...st/java/org/openhab/transform/basicprofiles/internal/profiles/StateFilterProfileTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -670,10 +670,13 @@ public void testComparingInputStateWithItem(GenericItem linkedItem, State inputS

public static Stream<Arguments> testFunctions() {
NumberItem powerItem = new NumberItem("Number:Power", "powerItem", UNIT_PROVIDER);
NumberItem percentItem = new NumberItem("Number:Dimensionless", "percentItem", UNIT_PROVIDER);
NumberItem decimalItem = new NumberItem("decimalItem");
List<Number> numbers = List.of(1, 2, 3, 4, 5);
List<Number> negatives = List.of(-1, -2, -3, -4, -5);
List<QuantityType> quantities = numbers.stream().map(n -> new QuantityType(n, Units.WATT)).toList();
List<QuantityType> percentQuantities = numbers.stream().map(n -> new QuantityType(String.format("%d %%", n)))
.toList();
List<DecimalType> decimals = numbers.stream().map(DecimalType::new).toList();
List<DecimalType> negativeDecimals = negatives.stream().map(DecimalType::new).toList();

Expand Down Expand Up @@ -711,14 +714,32 @@ public static Stream<Arguments> testFunctions() {
Arguments.of(decimalItem, "$DELTA_PERCENT < 10", decimals, DecimalType.valueOf("0.91"), true), //
Arguments.of(decimalItem, "$DELTA_PERCENT < 10", decimals, DecimalType.valueOf("0.89"), false), //

Arguments.of(decimalItem, "$DELTA_PERCENT < 10", negativeDecimals, DecimalType.valueOf("0"), false), //
Arguments.of(decimalItem, "10 > $DELTA_PERCENT", negativeDecimals, DecimalType.valueOf("0"), false), //
Arguments.of(decimalItem, "$DELTA_PERCENT < 10", negativeDecimals, DecimalType.valueOf("0"), false),
//
Arguments.of(decimalItem, "10 > $DELTA_PERCENT", negativeDecimals, DecimalType.valueOf("0"), false),
//

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

// Contrast a simple comparison against a Percent QuantityType vs delta percent check
Arguments.of(percentItem, "> 5%", percentQuantities, QuantityType.valueOf("5.1 %"), true), //
Arguments.of(percentItem, "$DELTA_PERCENT > 5", percentQuantities, QuantityType.valueOf("5.1 %"),
false), //

Arguments.of(percentItem, "> 5%", percentQuantities, QuantityType.valueOf("-10 %"), false), //
Arguments.of(percentItem, "$DELTA_PERCENT > 5", percentQuantities, QuantityType.valueOf("-10 %"), true), //

Arguments.of(percentItem, "< 200%", percentQuantities, QuantityType.valueOf("100 %"), true), //
Arguments.of(percentItem, "$DELTA_PERCENT < 200", percentQuantities, QuantityType.valueOf("100 %"),
false), //

Arguments.of(percentItem, "< 200%", percentQuantities, QuantityType.valueOf("-100 %"), true), //
Arguments.of(percentItem, "$DELTA_PERCENT < 200", percentQuantities, QuantityType.valueOf("-100 %"),
false), //

Arguments.of(decimalItem, "1 == $MIN", decimals, DecimalType.valueOf("20"), true), //
Arguments.of(decimalItem, "0 < $MIN", decimals, DecimalType.valueOf("20"), true), //
Arguments.of(decimalItem, "$MIN > 0", decimals, DecimalType.valueOf("20"), true), //
Expand Down