From 96fe905dd54912324dd919e1a13c381f0d42a76d Mon Sep 17 00:00:00 2001 From: Jimmy Tanagra Date: Sat, 11 Jan 2025 23:01:57 +1000 Subject: [PATCH 1/5] [basicprofiles] Fix division-by-zero error in $DELTA_PERCENT state filter Signed-off-by: Jimmy Tanagra --- .../internal/profiles/StateFilterProfile.java | 22 +++++++++++++++---- .../profiles/StateFilterProfileTest.java | 8 +++++++ 2 files changed, 26 insertions(+), 4 deletions(-) diff --git a/bundles/org.openhab.transform.basicprofiles/src/main/java/org/openhab/transform/basicprofiles/internal/profiles/StateFilterProfile.java b/bundles/org.openhab.transform.basicprofiles/src/main/java/org/openhab/transform/basicprofiles/internal/profiles/StateFilterProfile.java index 137cfd31509df..2674d12850bd0 100644 --- a/bundles/org.openhab.transform.basicprofiles/src/main/java/org/openhab/transform/basicprofiles/internal/profiles/StateFilterProfile.java +++ b/bundles/org.openhab.transform.basicprofiles/src/main/java/org/openhab/transform/basicprofiles/internal/profiles/StateFilterProfile.java @@ -339,8 +339,7 @@ public boolean check(State input) { if (rhsState == null) { rhsItem = getItemOrNull(rhsString); } else if (rhsState instanceof FunctionType rhsFunction) { - if (acceptedState == UnDefType.UNDEF && (rhsFunction.getType() == FunctionType.Function.DELTA - || rhsFunction.getType() == FunctionType.Function.DELTA_PERCENT)) { + if (rhsFunction.alwaysAccept()) { return true; } rhsItem = getLinkedItem(); @@ -378,8 +377,7 @@ public boolean check(State input) { } if (lhsState instanceof FunctionType lhsFunction) { - if (acceptedState == UnDefType.UNDEF && (lhsFunction.getType() == FunctionType.Function.DELTA - || lhsFunction.getType() == FunctionType.Function.DELTA_PERCENT)) { + if (lhsFunction.alwaysAccept()) { return true; } lhsItem = getLinkedItem(); @@ -567,6 +565,22 @@ public FunctionType(Function type, Optional windowSize) { }; } + public boolean alwaysAccept() { + if ((type == Function.DELTA || type == Function.DELTA_PERCENT) && acceptedState == UnDefType.UNDEF) { + return true; + } + if (type == Function.DELTA_PERCENT) { + // avoid division by zero + if (acceptedState instanceof QuantityType base) { + return base.toBigDecimal().compareTo(BigDecimal.ZERO) == 0; + } + if (acceptedState instanceof DecimalType base) { + return base.toBigDecimal().compareTo(BigDecimal.ZERO) == 0; + } + } + return false; + } + @Override public @Nullable T as(@Nullable Class target) { if (target == DecimalType.class || target == QuantityType.class) { diff --git a/bundles/org.openhab.transform.basicprofiles/src/test/java/org/openhab/transform/basicprofiles/internal/profiles/StateFilterProfileTest.java b/bundles/org.openhab.transform.basicprofiles/src/test/java/org/openhab/transform/basicprofiles/internal/profiles/StateFilterProfileTest.java index 706872ac33679..6286eeb7f2ad6 100644 --- a/bundles/org.openhab.transform.basicprofiles/src/test/java/org/openhab/transform/basicprofiles/internal/profiles/StateFilterProfileTest.java +++ b/bundles/org.openhab.transform.basicprofiles/src/test/java/org/openhab/transform/basicprofiles/internal/profiles/StateFilterProfileTest.java @@ -731,6 +731,14 @@ public static Stream testFunctions() { Arguments.of(decimalItem, "< 10%", decimals, DecimalType.valueOf("0.91"), true), // Arguments.of(decimalItem, "< 10%", decimals, DecimalType.valueOf("0.89"), false), // + // Check against possible division-by-zero errors in $DELTA_PERCENT + Arguments.of(decimalItem, "> 10%", List.of(DecimalType.ZERO), DecimalType.valueOf("1"), true), // + Arguments.of(decimalItem, "< 10%", List.of(DecimalType.ZERO), DecimalType.valueOf("1"), true), // + + // Check against possible division-by-zero errors in $DELTA_PERCENT + Arguments.of(decimalItem, "> 10%", List.of(DecimalType.ZERO), DecimalType.valueOf("1"), true), // + Arguments.of(decimalItem, "< 10%", List.of(DecimalType.ZERO), DecimalType.valueOf("1"), true), // + // 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 %"), From 0e07cd4e518fd3043c25981342fef54cc0c666e3 Mon Sep 17 00:00:00 2001 From: Jimmy Tanagra Date: Mon, 3 Feb 2025 15:47:52 +1000 Subject: [PATCH 2/5] remove duplicate tests Signed-off-by: Jimmy Tanagra --- .../internal/profiles/StateFilterProfileTest.java | 6 ------ 1 file changed, 6 deletions(-) diff --git a/bundles/org.openhab.transform.basicprofiles/src/test/java/org/openhab/transform/basicprofiles/internal/profiles/StateFilterProfileTest.java b/bundles/org.openhab.transform.basicprofiles/src/test/java/org/openhab/transform/basicprofiles/internal/profiles/StateFilterProfileTest.java index 6286eeb7f2ad6..3fc2468217406 100644 --- a/bundles/org.openhab.transform.basicprofiles/src/test/java/org/openhab/transform/basicprofiles/internal/profiles/StateFilterProfileTest.java +++ b/bundles/org.openhab.transform.basicprofiles/src/test/java/org/openhab/transform/basicprofiles/internal/profiles/StateFilterProfileTest.java @@ -722,9 +722,7 @@ public static Stream testFunctions() { 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, "< 10%", decimals, DecimalType.valueOf("1.09"), true), // Arguments.of(decimalItem, "< 10%", decimals, DecimalType.valueOf("1.11"), false), // @@ -735,10 +733,6 @@ public static Stream testFunctions() { Arguments.of(decimalItem, "> 10%", List.of(DecimalType.ZERO), DecimalType.valueOf("1"), true), // Arguments.of(decimalItem, "< 10%", List.of(DecimalType.ZERO), DecimalType.valueOf("1"), true), // - // Check against possible division-by-zero errors in $DELTA_PERCENT - Arguments.of(decimalItem, "> 10%", List.of(DecimalType.ZERO), DecimalType.valueOf("1"), true), // - Arguments.of(decimalItem, "< 10%", List.of(DecimalType.ZERO), DecimalType.valueOf("1"), true), // - // 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 %"), From fe4886acd9220fb3dfea9ebcc250a51dbdad70ef Mon Sep 17 00:00:00 2001 From: Jimmy Tanagra Date: Mon, 3 Feb 2025 20:20:27 +1000 Subject: [PATCH 3/5] use Optional for acceptedState Signed-off-by: Jimmy Tanagra --- .../internal/profiles/StateFilterProfile.java | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/bundles/org.openhab.transform.basicprofiles/src/main/java/org/openhab/transform/basicprofiles/internal/profiles/StateFilterProfile.java b/bundles/org.openhab.transform.basicprofiles/src/main/java/org/openhab/transform/basicprofiles/internal/profiles/StateFilterProfile.java index 2674d12850bd0..83c5a0b965911 100644 --- a/bundles/org.openhab.transform.basicprofiles/src/main/java/org/openhab/transform/basicprofiles/internal/profiles/StateFilterProfile.java +++ b/bundles/org.openhab.transform.basicprofiles/src/main/java/org/openhab/transform/basicprofiles/internal/profiles/StateFilterProfile.java @@ -103,7 +103,7 @@ public class StateFilterProfile implements StateProfile { private @Nullable Item linkedItem = null; private State newState = UnDefType.UNDEF; - private State acceptedState = UnDefType.UNDEF; + private Optional acceptedState = Optional.empty(); private LinkedList previousStates = new LinkedList<>(); private final int windowSize; @@ -230,7 +230,7 @@ private State checkCondition(State state) { } if (conditions.stream().allMatch(c -> c.check(state))) { - acceptedState = state; + acceptedState = Optional.of(state); return state; } else { return configMismatchState; @@ -566,15 +566,15 @@ public FunctionType(Function type, Optional windowSize) { } public boolean alwaysAccept() { - if ((type == Function.DELTA || type == Function.DELTA_PERCENT) && acceptedState == UnDefType.UNDEF) { + if ((type == Function.DELTA || type == Function.DELTA_PERCENT) && acceptedState.isEmpty()) { return true; } if (type == Function.DELTA_PERCENT) { // avoid division by zero - if (acceptedState instanceof QuantityType base) { + if (acceptedState.get() instanceof QuantityType base) { return base.toBigDecimal().compareTo(BigDecimal.ZERO) == 0; } - if (acceptedState instanceof DecimalType base) { + if (acceptedState.get() instanceof DecimalType base) { return base.toBigDecimal().compareTo(BigDecimal.ZERO) == 0; } } @@ -707,11 +707,11 @@ public String toString() { private @Nullable State calculateDelta() { if (newState instanceof QuantityType newStateQuantity) { - QuantityType result = newStateQuantity.subtract((QuantityType) acceptedState); + QuantityType result = newStateQuantity.subtract((QuantityType) acceptedState.get()); return result.toBigDecimal().compareTo(BigDecimal.ZERO) < 0 ? result.negate() : result; } BigDecimal result = ((DecimalType) newState).toBigDecimal() - .subtract(((DecimalType) acceptedState).toBigDecimal()) // + .subtract(((DecimalType) acceptedState.get()).toBigDecimal()) // .abs(); return new DecimalType(result); } @@ -720,13 +720,13 @@ public String toString() { State calculatedDelta = calculateDelta(); BigDecimal bdDelta; BigDecimal bdBase; - if (acceptedState instanceof QuantityType acceptedStateQuantity) { + if (acceptedState.get() instanceof QuantityType acceptedStateQuantity) { // Assume that delta and base are in the same unit bdDelta = ((QuantityType) calculatedDelta).toBigDecimal(); bdBase = acceptedStateQuantity.toBigDecimal(); } else { bdDelta = ((DecimalType) calculatedDelta).toBigDecimal(); - bdBase = ((DecimalType) acceptedState).toBigDecimal(); + bdBase = ((DecimalType) acceptedState.get()).toBigDecimal(); } bdBase = bdBase.abs(); BigDecimal percent = bdDelta.multiply(BigDecimal.valueOf(100)).divide(bdBase, 2, RoundingMode.HALF_EVEN); From 5de032e42857a719c24780beb03bf8e8b36370f0 Mon Sep 17 00:00:00 2001 From: Jimmy Tanagra Date: Mon, 3 Feb 2025 22:26:12 +1000 Subject: [PATCH 4/5] add empty checks Signed-off-by: Jimmy Tanagra --- .../basicprofiles/internal/profiles/StateFilterProfile.java | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/bundles/org.openhab.transform.basicprofiles/src/main/java/org/openhab/transform/basicprofiles/internal/profiles/StateFilterProfile.java b/bundles/org.openhab.transform.basicprofiles/src/main/java/org/openhab/transform/basicprofiles/internal/profiles/StateFilterProfile.java index 83c5a0b965911..69d37ff503ce2 100644 --- a/bundles/org.openhab.transform.basicprofiles/src/main/java/org/openhab/transform/basicprofiles/internal/profiles/StateFilterProfile.java +++ b/bundles/org.openhab.transform.basicprofiles/src/main/java/org/openhab/transform/basicprofiles/internal/profiles/StateFilterProfile.java @@ -706,6 +706,9 @@ public String toString() { } private @Nullable State calculateDelta() { + if (acceptedState.isEmpty()) { + return null; + } if (newState instanceof QuantityType newStateQuantity) { QuantityType result = newStateQuantity.subtract((QuantityType) acceptedState.get()); return result.toBigDecimal().compareTo(BigDecimal.ZERO) < 0 ? result.negate() : result; @@ -717,6 +720,9 @@ public String toString() { } private @Nullable State calculateDeltaPercent() { + if (acceptedState.isEmpty()) { + return null; + } State calculatedDelta = calculateDelta(); BigDecimal bdDelta; BigDecimal bdBase; From 8c3a3b694a21516913db876c4a08be9237f5f571 Mon Sep 17 00:00:00 2001 From: Jimmy Tanagra Date: Tue, 4 Feb 2025 23:16:52 +1000 Subject: [PATCH 5/5] add javadoc Signed-off-by: Jimmy Tanagra --- .../internal/profiles/StateFilterProfile.java | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/bundles/org.openhab.transform.basicprofiles/src/main/java/org/openhab/transform/basicprofiles/internal/profiles/StateFilterProfile.java b/bundles/org.openhab.transform.basicprofiles/src/main/java/org/openhab/transform/basicprofiles/internal/profiles/StateFilterProfile.java index 69d37ff503ce2..d717b25271064 100644 --- a/bundles/org.openhab.transform.basicprofiles/src/main/java/org/openhab/transform/basicprofiles/internal/profiles/StateFilterProfile.java +++ b/bundles/org.openhab.transform.basicprofiles/src/main/java/org/openhab/transform/basicprofiles/internal/profiles/StateFilterProfile.java @@ -565,6 +565,14 @@ public FunctionType(Function type, Optional windowSize) { }; } + /** + * If the profile uses the DELTA or DELTA_PERCENT functions, the new state value will always be accepted if the + * 'acceptedState' (prior state) has not yet been initialised, or -- in the case of the DELTA_PERCENT function + * only -- if 'acceptedState' has a zero value. This ensures that 'acceptedState' is always initialised. And it + * also ensures that the DELTA_PERCENT function cannot cause a divide by zero error. + * + * @return true if the new state value shall be accepted + */ public boolean alwaysAccept() { if ((type == Function.DELTA || type == Function.DELTA_PERCENT) && acceptedState.isEmpty()) { return true;