Skip to content

Rules DSL: Support arithmetic operations directly on Number Item's State #5618

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

Draft
jimtng wants to merge 1 commit into
base: main
Choose a base branch
from
+161 −8
Draft

Rules DSL: Support arithmetic operations directly on Number Item's State #5618

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
56 changes: 48 additions & 8 deletions ...el.script/src.moved/test/java/org/openhab/core/model/script/lib/NumberExtensionsTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,11 @@
import org.junit.jupiter.params.provider.ValueSource;
import org.openhab.core.library.types.DecimalType;
import org.openhab.core.library.types.HSBType;
import org.openhab.core.library.types.OnOffType;
import org.openhab.core.library.types.PercentType;
import org.openhab.core.library.types.QuantityType;
import org.openhab.core.library.unit.Units;
import org.openhab.core.types.State;
import org.openhab.core.types.Type;

/**
Expand All @@ -53,12 +55,12 @@ public class NumberExtensionsTest {

@Test
public void operatorPlusNumberNumber() {
assertThat(NumberExtensions.operator_plus(DECIMAL1, DECIMAL2), is(BigDecimal.valueOf(3)));
assertThat(NumberExtensions.operator_plus((Number) DECIMAL1, (Number) DECIMAL2), is(BigDecimal.valueOf(3)));

@wborn wborn Aug 11, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The need to add these casts to previously unambiguous calls looks like an API compatibility regression caused by the new overloads.

DecimalType implements both Number and State, so after adding both:

operator_plus(Number, Number)
operator_plus(State, State)

neither overload is more specific for a DecimalType argument and Java requires an explicit cast.

This package is exported by org.openhab.core.model.script, so external Java code directly using these extension methods could also stop compiling after updating openHAB, even though existing binaries would continue to work.

It would be preferable to avoid introducing this ambiguity rather than adapting the existing tests to it. It would also be useful to verify that equivalent Rules DSL expressions with variables statically typed as DecimalType, PercentType, etc. do not become ambiguous for the same reason.

}

@Test
public void operatorPlusNumberQuantityOne() {
assertThat(NumberExtensions.operator_plus(Q_ONE_1, DECIMAL2), is(BigDecimal.valueOf(3)));
assertThat(NumberExtensions.operator_plus((Number) Q_ONE_1, (Number) DECIMAL2), is(BigDecimal.valueOf(3)));
}

@Test
Expand All @@ -68,7 +70,7 @@ public void operatorPlusQuantityQuantity() {

@Test
public void operatorMinusNumber() {
assertThat(NumberExtensions.operator_minus(DECIMAL1), is(BigDecimal.valueOf(-1)));
assertThat(NumberExtensions.operator_minus((Number) DECIMAL1), is(BigDecimal.valueOf(-1)));
}

@Test
Expand All @@ -78,12 +80,12 @@ public void operatorMinusQuantity() {

@Test
public void operatorMinusNumberNumber() {
assertThat(NumberExtensions.operator_minus(DECIMAL2, DECIMAL1), is(BigDecimal.ONE));
assertThat(NumberExtensions.operator_minus((Number) DECIMAL2, (Number) DECIMAL1), is(BigDecimal.ONE));
}

@Test
public void operatorMinusNumberQuantityOne() {
assertThat(NumberExtensions.operator_minus(Q_ONE_2, DECIMAL1), is(BigDecimal.ONE));
assertThat(NumberExtensions.operator_minus((Number) Q_ONE_2, (Number) DECIMAL1), is(BigDecimal.ONE));
}

@Test
Expand All @@ -93,7 +95,8 @@ public void operatorMinusQuantityQuantity() {

@Test
public void operatorMultiplyNumberQuantity() {
assertThat(NumberExtensions.operator_multiply(DECIMAL2, Q_LENGTH_2_CM), is(QuantityType.valueOf("4 cm")));
assertThat(NumberExtensions.operator_multiply((Number) DECIMAL2, Q_LENGTH_2_CM),
is(QuantityType.valueOf("4 cm")));
}

@Test
Expand All @@ -103,7 +106,8 @@ public void operatorMultiplyQuantityQuantity() {

@Test
public void operatorDivideQuantityNumber() {
assertThat(NumberExtensions.operator_divide(Q_LENGTH_1_M, DECIMAL2), is(QuantityType.valueOf("0.5 m")));
assertThat(NumberExtensions.operator_divide(Q_LENGTH_1_M, (Number) DECIMAL2),
is(QuantityType.valueOf("0.5 m")));
}

@Test
Expand All @@ -113,7 +117,43 @@ public void operatorDivideQuantityQuantity() {

@Test
public void operatorDivideNumberQuantity() {
assertThat(NumberExtensions.operator_divide(DECIMAL1, Q_LENGTH_2_CM), is(QuantityType.valueOf("0.5 one/cm")));
assertThat(NumberExtensions.operator_divide((Number) DECIMAL1, Q_LENGTH_2_CM),
is(QuantityType.valueOf("0.5 one/cm")));
}

@Test
public void operatorMinusStateState() {

@wborn wborn Aug 11, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These tests verify the runtime dispatch helpers, but they bypass the Rules DSL/Xbase operator resolution that this PR is intended to change by explicitly calling NumberExtensions.operator_*.

Could this also be covered by Rules DSL-level tests that parse/type-resolve and execute representative expressions?

In particular, it would be useful to cover:

NumberItem.state - OtherNumberItem.state
NumberItem.state * 1.5
QuantityItem.state - OtherQuantityItem.state
(QuantityItem.state - OtherQuantityItem.state) * 2

as well as explicitly typed numeric variables, NULL/UNDEF, and a nonnumeric Item state.

That would catch both overload-resolution regressions and cases where the static type of an intermediate expression differs from its runtime numeric type.

assertThat(NumberExtensions.operator_minus((State) DECIMAL2, (State) DECIMAL1), is(BigDecimal.ONE));
}

@Test
public void operatorMinusQuantityStateState() {
assertThat(NumberExtensions.operator_minus((State) Q_LENGTH_1_M, (State) Q_LENGTH_2_CM),
is(QuantityType.valueOf("0.98 m")));
}

@Test
public void operatorMultiplyStateNumber() {
assertThat(NumberExtensions.operator_multiply((State) DECIMAL2, BigDecimal.valueOf(1.5)),
is(BigDecimal.valueOf(3.0)));
}

@Test
public void operatorMultiplyQuantityStateNumber() {
assertThat(NumberExtensions.operator_multiply((State) Q_LENGTH_2_CM, (Number) DECIMAL2),
is(QuantityType.valueOf("4 cm")));
}

@Test
public void operatorDivideNumberState() {
assertThat(NumberExtensions.operator_divide(BigDecimal.TEN, (State) DECIMAL2),
is(new BigDecimal("5.00000000")));
}

@Test
public void operatorNonNumericState() {
assertThrows(IllegalArgumentException.class,
() -> NumberExtensions.operator_multiply(OnOffType.ON, (Number) DECIMAL2));
}

@Test
Expand Down
113 changes: 113 additions & 0 deletions ...org.openhab.core.model.script/src/org/openhab/core/model/script/lib/NumberExtensions.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import org.openhab.core.library.types.DecimalType;
import org.openhab.core.library.types.QuantityType;
import org.openhab.core.library.unit.Units;
import org.openhab.core.types.State;
import org.openhab.core.types.Type;

/**
Expand Down Expand Up @@ -91,6 +92,60 @@ public static BigDecimal operator_divide(Number x, Number y) {
return xValue.divide(yValue, 8, RoundingMode.HALF_UP);
}

// Calculation operators for states

public static Number operator_plus(State x, State y) {

@wborn wborn Aug 11, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Number return type can lose the fact that the runtime result is a QuantityType when this expression is used as part of another expression.

For example, with two dimensional Number Items:

(LengthA.state - LengthB.state) * 2

the first operation can correctly produce a QuantityType at runtime, but its static result type is Number. Xbase will therefore resolve the following multiplication as a Number operation rather than a QuantityType operation, which converts the quantity to a BigDecimal and loses the unit.

The same issue occurs when such a result is assigned to an inferred variable and used later.

Could this be implemented without reducing a quantity result to Number in the DSL type system? Otherwise simple direct state arithmetic may work while equivalent chained arithmetic silently changes semantics.

return plus(stateToNumber(x), stateToNumber(y));
}

public static Number operator_plus(State x, Number y) {

@wborn wborn Aug 11, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This makes expressions such as:

Temperature.state + 1

valid, but when the state is a QuantityType, plus() only chooses the quantity-specific operator when both operands are QuantityType. A quantity plus a plain number therefore falls through to operator_plus(Number, Number), which converts the quantity to its system-unit numerical value and returns a BigDecimal.

The same problem applies to State - Number.

That seems particularly risky because previously discarding the unit required an explicit as Number conversion. With this change an apparently unit-aware Item expression can discard the unit implicitly.

Could mixed quantity/plain-number addition and subtraction either be rejected or given explicitly defined unit-safe semantics instead?

return plus(stateToNumber(x), y);
}

public static Number operator_plus(Number x, State y) {
return plus(x, stateToNumber(y));
}

public static Number operator_minus(State x) {
return minus(stateToNumber(x));
}

public static Number operator_minus(State x, State y) {
return minus(stateToNumber(x), stateToNumber(y));
}

public static Number operator_minus(State x, Number y) {
return minus(stateToNumber(x), y);
}

public static Number operator_minus(Number x, State y) {
return minus(x, stateToNumber(y));
}

public static Number operator_multiply(State x, State y) {
return multiply(stateToNumber(x), stateToNumber(y));
}

public static Number operator_multiply(State x, Number y) {
return multiply(stateToNumber(x), y);
}

public static Number operator_multiply(Number x, State y) {
return multiply(x, stateToNumber(y));
}

public static Number operator_divide(State x, State y) {
return divide(stateToNumber(x), stateToNumber(y));
}

public static Number operator_divide(State x, Number y) {
return divide(stateToNumber(x), y);
}

public static Number operator_divide(Number x, State y) {
return divide(x, stateToNumber(y));
}

// Comparison operations between numbers

public static boolean operator_equals(Number left, Number right) {
Expand Down Expand Up @@ -408,4 +463,62 @@ private static boolean oneIsQuantity(Number left, Number right) {
private static boolean isAbstractUnitOne(QuantityType<?> left) {
return Units.ONE.equals(left.getUnit());
}

private static Number stateToNumber(State state) {
if (state == null) {
return null;
}
if (state instanceof Number number) {
return number;
}
throw new IllegalArgumentException(
"State '" + state + "' of type '" + state.getClass().getSimpleName() + "' cannot be used as a number");
}

private static Number plus(Number x, Number y) {
if (x instanceof QuantityType<?> qx && y instanceof QuantityType<?> qy) {
return operator_plus(qx, qy);
}
return operator_plus(x, y);
}

private static Number minus(Number x) {
if (x instanceof QuantityType<?> qx) {
return operator_minus(qx);
}
return operator_minus(x);
}

private static Number minus(Number x, Number y) {
if (x instanceof QuantityType<?> qx && y instanceof QuantityType<?> qy) {
return operator_minus(qx, qy);
}
return operator_minus(x, y);
}

private static Number multiply(Number x, Number y) {
if (x instanceof QuantityType<?> qx && y instanceof QuantityType<?> qy) {
return operator_multiply(qx, qy);
}
if (x instanceof QuantityType<?> qx) {
return operator_multiply(qx, y);
}
if (y instanceof QuantityType<?> qy) {
return operator_multiply(x, qy);
}
return operator_multiply(x, y);
}

private static Number divide(Number x, Number y) {
if (x instanceof QuantityType<?> qx && y instanceof QuantityType<?> qy) {
return operator_divide(qx, qy);
}
if (x instanceof QuantityType<?> qx) {
return operator_divide(qx, y);
}
if (y instanceof QuantityType<?> qy) {
return operator_divide(x, qy);
}
return operator_divide(x, y);
}
}
Loading