Fix: GroupItem now uses GroupItem's system unit

bundles/org.openhab.core/src/main/java/org/openhab/core/internal/items/GroupFunctionHelper.java

@@ -15,7 +15,7 @@
 import java.util.ArrayList;
 import java.util.List;
 
-import javax.measure.Quantity;
+import javax.measure.Unit;
 
 import org.eclipse.jdt.annotation.NonNullByDefault;
 import org.eclipse.jdt.annotation.Nullable;
@@ -48,13 +48,12 @@ public class GroupFunctionHelper {
      * arithmetic group function will take unit conversion into account.
      *
      * @param function the {@link GroupFunctionDTO} describing the group function.
-     * @param baseItem an optional {@link Item} defining the dimension for unit conversion.
+     * @param baseItem an optional {@link Item} defining the dimension/unit for unit conversion.
      * @return a {@link GroupFunction} according to the given parameters.
      */
     public GroupFunction createGroupFunction(GroupFunctionDTO function, @Nullable Item baseItem) {
-        Class<? extends Quantity<?>> dimension = getDimension(baseItem);
-        if (dimension != null) {
-            return createDimensionGroupFunction(function, baseItem, dimension);
+        if (baseItem instanceof NumberItem baseNumberItem && baseNumberItem.getDimension() != null) {
+            return createDimensionGroupFunction(function, baseNumberItem);
         }
         return createDefaultGroupFunction(function, baseItem);
     }
@@ -78,30 +77,26 @@ private List<State> parseStates(@Nullable Item baseItem, String @Nullable [] par
         return states;
     }
 
-    private @Nullable Class<? extends Quantity<?>> getDimension(@Nullable Item baseItem) {
-        if (baseItem instanceof NumberItem numberItem) {
-            return numberItem.getDimension();
-        }
-        return null;
-    }
-
-    private GroupFunction createDimensionGroupFunction(GroupFunctionDTO function, @Nullable Item baseItem,
-            Class<? extends Quantity<?>> dimension) {
+    private GroupFunction createDimensionGroupFunction(GroupFunctionDTO function, NumberItem baseNumberItem) {
         final String functionName = function.name;
-        switch (functionName.toUpperCase()) {
-            case "AVG":
-                return new QuantityTypeArithmeticGroupFunction.Avg(dimension);
-            case "MEDIAN":
-                return new QuantityTypeArithmeticGroupFunction.Median(dimension, baseItem);
-            case "SUM":
-                return new QuantityTypeArithmeticGroupFunction.Sum(dimension);
-            case "MIN":
-                return new QuantityTypeArithmeticGroupFunction.Min(dimension);
-            case "MAX":
-                return new QuantityTypeArithmeticGroupFunction.Max(dimension);
-            default:
-                return createDefaultGroupFunction(function, baseItem);
+        Unit<?> targetUnit = baseNumberItem.getUnit();
+        if (targetUnit != null) {
+            targetUnit = targetUnit.getSystemUnit(); // use system unit so 'Sum' becomes zero based (absolute)
+            switch (functionName.toUpperCase()) {
+                case "AVG":
+                    return new QuantityTypeArithmeticGroupFunction.Avg(targetUnit);
+                case "MEDIAN":
+                    return new QuantityTypeArithmeticGroupFunction.Median(targetUnit);
+                case "SUM":
+                    return new QuantityTypeArithmeticGroupFunction.Sum(targetUnit);
+                case "MIN":
+                    return new QuantityTypeArithmeticGroupFunction.Min(targetUnit);
+                case "MAX":
+                    return new QuantityTypeArithmeticGroupFunction.Max(targetUnit);
+                default:
+            }
         }
+        return createDefaultGroupFunction(function, baseNumberItem);
     }
 
     private GroupFunction createDefaultGroupFunction(GroupFunctionDTO function, @Nullable Item baseItem) {

bundles/org.openhab.core/src/main/java/org/openhab/core/library/types/QuantityTypeArithmeticGroupFunction.java

@@ -13,20 +13,18 @@
 package org.openhab.core.library.types;
 
 import java.math.BigDecimal;
-import java.math.MathContext;
-import java.util.ArrayList;
 import java.util.List;
+import java.util.Objects;
+import java.util.Optional;
 import java.util.Set;
 
-import javax.measure.Quantity;
 import javax.measure.Unit;
 
 import org.eclipse.jdt.annotation.NonNullByDefault;
 import org.eclipse.jdt.annotation.Nullable;
 import org.openhab.core.items.GroupFunction;
 import org.openhab.core.items.GroupItem;
 import org.openhab.core.items.Item;
-import org.openhab.core.library.items.NumberItem;
 import org.openhab.core.types.State;
 import org.openhab.core.types.UnDefType;
 import org.openhab.core.util.Statistics;
@@ -35,16 +33,17 @@
  * This interface is a container for dimension based functions that require {@link QuantityType}s for its calculations.
  *
  * @author Henning Treu - Initial contribution
+ * @author Andrew Fiddian-Green - Normalise calculations based on the Unit of the GroupItem
  */
 @NonNullByDefault
 public interface QuantityTypeArithmeticGroupFunction extends GroupFunction {
 
     abstract class DimensionalGroupFunction implements GroupFunction {
 
-        protected final Class<? extends Quantity<?>> dimension;
+        protected final Unit<?> referenceUnit; // the reference unit for all group member calculations
 
-        public DimensionalGroupFunction(Class<? extends Quantity<?>> dimension) {
-            this.dimension = dimension;
+        public DimensionalGroupFunction(Unit<?> referenceUnit) {
+            this.referenceUnit = referenceUnit;
         }
 
         @Override
@@ -62,208 +61,148 @@ public State[] getParameters() {
             return new State[0];
         }
 
-        protected boolean isSameDimension(@Nullable Item item) {
-            if (item instanceof GroupItem groupItem) {
-                return isSameDimension(groupItem.getBaseItem());
-            }
-            return item instanceof NumberItem ni && dimension.equals(ni.getDimension());
+        /**
+         * Convert the given item {@link State} to a {@link QuantityType} based on the {@link Unit} of the
+         * {@link GroupItem} i.e. 'referenceUnit'. Returns null if the {@link State} is not a {@link QuantityType} or
+         * if the {@link QuantityType} could not be converted to 'referenceUnit'.
+         *
+         * The conversion can be made to both inverted and non-inverted units, so invertible type conversions (e.g.
+         * Mirek <=> Kelvin) are supported.
+         *
+         * @param state the State of any given group member item
+         * @return a QuantityType or null
+         */
+        private @Nullable QuantityType<?> referenceUnitQuantityType(@Nullable State state) {
+            return state instanceof QuantityType<?> quantity ? quantity.toInvertibleUnit(referenceUnit) : null;
+        }
+
+        /**
+         * Convert a set of {@link Item} to a respective list of {@link QuantityType}. Exclude any {@link Item}s whose
+         * current {@link State} is not a {@link QuantityType}. Convert any remaining {@link QuantityType} to the
+         * 'referenceUnit' and exclude any values that did not convert.
+         *
+         * @param items a list of {@link Item}
+         * @return a list of {@link QuantityType} converted to the 'referenceUnit'
+         */
+        @SuppressWarnings({ "rawtypes" })
+        protected List<QuantityType> referenceUnitQuantityTypes(Set<Item> items) {
+            return items.stream().map(i -> i.getState()).map(s -> referenceUnitQuantityType(s)).filter(Objects::nonNull)
+                    .map(s -> (QuantityType) s).toList();
         }
     }
 
     /**
-     * This calculates the numeric average over all item states of {@link QuantityType}.
+     * Calculates the average of a set of item states whose value could be converted to the 'referenceUnit'.
      */
     class Avg extends DimensionalGroupFunction {
 
-        public Avg(Class<? extends Quantity<?>> dimension) {
-            super(dimension);
+        public Avg(Unit<?> targetUnit) {
+            super(targetUnit);
         }
 
         @Override
         @SuppressWarnings({ "unchecked", "rawtypes" })
         public State calculate(@Nullable Set<Item> items) {
-            if (items == null || items.isEmpty()) {
-                return UnDefType.UNDEF;
-            }
-
-            QuantityType<?> sum = null;
-            int count = 0;
-            for (Item item : items) {
-                if (isSameDimension(item)) {
-                    QuantityType itemState = item.getStateAs(QuantityType.class);
-                    if (itemState != null) {
-                        if (sum == null) {
-                            sum = itemState; // initialise the sum from the first item
-                            count++;
-                        } else {
-                            itemState = itemState.toInvertibleUnit(sum.getUnit());
-                            if (itemState != null) {
-                                sum = sum.add(itemState);
-                                count++;
-                            }
-                        }
-                    }
+            if (items != null) {
+                List<QuantityType> referenceUnitQuantities = referenceUnitQuantityTypes(items);
+                if (!referenceUnitQuantities.isEmpty()) {
+                    return referenceUnitQuantities.stream()
+                            .reduce(new QuantityType<>(0, referenceUnit), QuantityType::add)
+                            .divide(BigDecimal.valueOf(referenceUnitQuantities.size()));
                 }
             }
-
-            if (sum != null && count > 0) {
-                BigDecimal result = sum.toBigDecimal().divide(BigDecimal.valueOf(count), MathContext.DECIMAL128);
-                return new QuantityType(result, sum.getUnit());
-            }
-
             return UnDefType.UNDEF;
         }
     }
 
     /**
-     * This calculates the numeric median over all item states of {@link QuantityType}.
+     * Calculates the median of a set of item states whose value could be converted to the 'referenceUnit'.
      */
     class Median extends DimensionalGroupFunction {
 
-        private @Nullable Item baseItem;
-
-        public Median(Class<? extends Quantity<?>> dimension, @Nullable Item baseItem) {
-            super(dimension);
-            this.baseItem = baseItem;
+        public Median(Unit<?> targetUnit) {
+            super(targetUnit);
         }
 
         @Override
-        @SuppressWarnings({ "unchecked", "rawtypes" })
         public State calculate(@Nullable Set<Item> items) {
             if (items != null) {
-                List<BigDecimal> values = new ArrayList<>();
-                Unit<?> unit = null;
-                if (baseItem instanceof NumberItem numberItem) {
-                    unit = numberItem.getUnit();
-                }
-                for (Item item : items) {
-                    if (!isSameDimension(item)) {
-                        continue;
-                    }
-                    QuantityType itemState = item.getStateAs(QuantityType.class);
-                    if (itemState == null) {
-                        continue;
-                    }
-                    if (unit == null) {
-                        unit = itemState.getUnit(); // set it to the first item's unit
-                    }
-                    if (itemState.toInvertibleUnit(unit) instanceof QuantityType<?> inverted) {
-                        values.add(inverted.toBigDecimal());
-                    }
-                }
-
-                if (!values.isEmpty()) {
-                    BigDecimal median = Statistics.median(values);
-                    if (median != null && unit != null) {
-                        return new QuantityType<>(median, unit);
-                    }
-
+                BigDecimal median = Statistics
+                        .median(referenceUnitQuantityTypes(items).stream().map(q -> q.toBigDecimal()).toList());
+                if (median != null) {
+                    return new QuantityType<>(median, referenceUnit);
                 }
             }
             return UnDefType.UNDEF;
         }
     }
 
     /**
-     * This calculates the numeric sum over all item states of {@link QuantityType}.
+     * Calculates the sum of a set of item states whose value could be converted to the 'referenceUnit'.
+     *
+     * Uses the {@link QuanitityType.add()} method so the result is an incremental sum based on the 'referenceUnit'. As
+     * a general rule this class is instantiated with a 'referenceUnit' that is a "system unit" (which are zero based)
+     * so such incremental sum is in fact also an absolute sum. However the class COULD be instantiated with a "non-
+     * system unit" (e.g. °C, °F) in which case the result would be an incremental sum based on that unit.
+     *
      */
     class Sum extends DimensionalGroupFunction {
 
-        public Sum(Class<? extends Quantity<?>> dimension) {
-            super(dimension);
+        public Sum(Unit<?> targetUnit) {
+            super(targetUnit);
         }
 
         @Override
         @SuppressWarnings({ "unchecked", "rawtypes" })
         public State calculate(@Nullable Set<Item> items) {
-            if (items == null || items.isEmpty()) {
-                return UnDefType.UNDEF;
-            }
-
-            QuantityType<?> sum = null;
-            for (Item item : items) {
-                if (isSameDimension(item)) {
-                    QuantityType itemState = item.getStateAs(QuantityType.class);
-                    if (itemState != null) {
-                        if (sum == null) {
-                            sum = itemState; // initialise the sum from the first item
-                        } else {
-                            itemState = itemState.toUnit(sum.getUnit());
-                            if (itemState != null) {
-                                sum = sum.add(itemState);
-                            }
-                        }
-                    }
+            if (items != null) {
+                List<QuantityType> referenceUnitQuantities = referenceUnitQuantityTypes(items);
+                if (!referenceUnitQuantities.isEmpty()) {
+                    return referenceUnitQuantities.stream().reduce(new QuantityType<>(0, referenceUnit),
+                            QuantityType::add);
                 }
             }
-
-            return sum != null ? sum : UnDefType.UNDEF;
+            return UnDefType.UNDEF;
         }
     }
 
     /**
-     * This calculates the minimum value of all item states of {@link QuantityType}.
+     * Calculates the minimum of a set of item states whose value could be converted to the 'referenceUnit'.
      */
     class Min extends DimensionalGroupFunction {
 
-        public Min(Class<? extends Quantity<?>> dimension) {
-            super(dimension);
+        public Min(Unit<?> targetUnit) {
+            super(targetUnit);
         }
 
         @Override
         @SuppressWarnings({ "unchecked", "rawtypes" })
         public State calculate(@Nullable Set<Item> items) {
-            if (items == null || items.isEmpty()) {
-                return UnDefType.UNDEF;
-            }
-
-            QuantityType<?> min = null;
-            for (Item item : items) {
-                if (isSameDimension(item)) {
-                    QuantityType itemState = item.getStateAs(QuantityType.class);
-                    if (itemState != null) {
-                        if (min == null
-                                || (min.getUnit().isCompatible(itemState.getUnit()) && min.compareTo(itemState) > 0)) {
-                            min = itemState;
-                        }
-                    }
-                }
+            if (items != null) {
+                Optional<QuantityType> min = referenceUnitQuantityTypes(items).stream().min(QuantityType::compareTo);
+                return min.isPresent() ? min.get() : UnDefType.UNDEF;
             }
-
-            return min != null ? min : UnDefType.UNDEF;
+            return UnDefType.UNDEF;
         }
     }
 
     /**
-     * This calculates the maximum value of all item states of {@link QuantityType}.
+     * Calculates the maximum of a set of item states whose value could be converted to the 'referenceUnit'.
      */
     class Max extends DimensionalGroupFunction {
 
-        public Max(Class<? extends Quantity<?>> dimension) {
-            super(dimension);
+        public Max(Unit<?> targetUnit) {
+            super(targetUnit);
         }
 
         @Override
         @SuppressWarnings({ "unchecked", "rawtypes" })
         public State calculate(@Nullable Set<Item> items) {
-            if (items == null || items.isEmpty()) {
-                return UnDefType.UNDEF;
-            }
-
-            QuantityType<?> max = null;
-            for (Item item : items) {
-                if (isSameDimension(item)) {
-                    QuantityType itemState = item.getStateAs(QuantityType.class);
-                    if (itemState != null) {
-                        if (max == null
-                                || (max.getUnit().isCompatible(itemState.getUnit()) && max.compareTo(itemState) < 0)) {
-                            max = itemState;
-                        }
-                    }
-                }
+            if (items != null) {
+                Optional<QuantityType> max = referenceUnitQuantityTypes(items).stream().max(QuantityType::compareTo);
+                return max.isPresent() ? max.get() : UnDefType.UNDEF;
             }
-
-            return max != null ? max : UnDefType.UNDEF;
+            return UnDefType.UNDEF;
         }
     }
 }