[mqtt.generic] separate command parsing from cached value updating (#12238)

* [mqtt.generic] separate command parsing from cached value updating

fixes #12150

Previously, Value.update would parse the command, _and_ update the cached
value with that command. Which means that when sending a command towards
MQTT (instead of processing an update from MQTT), the cached value was
unintentionally updated. This prevented the REFRESH command from returning
the most recent value received from the device.

Separating the two concerns also makes the test more obvious what they are
testing, and vastly simplified a kludgy workaround that RollershutterValue
was using to be able to process Commands that aren't States.

Signed-off-by: Cody Cutrer <cody@cutrer.us>

* [mqtt.generic] split Value::parseCommand into parseMessage

so that a particular value type subclass can have varying implementations
if it desires

---------

Signed-off-by: Cody Cutrer <cody@cutrer.us>

bundles/org.openhab.binding.mqtt.generic/src/main/java/org/openhab/binding/mqtt/generic/ChannelState.java

@@ -31,6 +31,7 @@ import org.openhab.core.io.transport.mqtt.MqttMessageSubscriber;
 import org.openhab.core.library.types.StringType;
 import org.openhab.core.thing.ChannelUID;
 import org.openhab.core.types.Command;
+import org.openhab.core.types.State;
 import org.openhab.core.types.TypeParser;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -197,16 +198,10 @@ public class ChannelState implements MqttMessageSubscriber {
             return;
         }
 
-        Command postOnlyCommand = cachedValue.isPostOnly(command);
+        Command parsedCommand;
-        if (postOnlyCommand != null) {
-            channelStateUpdateListener.postChannelCommand(channelUID, postOnlyCommand);
-            receivedOrTimeout();
-            return;
-        }
-
         // Map the string to a command, update the cached value and post the command to the framework
         try {
-            cachedValue.update(command);
+            parsedCommand = cachedValue.parseMessage(command);
         } catch (IllegalArgumentException | IllegalStateException e) {
             logger.warn("Command '{}' from channel '{}' not supported by type '{}': {}", strValue, channelUID,
                     cachedValue.getClass().getSimpleName(), e.getMessage());
@@ -214,6 +209,14 @@ public class ChannelState implements MqttMessageSubscriber {
             return;
         }
 
+        // things that are only Commands _must_ be posted as a command (like STOP)
+        if (!(parsedCommand instanceof State)) {
+            channelStateUpdateListener.postChannelCommand(channelUID, parsedCommand);
+            receivedOrTimeout();
+            return;
+        }
+        cachedValue.update((State) parsedCommand);
+
         if (config.postCommand) {
             channelStateUpdateListener.postChannelCommand(channelUID, (Command) cachedValue.getChannelState());
         } else {
@@ -348,10 +351,6 @@ public class ChannelState implements MqttMessageSubscriber {
      *         and exceptionally otherwise.
      */
     public CompletableFuture<Boolean> publishValue(Command command) {
-        cachedValue.update(command);
-
-        Value mqttCommandValue = cachedValue;
-
         final MqttBrokerConnection connection = this.connection;
 
         if (connection == null) {
@@ -361,6 +360,9 @@ public class ChannelState implements MqttMessageSubscriber {
             return f;
         }
 
+        Command mqttCommandValue = cachedValue.parseCommand(command);
+        Value mqttFormatter = cachedValue;
+
         if (readOnly) {
             logger.debug(
                     "You have tried to publish {} to the mqtt topic '{}' that was marked read-only. You can't 'set' anything on a sensor state topic for example.",
@@ -370,12 +372,11 @@ public class ChannelState implements MqttMessageSubscriber {
 
         // Outgoing transformations
         for (ChannelStateTransformation t : transformationsOut) {
-            String commandString = mqttCommandValue.getMQTTpublishValue(null);
+            String commandString = mqttFormatter.getMQTTpublishValue(mqttCommandValue, null);
             String transformedValue = t.processValue(commandString);
             if (transformedValue != null) {
-                Value textValue = new TextValue();
+                mqttFormatter = new TextValue();
-                textValue.update(new StringType(transformedValue));
+                mqttCommandValue = new StringType(transformedValue);
-                mqttCommandValue = textValue;
             } else {
                 logger.debug("Transformation '{}' returned null on '{}', discarding message", mqttCommandValue,
                         t.serviceName);
@@ -388,13 +389,13 @@ public class ChannelState implements MqttMessageSubscriber {
         // Formatter: Applied before the channel state value is published to the MQTT broker.
         if (config.formatBeforePublish.length() > 0) {
             try {
-                commandString = mqttCommandValue.getMQTTpublishValue(config.formatBeforePublish);
+                commandString = mqttFormatter.getMQTTpublishValue(mqttCommandValue, config.formatBeforePublish);
             } catch (IllegalFormatException e) {
                 logger.debug("Format pattern incorrect for {}", channelUID, e);
-                commandString = mqttCommandValue.getMQTTpublishValue(null);
+                commandString = mqttFormatter.getMQTTpublishValue(mqttCommandValue, null);
             }
         } else {
-            commandString = mqttCommandValue.getMQTTpublishValue(null);
+            commandString = mqttFormatter.getMQTTpublishValue(mqttCommandValue, null);
         }
 
         int qos = (config.qos != null) ? config.qos : connection.getQos();

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

@@ -80,24 +80,24 @@ public class ColorValue extends Value {
      * Updates the color state.
      */
     @Override
-    public void update(Command command) throws IllegalArgumentException {
+    public HSBType parseCommand(Command command) throws IllegalArgumentException {
         HSBType oldvalue = (state == UnDefType.UNDEF) ? new HSBType() : (HSBType) state;
         if (command instanceof HSBType) {
-            state = (HSBType) command;
+            return (HSBType) command;
         } else if (command instanceof OnOffType) {
             OnOffType boolValue = ((OnOffType) command);
             PercentType minOn = new PercentType(Math.max(oldvalue.getBrightness().intValue(), onBrightness));
-            state = new HSBType(oldvalue.getHue(), oldvalue.getSaturation(),
+            return new HSBType(oldvalue.getHue(), oldvalue.getSaturation(),
                     boolValue == OnOffType.ON ? minOn : new PercentType(0));
         } else if (command instanceof PercentType) {
-            state = new HSBType(oldvalue.getHue(), oldvalue.getSaturation(), (PercentType) command);
+            return new HSBType(oldvalue.getHue(), oldvalue.getSaturation(), (PercentType) command);
         } else {
             final String updatedValue = command.toString();
             if (onValue.equals(updatedValue)) {
                 PercentType minOn = new PercentType(Math.max(oldvalue.getBrightness().intValue(), onBrightness));
-                state = new HSBType(oldvalue.getHue(), oldvalue.getSaturation(), minOn);
+                return new HSBType(oldvalue.getHue(), oldvalue.getSaturation(), minOn);
             } else if (offValue.equals(updatedValue)) {
-                state = new HSBType(oldvalue.getHue(), oldvalue.getSaturation(), new PercentType(0));
+                return new HSBType(oldvalue.getHue(), oldvalue.getSaturation(), new PercentType(0));
             } else {
                 String[] split = updatedValue.split(",");
                 if (split.length != 3) {
@@ -105,18 +105,15 @@ public class ColorValue extends Value {
                 }
                 switch (this.colorMode) {
                     case HSB:
-                        state = new HSBType(updatedValue);
+                        return new HSBType(updatedValue);
-                        break;
                     case RGB:
-                        state = HSBType.fromRGB(Integer.parseInt(split[0]), Integer.parseInt(split[1]),
+                        return HSBType.fromRGB(Integer.parseInt(split[0]), Integer.parseInt(split[1]),
                                 Integer.parseInt(split[2]));
-                        break;
                     case XYY:
                         HSBType tempState = HSBType.fromXY(Float.parseFloat(split[0]), Float.parseFloat(split[1]));
-                        state = new HSBType(tempState.getHue(), tempState.getSaturation(), new PercentType(split[2]));
+                        return new HSBType(tempState.getHue(), tempState.getSaturation(), new PercentType(split[2]));
-                        break;
                     default:
-                        logger.warn("Non supported color mode");
+                        throw new IllegalArgumentException("Non supported color mode");
                 }
             }
         }
@@ -130,11 +127,7 @@ public class ColorValue extends Value {
      *         ("0.419321,0.505255,100.00").
      */
     @Override
-    public String getMQTTpublishValue(@Nullable String pattern) {
+    public String getMQTTpublishValue(Command command, @Nullable String pattern) {
-        if (state == UnDefType.UNDEF) {
-            return "";
-        }
-
         String formatPattern = pattern;
         if (formatPattern == null || "%s".equals(formatPattern)) {
             if (this.colorMode == ColorMode.XYY) {
@@ -144,7 +137,7 @@ public class ColorValue extends Value {
             }
         }
 
-        HSBType hsbState = (HSBType) state;
+        HSBType hsbState = (HSBType) command;
 
         switch (this.colorMode) {
             case HSB:

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

@@ -21,7 +21,6 @@ import org.openhab.core.library.CoreItemFactory;
 import org.openhab.core.library.types.DateTimeType;
 import org.openhab.core.library.types.StringType;
 import org.openhab.core.types.Command;
-import org.openhab.core.types.UnDefType;
 
 /**
  * Implements a datetime value.
@@ -35,23 +34,20 @@ public class DateTimeValue extends Value {
     }
 
     @Override
-    public void update(Command command) throws IllegalArgumentException {
+    public DateTimeType parseCommand(Command command) throws IllegalArgumentException {
         if (command instanceof DateTimeType) {
-            state = ((DateTimeType) command);
+            return ((DateTimeType) command);
         } else {
-            state = DateTimeType.valueOf(command.toString());
+            return DateTimeType.valueOf(command.toString());
         }
     }
 
     @Override
-    public String getMQTTpublishValue(@Nullable String pattern) {
+    public String getMQTTpublishValue(Command command, @Nullable String pattern) {
-        if (state == UnDefType.UNDEF) {
-            return "";
-        }
         String formatPattern = pattern;
         if (formatPattern == null || "%s".contentEquals(formatPattern)) {
-            return DateTimeFormatter.ISO_LOCAL_DATE_TIME.format(((DateTimeType) state).getZonedDateTime());
+            return DateTimeFormatter.ISO_LOCAL_DATE_TIME.format(((DateTimeType) command).getZonedDateTime());
         }
-        return String.format(formatPattern, ((DateTimeType) state).getZonedDateTime());
+        return String.format(formatPattern, ((DateTimeType) command).getZonedDateTime());
     }
 }

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

@@ -30,7 +30,7 @@ public class ImageValue extends Value {
     }
 
     @Override
-    public void update(Command command) throws IllegalArgumentException {
+    public Command parseCommand(Command command) throws IllegalArgumentException {
         throw new IllegalArgumentException("Binary type. Command not allowed");
     }
 

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

@@ -35,9 +35,9 @@ public class LocationValue extends Value {
     }
 
     @Override
-    public String getMQTTpublishValue(@Nullable String pattern) {
+    public String getMQTTpublishValue(Command command, @Nullable String pattern) {
         String formatPattern = pattern;
-        PointType point = ((PointType) state);
+        PointType point = (PointType) command;
 
         if (formatPattern == null || "%s".equals(formatPattern)) {
             if (point.getAltitude().toBigDecimal().equals(BigDecimal.ZERO)) {
@@ -51,11 +51,11 @@ public class LocationValue extends Value {
     }
 
     @Override
-    public void update(Command command) throws IllegalArgumentException {
+    public PointType parseCommand(Command command) throws IllegalArgumentException {
         if (command instanceof PointType) {
-            state = ((PointType) command);
+            return ((PointType) command);
         } else {
-            state = PointType.valueOf(command.toString());
+            return PointType.valueOf(command.toString());
         }
     }
 }

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

@@ -27,7 +27,6 @@ import org.openhab.core.library.types.UpDownType;
 import org.openhab.core.library.unit.Units;
 import org.openhab.core.types.Command;
 import org.openhab.core.types.StateDescriptionFragmentBuilder;
-import org.openhab.core.types.UnDefType;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -75,21 +74,17 @@ public class NumberValue extends Value {
     }
 
     @Override
-    public String getMQTTpublishValue(@Nullable String pattern) {
+    public String getMQTTpublishValue(Command command, @Nullable String pattern) {
-        if (state == UnDefType.UNDEF) {
-            return "";
-        }
-
         String formatPattern = pattern;
         if (formatPattern == null) {
             formatPattern = "%s";
         }
 
-        return state.format(formatPattern);
+        return command.format(formatPattern);
     }
 
     @Override
-    public void update(Command command) throws IllegalArgumentException {
+    public Command parseCommand(Command command) throws IllegalArgumentException {
         BigDecimal newValue = null;
         if (command instanceof DecimalType) {
             newValue = ((DecimalType) command).toBigDecimal();
@@ -106,14 +101,14 @@ public class NumberValue extends Value {
             newValue = new BigDecimal(command.toString());
         }
         if (!checkConditions(newValue)) {
-            return;
+            throw new IllegalArgumentException(newValue + " is out of range");
         }
         // 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);
+            return new QuantityType<>(newValue, unit);
         } else {
-            state = new DecimalType(newValue);
+            return new DecimalType(newValue);
         }
     }
 

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

@@ -72,29 +72,29 @@ public class OnOffValue extends Value {
     }
 
     @Override
-    public void update(Command command) throws IllegalArgumentException {
+    public OnOffType parseCommand(Command command) throws IllegalArgumentException {
         if (command instanceof OnOffType) {
-            state = (OnOffType) command;
+            return (OnOffType) command;
         } else {
             final String updatedValue = command.toString();
             if (onState.equals(updatedValue)) {
-                state = OnOffType.ON;
+                return OnOffType.ON;
             } else if (offState.equals(updatedValue)) {
-                state = OnOffType.OFF;
+                return OnOffType.OFF;
             } else {
-                state = OnOffType.valueOf(updatedValue);
+                return OnOffType.valueOf(updatedValue);
             }
         }
     }
 
     @Override
-    public String getMQTTpublishValue(@Nullable String pattern) {
+    public String getMQTTpublishValue(Command command, @Nullable String pattern) {
         String formatPattern = pattern;
         if (formatPattern == null) {
             formatPattern = "%s";
         }
 
-        return String.format(formatPattern, state == OnOffType.ON ? onCommand : offCommand);
+        return String.format(formatPattern, command == OnOffType.ON ? onCommand : offCommand);
     }
 
     @Override

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

@@ -53,28 +53,28 @@ public class OpenCloseValue extends Value {
     }
 
     @Override
-    public void update(Command command) throws IllegalArgumentException {
+    public OpenClosedType parseCommand(Command command) throws IllegalArgumentException {
         if (command instanceof OpenClosedType) {
-            state = (OpenClosedType) command;
+            return (OpenClosedType) command;
         } else {
             final String updatedValue = command.toString();
             if (openString.equals(updatedValue)) {
-                state = OpenClosedType.OPEN;
+                return OpenClosedType.OPEN;
             } else if (closeString.equals(updatedValue)) {
-                state = OpenClosedType.CLOSED;
+                return OpenClosedType.CLOSED;
             } else {
-                state = OpenClosedType.valueOf(updatedValue);
+                return OpenClosedType.valueOf(updatedValue);
             }
         }
     }
 
     @Override
-    public String getMQTTpublishValue(@Nullable String pattern) {
+    public String getMQTTpublishValue(Command command, @Nullable String pattern) {
         String formatPattern = pattern;
         if (formatPattern == null) {
             formatPattern = "%s";
         }
 
-        return String.format(formatPattern, state == OpenClosedType.OPEN ? openString : closeString);
+        return String.format(formatPattern, command == OpenClosedType.OPEN ? openString : closeString);
     }
 }

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

@@ -72,17 +72,17 @@ public class PercentageValue extends Value {
     }
 
     @Override
-    public void update(Command command) throws IllegalArgumentException {
+    public PercentType parseCommand(Command command) throws IllegalArgumentException {
         PercentType oldvalue = (state == UnDefType.UNDEF) ? new PercentType() : (PercentType) state;
         // Nothing do to -> We have received a percentage
         if (command instanceof PercentType) {
-            state = (PercentType) command;
+            return (PercentType) command;
         } else //
                // A decimal type need to be converted according to the current min/max values
         if (command instanceof DecimalType) {
             BigDecimal v = ((DecimalType) command).toBigDecimal();
             v = v.subtract(min).multiply(HUNDRED).divide(max.subtract(min), MathContext.DECIMAL128);
-            state = new PercentType(v);
+            return new PercentType(v);
         } else //
                // A quantity type need to be converted according to the current min/max values
         if (command instanceof QuantityType) {
@@ -90,57 +90,55 @@ public class PercentageValue extends Value {
             if (qty != null) {
                 BigDecimal v = qty.toBigDecimal();
                 v = v.subtract(min).multiply(HUNDRED).divide(max.subtract(min), MathContext.DECIMAL128);
-                state = new PercentType(v);
+                return new PercentType(v);
             }
+            return oldvalue;
         } else //
                // Increase or decrease by "step"
         if (command instanceof IncreaseDecreaseType) {
             if (((IncreaseDecreaseType) command) == IncreaseDecreaseType.INCREASE) {
                 final BigDecimal v = oldvalue.toBigDecimal().add(stepPercent);
-                state = v.compareTo(HUNDRED) <= 0 ? new PercentType(v) : PercentType.HUNDRED;
+                return v.compareTo(HUNDRED) <= 0 ? new PercentType(v) : PercentType.HUNDRED;
             } else {
                 final BigDecimal v = oldvalue.toBigDecimal().subtract(stepPercent);
-                state = v.compareTo(BigDecimal.ZERO) >= 0 ? new PercentType(v) : PercentType.ZERO;
+                return v.compareTo(BigDecimal.ZERO) >= 0 ? new PercentType(v) : PercentType.ZERO;
             }
         } else //
                // On/Off equals 100 or 0 percent
         if (command instanceof OnOffType) {
-            state = ((OnOffType) command) == OnOffType.ON ? PercentType.HUNDRED : PercentType.ZERO;
+            return ((OnOffType) command) == OnOffType.ON ? PercentType.HUNDRED : PercentType.ZERO;
         } else//
               // Increase or decrease by "step"
         if (command instanceof UpDownType) {
             if (((UpDownType) command) == UpDownType.UP) {
                 final BigDecimal v = oldvalue.toBigDecimal().add(stepPercent);
-                state = v.compareTo(HUNDRED) <= 0 ? new PercentType(v) : PercentType.HUNDRED;
+                return v.compareTo(HUNDRED) <= 0 ? new PercentType(v) : PercentType.HUNDRED;
             } else {
                 final BigDecimal v = oldvalue.toBigDecimal().subtract(stepPercent);
-                state = v.compareTo(BigDecimal.ZERO) >= 0 ? new PercentType(v) : PercentType.ZERO;
+                return v.compareTo(BigDecimal.ZERO) >= 0 ? new PercentType(v) : PercentType.ZERO;
             }
         } else //
                // Check against custom on/off values
         if (command instanceof StringType) {
             if (onValue != null && command.toString().equals(onValue)) {
-                state = new PercentType(max);
+                return new PercentType(max);
             } else if (offValue != null && command.toString().equals(offValue)) {
-                state = new PercentType(min);
+                return new PercentType(min);
             } else {
-                throw new IllegalStateException("Unknown String!");
+                throw new IllegalStateException("Unable to parse " + command.toString() + " as a percent.");
             }
         } else {
             // We are desperate -> Try to parse the command as number value
-            state = PercentType.valueOf(command.toString());
+            return PercentType.valueOf(command.toString());
         }
     }
 
     @Override
-    public String getMQTTpublishValue(@Nullable String pattern) {
+    public String getMQTTpublishValue(Command command, @Nullable String pattern) {
-        if (state == UnDefType.UNDEF) {
-            return "";
-        }
         // Formula: From percentage to custom min/max: value*span/100+min
         // Calculation need to happen with big decimals to either return a straight integer or a decimal depending on
         // the value.
-        BigDecimal value = ((PercentType) state).toBigDecimal().multiply(span).divide(HUNDRED, MathContext.DECIMAL128)
+        BigDecimal value = ((PercentType) command).toBigDecimal().multiply(span).divide(HUNDRED, MathContext.DECIMAL128)
                 .add(min).stripTrailingZeros();
 
         String formatPattern = pattern;

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

@@ -39,7 +39,6 @@ public class RollershutterValue extends Value {
     private final @Nullable String upString;
     private final @Nullable String downString;
     private final String stopString;
-    private boolean nextIsStop = false; // If set: getMQTTpublishValue will return the stop string
 
     /**
      * Creates a new rollershutter value.
@@ -57,76 +56,75 @@ public class RollershutterValue extends Value {
     }
 
     @Override
-    public void update(Command command) throws IllegalArgumentException {
+    public Command parseCommand(Command command) throws IllegalArgumentException {
-        nextIsStop = false;
         if (command instanceof StopMoveType) {
-            nextIsStop = (((StopMoveType) command) == StopMoveType.STOP);
+            if (command == StopMoveType.STOP) {
-            return;
+                return command;
-        } else if (command instanceof UpDownType) {
+            } else {
-            state = ((UpDownType) command) == UpDownType.UP ? PercentType.ZERO : PercentType.HUNDRED;
+                throw new IllegalArgumentException(command.toString() + " is not a valid command for MQTT.");
-            return;
-        } else if (command instanceof PercentType) {
-            state = (PercentType) command;
-            return;
-        } else if (command instanceof StringType) {
-            final String updatedValue = command.toString();
-            if (updatedValue.equals(upString)) {
-                state = PercentType.ZERO;
-                return;
-            } else if (updatedValue.equals(downString)) {
-                state = PercentType.HUNDRED;
-                return;
-            } else if (updatedValue.equals(stopString)) {
-                nextIsStop = true;
-                return;
             }
-        }
+        } else if (command instanceof UpDownType) {
-        throw new IllegalStateException("Cannot call update() with " + command.toString());
+            if (command == UpDownType.UP) {
-    }
+                if (upString != null) {
-
+                    return command;
-    /**
+                } else {
-     * The stop command will not update the internal state and is posted to the framework.
+                    return PercentType.ZERO;
-     * <p>
+                }
-     * The Up/Down commands (100%/0%) are not updating the state directly and are also
+            } else {
-     * posted as percent value to the framework. It is up to the user if the posted values
+                if (downString != null) {
-     * are applied to the item state immediately (autoupdate=true) or not.
+                    return command;
-     */
+                } else {
-    @Override
+                    return PercentType.HUNDRED;
-    public @Nullable Command isPostOnly(Command command) {
+                }
-        if (command instanceof UpDownType) {
+            }
-            return command;
+        } else if (command instanceof PercentType) {
-        } else if (command instanceof StopMoveType) {
+            return (PercentType) command;
-            return command;
         } else if (command instanceof StringType) {
             final String updatedValue = command.toString();
             if (updatedValue.equals(upString)) {
-                return UpDownType.UP.as(PercentType.class);
+                return UpDownType.UP;
             } else if (updatedValue.equals(downString)) {
-                return UpDownType.DOWN.as(PercentType.class);
+                return UpDownType.DOWN;
             } else if (updatedValue.equals(stopString)) {
                 return StopMoveType.STOP;
             }
         }
-        return null;
+        throw new IllegalStateException("Cannot call parseCommand() with " + command.toString());
     }
 
     @Override
-    public String getMQTTpublishValue(@Nullable String pattern) {
+    public String getMQTTpublishValue(Command command, @Nullable String pattern) {
         final String upString = this.upString;
         final String downString = this.downString;
-        if (this.nextIsStop) {
+        final String stopString = this.stopString;
-            this.nextIsStop = false;
+        if (command == UpDownType.UP) {
-            return stopString;
+            if (upString != null) {
-        } else if (state instanceof PercentType) {
-            if (state.equals(PercentType.HUNDRED) && downString != null) {
-                return downString;
-            } else if (state.equals(PercentType.ZERO) && upString != null) {
                 return upString;
             } else {
-                return String.valueOf(((PercentType) state).intValue());
+                return ((UpDownType) command).name();
+            }
+        } else if (command == UpDownType.DOWN) {
+            if (downString != null) {
+                return downString;
+            } else {
+                return ((UpDownType) command).name();
+            }
+        } else if (command == StopMoveType.STOP) {
+            if (stopString != null) {
+                return stopString;
+            } else {
+                return ((StopMoveType) command).name();
+            }
+        } else if (command instanceof PercentType) {
+            if (command.equals(PercentType.HUNDRED) && downString != null) {
+                return downString;
+            } else if (command.equals(PercentType.ZERO) && upString != null) {
+                return upString;
+            } else {
+                return String.valueOf(((PercentType) command).intValue());
             }
         } else {
-            return "UNDEF";
+            throw new IllegalArgumentException("Invalid command type for Rollershutter item");
         }
     }
 }

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

@@ -60,13 +60,13 @@ public class TextValue extends Value {
     }
 
     @Override
-    public void update(Command command) throws IllegalArgumentException {
+    public StringType parseCommand(Command command) throws IllegalArgumentException {
         final Set<String> states = this.states;
         String valueStr = command.toString();
         if (states != null && !states.contains(valueStr)) {
             throw new IllegalArgumentException("Value " + valueStr + " not within range");
         }
-        state = new StringType(valueStr);
+        return new StringType(valueStr);
     }
 
     /**

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

@@ -94,11 +94,11 @@ public abstract class Value {
         return state;
     }
 
-    public String getMQTTpublishValue(@Nullable String pattern) {
+    public String getMQTTpublishValue(Command command, @Nullable String pattern) {
         if (pattern == null) {
-            return state.format("%s");
+            return command.format("%s");
         }
-        return state.format(pattern);
+        return command.format(pattern);
     }
 
     /**
@@ -118,22 +118,35 @@ public abstract class Value {
     }
 
     /**
-     * Updates the internal value state with the given command.
+     * Updates the internal value state with the given state.
      *
-     * @param command The command to update the internal value.
+     * @param newState The new state to update the internal value.
      * @exception IllegalArgumentException Thrown if for example a text is assigned to a number type.
      */
-    public abstract void update(Command command) throws IllegalArgumentException;
+    public void update(State newState) throws IllegalArgumentException {
+        state = newState;
+    }
 
     /**
-     * Returns the given command if it cannot be handled by {@link #update(Command)}
+     * Parses a given command into the proper type for this Value type. This will usually be a State,
-     * or {@link #update(byte[])} and need to be posted straight to the framework instead.
+     * but can be a Command.
-     * Returns null otherwise.
      *
-     * @param command The command to decide about
+     * @param command The command to parse.
+     * @exception IllegalArgumentException Thrown if for example a text is assigned to a number type.
      */
-    public @Nullable Command isPostOnly(Command command) {
+    public abstract Command parseCommand(Command command) throws IllegalArgumentException;
-        return null;
+
+    /**
+     * Parses a given command from MQTT into the proper type for this Value type. This will usually
+     * be a State, but can be a non-State Command, in which case the channel will be commanded instead
+     * of updated, regardless of postCommand setting. The default implementation just calls
+     * parseCommand, so that both directions have the same logic.
+     *
+     * @param command The command to parse.
+     * @exception IllegalArgumentException Thrown if for example a text is assigned to a number type.
+     */
+    public Command parseMessage(Command command) throws IllegalArgumentException {
+        return parseCommand(command);
     }
 
     /**

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

@@ -53,6 +53,7 @@ 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;
+import org.openhab.core.types.Command;
 
 /**
  * Tests the {@link ChannelState} class.
@@ -234,8 +235,8 @@ public class ChannelStateTests {
 
         c.processMessage("state", "INCREASE".getBytes());
         assertThat(value.getChannelState().toString(), is("55"));
-        assertThat(value.getMQTTpublishValue(null), is("10"));
+        assertThat(value.getMQTTpublishValue((Command) value.getChannelState(), null), is("10"));
-        assertThat(value.getMQTTpublishValue("%03.0f"), is("010"));
+        assertThat(value.getMQTTpublishValue((Command) value.getChannelState(), "%03.0f"), is("010"));
     }
 
     @Test
@@ -246,23 +247,23 @@ public class ChannelStateTests {
 
         c.processMessage("state", "ON".getBytes()); // Normal on state
         assertThat(value.getChannelState().toString(), is("0,0,10"));
-        assertThat(value.getMQTTpublishValue(null), is("25,25,25"));
+        assertThat(value.getMQTTpublishValue((Command) value.getChannelState(), null), is("25,25,25"));
 
         c.processMessage("state", "FOFF".getBytes()); // Custom off state
         assertThat(value.getChannelState().toString(), is("0,0,0"));
-        assertThat(value.getMQTTpublishValue(null), is("0,0,0"));
+        assertThat(value.getMQTTpublishValue((Command) value.getChannelState(), null), is("0,0,0"));
 
         c.processMessage("state", "10".getBytes()); // Brightness only
         assertThat(value.getChannelState().toString(), is("0,0,10"));
-        assertThat(value.getMQTTpublishValue(null), is("25,25,25"));
+        assertThat(value.getMQTTpublishValue((Command) value.getChannelState(), null), is("25,25,25"));
 
         HSBType t = HSBType.fromRGB(12, 18, 231);
 
         c.processMessage("state", "12,18,231".getBytes());
         assertThat(value.getChannelState(), is(t)); // HSB
         // rgb -> hsv -> rgb is quite lossy
-        assertThat(value.getMQTTpublishValue(null), is("13,20,229"));
+        assertThat(value.getMQTTpublishValue((Command) value.getChannelState(), null), is("13,20,229"));
-        assertThat(value.getMQTTpublishValue("%3$d,%2$d,%1$d"), is("229,20,13"));
+        assertThat(value.getMQTTpublishValue((Command) value.getChannelState(), "%3$d,%2$d,%1$d"), is("229,20,13"));
     }
 
     @Test
@@ -273,19 +274,19 @@ public class ChannelStateTests {
 
         c.processMessage("state", "ON".getBytes()); // Normal on state
         assertThat(value.getChannelState().toString(), is("0,0,10"));
-        assertThat(value.getMQTTpublishValue(null), is("0,0,10"));
+        assertThat(value.getMQTTpublishValue((Command) value.getChannelState(), null), is("0,0,10"));
 
         c.processMessage("state", "FOFF".getBytes()); // Custom off state
         assertThat(value.getChannelState().toString(), is("0,0,0"));
-        assertThat(value.getMQTTpublishValue(null), is("0,0,0"));
+        assertThat(value.getMQTTpublishValue((Command) value.getChannelState(), null), is("0,0,0"));
 
         c.processMessage("state", "10".getBytes()); // Brightness only
         assertThat(value.getChannelState().toString(), is("0,0,10"));
-        assertThat(value.getMQTTpublishValue(null), is("0,0,10"));
+        assertThat(value.getMQTTpublishValue((Command) value.getChannelState(), null), is("0,0,10"));
 
         c.processMessage("state", "12,18,100".getBytes());
         assertThat(value.getChannelState().toString(), is("12,18,100"));
-        assertThat(value.getMQTTpublishValue(null), is("12,18,100"));
+        assertThat(value.getMQTTpublishValue((Command) value.getChannelState(), null), is("12,18,100"));
     }
 
     @Test
@@ -296,22 +297,23 @@ public class ChannelStateTests {
 
         c.processMessage("state", "ON".getBytes()); // Normal on state
         assertThat(value.getChannelState().toString(), is("0,0,10"));
-        assertThat(value.getMQTTpublishValue(null), is("0.312716,0.329002,10.00"));
+        assertThat(value.getMQTTpublishValue((Command) value.getChannelState(), null), is("0.312716,0.329002,10.00"));
 
         c.processMessage("state", "FOFF".getBytes()); // Custom off state
         assertThat(value.getChannelState().toString(), is("0,0,0"));
-        assertThat(value.getMQTTpublishValue(null), is("0.312716,0.329002,0.00"));
+        assertThat(value.getMQTTpublishValue((Command) value.getChannelState(), null), is("0.312716,0.329002,0.00"));
 
         c.processMessage("state", "10".getBytes()); // Brightness only
         assertThat(value.getChannelState().toString(), is("0,0,10"));
-        assertThat(value.getMQTTpublishValue(null), is("0.312716,0.329002,10.00"));
+        assertThat(value.getMQTTpublishValue((Command) value.getChannelState(), null), is("0.312716,0.329002,10.00"));
 
         HSBType t = HSBType.fromXY(0.3f, 0.6f);
 
         c.processMessage("state", "0.3,0.6,100".getBytes());
         assertThat(value.getChannelState(), is(t)); // HSB
-        assertThat(value.getMQTTpublishValue(null), is("0.300000,0.600000,100.00"));
+        assertThat(value.getMQTTpublishValue((Command) value.getChannelState(), null), is("0.300000,0.600000,100.00"));
-        assertThat(value.getMQTTpublishValue("%3$.1f,%2$.4f,%1$.4f"), is("100.0,0.6000,0.3000"));
+        assertThat(value.getMQTTpublishValue((Command) value.getChannelState(), "%3$.1f,%2$.4f,%1$.4f"),
+                is("100.0,0.6000,0.3000"));
     }
 
     @Test
@@ -322,7 +324,7 @@ public class ChannelStateTests {
 
         c.processMessage("state", "46.833974, 7.108433".getBytes());
         assertThat(value.getChannelState().toString(), is("46.833974,7.108433"));
-        assertThat(value.getMQTTpublishValue(null), is("46.833974,7.108433"));
+        assertThat(value.getMQTTpublishValue((Command) value.getChannelState(), null), is("46.833974,7.108433"));
     }
 
     @Test
@@ -339,7 +341,7 @@ public class ChannelStateTests {
         String channelState = value.getChannelState().toString();
         assertTrue(channelState.startsWith(datetime),
                 "Expected '" + channelState + "' to start with '" + datetime + "'");
-        assertThat(value.getMQTTpublishValue(null), is(datetime));
+        assertThat(value.getMQTTpublishValue((Command) value.getChannelState(), null), is(datetime));
     }
 
     @Test

bundles/org.openhab.binding.mqtt.generic/src/test/java/org/openhab/binding/mqtt/generic/internal/handler/GenericThingHandlerTests.java

@@ -42,7 +42,6 @@ import org.openhab.binding.mqtt.generic.values.ValueFactory;
 import org.openhab.binding.mqtt.handler.AbstractBrokerHandler;
 import org.openhab.core.config.core.Configuration;
 import org.openhab.core.io.transport.mqtt.MqttBrokerConnection;
-import org.openhab.core.library.types.OnOffType;
 import org.openhab.core.library.types.StringType;
 import org.openhab.core.thing.ChannelUID;
 import org.openhab.core.thing.Thing;
@@ -51,6 +50,7 @@ import org.openhab.core.thing.ThingStatusDetail;
 import org.openhab.core.thing.ThingStatusInfo;
 import org.openhab.core.thing.binding.ThingHandlerCallback;
 import org.openhab.core.types.RefreshType;
+import org.openhab.core.types.UnDefType;
 
 /**
  * Tests cases for {@link GenericMQTTThingHandler}.
@@ -156,8 +156,9 @@ public class GenericThingHandlerTests {
 
         StringType updateValue = new StringType("UPDATE");
         thingHandler.handleCommand(TEXT_CHANNEL_UID, updateValue);
-        verify(value).update(eq(updateValue));
+        verify(value).parseCommand(eq(updateValue));
-        assertThat(channelConfig.getCache().getChannelState().toString(), is("UPDATE"));
+        // It didn't update the cached state
+        assertThat(value.getChannelState(), is(UnDefType.UNDEF));
     }
 
     @Test
@@ -173,8 +174,7 @@ public class GenericThingHandlerTests {
         StringType updateValue = new StringType("ON");
         thingHandler.handleCommand(TEXT_CHANNEL_UID, updateValue);
 
-        verify(value).update(eq(updateValue));
+        verify(value).parseCommand(eq(updateValue));
-        assertThat(channelConfig.getCache().getChannelState(), is(OnOffType.ON));
     }
 
     @Test

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

@@ -29,11 +29,13 @@ 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.StopMoveType;
 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.State;
 import org.openhab.core.types.TypeParser;
 
 /**
@@ -55,115 +57,106 @@ public class ValueTests {
     @Test
     public void illegalTextStateUpdate() {
         TextValue v = new TextValue("one,two".split(","));
-        assertThrows(IllegalArgumentException.class, () -> v.update(p(v, "three")));
+        assertThrows(IllegalArgumentException.class, () -> v.parseCommand(p(v, "three")));
     }
 
+    @Test
     public void textStateUpdate() {
         TextValue v = new TextValue("one,two".split(","));
-        v.update(p(v, "one"));
+        v.parseCommand(p(v, "one"));
     }
 
+    @Test
     public void colorUpdate() {
         ColorValue v = new ColorValue(ColorMode.RGB, "fancyON", "fancyOFF", 77);
-        v.update(p(v, "255, 255, 255"));
+        v.update((State) v.parseCommand(p(v, "255,255,255")));
 
-        v.update(p(v, "OFF"));
+        HSBType hsb = (HSBType) v.parseCommand(p(v, "OFF"));
-        assertThat(((HSBType) v.getChannelState()).getBrightness().intValue(), is(0));
+        assertThat(hsb.getBrightness().intValue(), is(0));
-        v.update(p(v, "ON"));
+        v.update(hsb);
-        assertThat(((HSBType) v.getChannelState()).getBrightness().intValue(), is(77));
+        hsb = (HSBType) v.parseCommand(p(v, "ON"));
+        assertThat(hsb.getBrightness().intValue(), is(77));
 
-        v.update(p(v, "0"));
+        hsb = (HSBType) v.parseCommand(p(v, "0"));
-        assertThat(((HSBType) v.getChannelState()).getBrightness().intValue(), is(0));
+        assertThat(hsb.getBrightness().intValue(), is(0));
-        v.update(p(v, "1"));
+        hsb = (HSBType) v.parseCommand(p(v, "1"));
-        assertThat(((HSBType) v.getChannelState()).getBrightness().intValue(), is(1));
+        assertThat(hsb.getBrightness().intValue(), is(1));
     }
 
     @Test
     public void illegalColorUpdate() {
         ColorValue v = new ColorValue(ColorMode.RGB, null, null, 10);
-        assertThrows(IllegalArgumentException.class, () -> v.update(p(v, "255,255,abc")));
+        assertThrows(IllegalArgumentException.class, () -> v.parseCommand(p(v, "255,255,abc")));
     }
 
     @Test
     public void illegalNumberCommand() {
         NumberValue v = new NumberValue(null, null, null, null);
-        assertThrows(IllegalArgumentException.class, () -> v.update(OnOffType.OFF));
+        assertThrows(IllegalArgumentException.class, () -> v.parseCommand(OnOffType.OFF));
     }
 
     @Test
     public void illegalPercentCommand() {
         PercentageValue v = new PercentageValue(null, null, null, null, null);
-        assertThrows(IllegalStateException.class, () -> v.update(new StringType("demo")));
+        assertThrows(IllegalStateException.class, () -> v.parseCommand(new StringType("demo")));
     }
 
     @Test
     public void illegalOnOffCommand() {
         OnOffValue v = new OnOffValue(null, null);
-        assertThrows(IllegalArgumentException.class, () -> v.update(new DecimalType(101.0)));
+        assertThrows(IllegalArgumentException.class, () -> v.parseCommand(new DecimalType(101.0)));
     }
 
     @Test
     public void illegalPercentUpdate() {
         PercentageValue v = new PercentageValue(null, null, null, null, null);
-        assertThrows(IllegalArgumentException.class, () -> v.update(new DecimalType(101.0)));
+        assertThrows(IllegalArgumentException.class, () -> v.parseCommand(new DecimalType(101.0)));
     }
 
     @Test
     public void onoffUpdate() {
         OnOffValue v = new OnOffValue("fancyON", "fancyOff");
+
         // Test with command
-        v.update(OnOffType.OFF);
+        assertThat(v.parseCommand(OnOffType.OFF), is(OnOffType.OFF));
-        assertThat(v.getMQTTpublishValue(null), is("fancyOff"));
+        assertThat(v.parseCommand(OnOffType.ON), is(OnOffType.ON));
-        assertThat(v.getChannelState(), is(OnOffType.OFF));
-        v.update(OnOffType.ON);
-        assertThat(v.getMQTTpublishValue(null), is("fancyON"));
-        assertThat(v.getChannelState(), is(OnOffType.ON));
 
         // Test with string, representing the command
-        v.update(new StringType("OFF"));
+        assertThat(v.parseCommand(new StringType("OFF")), is(OnOffType.OFF));
-        assertThat(v.getMQTTpublishValue(null), is("fancyOff"));
+        assertThat(v.parseCommand(new StringType("ON")), is(OnOffType.ON));
-        assertThat(v.getChannelState(), is(OnOffType.OFF));
-        v.update(new StringType("ON"));
-        assertThat(v.getMQTTpublishValue(null), is("fancyON"));
-        assertThat(v.getChannelState(), is(OnOffType.ON));
 
         // Test with custom string, setup in the constructor
-        v.update(new StringType("fancyOff"));
+        assertThat(v.parseCommand(new StringType("fancyOff")), is(OnOffType.OFF));
-        assertThat(v.getMQTTpublishValue(null), is("fancyOff"));
+        assertThat(v.parseCommand(new StringType("fancyON")), is(OnOffType.ON));
-        assertThat(v.getMQTTpublishValue("=%s"), is("=fancyOff"));
+
-        assertThat(v.getChannelState(), is(OnOffType.OFF));
+        // Test basic formatting
-        v.update(new StringType("fancyON"));
+        assertThat(v.getMQTTpublishValue(OnOffType.ON, null), is("fancyON"));
-        assertThat(v.getMQTTpublishValue(null), is("fancyON"));
+        assertThat(v.getMQTTpublishValue(OnOffType.OFF, null), is("fancyOff"));
-        assertThat(v.getMQTTpublishValue("=%s"), is("=fancyON"));
+
-        assertThat(v.getChannelState(), is(OnOffType.ON));
+        // Test custom formatting
+        assertThat(v.getMQTTpublishValue(OnOffType.OFF, "=%s"), is("=fancyOff"));
+        assertThat(v.getMQTTpublishValue(OnOffType.ON, "=%s"), is("=fancyON"));
     }
 
     @Test
     public void openCloseUpdate() {
         OpenCloseValue v = new OpenCloseValue("fancyON", "fancyOff");
+
         // Test with command
-        v.update(OpenClosedType.CLOSED);
+        assertThat(v.parseCommand(OpenClosedType.CLOSED), is(OpenClosedType.CLOSED));
-        assertThat(v.getMQTTpublishValue(null), is("fancyOff"));
+        assertThat(v.parseCommand(OpenClosedType.OPEN), is(OpenClosedType.OPEN));
-        assertThat(v.getChannelState(), is(OpenClosedType.CLOSED));
-        v.update(OpenClosedType.OPEN);
-        assertThat(v.getMQTTpublishValue(null), is("fancyON"));
-        assertThat(v.getChannelState(), is(OpenClosedType.OPEN));
 
         // Test with string, representing the command
-        v.update(new StringType("CLOSED"));
+        assertThat(v.parseCommand(new StringType("CLOSED")), is(OpenClosedType.CLOSED));
-        assertThat(v.getMQTTpublishValue(null), is("fancyOff"));
+        assertThat(v.parseCommand(new StringType("OPEN")), is(OpenClosedType.OPEN));
-        assertThat(v.getChannelState(), is(OpenClosedType.CLOSED));
-        v.update(new StringType("OPEN"));
-        assertThat(v.getMQTTpublishValue(null), is("fancyON"));
-        assertThat(v.getChannelState(), is(OpenClosedType.OPEN));
 
         // Test with custom string, setup in the constructor
-        v.update(new StringType("fancyOff"));
+        assertThat(v.parseCommand(new StringType("fancyOff")), is(OpenClosedType.CLOSED));
-        assertThat(v.getMQTTpublishValue(null), is("fancyOff"));
+        assertThat(v.parseCommand(new StringType("fancyON")), is(OpenClosedType.OPEN));
-        assertThat(v.getChannelState(), is(OpenClosedType.CLOSED));
+
-        v.update(new StringType("fancyON"));
+        // Test basic formatting
-        assertThat(v.getMQTTpublishValue(null), is("fancyON"));
+        assertThat(v.getMQTTpublishValue(OpenClosedType.CLOSED, null), is("fancyOff"));
-        assertThat(v.getChannelState(), is(OpenClosedType.OPEN));
+        assertThat(v.getMQTTpublishValue(OpenClosedType.OPEN, null), is("fancyON"));
     }
 
     @Test
@@ -171,25 +164,25 @@ public class ValueTests {
         NumberValue v = new NumberValue(null, null, new BigDecimal(10), Units.WATT);
 
         // Test with command with units
-        v.update(new QuantityType<>(20, Units.WATT));
+        Command command = v.parseCommand(new QuantityType<>(20, Units.WATT));
-        assertThat(v.getMQTTpublishValue(null), is("20"));
+        assertThat(command, is(new QuantityType<>(20, Units.WATT)));
-        assertThat(v.getChannelState(), is(new QuantityType<>(20, Units.WATT)));
+        assertThat(v.getMQTTpublishValue(command, null), is("20"));
-        v.update(new QuantityType<>(20, MetricPrefix.KILO(Units.WATT)));
+        command = v.parseCommand(new QuantityType<>(20, MetricPrefix.KILO(Units.WATT)));
-        assertThat(v.getMQTTpublishValue(null), is("20000"));
+        assertThat(command, is(new QuantityType<>(20, MetricPrefix.KILO(Units.WATT))));
-        assertThat(v.getChannelState(), is(new QuantityType<>(20, MetricPrefix.KILO(Units.WATT))));
+        assertThat(v.getMQTTpublishValue(command, null), is("20000"));
 
         // Test with command without units
-        v.update(new QuantityType<>("20"));
+        command = v.parseCommand(new QuantityType<>("20"));
-        assertThat(v.getMQTTpublishValue(null), is("20"));
+        assertThat(command, is(new QuantityType<>(20, Units.WATT)));
-        assertThat(v.getChannelState(), is(new QuantityType<>(20, Units.WATT)));
+        assertThat(v.getMQTTpublishValue(command, null), is("20"));
     }
 
     @Test
     public void numberUpdateMireds() {
         NumberValue v = new NumberValue(null, null, new BigDecimal(10), Units.MIRED);
 
-        v.update(new QuantityType<>(2700, Units.KELVIN));
+        Command command = v.parseCommand(new QuantityType<>(2700, Units.KELVIN));
-        assertThat(v.getMQTTpublishValue("%.0f"), is("370"));
+        assertThat(v.getMQTTpublishValue(command, "%.0f"), is("370"));
     }
 
     @Test
@@ -197,87 +190,76 @@ public class ValueTests {
         NumberValue v = new NumberValue(null, null, new BigDecimal(10), Units.PERCENT);
 
         // Test with command with units
-        v.update(new QuantityType<>(20, Units.PERCENT));
+        Command command = v.parseCommand(new QuantityType<>(20, Units.PERCENT));
-        assertThat(v.getMQTTpublishValue(null), is("20"));
+        assertThat(command, is(new QuantityType<>(20, Units.PERCENT)));
-        assertThat(v.getChannelState(), is(new QuantityType<>(20, Units.PERCENT)));
+        assertThat(v.getMQTTpublishValue(command, null), is("20"));
 
         // Test with command without units
-        v.update(new QuantityType<>("20"));
+        command = v.parseCommand(new QuantityType<>("20"));
-        assertThat(v.getMQTTpublishValue(null), is("20"));
+        assertThat(command, is(new QuantityType<>(20, Units.PERCENT)));
-        assertThat(v.getChannelState(), is(new QuantityType<>(20, Units.PERCENT)));
+        assertThat(v.getMQTTpublishValue(command, null), is("20"));
     }
 
     @Test
     public void rollershutterUpdateWithStrings() {
         RollershutterValue v = new RollershutterValue("fancyON", "fancyOff", "fancyStop");
-        // Test with command
+        // Test with UP/DOWN/STOP command
-        v.update(UpDownType.UP);
+        assertThat(v.parseCommand(UpDownType.UP), is(UpDownType.UP));
-        assertThat(v.getMQTTpublishValue(null), is("fancyON"));
+        assertThat(v.getMQTTpublishValue(UpDownType.UP, null), is("fancyON"));
-        assertThat(v.getChannelState(), is(PercentType.ZERO));
+        assertThat(v.parseCommand(UpDownType.DOWN), is(UpDownType.DOWN));
-        v.update(UpDownType.DOWN);
+        assertThat(v.getMQTTpublishValue(UpDownType.DOWN, null), is("fancyOff"));
-        assertThat(v.getMQTTpublishValue(null), is("fancyOff"));
+        assertThat(v.parseCommand(StopMoveType.STOP), is(StopMoveType.STOP));
-        assertThat(v.getChannelState(), is(PercentType.HUNDRED));
+        assertThat(v.getMQTTpublishValue(StopMoveType.STOP, null), is("fancyStop"));
 
         // Test with custom string
-        v.update(new StringType("fancyON"));
+        assertThat(v.parseCommand(new StringType("fancyON")), is(UpDownType.UP));
-        assertThat(v.getMQTTpublishValue(null), is("fancyON"));
+        assertThat(v.parseCommand(new StringType("fancyOff")), is(UpDownType.DOWN));
-        assertThat(v.getChannelState(), is(PercentType.ZERO));
+
-        v.update(new StringType("fancyOff"));
+        // Test with exact percent
-        assertThat(v.getMQTTpublishValue(null), is("fancyOff"));
+        Command command = new PercentType(27);
-        assertThat(v.getChannelState(), is(PercentType.HUNDRED));
+        assertThat(v.parseCommand((Command) command), is(command));
-        v.update(new PercentType(27));
+        assertThat(v.getMQTTpublishValue(command, null), is("27"));
-        assertThat(v.getMQTTpublishValue(null), is("27"));
+
-        assertThat(v.getChannelState(), is(new PercentType(27)));
+        // Test formatting 0/100
+        assertThat(v.getMQTTpublishValue(PercentType.ZERO, null), is("fancyON"));
+        assertThat(v.getMQTTpublishValue(PercentType.HUNDRED, null), is("fancyOff"));
     }
 
     @Test
     public void rollershutterUpdateWithOutStrings() {
         RollershutterValue v = new RollershutterValue(null, null, "fancyStop");
         // Test with command
-        v.update(UpDownType.UP);
+        assertThat(v.parseCommand(UpDownType.UP), is(PercentType.ZERO));
-        assertThat(v.getMQTTpublishValue(null), is("0"));
+        assertThat(v.parseCommand(UpDownType.DOWN), is(PercentType.HUNDRED));
-        assertThat(v.getChannelState(), is(PercentType.ZERO));
-        v.update(UpDownType.DOWN);
-        assertThat(v.getMQTTpublishValue(null), is("100"));
-        assertThat(v.getChannelState(), is(PercentType.HUNDRED));
 
         // Test with custom string
-        v.update(PercentType.ZERO);
+        // Test formatting 0/100
-        assertThat(v.getMQTTpublishValue(null), is("0"));
+        assertThat(v.getMQTTpublishValue(PercentType.ZERO, null), is("0"));
-        assertThat(v.getChannelState(), is(PercentType.ZERO));
+        assertThat(v.getMQTTpublishValue(PercentType.HUNDRED, null), is("100"));
-        v.update(PercentType.HUNDRED);
-        assertThat(v.getMQTTpublishValue(null), is("100"));
-        assertThat(v.getChannelState(), is(PercentType.HUNDRED));
-        v.update(new PercentType(27));
-        assertThat(v.getMQTTpublishValue(null), is("27"));
-        assertThat(v.getChannelState(), is(new PercentType(27)));
     }
 
     @Test
     public void percentCalc() {
         PercentageValue v = new PercentageValue(new BigDecimal(10.0), new BigDecimal(110.0), new BigDecimal(1.0), null,
                 null);
-        v.update(new DecimalType("110.0"));
+        assertThat(v.parseCommand(new DecimalType("110.0")), is(PercentType.HUNDRED));
-        assertThat((PercentType) v.getChannelState(), is(new PercentType(100)));
+        assertThat(v.getMQTTpublishValue(PercentType.HUNDRED, null), is("110"));
-        assertThat(v.getMQTTpublishValue(null), is("110"));
+        assertThat(v.parseCommand(new DecimalType(10.0)), is(PercentType.ZERO));
-        v.update(new DecimalType(10.0));
+        assertThat(v.getMQTTpublishValue(PercentType.ZERO, null), is("10"));
-        assertThat((PercentType) v.getChannelState(), is(new PercentType(0)));
-        assertThat(v.getMQTTpublishValue(null), is("10"));
 
-        v.update(OnOffType.ON);
+        assertThat(v.parseCommand(OnOffType.ON), is(PercentType.HUNDRED));
-        assertThat((PercentType) v.getChannelState(), is(new PercentType(100)));
+        assertThat(v.parseCommand(OnOffType.OFF), is(PercentType.ZERO));
-        v.update(OnOffType.OFF);
-        assertThat((PercentType) v.getChannelState(), is(new PercentType(0)));
     }
 
     @Test
     public void percentMQTTValue() {
         PercentageValue v = new PercentageValue(null, null, null, null, null);
-        v.update(new DecimalType("10.10000"));
+        assertThat(v.parseCommand(new DecimalType("10.10000")), is(new PercentType("10.1")));
-        assertThat(v.getMQTTpublishValue(null), is("10.1"));
+        assertThat(v.getMQTTpublishValue(new PercentType("10.1"), null), is("10.1"));
+        Command command;
         for (int i = 0; i <= 100; i++) {
-            v.update(new DecimalType(i));
+            command = v.parseCommand(new DecimalType(i));
-            assertThat(v.getMQTTpublishValue(null), is("" + i));
+            assertThat(v.getMQTTpublishValue(command, null), is("" + i));
         }
     }
 
@@ -285,22 +267,18 @@ public class ValueTests {
     public void percentCustomOnOff() {
         PercentageValue v = new PercentageValue(new BigDecimal("0.0"), new BigDecimal("100.0"), new BigDecimal("1.0"),
                 "on", "off");
-        v.update(new StringType("on"));
+        assertThat(v.parseCommand(new StringType("on")), is(PercentType.HUNDRED));
-        assertThat((PercentType) v.getChannelState(), is(new PercentType(100)));
+        assertThat(v.parseCommand(new StringType("off")), is(PercentType.ZERO));
-        v.update(new StringType("off"));
-        assertThat((PercentType) v.getChannelState(), is(new PercentType(0)));
     }
 
     @Test
     public void decimalCalc() {
         PercentageValue v = new PercentageValue(new BigDecimal("0.1"), new BigDecimal("1.0"), new BigDecimal("0.1"),
                 null, null);
-        v.update(new DecimalType(1.0));
+        assertThat(v.parseCommand(new DecimalType(1.0)), is(PercentType.HUNDRED));
-        assertThat((PercentType) v.getChannelState(), is(new PercentType(100)));
+        assertThat(v.parseCommand(new DecimalType(0.1)), is(PercentType.ZERO));
-        v.update(new DecimalType(0.1));
+        PercentType command = (PercentType) v.parseCommand(new DecimalType(0.2));
-        assertThat((PercentType) v.getChannelState(), is(new PercentType(0)));
+        assertEquals(command.floatValue(), 11.11f, 0.01f);
-        v.update(new DecimalType(0.2));
-        assertEquals(((PercentType) v.getChannelState()).floatValue(), 11.11f, 0.01f);
     }
 
     @Test
@@ -309,25 +287,27 @@ public class ValueTests {
                 null, null);
 
         // Normal operation.
-        v.update(new DecimalType("6.0"));
+        PercentType command = (PercentType) v.parseCommand(new DecimalType("6.0"));
-        assertEquals(((PercentType) v.getChannelState()).floatValue(), 50.0f, 0.01f);
+        assertEquals(command.floatValue(), 50.0f, 0.01f);
-        v.update(IncreaseDecreaseType.INCREASE);
+        v.update(command);
-        assertEquals(((PercentType) v.getChannelState()).floatValue(), 55.0f, 0.01f);
+        command = (PercentType) v.parseCommand(IncreaseDecreaseType.INCREASE);
-        v.update(IncreaseDecreaseType.DECREASE);
+        assertEquals(command.floatValue(), 55.0f, 0.01f);
-        v.update(IncreaseDecreaseType.DECREASE);
+        command = (PercentType) v.parseCommand(IncreaseDecreaseType.DECREASE);
-        assertEquals(((PercentType) v.getChannelState()).floatValue(), 45.0f, 0.01f);
+        assertEquals(command.floatValue(), 45.0f, 0.01f);
 
         // Lower limit.
-        v.update(new DecimalType("1.1"));
+        command = (PercentType) v.parseCommand(new DecimalType("1.1"));
-        assertEquals(((PercentType) v.getChannelState()).floatValue(), 1.0f, 0.01f);
+        assertEquals(command.floatValue(), 1.0f, 0.01f);
-        v.update(IncreaseDecreaseType.DECREASE);
+        v.update(command);
-        assertEquals(((PercentType) v.getChannelState()).floatValue(), 0.0f, 0.01f);
+        command = (PercentType) v.parseCommand(IncreaseDecreaseType.DECREASE);
+        assertEquals(command.floatValue(), 0.0f, 0.01f);
 
         // Upper limit.
-        v.update(new DecimalType("10.8"));
+        command = (PercentType) v.parseCommand(new DecimalType("10.8"));
-        assertEquals(((PercentType) v.getChannelState()).floatValue(), 98.0f, 0.01f);
+        assertEquals(command.floatValue(), 98.0f, 0.01f);
-        v.update(IncreaseDecreaseType.INCREASE);
+        v.update(command);
-        assertEquals(((PercentType) v.getChannelState()).floatValue(), 100.0f, 0.01f);
+        command = (PercentType) v.parseCommand(IncreaseDecreaseType.INCREASE);
+        assertEquals(command.floatValue(), 100.0f, 0.01f);
     }
 
     @Test
@@ -336,31 +316,33 @@ public class ValueTests {
                 null, null);
 
         // Normal operation.
-        v.update(new DecimalType("6.0"));
+        PercentType command = (PercentType) v.parseCommand(new DecimalType("6.0"));
-        assertEquals(((PercentType) v.getChannelState()).floatValue(), 50.0f, 0.01f);
+        assertEquals(command.floatValue(), 50.0f, 0.01f);
-        v.update(UpDownType.UP);
+        v.update(command);
-        assertEquals(((PercentType) v.getChannelState()).floatValue(), 55.0f, 0.01f);
+        command = (PercentType) v.parseCommand(UpDownType.UP);
-        v.update(UpDownType.DOWN);
+        assertEquals(command.floatValue(), 55.0f, 0.01f);
-        v.update(UpDownType.DOWN);
+        command = (PercentType) v.parseCommand(UpDownType.DOWN);
-        assertEquals(((PercentType) v.getChannelState()).floatValue(), 45.0f, 0.01f);
+        assertEquals(command.floatValue(), 45.0f, 0.01f);
 
         // Lower limit.
-        v.update(new DecimalType("1.1"));
+        command = (PercentType) v.parseCommand(new DecimalType("1.1"));
-        assertEquals(((PercentType) v.getChannelState()).floatValue(), 1.0f, 0.01f);
+        assertEquals(command.floatValue(), 1.0f, 0.01f);
-        v.update(UpDownType.DOWN);
+        v.update(command);
-        assertEquals(((PercentType) v.getChannelState()).floatValue(), 0.0f, 0.01f);
+        command = (PercentType) v.parseCommand(UpDownType.DOWN);
+        assertEquals(command.floatValue(), 0.0f, 0.01f);
 
         // Upper limit.
-        v.update(new DecimalType("10.8"));
+        command = (PercentType) v.parseCommand(new DecimalType("10.8"));
-        assertEquals(((PercentType) v.getChannelState()).floatValue(), 98.0f, 0.01f);
+        assertEquals(command.floatValue(), 98.0f, 0.01f);
-        v.update(UpDownType.UP);
+        v.update(command);
-        assertEquals(((PercentType) v.getChannelState()).floatValue(), 100.0f, 0.01f);
+        command = (PercentType) v.parseCommand(UpDownType.UP);
+        assertEquals(command.floatValue(), 100.0f, 0.01f);
     }
 
     @Test
     public void percentCalcInvalid() {
         PercentageValue v = new PercentageValue(new BigDecimal(10.0), new BigDecimal(110.0), new BigDecimal(1.0), null,
                 null);
-        assertThrows(IllegalArgumentException.class, () -> v.update(new DecimalType(9.0)));
+        assertThrows(IllegalArgumentException.class, () -> v.parseCommand(new DecimalType(9.0)));
     }
 }

bundles/org.openhab.binding.mqtt.homie/src/test/java/org/openhab/binding/mqtt/homie/internal/handler/HomieThingHandlerTests.java

@@ -46,7 +46,6 @@ import org.openhab.binding.mqtt.generic.mapping.AbstractMqttAttributeClass;
 import org.openhab.binding.mqtt.generic.mapping.SubscribeFieldToMQTTtopic;
 import org.openhab.binding.mqtt.generic.tools.ChildMap;
 import org.openhab.binding.mqtt.generic.tools.DelayedBatchProcessing;
-import org.openhab.binding.mqtt.generic.values.Value;
 import org.openhab.binding.mqtt.handler.AbstractBrokerHandler;
 import org.openhab.binding.mqtt.homie.ChannelStateHelper;
 import org.openhab.binding.mqtt.homie.ThingHandlerHelper;
@@ -71,9 +70,7 @@ import org.openhab.core.thing.binding.ThingHandlerCallback;
 import org.openhab.core.thing.binding.builder.ThingBuilder;
 import org.openhab.core.thing.type.ChannelKind;
 import org.openhab.core.thing.type.ThingTypeRegistry;
-import org.openhab.core.types.Command;
 import org.openhab.core.types.RefreshType;
-import org.openhab.core.types.TypeParser;
 
 /**
  * Tests cases for {@link HomieThingHandler}.
@@ -258,25 +255,19 @@ public class HomieThingHandlerTests {
 
         StringType updateValue = new StringType("UPDATE");
         thingHandler.handleCommand(property.channelUID, updateValue);
-
-        assertThat(property.getChannelState().getCache().getChannelState().toString(), is("UPDATE"));
         verify(connectionMock, times(1)).publish(any(), any(), anyInt(), anyBoolean());
 
         // Check non writable property
         property.attributes.settable = false;
         property.attributesReceived();
         // Assign old value
-        Value value = property.getChannelState().getCache();
+        property.getChannelState().getCache().update(new StringType("OLDVALUE"));
-        Command command = TypeParser.parseCommand(value.getSupportedCommandTypes(), "OLDVALUE");
+        // Try to update with new value
-        if (command != null) {
+        updateValue = new StringType("SOMETHINGNEW");
-            property.getChannelState().getCache().update(command);
+        thingHandler.handleCommand(property.channelUID, updateValue);
-            // Try to update with new value
+        // Expect old value and no MQTT publish
-            updateValue = new StringType("SOMETHINGNEW");
+        assertThat(property.getChannelState().getCache().getChannelState().toString(), is("OLDVALUE"));
-            thingHandler.handleCommand(property.channelUID, updateValue);
+        verify(connectionMock, times(1)).publish(any(), any(), anyInt(), anyBoolean());
-            // Expect old value and no MQTT publish
-            assertThat(property.getChannelState().getCache().getChannelState().toString(), is("OLDVALUE"));
-            verify(connectionMock, times(1)).publish(any(), any(), anyInt(), anyBoolean());
-        }
     }
 
     public Object createSubscriberAnswer(InvocationOnMock invocation) {