Fix FunctionPropertyExt* and FunctionProperty for PDT_CONTROL

src/knx/bau_systemB.cpp

@@ -384,7 +384,16 @@ void BauSystemB::functionPropertyCommandIndication(Priority priority, HopCountTy
 
     InterfaceObject* obj = getInterfaceObject(objectIndex);
     if(obj)
+    {
+        if (obj->property((PropertyID)propertyId)->Type() == PDT_FUNCTION)
+        {
             obj->command((PropertyID)propertyId, data, length, resultData, resultLength);
+        }
+        else
+        {
+            resultLength = 0; // We must not send a return code or any data fields
+        }
+    }
 
     _appLayer.functionPropertyStateResponse(AckRequested, priority, hopType, asap, secCtrl, objectIndex, propertyId, resultData, resultLength);
 }
@@ -397,7 +406,16 @@ void BauSystemB::functionPropertyStateIndication(Priority priority, HopCountType
 
     InterfaceObject* obj = getInterfaceObject(objectIndex);
     if(obj)
+    {
+        if (obj->property((PropertyID)propertyId)->Type() == PDT_FUNCTION)
+        {
             obj->state((PropertyID)propertyId, data, length, resultData, resultLength);
+        }
+        else
+        {
+            resultLength = 0; // We must not send a return code or any data fields
+        }
+    }
 
     _appLayer.functionPropertyStateResponse(AckRequested, priority, hopType, asap, secCtrl, objectIndex, propertyId, resultData, resultLength);
 }
@@ -406,24 +424,54 @@ void BauSystemB::functionPropertyExtCommandIndication(Priority priority, HopCoun
                                                       uint8_t propertyId, uint8_t* data, uint8_t length)
 {
     uint8_t resultData[kFunctionPropertyResultBufferMaxSize];
-    uint8_t resultLength = sizeof(resultData); // tell the callee the maximum size of the buffer
+    uint8_t resultLength = 1; // we always have to include the return code at least
 
     InterfaceObject* obj = getInterfaceObject(objectType, objectInstance);
     if(obj)
     {
-        if (obj->property((PropertyID)propertyId)->Type() == PDT_CONTROL)
+        PropertyDataType propType = obj->property((PropertyID)propertyId)->Type();
+
+        if (propType == PDT_FUNCTION)
+        {
+            // The first byte is reserved and 0 for PDT_FUNCTION
+            uint8_t reservedByte = data[0];
+            if (reservedByte != 0x00)
+            {
+                resultData[0] = ReturnCodes::DataVoid;
+            }
+            else
+            {
+                resultLength = sizeof(resultData); // tell the callee the maximum size of the buffer
+                obj->command((PropertyID)propertyId, data, length, resultData, resultLength);
+                // resultLength was modified by the callee
+            }
+        }
+        else if (propType == PDT_CONTROL)
         {
             uint8_t count = 1;
+            // write the event
             obj->writeProperty((PropertyID)propertyId, 1, data, count);
+            if (count == 1)
+            {
+                // Read the current state (one byte only) for the response
+                obj->readProperty((PropertyID)propertyId, 1, count, &resultData[1]);
                 resultLength = count ? 2 : 1;
                 resultData[0] = count ? ReturnCodes::Success : ReturnCodes::DataVoid;
-            resultData[1] = data[0];
             }
-        else if (obj->property((PropertyID)propertyId)->Type() == PDT_FUNCTION)
+            else
             {
-            obj->command((PropertyID)propertyId, data, length, resultData, resultLength);
+                resultData[0] = ReturnCodes::AddressVoid;
             }
         }
+        else
+        {
+            resultData[0] = ReturnCodes::DataTypeConflict;
+        }
+    }
+    else
+    {
+        resultData[0] = ReturnCodes::GenericError;
+    }
 
     _appLayer.functionPropertyExtStateResponse(AckRequested, priority, hopType, asap, secCtrl, objectType, objectInstance, propertyId, resultData, resultLength);
 }
