[homematic] Validate datapoint values before writing to config (#12557)

* [homematic] Validate datapoint values before writing to config

HM config datapoint values on some devices can be out of their valid
range in some cases, particularly if they are unused by the device
currently [1]. Since openhab-core now validates attempts to update
configuration by the thing handlers, make sure we always send a valid
configuration even for those affected datapoints.

[1] One example for such behaviour is HmIPW-DRBL4, which has multiple
    datapoints which depend on each other
    - POWERUP_JUMPTARGET can have values OFF, ON, OFF_DELAY, ON_DELAY
    - POWERUP_ONDELAY_VALUE, POWERUP_ONDELAY_UNIT are only used if
      POWERUP_JUMPTARGET has value ON_DELAY
    - likewise, POWERUP_OFFDELAY_VALUE and POWERUP_OFFDELAY_UNIT are
      only used if POWERUP_JUMPTARGET has value OFF_DELAY
    - if e.g. the POWERUP_JUMPTARGET is OFF, e.g. POWERUP_ONDELAY_VALUE
      might stay uninitialized by CCU and/or device itself, in which
      case it may be reported with an invalid value

Signed-off-by: Danny Baumann <dannybaumann@web.de>

diff --git a/bundles/org.openhab.binding.homematic/src/main/java/org/openhab/binding/homematic/internal/handler/HomematicThingHandler.java b/bundles/org.openhab.binding.homematic/src/main/java/org/openhab/binding/homematic/internal/handler/HomematicThingHandler.java
index 1b11998d8058c43e83b2b807742567cd1f65c33d..33411cdd7592b8679d58b12be82357329a065c97 100644 (file)
--- a/bundles/org.openhab.binding.homematic/src/main/java/org/openhab/binding/homematic/internal/handler/HomematicThingHandler.java
+++ b/bundles/org.openhab.binding.homematic/src/main/java/org/openhab/binding/homematic/internal/handler/HomematicThingHandler.java
@@ -26,6 +26,7 @@ import java.util.Map.Entry;
 import java.util.Objects;
 import java.util.concurrent.Future;
 
+import org.eclipse.jdt.annotation.Nullable;
 import org.openhab.binding.homematic.internal.HomematicBindingConstants;
 import org.openhab.binding.homematic.internal.common.HomematicConfig;
 import org.openhab.binding.homematic.internal.communicator.HomematicGateway;
@@ -140,8 +141,7 @@ public class HomematicThingHandler extends BaseThingHandler {
             loadHomematicChannelValues(channel);
             for (HmDatapoint dp : channel.getDatapoints()) {
                 if (dp.getParamsetType() == HmParamsetType.MASTER) {
-                    config.put(MetadataUtils.getParameterName(dp),
-                            dp.isEnumType() ? dp.getOptionValue() : dp.getValue());
+                    config.put(MetadataUtils.getParameterName(dp), getValueForConfiguration(dp));
                 }
             }
         }
@@ -401,7 +401,7 @@ public class HomematicThingHandler extends BaseThingHandler {
             if (dp.getParamsetType() == HmParamsetType.MASTER) {
                 // update configuration
                 Configuration config = editConfiguration();
-                config.put(MetadataUtils.getParameterName(dp), dp.isEnumType() ? dp.getOptionValue() : dp.getValue());
+                config.put(MetadataUtils.getParameterName(dp), getValueForConfiguration(dp));
                 updateConfiguration(config);
             } else if (!HomematicTypeGeneratorImpl.isIgnoredDatapoint(dp)) {
                 // update channel
@@ -420,6 +420,37 @@ public class HomematicThingHandler extends BaseThingHandler {
         }
     }
 
+    private @Nullable Object getValueForConfiguration(HmDatapoint dp) {
+        if (dp.getValue() == null) {
+            return null;
+        }
+        if (dp.isEnumType()) {
+            return dp.getOptionValue();
+        }
+        if (dp.isNumberType()) {
+            // For number datapoints that are only used depending on the value of other datapoints,
+            // the CCU may return invalid (out of range) values if the datapoint currently is not in use.
+            // Make sure to not invalidate the whole configuration by returning the datapoint's default
+            // value in that case.
+            final boolean minValid, maxValid;
+            if (dp.isFloatType()) {
+                Double numValue = dp.getDoubleValue();
+                minValid = dp.getMinValue() == null || numValue >= dp.getMinValue().doubleValue();
+                maxValid = dp.getMaxValue() == null || numValue <= dp.getMaxValue().doubleValue();
+            } else {
+                Integer numValue = dp.getIntegerValue();
+                minValid = dp.getMinValue() == null || numValue >= dp.getMinValue().intValue();
+                maxValid = dp.getMaxValue() == null || numValue <= dp.getMaxValue().intValue();
+            }
+            if (minValid && maxValid) {
+                return dp.getValue();
+            }
+            logger.warn("Value for datapoint {} is outside of valid range, using default value for config.", dp);
+            return dp.getDefaultValue();
+        }
+        return dp.getValue();
+    }
+
     /**
      * Converts the value of the datapoint to a State, updates the channel and also sets the thing status if necessary.
      */

diff --git a/bundles/org.openhab.binding.homematic/src/main/java/org/openhab/binding/homematic/internal/model/HmDatapoint.java b/bundles/org.openhab.binding.homematic/src/main/java/org/openhab/binding/homematic/internal/model/HmDatapoint.java
index e334c06fc337742b4bbbefc1643692f09fa803c1..7c7f73042bf4146b1f54b8596a46445e9178565d 100644 (file)
--- a/bundles/org.openhab.binding.homematic/src/main/java/org/openhab/binding/homematic/internal/model/HmDatapoint.java
+++ b/bundles/org.openhab.binding.homematic/src/main/java/org/openhab/binding/homematic/internal/model/HmDatapoint.java
@@ -12,6 +12,7 @@
  */
 package org.openhab.binding.homematic.internal.model;
 
+import org.eclipse.jdt.annotation.Nullable;
 import org.openhab.binding.homematic.internal.misc.MiscUtils;
 
 /**
@@ -148,21 +149,34 @@ public class HmDatapoint implements Cloneable {
     /**
      * Returns the value of a option list.
      */
-    public String getOptionValue() {
-        if (options != null && value != null) {
-            int idx = 0;
-            if (value instanceof Integer) {
-                idx = (int) value;
-            } else {
-                idx = Integer.parseInt(value.toString());
-            }
-            if (idx < options.length) {
-                return options[idx];
-            }
+    public @Nullable String getOptionValue() {
+        Integer idx = getIntegerValue();
+        if (options != null && idx != null && idx < options.length) {
+            return options[idx];
         }
         return null;
     }
 
+    public @Nullable Integer getIntegerValue() {
+        if (value instanceof Integer) {
+            return (int) value;
+        } else if (value != null) {
+            return Integer.parseInt(value.toString());
+        } else {
+            return null;
+        }
+    }
+
+    public @Nullable Double getDoubleValue() {
+        if (value instanceof Double) {
+            return (double) value;
+        } else if (value != null) {
+            return Double.parseDouble(value.toString());
+        } else {
+            return null;
+        }
+    }
+
     /**
      * Returns the max value.
      */