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

maniac103 · 2022-04-01T12:32:24Z

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. 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.

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

Fixes the issue reported here

maniac103 · 2022-04-01T12:33:03Z

@fwolter @MHerbst Please review as well ;-)

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>

...atic/src/main/java/org/openhab/binding/homematic/internal/handler/HomematicThingHandler.java

lolodomo · 2022-04-02T08:50:34Z

...atic/src/main/java/org/openhab/binding/homematic/internal/handler/HomematicThingHandler.java

@@ -140,8 +140,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));


As getValueForConfiguration can retrun null, I am not sure whether this config entry should be set to null, or not set at all ?
I am even not sure if this is valid to set a null value for a config parameter.

The only case under which getValueForConfiguration() returns null is if the datapoint holds a null value. In that case I'd say that writing null into the config is correct.
The places I checked that care about the keys present in the map (those that call containsKey() and keySet() seem to be able to deal with null values being present. Not sure about e.g. the UI, but given the previous code put null values in this case (value being null) as well, I guess it should be fine.

...inding.homematic/src/main/java/org/openhab/binding/homematic/internal/model/HmDatapoint.java

...atic/src/main/java/org/openhab/binding/homematic/internal/handler/HomematicThingHandler.java

...inding.homematic/src/main/java/org/openhab/binding/homematic/internal/model/HmDatapoint.java

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

lolodomo

LGTM

…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 <dannybaumann@web.de> Signed-off-by: Nick Waterton <n.waterton@outlook.com>

openhab-bot · 2022-05-06T21:09:00Z

This pull request has been mentioned on openHAB Community. There might be relevant details there:

https://community.openhab.org/t/openhab-3-3-milestone-discussion/132715/130

…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 <dannybaumann@web.de>

…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 <dannybaumann@web.de> Signed-off-by: Andras Uhrin <andras.uhrin@gmail.com>

…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 <dannybaumann@web.de>

maniac103 requested a review from gerrieg as a code owner April 1, 2022 12:32

maniac103 force-pushed the hm-config-validation branch from ac8ae99 to fa4d32b Compare April 1, 2022 12:37

jlaur added the bug An unexpected problem or unintended behavior of an add-on label Apr 2, 2022

lolodomo requested changes Apr 2, 2022

View reviewed changes

Address review comments

4fac861

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

lolodomo approved these changes Apr 3, 2022

View reviewed changes

lolodomo merged commit 28ce7eb into openhab:main Apr 3, 2022

lolodomo added this to the 3.3 milestone Apr 3, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

maniac103 commented Apr 1, 2022

maniac103 commented Apr 1, 2022

lolodomo Apr 2, 2022

maniac103 Apr 2, 2022

lolodomo left a comment

openhab-bot commented May 6, 2022

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

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

Conversation

maniac103 commented Apr 1, 2022

maniac103 commented Apr 1, 2022

lolodomo Apr 2, 2022

Choose a reason for hiding this comment

maniac103 Apr 2, 2022

Choose a reason for hiding this comment

lolodomo left a comment

Choose a reason for hiding this comment

openhab-bot commented May 6, 2022