From 28ce7ebaedf3729cfa246b8a4896b6eedb0eb529 Mon Sep 17 00:00:00 2001 From: maniac103 Date: Sun, 3 Apr 2022 11:39:59 +0200 Subject: [PATCH] [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 --- .../handler/HomematicThingHandler.java | 37 +++++++++++++++++-- .../homematic/internal/model/HmDatapoint.java | 36 ++++++++++++------ 2 files changed, 59 insertions(+), 14 deletions(-) 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 1b11998d8..33411cdd7 100644 --- 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 e334c06fc..7c7f73042 100644 --- 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. */