[mqtt.generic] Add UOM to inbound values for MQTT Channels

bundles/org.openhab.binding.mqtt.generic/src/main/java/org/openhab/binding/mqtt/generic/values/NumberValue.java

@@ -16,6 +16,8 @@
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
 
+import javax.measure.Unit;
+
 import org.eclipse.jdt.annotation.NonNullByDefault;
 import org.eclipse.jdt.annotation.Nullable;
 import org.openhab.core.library.CoreItemFactory;
@@ -47,19 +49,19 @@ public class NumberValue extends Value {
     private final @Nullable BigDecimal min;
     private final @Nullable BigDecimal max;
     private final BigDecimal step;
-    private final String unit;
+    private final Unit<?> unit;
 
     public NumberValue(@Nullable BigDecimal min, @Nullable BigDecimal max, @Nullable BigDecimal step,
-            @Nullable String unit) {
+            @Nullable Unit<?> unit) {
         super(CoreItemFactory.NUMBER, Stream.of(QuantityType.class, IncreaseDecreaseType.class, UpDownType.class)
                 .collect(Collectors.toList()));
         this.min = min;
         this.max = max;
         this.step = step == null ? BigDecimal.ONE : step;
-        this.unit = unit == null ? "" : unit;
+        this.unit = unit != null ? unit : Units.ONE;
     }
 
-    protected boolean checkConditions(BigDecimal newValue, DecimalType oldvalue) {
+    protected boolean checkConditions(BigDecimal newValue) {
         BigDecimal min = this.min;
         if (min != null && newValue.compareTo(min) == -1) {
             logger.trace("Number not accepted as it is below the configured minimum");
@@ -90,49 +92,54 @@ public String getMQTTpublishValue(@Nullable String pattern) {
 
     @Override
     public void update(Command command) throws IllegalArgumentException {
-        DecimalType oldvalue = (state == UnDefType.UNDEF) ? new DecimalType() : (DecimalType) state;
         BigDecimal newValue = null;
         if (command instanceof DecimalType) {
-            if (!checkConditions(((DecimalType) command).toBigDecimal(), oldvalue)) {
-                return;
-            }
-            state = (DecimalType) command;
+            newValue = ((DecimalType) command).toBigDecimal();
         } else if (command instanceof IncreaseDecreaseType || command instanceof UpDownType) {
+            BigDecimal oldValue = getOldValue();
             if (command == IncreaseDecreaseType.INCREASE || command == UpDownType.UP) {
-                newValue = oldvalue.toBigDecimal().add(step);
+                newValue = oldValue.add(step);
             } else {
-                newValue = oldvalue.toBigDecimal().subtract(step);
-            }
-            if (!checkConditions(newValue, oldvalue)) {
-                return;
+                newValue = oldValue.subtract(step);
             }
-            state = new DecimalType(newValue);
         } else if (command instanceof QuantityType<?>) {
-            QuantityType<?> qType = (QuantityType<?>) command;
-
-            if (qType.getUnit().isCompatible(Units.ONE)) {
-                newValue = qType.toBigDecimal();
-            } else {
-                qType = qType.toUnit(unit);
-                if (qType != null) {
-                    newValue = qType.toBigDecimal();
-                }
-            }
-            if (newValue != null) {
-                if (!checkConditions(newValue, oldvalue)) {
-                    return;
-                }
-                state = new DecimalType(newValue);
-            }
+            newValue = getQuantityTypeAsDecimal((QuantityType<?>) command);
         } else {
             newValue = new BigDecimal(command.toString());
-            if (!checkConditions(newValue, oldvalue)) {
-                return;
-            }
+        }
+        if (!checkConditions(newValue)) {
+            return;
+        }
+        // items with units specified in the label in the UI but no unit on mqtt are stored as
+        // DecimalType to avoid conversions (e.g. % expects 0-1 rather than 0-100)
+        if (!Units.ONE.equals(unit)) {
+            state = new QuantityType<>(newValue, unit);
+        } else {
             state = new DecimalType(newValue);
         }
     }
 
+    private BigDecimal getOldValue() {
+        BigDecimal val = BigDecimal.ZERO;
+        if (state instanceof DecimalType) {
+            val = ((DecimalType) state).toBigDecimal();
+        } else if (state instanceof QuantityType<?>) {
+            val = ((QuantityType<?>) state).toBigDecimal();
+        }
+        return val;
+    }
+
+    private BigDecimal getQuantityTypeAsDecimal(QuantityType<?> qType) {
+        BigDecimal val = qType.toBigDecimal();
+        if (!qType.getUnit().isCompatible(Units.ONE)) {
+            QuantityType<?> convertedType = qType.toUnit(unit);
+            if (convertedType != null) {
+                val = convertedType.toBigDecimal();
+            }
+        }
+        return val;
+    }
+
     @Override
     public StateDescriptionFragmentBuilder createStateDescription(boolean readOnly) {
         StateDescriptionFragmentBuilder builder = super.createStateDescription(readOnly);
@@ -144,10 +151,6 @@ public StateDescriptionFragmentBuilder createStateDescription(boolean readOnly)
         if (min != null) {
             builder = builder.withMinimum(min);
         }
-        builder = builder.withStep(step);
-        if (this.unit.length() > 0) {
-            builder = builder.withPattern("%s " + this.unit.replace("%", "%%"));
-        }
-        return builder;
+        return builder.withStep(step).withPattern("%s %unit%");
     }
 }

bundles/org.openhab.binding.mqtt.generic/src/main/java/org/openhab/binding/mqtt/generic/values/ValueFactory.java

@@ -16,6 +16,7 @@
 import org.openhab.binding.mqtt.generic.ChannelConfig;
 import org.openhab.binding.mqtt.generic.internal.MqttBindingConstants;
 import org.openhab.binding.mqtt.generic.mapping.ColorMode;
+import org.openhab.core.types.util.UnitUtils;
 
 /**
  * A factory t
@@ -24,6 +25,7 @@
  */
 @NonNullByDefault
 public class ValueFactory {
+
     /**
      * Creates a new channel state value.
      *
@@ -47,7 +49,7 @@ public static Value createValueState(ChannelConfig config, String channelTypeID)
                 value = new LocationValue();
                 break;
             case MqttBindingConstants.NUMBER:
-                value = new NumberValue(config.min, config.max, config.step, config.unit);
+                value = new NumberValue(config.min, config.max, config.step, UnitUtils.parseUnit(config.unit));
                 break;
             case MqttBindingConstants.DIMMER:
                 value = new PercentageValue(config.min, config.max, config.step, config.on, config.off);

bundles/org.openhab.binding.mqtt.generic/src/test/java/org/openhab/binding/mqtt/generic/ChannelStateTests.java

@@ -52,6 +52,7 @@
 import org.openhab.core.library.types.HSBType;
 import org.openhab.core.library.types.RawType;
 import org.openhab.core.library.types.StringType;
+import org.openhab.core.library.unit.Units;
 import org.openhab.core.thing.ChannelUID;
 
 /**
@@ -185,6 +186,36 @@ public void receiveDecimalFractionalTest() {
         assertThat(value.getChannelState().toString(), is("16.0"));
     }
 
+    @Test
+    public void receiveDecimalUnitTest() {
+        NumberValue value = new NumberValue(null, null, new BigDecimal(10), Units.WATT);
+        ChannelState c = spy(new ChannelState(config, channelUID, value, channelStateUpdateListener));
+        c.start(connection, mock(ScheduledExecutorService.class), 100);
+
+        c.processMessage("state", "15".getBytes());
+        assertThat(value.getChannelState().toString(), is("15 W"));
+
+        c.processMessage("state", "INCREASE".getBytes());
+        assertThat(value.getChannelState().toString(), is("25 W"));
+
+        c.processMessage("state", "DECREASE".getBytes());
+        assertThat(value.getChannelState().toString(), is("15 W"));
+
+        verify(channelStateUpdateListener, times(3)).updateChannelState(eq(channelUID), any());
+    }
+
+    @Test
+    public void receiveDecimalAsPercentageUnitTest() {
+        NumberValue value = new NumberValue(null, null, new BigDecimal(10), Units.PERCENT);
+        ChannelState c = spy(new ChannelState(config, channelUID, value, channelStateUpdateListener));
+        c.start(connection, mock(ScheduledExecutorService.class), 100);
+
+        c.processMessage("state", "63.7".getBytes());
+        assertThat(value.getChannelState().toString(), is("63.7 %"));
+
+        verify(channelStateUpdateListener, times(1)).updateChannelState(eq(channelUID), any());
+    }
+
     @Test
     public void receivePercentageTest() {
         PercentageValue value = new PercentageValue(new BigDecimal(-100), new BigDecimal(100), new BigDecimal(10), null,

bundles/org.openhab.binding.mqtt.generic/src/test/java/org/openhab/binding/mqtt/generic/values/ValueTests.java

@@ -26,8 +26,11 @@
 import org.openhab.core.library.types.OnOffType;
 import org.openhab.core.library.types.OpenClosedType;
 import org.openhab.core.library.types.PercentType;
+import org.openhab.core.library.types.QuantityType;
 import org.openhab.core.library.types.StringType;
 import org.openhab.core.library.types.UpDownType;
+import org.openhab.core.library.unit.MetricPrefix;
+import org.openhab.core.library.unit.Units;
 import org.openhab.core.types.Command;
 import org.openhab.core.types.TypeParser;
 
@@ -160,6 +163,39 @@ public void openCloseUpdate() {
         assertThat(v.getChannelState(), is(OpenClosedType.OPEN));
     }
 
+    @Test
+    public void numberUpdate() {
+        NumberValue v = new NumberValue(null, null, new BigDecimal(10), Units.WATT);
+
+        // Test with command with units
+        v.update(new QuantityType<>(20, Units.WATT));
+        assertThat(v.getMQTTpublishValue(null), is("20"));
+        assertThat(v.getChannelState(), is(new QuantityType<>(20, Units.WATT)));
+        v.update(new QuantityType<>(20, MetricPrefix.KILO(Units.WATT)));
+        assertThat(v.getMQTTpublishValue(null), is("20000"));
+        assertThat(v.getChannelState(), is(new QuantityType<>(20, MetricPrefix.KILO(Units.WATT))));
+
+        // Test with command without units
+        v.update(new QuantityType<>("20"));
+        assertThat(v.getMQTTpublishValue(null), is("20"));
+        assertThat(v.getChannelState(), is(new QuantityType<>(20, Units.WATT)));
+    }
+
+    @Test
+    public void numberPercentageUpdate() {
+        NumberValue v = new NumberValue(null, null, new BigDecimal(10), Units.PERCENT);
+
+        // Test with command with units
+        v.update(new QuantityType<>(20, Units.PERCENT));
+        assertThat(v.getMQTTpublishValue(null), is("20"));
+        assertThat(v.getChannelState(), is(new QuantityType<>(20, Units.PERCENT)));
+
+        // Test with command without units
+        v.update(new QuantityType<>("20"));
+        assertThat(v.getMQTTpublishValue(null), is("20"));
+        assertThat(v.getChannelState(), is(new QuantityType<>(20, Units.PERCENT)));
+    }
+
     @Test
     public void rollershutterUpdateWithStrings() {
         RollershutterValue v = new RollershutterValue("fancyON", "fancyOff", "fancyStop");

bundles/org.openhab.binding.mqtt.homeassistant/src/main/java/org/openhab/binding/mqtt/homeassistant/internal/component/Climate.java

@@ -17,6 +17,9 @@
 import java.util.List;
 import java.util.function.Predicate;
 
+import javax.measure.Unit;
+import javax.measure.quantity.Temperature;
+
 import org.eclipse.jdt.annotation.NonNullByDefault;
 import org.eclipse.jdt.annotation.Nullable;
 import org.openhab.binding.mqtt.generic.ChannelStateUpdateListener;
@@ -27,6 +30,8 @@
 import org.openhab.binding.mqtt.homeassistant.internal.ComponentChannel;
 import org.openhab.binding.mqtt.homeassistant.internal.config.dto.AbstractChannelConfiguration;
 import org.openhab.core.library.types.StringType;
+import org.openhab.core.library.unit.ImperialUnits;
+import org.openhab.core.library.unit.SIUnits;
 import org.openhab.core.types.Command;
 import org.openhab.core.types.State;
 
@@ -53,10 +58,28 @@ public class Climate extends AbstractComponent<Climate.ChannelConfiguration> {
     public static final String TEMPERATURE_LOW_CH_ID = "temperatureLow";
     public static final String POWER_CH_ID = "power";
 
-    private static final String CELSIUM = "C";
-    private static final String FAHRENHEIT = "F";
-    private static final float DEFAULT_CELSIUM_PRECISION = 0.1f;
-    private static final float DEFAULT_FAHRENHEIT_PRECISION = 1f;
+    public static enum TemperatureUnit {
+        @SerializedName("C")
+        CELSIUS(SIUnits.CELSIUS, new BigDecimal("0.1")),
+        @SerializedName("F")
+        FAHRENHEIT(ImperialUnits.FAHRENHEIT, BigDecimal.ONE);
+
+        private final Unit<Temperature> unit;
+        private final BigDecimal defaultPrecision;
+
+        TemperatureUnit(Unit<Temperature> unit, BigDecimal defaultPrecision) {
+            this.unit = unit;
+            this.defaultPrecision = defaultPrecision;
+        }
+
+        public Unit<Temperature> getUnit() {
+            return unit;
+        }
+
+        public BigDecimal getDefaultPrecision() {
+            return defaultPrecision;
+        }
+    }
 
     private static final String ACTION_OFF = "off";
     private static final State ACTION_OFF_STATE = new StringType(ACTION_OFF);
@@ -175,28 +198,23 @@ static class ChannelConfiguration extends AbstractChannelConfiguration {
 
         protected Integer initial = 21;
         @SerializedName("max_temp")
-        protected @Nullable Float maxTemp;
+        protected @Nullable BigDecimal maxTemp;
         @SerializedName("min_temp")
-        protected @Nullable Float minTemp;
+        protected @Nullable BigDecimal minTemp;
         @SerializedName("temperature_unit")
-        protected String temperatureUnit = CELSIUM; // System unit by default
+        protected TemperatureUnit temperatureUnit = TemperatureUnit.CELSIUS; // System unit by default
         @SerializedName("temp_step")
-        protected Float tempStep = 1f;
-        protected @Nullable Float precision;
+        protected BigDecimal tempStep = BigDecimal.ONE;
+        protected @Nullable BigDecimal precision;
         @SerializedName("send_if_off")
         protected Boolean sendIfOff = true;
     }
 
     public Climate(ComponentFactory.ComponentConfiguration componentConfiguration) {
         super(componentConfiguration, ChannelConfiguration.class);
 
-        BigDecimal minTemp = channelConfiguration.minTemp != null ? BigDecimal.valueOf(channelConfiguration.minTemp)
-                : null;
-        BigDecimal maxTemp = channelConfiguration.maxTemp != null ? BigDecimal.valueOf(channelConfiguration.maxTemp)
-                : null;
-        float precision = channelConfiguration.precision != null ? channelConfiguration.precision
-                : (FAHRENHEIT.equals(channelConfiguration.temperatureUnit) ? DEFAULT_FAHRENHEIT_PRECISION
-                        : DEFAULT_CELSIUM_PRECISION);
+        BigDecimal precision = channelConfiguration.precision != null ? channelConfiguration.precision
+                : channelConfiguration.temperatureUnit.getDefaultPrecision();
         final ChannelStateUpdateListener updateListener = componentConfiguration.getUpdateListener();
 
         ComponentChannel actionChannel = buildOptionalChannel(ACTION_CH_ID,
@@ -214,7 +232,8 @@ public Climate(ComponentFactory.ComponentConfiguration componentConfiguration) {
                 channelConfiguration.awayModeStateTopic, commandFilter);
 
         buildOptionalChannel(CURRENT_TEMPERATURE_CH_ID,
-                new NumberValue(minTemp, maxTemp, BigDecimal.valueOf(precision), channelConfiguration.temperatureUnit),
+                new NumberValue(channelConfiguration.minTemp, channelConfiguration.maxTemp, precision,
+                        channelConfiguration.temperatureUnit.getUnit()),
                 updateListener, null, null, channelConfiguration.currentTemperatureTemplate,
                 channelConfiguration.currentTemperatureTopic, commandFilter);
 
@@ -237,22 +256,22 @@ public Climate(ComponentFactory.ComponentConfiguration componentConfiguration) {
                 channelConfiguration.swingStateTemplate, channelConfiguration.swingStateTopic, commandFilter);
 
         buildOptionalChannel(TEMPERATURE_CH_ID,
-                new NumberValue(minTemp, maxTemp, BigDecimal.valueOf(channelConfiguration.tempStep),
-                        channelConfiguration.temperatureUnit),
+                new NumberValue(channelConfiguration.minTemp, channelConfiguration.maxTemp,
+                        channelConfiguration.tempStep, channelConfiguration.temperatureUnit.getUnit()),
                 updateListener, channelConfiguration.temperatureCommandTemplate,
                 channelConfiguration.temperatureCommandTopic, channelConfiguration.temperatureStateTemplate,
                 channelConfiguration.temperatureStateTopic, commandFilter);
 
         buildOptionalChannel(TEMPERATURE_HIGH_CH_ID,
-                new NumberValue(minTemp, maxTemp, BigDecimal.valueOf(channelConfiguration.tempStep),
-                        channelConfiguration.temperatureUnit),
+                new NumberValue(channelConfiguration.minTemp, channelConfiguration.maxTemp,
+                        channelConfiguration.tempStep, channelConfiguration.temperatureUnit.getUnit()),
                 updateListener, channelConfiguration.temperatureHighCommandTemplate,
                 channelConfiguration.temperatureHighCommandTopic, channelConfiguration.temperatureHighStateTemplate,
                 channelConfiguration.temperatureHighStateTopic, commandFilter);
 
         buildOptionalChannel(TEMPERATURE_LOW_CH_ID,
-                new NumberValue(minTemp, maxTemp, BigDecimal.valueOf(channelConfiguration.tempStep),
-                        channelConfiguration.temperatureUnit),
+                new NumberValue(channelConfiguration.minTemp, channelConfiguration.maxTemp,
+                        channelConfiguration.tempStep, channelConfiguration.temperatureUnit.getUnit()),
                 updateListener, channelConfiguration.temperatureLowCommandTemplate,
                 channelConfiguration.temperatureLowCommandTopic, channelConfiguration.temperatureLowStateTemplate,
                 channelConfiguration.temperatureLowStateTopic, commandFilter);

bundles/org.openhab.binding.mqtt.homeassistant/src/main/java/org/openhab/binding/mqtt/homeassistant/internal/component/Sensor.java

@@ -23,6 +23,7 @@
 import org.openhab.binding.mqtt.generic.values.Value;
 import org.openhab.binding.mqtt.homeassistant.internal.config.dto.AbstractChannelConfiguration;
 import org.openhab.binding.mqtt.homeassistant.internal.listener.ExpireUpdateStateListener;
+import org.openhab.core.types.util.UnitUtils;
 
 import com.google.gson.annotations.SerializedName;
 
@@ -71,7 +72,7 @@ public Sensor(ComponentFactory.ComponentConfiguration componentConfiguration) {
         String uom = channelConfiguration.unitOfMeasurement;
 
         if (uom != null && !uom.isBlank()) {
-            value = new NumberValue(null, null, null, uom);
+            value = new NumberValue(null, null, null, UnitUtils.parseUnit(uom));
         } else {
             value = new TextValue();
         }