Merge pull request #300 from cornelius-koepp/fix/group-object-setting-invalid-value--thelsing/master

Fix Sending Wrong Values on Failed Conversion to DPT

Author: thelsing

src/knx/group_object.cpp

@@ -242,10 +242,15 @@ GroupObjectUpdatedHandler GroupObject::callback()
 }
 #endif
 
-void GroupObject::value(const KNXValue& value, const Dpt& type)
+bool GroupObject::value(const KNXValue& value, const Dpt& type)
 {
-    valueNoSend(value, type);
-    objectWritten();
+    if (valueNoSend(value, type))
+    {
+        // write on successful conversion/setting value only
+        objectWritten();
+        return true;
+    }
+    return false;
 }
 
 
@@ -280,9 +285,9 @@ bool GroupObject::tryValue(KNXValue& value)
 }
 
 
-void GroupObject::value(const KNXValue& value)
+bool GroupObject::value(const KNXValue& value)
 {
-    this->value(value, _datapointType);
+    return this->value(value, _datapointType);
 }
 
 
@@ -292,34 +297,42 @@ KNXValue GroupObject::value()
 }
 
 
-void GroupObject::valueNoSend(const KNXValue& value)
+bool GroupObject::valueNoSend(const KNXValue& value)
 {
-    valueNoSend(value, _datapointType);
+    return valueNoSend(value, _datapointType);
 }
 #endif
 
-void GroupObject::valueNoSend(const KNXValue& value, const Dpt& type)
+bool GroupObject::valueNoSend(const KNXValue& value, const Dpt& type)
 {
-    if (_uninitialized)
+    const bool encodingDone = KNX_Encode_Value(value, _data, _dataLength, type);
+
+    // initialize on succesful conversion only
+    if (encodingDone && _uninitialized)
         commFlag(Ok);
 
-    KNX_Encode_Value(value, _data, _dataLength, type);
+    return encodingDone;
 }
 
 bool GroupObject::valueNoSendCompare(const KNXValue& value, const Dpt& type)
 {
     if (_uninitialized)
     {
         // always set first value
-        this->valueNoSend(value, type);
-        return true;
+        return valueNoSend(value, type);
     }
     else
     {
-        // convert new value to given dtp
+        // convert new value to given DPT
         uint8_t newData[_dataLength];
         memset(newData, 0, _dataLength);
-        KNX_Encode_Value(value, newData, _dataLength, type);
+        const bool encodingDone = KNX_Encode_Value(value, newData, _dataLength, type);
+        if (!encodingDone)
+        {
+            // value conversion to DPT failed
+            // do NOT update the value of the KO!
+            return false;
+        }
 
         // check for change in converted value / update value on change only
         const bool dataChanged = memcmp(_data, newData, _dataLength);

src/knx/group_object.h

@@ -162,8 +162,10 @@ class GroupObject
          * @param type the datapoint type used for the conversion.
          *
          * The parameters must fit the group object. Otherwise it will stay unchanged.
+         *
+         * @returns true if the value was converted successfully to the datapoint type and the group object was updated.
          */
-        void value(const KNXValue& value, const Dpt& type);
+        bool value(const KNXValue& value, const Dpt& type);
 
         /**
          * Check if the value (after conversion to dpt) will differ from current value of the group object and changes the state of the group object to ::WriteRequest if different.
@@ -173,18 +175,20 @@ class GroupObject
          *
          * The parameters must fit the group object. Otherwise it will stay unchanged.
          *
-         * @returns true if the value of the group object has changed
+         * @returns true if the value of the group object has changed, false if conversion results in same value as stored in group object or failed.
          */
         bool valueCompare(const KNXValue& value, const Dpt& type);
 
         /**
-         * set the current value of the group object.
+         * set the current value of the group objectand show success.
          * @param value the value the group object is set to
          * @param type the datapoint type used for the conversion.
          *
          * The parameters must fit the group object. Otherwise it will stay unchanged.
+         *
+         * @returns true if value was converted successfully to the datapoint type and the group object was updated.
          */
-        void valueNoSend(const KNXValue& value, const Dpt& type);
+        bool valueNoSend(const KNXValue& value, const Dpt& type);
 
         /**
          * Check if the value (after conversion to dpt) will differ from current value of the group object and update if necessary.
@@ -194,7 +198,7 @@ class GroupObject
          *
          * The parameters must fit the group object. Otherwise it will stay unchanged.
          *
-         * @returns true if the value of the group object has changed
+         * @returns true if the value of the group object has changed, false if conversion results in same value as stored in group object or failed.
          */
         bool valueNoSendCompare(const KNXValue& value, const Dpt& type);
 
@@ -220,15 +224,19 @@ class GroupObject
          * @param value the value the group object is set to
          *
          * The parameters must fit the group object and dhe datapoint type must be set with dataPointType(). Otherwise it will stay unchanged.
+         *
+         * @returns true if the value was converted successfully to the datapoint type and the group object was updated.
          */
-        void value(const KNXValue& value);
+        bool value(const KNXValue& value);
         /**
          * set the current value of the group object.
          * @param value the value the group object is set to
          *
-         * The parameters must fit the group object and dhe datapoint type must be set with dataPointType(). Otherwise it will stay unchanged.
+         * The parameters must fit the group object and the datapoint type must be set with dataPointType(). Otherwise it will stay unchanged.
+         *
+         * @returns true if the value was converted successfully to the datapoint type and the group object was updated.
          */
-        void valueNoSend(const KNXValue& value);
+        bool valueNoSend(const KNXValue& value);
         /**
          * set the current value of the group object.
          * @param value the value the group object is set to