@@ -437,18 +485,41 @@ void BauSystemB::functionPropertyExtStateIndication(Priority priority, HopCountT
     InterfaceObject* obj = getInterfaceObject(objectType, objectInstance);
     if(obj)
     {
-        if (obj->property((PropertyID)propertyId)->Type() == PDT_CONTROL)
+        PropertyDataType propType = obj->property((PropertyID)propertyId)->Type();
+
+        if (propType == PDT_FUNCTION)
+        {
+            // The first byte is reserved and 0 for PDT_FUNCTION
+            uint8_t reservedByte = data[0];
+            if (reservedByte != 0x00)
+            {
+                resultData[0] = ReturnCodes::DataVoid;
+            }
+            else
+            {
+                resultLength = sizeof(resultData); // tell the callee the maximum size of the buffer
+                obj->state((PropertyID)propertyId, data, length, resultData, resultLength);
+                // resultLength was modified by the callee
+            }
+        }
+        else if (propType == PDT_CONTROL)
         {
             uint8_t count = 1;
+            // Read the current state (one byte only) for the response
             obj->readProperty((PropertyID)propertyId, 1, count, &resultData[1]);
             resultLength = count ? 2 : 1;
             resultData[0] = count ? ReturnCodes::Success : ReturnCodes::DataVoid;
         }
-        else if (obj->property((PropertyID)propertyId)->Type() == PDT_FUNCTION)
+        else
         {
-            obj->state((PropertyID)propertyId, data, length, resultData, resultLength);
+            resultData[0] = ReturnCodes::DataTypeConflict;
         }
     }
+    else
+    {
+        resultData[0] = ReturnCodes::GenericError;
+    }
+
     _appLayer.functionPropertyExtStateResponse(AckRequested, priority, hopType, asap, secCtrl, objectType, objectInstance, propertyId, resultData, resultLength);
 }
 

src/knx/security_interface_object.cpp

@@ -30,7 +30,7 @@ SecurityInterfaceObject::SecurityInterfaceObject()
             },
             // WriteCallback of PID_LOAD_STATE_CONTROL
             [](SecurityInterfaceObject* obj, uint16_t start, uint8_t count, const uint8_t* data) -> uint8_t {
-                obj->_state = (LoadState) data[0];
+                obj->loadEvent(data);
                 return 1;
             }),
         new FunctionProperty<SecurityInterfaceObject>(this, PID_SECURITY_MODE,
@@ -157,6 +157,7 @@ SecurityInterfaceObject::SecurityInterfaceObject()
         new DataProperty( PID_ZONE_KEY_TABLE, true, PDT_GENERIC_19, 32, ReadLv3 | WriteLv0 ), // written by ETS
         new DataProperty( PID_GO_SECURITY_FLAGS, true, PDT_GENERIC_01, 32, ReadLv3 | WriteLv0 ), // written by ETS
         new DataProperty( PID_ROLE_TABLE, true, PDT_GENERIC_01, 32, ReadLv3 | WriteLv0 ), // written by ETS
+        new DataProperty( PID_ERROR_CODE, false, PDT_ENUM8, 1, ReadLv3 | WriteLv0, (uint8_t)E_NO_FAULT),
         new DataProperty( PID_TOOL_SEQUENCE_NUMBER_SENDING, true, PDT_GENERIC_06, 1, ReadLv3 | WriteLv0 ) // Updated by our device accordingly (non-standardized!)
     };
     initializeProperties(sizeof(properties), properties);
@@ -187,6 +188,126 @@ bool SecurityInterfaceObject::isLoaded()
     return _state == LS_LOADED;
 }
 
