Skip to content

Commit

Permalink
[homematic] Validate datapoint values before writing to config (openh…
Browse files Browse the repository at this point in the history
…ab#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 <[email protected]>
  • Loading branch information
maniac103 authored and psmedley committed Feb 23, 2023
1 parent b9ec282 commit ffd4aee
Show file tree
Hide file tree
Showing 2 changed files with 59 additions and 14 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
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;
Expand Down Expand Up @@ -140,8 +141,7 @@ private void doInitializeInBackground() throws GatewayNotAvailableException, Hom
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));
}
}
}
Expand Down Expand Up @@ -401,7 +401,7 @@ protected void updateDatapointState(HmDatapoint dp) {
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
Expand All @@ -420,6 +420,37 @@ protected void updateDatapointState(HmDatapoint dp) {
}
}

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.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

/**
Expand Down Expand Up @@ -148,21 +149,34 @@ public int getOptionIndex(String option) {
/**
* 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.
*/
Expand Down

0 comments on commit ffd4aee

Please sign in to comment.