+void SecurityInterfaceObject::loadEvent(const uint8_t* data)
+{
+    switch (_state)
+    {
+        case LS_UNLOADED:
+            loadEventUnloaded(data);
+            break;
+        case LS_LOADING:
+            loadEventLoading(data);
+            break;
+        case LS_LOADED:
+            loadEventLoaded(data);
+            break;
+        case LS_ERROR:
+            loadEventError(data);
+            break;
+        default:
+            /* do nothing */
+            break;
+    }
+}
+
+void SecurityInterfaceObject::loadEventUnloaded(const uint8_t* data)
+{
+    uint8_t event = data[0];
+    switch (event)
+    {
+        case LE_NOOP:
+        case LE_LOAD_COMPLETED:
+        case LE_ADDITIONAL_LOAD_CONTROLS:
+        case LE_UNLOAD:
+            break;
+        case LE_START_LOADING:
+            loadState(LS_LOADING);
+            break;
+        default:
+            loadState(LS_ERROR);
+            errorCode(E_GOT_UNDEF_LOAD_CMD);
+    }
+}
+
+void SecurityInterfaceObject::loadEventLoading(const uint8_t* data)
+{
+    uint8_t event = data[0];
+    switch (event)
+    {
+        case LE_NOOP:
+        case LE_START_LOADING:
+            break;
+        case LE_LOAD_COMPLETED:
+            loadState(LS_LOADED);
+            break;
+        case LE_UNLOAD:
+            loadState(LS_UNLOADED);
+            break;
+        case LE_ADDITIONAL_LOAD_CONTROLS: // Not supported here
+        default:
+            loadState(LS_ERROR);
+            errorCode(E_GOT_UNDEF_LOAD_CMD);
+    }
+}
+
+void SecurityInterfaceObject::loadEventLoaded(const uint8_t* data)
+{
+    uint8_t event = data[0];
+    switch (event)
+    {
+        case LE_NOOP:
+        case LE_LOAD_COMPLETED:
+            break;
+        case LE_START_LOADING:
+            loadState(LS_LOADING);
+            break;
+        case LE_UNLOAD:
+            loadState(LS_UNLOADED);
+            break;
+        case LE_ADDITIONAL_LOAD_CONTROLS:
+            loadState(LS_ERROR);
+            errorCode(E_INVALID_OPCODE);
+            break;
+        default:
+            loadState(LS_ERROR);
+            errorCode(E_GOT_UNDEF_LOAD_CMD);
+    }
+}
+
+void SecurityInterfaceObject::loadEventError(const uint8_t* data)
+{
+    uint8_t event = data[0];
+    switch (event)
+    {
+        case LE_NOOP:
+        case LE_LOAD_COMPLETED:
+        case LE_ADDITIONAL_LOAD_CONTROLS:
+        case LE_START_LOADING:
+            break;
+        case LE_UNLOAD:
+            loadState(LS_UNLOADED);
+            break;
+        default:
+            loadState(LS_ERROR);
+            errorCode(E_GOT_UNDEF_LOAD_CMD);
+    }
+}
+
+void SecurityInterfaceObject::loadState(LoadState newState)
+{
+    if (newState == _state)
+        return;
+    //beforeStateChange(newState);
+    _state = newState;
+}
+
+void SecurityInterfaceObject::errorCode(ErrorCode errorCode)
+{
+    uint8_t data = errorCode;
+    Property* prop = property(PID_ERROR_CODE);
+    prop->write(data);
+}
+
 void SecurityInterfaceObject::factoryReset()
 {
     _secAppLayer->setSecurityMode(false);

src/knx/security_interface_object.h

@@ -25,6 +25,15 @@ public:
 private:
   SecureApplicationLayer* _secAppLayer = nullptr;
 
+  void errorCode(ErrorCode errorCode);
+
+  void loadEvent(const uint8_t* data);
+  void loadEventUnloaded(const uint8_t* data);
+  void loadEventLoading(const uint8_t* data);
+  void loadEventLoaded(const uint8_t* data);
+  void loadEventError(const uint8_t* data);
+
+  void loadState(LoadState newState);
   LoadState _state = LS_UNLOADED;
 
   // Our FDSK