From 48b492c451ab80f44d2d503307d5244f24fd4800 Mon Sep 17 00:00:00 2001
From: Julian Tusch <urhrf@student.kit.edu>
Date: Sat, 13 Apr 2024 05:15:23 +0200
Subject: [PATCH] changes: - formatting, cleanup - moved update skill into
 FluxioSkill - implemented core mutex functionality - improved return value
 and error handling - added armarx warning messages - store root profile
 defaults as stringified json in fluxio values

---
 .../libraries/skills/core/FluxioEdge.cpp      |  47 ++-
 .../libraries/skills/core/FluxioEdge.h        |  11 +-
 .../libraries/skills/core/FluxioNode.cpp      |  16 +-
 .../libraries/skills/core/FluxioNode.h        |   8 +-
 .../libraries/skills/core/FluxioParameter.cpp |  67 +++-
 .../libraries/skills/core/FluxioParameter.h   |  10 +-
 .../skills/core/FluxioParameterNode.cpp       |  29 +-
 .../skills/core/FluxioParameterNode.h         |  11 +-
 .../libraries/skills/core/FluxioProfile.cpp   |   4 +-
 .../libraries/skills/core/FluxioProfile.h     |  10 +-
 .../libraries/skills/core/FluxioProvider.cpp  |   4 +-
 .../libraries/skills/core/FluxioProvider.h    |   7 +-
 .../libraries/skills/core/FluxioSkill.cpp     | 309 +++++++++++++++---
 .../libraries/skills/core/FluxioSkill.h       |  32 +-
 .../skills/core/FluxioSubSkillNode.cpp        |  35 +-
 .../skills/core/FluxioSubSkillNode.h          |  13 +-
 .../libraries/skills/core/FluxioValue.cpp     |  23 +-
 .../libraries/skills/core/FluxioValue.h       |  10 +-
 .../manager/SkillManagerComponentPlugin.cpp   | 166 ++++------
 .../manager/SkillManagerComponentPlugin.h     |  33 +-
 .../SkillManagerComponentPluginUser.cpp       |  94 ++++--
 .../manager/SkillManagerComponentPluginUser.h |  53 ++-
 22 files changed, 703 insertions(+), 289 deletions(-)

diff --git a/source/RobotAPI/libraries/skills/core/FluxioEdge.cpp b/source/RobotAPI/libraries/skills/core/FluxioEdge.cpp
index b69c0a461..b510df20c 100644
--- a/source/RobotAPI/libraries/skills/core/FluxioEdge.cpp
+++ b/source/RobotAPI/libraries/skills/core/FluxioEdge.cpp
@@ -1,4 +1,9 @@
 #include "FluxioEdge.h"
+
+#include <optional>
+
+#include <ArmarXCore/core/logging/Logging.h>
+
 #include "FluxioNode.h"
 #include "FluxioParameter.h"
 
@@ -6,9 +11,16 @@ namespace armarx
 {
     namespace skills
     {
-        manager::dto::FluxioEdge
+        std::optional<manager::dto::FluxioEdge>
         FluxioEdge::toManagerIce() const
         {
+            if (fromNodePtr == nullptr || fromParameterPtr == nullptr || toNodePtr == nullptr ||
+                toParameterPtr == nullptr)
+            {
+                ARMARX_WARNING << "Could not create FluxioEdge DTO";
+                return std::nullopt;
+            }
+
             manager::dto::FluxioEdge ret;
 
             ret.fromNodeId = fromNodePtr->toFluxioIdentificatorIce();
@@ -19,19 +31,34 @@ namespace armarx
             return ret;
         }
 
-        FluxioEdge
+        std::optional<FluxioEdge>
         FluxioEdge::FromIce(const manager::dto::FluxioEdge& i,
-                            std::vector<FluxioNode>& nodes,
-                            std::vector<FluxioParameter>& parameters)
+                            std::map<std::string, FluxioNode*>& nodesMap,
+                            std::map<std::string, FluxioParameter>& parametersMap)
         {
+
+            skills::FluxioNode* fromNodePtr =
+                FluxioNode::FromFluxioIdentificatorIce(i.fromNodeId, nodesMap);
+            skills::FluxioParameter* fromParameterPtr =
+                FluxioParameter::FromFluxioIdentificatorIce(i.fromParameterId, parametersMap);
+            skills::FluxioNode* toNodePtr =
+                FluxioNode::FromFluxioIdentificatorIce(i.toNodeId, nodesMap);
+            skills::FluxioParameter* toParameterPtr =
+                FluxioParameter::FromFluxioIdentificatorIce(i.toParameterId, parametersMap);
+
+            if (fromNodePtr == nullptr || fromParameterPtr == nullptr || toNodePtr == nullptr ||
+                toParameterPtr == nullptr)
+            {
+                ARMARX_WARNING << "Could not create FluxioEdge";
+                return std::nullopt;
+            }
+
             FluxioEdge ret;
 
-            ret.fromNodePtr = FluxioNode::FromFluxioIdentificatorIce(i.fromNodeId, nodes);
-            ret.fromParameterPtr =
-                FluxioParameter::FromFluxioIdentificatorIce(i.fromParameterId, parameters);
-            ret.toNodePtr = FluxioNode::FromFluxioIdentificatorIce(i.toNodeId, nodes);
-            ret.toParameterPtr =
-                FluxioParameter::FromFluxioIdentificatorIce(i.toParameterId, parameters);
+            ret.fromNodePtr = fromNodePtr;
+            ret.fromParameterPtr = fromParameterPtr;
+            ret.toNodePtr = toNodePtr;
+            ret.toParameterPtr = toParameterPtr;
 
             return ret;
         }
diff --git a/source/RobotAPI/libraries/skills/core/FluxioEdge.h b/source/RobotAPI/libraries/skills/core/FluxioEdge.h
index 856952964..a3a4cb251 100644
--- a/source/RobotAPI/libraries/skills/core/FluxioEdge.h
+++ b/source/RobotAPI/libraries/skills/core/FluxioEdge.h
@@ -1,7 +1,7 @@
 #pragma once
 
-#include <vector>
 #include <RobotAPI/interface/skills/SkillManagerInterface.h>
+
 #include "FluxioNode.h"
 #include "FluxioParameter.h"
 
@@ -16,11 +16,12 @@ namespace armarx
             FluxioNode* toNodePtr;
             FluxioParameter* toParameterPtr;
 
-            manager::dto::FluxioEdge toManagerIce() const;
+            std::optional<manager::dto::FluxioEdge> toManagerIce() const;
 
-            static FluxioEdge FromIce(const manager::dto::FluxioEdge& i,
-                                      std::vector<FluxioNode>& nodes,
-                                      std::vector<FluxioParameter>& parameters);
+            static std::optional<FluxioEdge>
+            FromIce(const manager::dto::FluxioEdge& i,
+                    std::map<std::string, FluxioNode*>& nodesMap,
+                    std::map<std::string, FluxioParameter>& parametersMap);
         };
     } // namespace skills
 } // namespace armarx
diff --git a/source/RobotAPI/libraries/skills/core/FluxioNode.cpp b/source/RobotAPI/libraries/skills/core/FluxioNode.cpp
index 2e70e0203..814c6b0c1 100644
--- a/source/RobotAPI/libraries/skills/core/FluxioNode.cpp
+++ b/source/RobotAPI/libraries/skills/core/FluxioNode.cpp
@@ -1,11 +1,14 @@
 #include "FluxioNode.h"
+
+#include <ArmarXCore/core/logging/Logging.h>
+
 #include <RobotAPI/interface/skills/SkillManagerInterface.h>
 
 namespace armarx
 {
     namespace skills
     {
-        manager::dto::FluxioNode
+        std::optional<manager::dto::FluxioNode>
         FluxioNode::toManagerIce() const
         {
             manager::dto::FluxioNode ret;
@@ -40,18 +43,17 @@ namespace armarx
 
         FluxioNode*
         FluxioNode::FromFluxioIdentificatorIce(const manager::dto::FluxioIdentificator& i,
-                                               std::vector<FluxioNode>& nodes)
+                                               std::map<std::string, FluxioNode*>& nodesMap)
         {
-            const auto& nodeIt = std::find_if(
-                nodes.begin(), nodes.end(), [&i](FluxioNode const& n) { return n.nodeId == i.id; });
+            const auto& nodeIt = nodesMap.find(i.id);
 
-            if (nodeIt == nodes.end())
+            if (nodeIt == nodesMap.end())
             {
-                // TODO: throw error or show warning
+                ARMARX_WARNING << "Node with id " << i.id << " not found";
                 return nullptr;
             }
 
-            return &(*nodeIt);
+            return nodesMap[nodeIt->first];
         }
 
         FluxioNode
diff --git a/source/RobotAPI/libraries/skills/core/FluxioNode.h b/source/RobotAPI/libraries/skills/core/FluxioNode.h
index 1e8b3302a..49df5019f 100644
--- a/source/RobotAPI/libraries/skills/core/FluxioNode.h
+++ b/source/RobotAPI/libraries/skills/core/FluxioNode.h
@@ -1,6 +1,7 @@
 #pragma once
 
 #include <string>
+
 #include <RobotAPI/interface/skills/SkillManagerInterface.h>
 
 namespace armarx
@@ -9,16 +10,19 @@ namespace armarx
     {
         struct FluxioNode
         {
+            virtual ~FluxioNode() = default;
             std::string nodeType;
             std::string nodeId;
             std::string name;
             float xPos = 0;
             float yPos = 0;
 
-            virtual manager::dto::FluxioNode toManagerIce() const;
+            virtual std::optional<manager::dto::FluxioNode> toManagerIce() const;
             manager::dto::FluxioIdentificator toFluxioIdentificatorIce() const;
 
-            static FluxioNode* FromFluxioIdentificatorIce(const manager::dto::FluxioIdentificator& i, std::vector<FluxioNode>& nodes);
+            static FluxioNode*
+            FromFluxioIdentificatorIce(const manager::dto::FluxioIdentificator& i,
+                                       std::map<std::string, FluxioNode*>& nodesMap);
             static FluxioNode FromIce(const manager::dto::FluxioNode& i);
         };
     } // namespace skills
diff --git a/source/RobotAPI/libraries/skills/core/FluxioParameter.cpp b/source/RobotAPI/libraries/skills/core/FluxioParameter.cpp
index bdc8e4086..4f0c2f0c0 100644
--- a/source/RobotAPI/libraries/skills/core/FluxioParameter.cpp
+++ b/source/RobotAPI/libraries/skills/core/FluxioParameter.cpp
@@ -1,8 +1,10 @@
 #include "FluxioParameter.h"
-#include <algorithm>
-#include <vector>
+
+#include <ArmarXCore/core/logging/Logging.h>
+
 #include "RobotAPI/libraries/skills/core/FluxioProfile.h"
 #include <RobotAPI/interface/skills/SkillManagerInterface.h>
+
 #include "FluxioValue.h"
 
 namespace armarx
@@ -24,7 +26,15 @@ namespace armarx
             skills::manager::dto::FluxioValueList ret_values;
             for (const FluxioValue& value : values)
             {
-                ret_values.push_back(value.toManagerIce());
+                const auto& v = value.toManagerIce();
+
+                if (!v.has_value())
+                {
+                    ARMARX_WARNING << "Value could not be converted.";
+                    continue;
+                }
+
+                ret_values.push_back(*v);
             }
             ret.values = ret_values;
 
@@ -40,22 +50,45 @@ namespace armarx
             return ret;
         }
 
+        void
+        FluxioParameter::updateFromIce(const manager::dto::FluxioParameter& i,
+                                       std::map<std::string, FluxioProfile>& profilesMap)
+        {
+            name = i.name;
+            description = i.description;
+            type = i.type;
+            required = i.required;
+            isInput = i.isInput;
+
+            values.clear(); // clear old values and create them anew
+            for (const auto& value : i.values)
+            {
+                const auto& v = FluxioValue::FromIce(value, profilesMap);
+
+                if (!v.has_value())
+                {
+                    ARMARX_WARNING << "Value could not be converted.";
+                    continue;
+                }
+
+                values.push_back(*v);
+            }
+        }
+
         FluxioParameter*
-        FluxioParameter::FromFluxioIdentificatorIce(const manager::dto::FluxioIdentificator& i,
-                                                    std::vector<FluxioParameter>& parameters)
+        FluxioParameter::FromFluxioIdentificatorIce(
+            const manager::dto::FluxioIdentificator& i,
+            std::map<std::string, FluxioParameter>& parametersMap)
         {
-            const auto& parameterIt =
-                std::find_if(parameters.begin(),
-                             parameters.end(),
-                             [&i](const FluxioParameter& p) { return p.id == i.id; });
+            const auto& parameterIt = parametersMap.find(i.id);
 
-            if (parameterIt == parameters.end())
+            if (parameterIt == parametersMap.end())
             {
-                // TODO: throw error or show warning
+                ARMARX_WARNING << "Parameter with id " << i.id << " not found";
                 return nullptr;
             }
 
-            return &(*parameterIt);
+            return &(parametersMap[parameterIt->first]);
         }
 
         FluxioParameter
@@ -73,7 +106,15 @@ namespace armarx
 
             for (const manager::dto::FluxioValue& value : i.values)
             {
-                ret.values.push_back(FluxioValue::FromIce(value, profilesMap));
+                const auto& v = FluxioValue::FromIce(value, profilesMap);
+
+                if (!v.has_value())
+                {
+                    ARMARX_WARNING << "Value could not be converted.";
+                    continue;
+                }
+
+                ret.values.push_back(*v);
             }
 
             return ret;
diff --git a/source/RobotAPI/libraries/skills/core/FluxioParameter.h b/source/RobotAPI/libraries/skills/core/FluxioParameter.h
index eb228686d..124d8690e 100644
--- a/source/RobotAPI/libraries/skills/core/FluxioParameter.h
+++ b/source/RobotAPI/libraries/skills/core/FluxioParameter.h
@@ -1,8 +1,10 @@
 #pragma once
 
+#include <list>
 #include <string>
-#include <vector>
+
 #include <RobotAPI/interface/skills/SkillManagerInterface.h>
+
 #include "FluxioValue.h"
 
 namespace armarx
@@ -17,14 +19,16 @@ namespace armarx
             std::string type;
             bool required = true;
             bool isInput;
-            std::vector<FluxioValue> values = {};
+            std::list<FluxioValue> values = {};
 
             manager::dto::FluxioParameter toManagerIce() const;
             manager::dto::FluxioIdentificator toFluxioIdentificatorIce() const;
+            void updateFromIce(const manager::dto::FluxioParameter& i,
+                               std::map<std::string, FluxioProfile>& profilesMap);
 
             static FluxioParameter*
             FromFluxioIdentificatorIce(const manager::dto::FluxioIdentificator& i,
-                                       std::vector<FluxioParameter>& parameters);
+                                       std::map<std::string, FluxioParameter>& parametersMap);
             static FluxioParameter FromIce(const manager::dto::FluxioParameter& i,
                                            std::map<std::string, FluxioProfile>& profilesMap);
         };
diff --git a/source/RobotAPI/libraries/skills/core/FluxioParameterNode.cpp b/source/RobotAPI/libraries/skills/core/FluxioParameterNode.cpp
index bcb68caaa..2c2aee3af 100644
--- a/source/RobotAPI/libraries/skills/core/FluxioParameterNode.cpp
+++ b/source/RobotAPI/libraries/skills/core/FluxioParameterNode.cpp
@@ -1,18 +1,24 @@
 #include "FluxioParameterNode.h"
+
+#include <optional>
+
+#include <ArmarXCore/core/logging/Logging.h>
+
 #include <RobotAPI/interface/skills/SkillManagerInterface.h>
+
 #include "FluxioParameter.h"
 
 namespace armarx
 {
     namespace skills
     {
-        manager::dto::FluxioNode
+        std::optional<manager::dto::FluxioNode>
         FluxioParameterNode::toManagerIce() const
         {
             if (parameterPtr == nullptr)
             {
-                // TODO: throw error or show warning
-                return {};
+                ARMARX_WARNING << "Parameter for Parameternode with id " << nodeId << " not set";
+                return std::nullopt;
             }
 
             manager::dto::FluxioNode ret;
@@ -31,18 +37,27 @@ namespace armarx
             return ret;
         }
 
-        FluxioParameterNode
+        std::optional<FluxioParameterNode>
         FluxioParameterNode::FromIce(const manager::dto::FluxioNode& i,
-                                     std::vector<FluxioParameter>& parameters)
+                                     std::map<std::string, FluxioParameter>& parametersMap)
         {
+
+            skills::FluxioParameter* parameterPtr =
+                FluxioParameter::FromFluxioIdentificatorIce(i.parameterId, parametersMap);
+
+            if (parameterPtr == nullptr)
+            {
+                return std::nullopt;
+            }
+
             FluxioParameterNode ret;
+
             ret.nodeId = i.nodeId;
             ret.nodeType = i.nodeType;
             ret.name = i.name;
             ret.xPos = i.xPos;
             ret.yPos = i.yPos;
-            ret.parameterPtr =
-                FluxioParameter::FromFluxioIdentificatorIce(i.parameterId, parameters);
+            ret.parameterPtr = parameterPtr;
 
             return ret;
         }
diff --git a/source/RobotAPI/libraries/skills/core/FluxioParameterNode.h b/source/RobotAPI/libraries/skills/core/FluxioParameterNode.h
index 4c227dd9a..0c31d3b49 100644
--- a/source/RobotAPI/libraries/skills/core/FluxioParameterNode.h
+++ b/source/RobotAPI/libraries/skills/core/FluxioParameterNode.h
@@ -1,7 +1,7 @@
 #pragma once
 
-#include <vector>
 #include <RobotAPI/interface/skills/SkillManagerInterface.h>
+
 #include "FluxioNode.h"
 #include "FluxioParameter.h"
 
@@ -9,14 +9,15 @@ namespace armarx
 {
     namespace skills
     {
-        struct FluxioParameterNode : FluxioNode
+        struct FluxioParameterNode : public FluxioNode
         {
             FluxioParameter* parameterPtr;
 
-            manager::dto::FluxioNode toManagerIce() const override;
+            std::optional<manager::dto::FluxioNode> toManagerIce() const override;
 
-            static FluxioParameterNode FromIce(const manager::dto::FluxioNode& i,
-                                               std::vector<FluxioParameter>& parameters);
+            static std::optional<FluxioParameterNode>
+            FromIce(const manager::dto::FluxioNode& i,
+                    std::map<std::string, FluxioParameter>& parametersMap);
         };
     } // namespace skills
 } // namespace armarx
diff --git a/source/RobotAPI/libraries/skills/core/FluxioProfile.cpp b/source/RobotAPI/libraries/skills/core/FluxioProfile.cpp
index ea5841932..7f8b73f39 100644
--- a/source/RobotAPI/libraries/skills/core/FluxioProfile.cpp
+++ b/source/RobotAPI/libraries/skills/core/FluxioProfile.cpp
@@ -1,5 +1,7 @@
 #include "FluxioProfile.h"
+
 #include <ArmarXCore/core/logging/Logging.h>
+
 #include <RobotAPI/interface/skills/SkillManagerInterface.h>
 
 namespace armarx
@@ -48,7 +50,7 @@ namespace armarx
 
             if (profilesEntry == profilesMap.end())
             {
-                // TODO: throw error or show warning
+                ARMARX_WARNING << "Profile with id " << i.id << " not found";
                 return nullptr;
             }
 
diff --git a/source/RobotAPI/libraries/skills/core/FluxioProfile.h b/source/RobotAPI/libraries/skills/core/FluxioProfile.h
index 65a5471e6..043a502d6 100644
--- a/source/RobotAPI/libraries/skills/core/FluxioProfile.h
+++ b/source/RobotAPI/libraries/skills/core/FluxioProfile.h
@@ -1,6 +1,7 @@
 #pragma once
 
 #include <string>
+
 #include <RobotAPI/interface/skills/SkillManagerInterface.h>
 
 namespace armarx
@@ -12,13 +13,16 @@ namespace armarx
             std::string id;
             std::string name;
             std::string description = "";
-            FluxioProfile *parentPtr = nullptr;
+            FluxioProfile* parentPtr = nullptr;
 
             manager::dto::FluxioProfile toManagerIce() const;
             manager::dto::FluxioIdentificator toFluxioIdentificatorIce() const;
 
-            static FluxioProfile* FromFluxioIdentificatorIce(const manager::dto::FluxioIdentificator& i, std::map<std::string,FluxioProfile>& profilesMap);
-            static FluxioProfile FromIce(const manager::dto::FluxioProfile& i, std::map<std::string,FluxioProfile>& profilesMap);
+            static FluxioProfile*
+            FromFluxioIdentificatorIce(const manager::dto::FluxioIdentificator& i,
+                                       std::map<std::string, FluxioProfile>& profilesMap);
+            static FluxioProfile FromIce(const manager::dto::FluxioProfile& i,
+                                         std::map<std::string, FluxioProfile>& profilesMap);
         };
     } // namespace skills
 } // namespace armarx
diff --git a/source/RobotAPI/libraries/skills/core/FluxioProvider.cpp b/source/RobotAPI/libraries/skills/core/FluxioProvider.cpp
index 82723afa6..4151de3b4 100644
--- a/source/RobotAPI/libraries/skills/core/FluxioProvider.cpp
+++ b/source/RobotAPI/libraries/skills/core/FluxioProvider.cpp
@@ -1,5 +1,7 @@
 #include "FluxioProvider.h"
 
+#include <ArmarXCore/core/logging/Logging.h>
+
 namespace armarx
 {
     namespace skills
@@ -35,7 +37,7 @@ namespace armarx
 
             if (providerIt == providersMap.end())
             {
-                // TODO: throw error or show warning
+                ARMARX_WARNING << "Provider with id " << i.id << " not found";
                 return nullptr;
             }
 
diff --git a/source/RobotAPI/libraries/skills/core/FluxioProvider.h b/source/RobotAPI/libraries/skills/core/FluxioProvider.h
index bfa749b5e..500affcea 100644
--- a/source/RobotAPI/libraries/skills/core/FluxioProvider.h
+++ b/source/RobotAPI/libraries/skills/core/FluxioProvider.h
@@ -1,6 +1,7 @@
 #pragma once
 
 #include <string>
+
 #include <RobotAPI/interface/skills/SkillManagerInterface.h>
 
 namespace armarx
@@ -11,11 +12,13 @@ namespace armarx
         {
             std::string id;
             std::string name;
-            
+
             manager::dto::FluxioProvider toManagerIce() const;
             manager::dto::FluxioIdentificator toFluxioIdentificatorIce() const;
 
-            static FluxioProvider* FromFluxioIdentificatorIce(const manager::dto::FluxioIdentificator& i, std::map<std::string, FluxioProvider>& providersMap);
+            static FluxioProvider*
+            FromFluxioIdentificatorIce(const manager::dto::FluxioIdentificator& i,
+                                       std::map<std::string, FluxioProvider>& providersMap);
             static FluxioProvider FromIce(const manager::dto::FluxioProvider& i);
         };
     } // namespace skills
diff --git a/source/RobotAPI/libraries/skills/core/FluxioSkill.cpp b/source/RobotAPI/libraries/skills/core/FluxioSkill.cpp
index 247bfc006..d14138cae 100644
--- a/source/RobotAPI/libraries/skills/core/FluxioSkill.cpp
+++ b/source/RobotAPI/libraries/skills/core/FluxioSkill.cpp
@@ -1,7 +1,12 @@
 #include "FluxioSkill.h"
-#include <vector>
+
 #include <ArmarXCore/core/logging/Logging.h>
+#include <ArmarXCore/core/time/DateTime.h>
+
+#include "RobotAPI/libraries/skills/core/FluxioParameter.h"
+#include "RobotAPI/libraries/skills/core/FluxioProvider.h"
 #include <RobotAPI/interface/skills/SkillManagerInterface.h>
+
 #include "FluxioEdge.h"
 #include "FluxioNode.h"
 #include "FluxioParameterNode.h"
@@ -11,14 +16,13 @@ namespace armarx
 {
     namespace skills
     {
-        manager::dto::FluxioSkill
+        std::optional<manager::dto::FluxioSkill>
         FluxioSkill::toManagerIce() const
         {
             if (skillProviderPtr == nullptr)
             {
-                ARMARX_WARNING << "SkillProvider for Skill with id " << id
-                               << " not set"; // TODO: create general error message structure
-                return {};
+                ARMARX_WARNING << "SkillProvider for Skill with id " << id << " not set";
+                return std::nullopt;
             }
 
             manager::dto::FluxioSkill ret;
@@ -32,42 +36,51 @@ namespace armarx
             ret.skillProviderId = skillProviderPtr->toFluxioIdentificatorIce();
 
             skills::manager::dto::FluxioParameterList ret_parameters;
-            for (const auto& parameter : parameters)
+            for (const auto& [id, parameter] : parameters)
             {
                 ret_parameters.push_back(parameter.toManagerIce());
             }
             ret.parameters = ret_parameters;
 
             skills::manager::dto::FluxioNodeList ret_nodes;
-            if (nodesPtr == nullptr)
+            skills::manager::dto::FluxioEdgeList ret_edges;
+            if (native)
             {
                 ret.nodesHasValue = false;
+                ret.edgesHasValue = false;
             }
             else
             {
                 ret.nodesHasValue = true;
+                ret.edgesHasValue = true;
 
-                for (const auto& node : *nodesPtr)
+                for (const auto& [id, nodePtr] : nodes)
                 {
-                    ret_nodes.push_back(node.toManagerIce());
-                }
-            }
-            ret.nodes = ret_nodes;
+                    const auto& node = nodePtr->toManagerIce();
 
-            skills::manager::dto::FluxioEdgeList ret_edges;
-            if (edgesPtr == nullptr)
-            {
-                ret.edgesHasValue = false;
-            }
-            else
-            {
-                ret.edgesHasValue = true;
+                    if (!node.has_value())
+                    {
+                        ARMARX_WARNING << "Node with id " << id << " could not be converted";
+                        continue;
+                    }
+
+                    ret_nodes.push_back(*node);
+                }
 
-                for (const auto& edge : *edgesPtr)
+                for (const auto& edge : edges)
                 {
-                    ret_edges.push_back(edge.toManagerIce());
+                    const auto& edgeDTO = edge.toManagerIce();
+
+                    if (!edgeDTO.has_value())
+                    {
+                        ARMARX_WARNING << "Edge could not be converted";
+                        continue;
+                    }
+
+                    ret_edges.push_back(*edgeDTO);
                 }
             }
+            ret.nodes = ret_nodes;
             ret.edges = ret_edges;
 
             return ret;
@@ -84,6 +97,170 @@ namespace armarx
             return ret;
         }
 
+        bool
+        FluxioSkill::updateFromIce(const manager::dto::FluxioSkill& i,
+                                   std::map<std::string, FluxioProvider>& providersMap,
+                                   std::map<std::string, FluxioProfile>& profilesMap,
+                                   std::map<std::string, FluxioSkill>& skillsMap)
+        {
+            if (id != i.id)
+            {
+                ARMARX_WARNING << "Trying to update Skill with id " << id << " with Skill with id "
+                               << i.id;
+                return false;
+            }
+
+            if (native != i.native)
+            {
+                ARMARX_WARNING << "Trying to update native state of Skill with id " << id;
+                return false;
+            }
+
+            // TODO: add corrupted flag to FluxioSkill to indicate invalid states
+
+            name = i.name;
+            description = i.description;
+            lastChanged = armarx::DateTime::Now().toDateTimeString();
+            executable = i.executable;
+            skillProviderPtr =
+                skills::FluxioProvider::FromFluxioIdentificatorIce(i.skillProviderId, providersMap);
+
+            // parameters
+            // create a parameter dto map for convenience
+            std::map<std::string, manager::dto::FluxioParameter> parameterDTOMap;
+            for (const auto& parameter : i.parameters)
+            {
+                parameterDTOMap[parameter.id] = parameter;
+            }
+
+            // find parameters to delete
+            std::list<std::string> parameterIdsToDelete;
+            for (const auto& [id, param] : parameters)
+            {
+                const auto& paramIt = parameterDTOMap.find(id);
+                if (paramIt == parameterDTOMap.end())
+                {
+                    parameterIdsToDelete.push_back(id);
+                }
+            }
+
+            // handle parameter deletion
+            for (const std::string& paramId : parameterIdsToDelete)
+            {
+                for (auto& [k, s] : skillsMap)
+                {
+                    s.deleteParameter(paramId, id);
+                }
+            }
+
+            for (const auto& [id, paramDTO] : parameterDTOMap)
+            {
+                const auto& paramIt = parameters.find(id);
+                if (paramIt == parameters.end()) // add new parameter
+                {
+                    parameters[id] = FluxioParameter::FromIce(paramDTO, profilesMap);
+                }
+                else // update existing parameter
+                {
+                    parameters[id].updateFromIce(paramDTO, profilesMap);
+                }
+            }
+
+            // nodes
+            if (i.nodesHasValue)
+            {
+                nodes.clear(); // create all nodes anew
+                for (const manager::dto::FluxioNode& node : i.nodes)
+                {
+                    FluxioNode* n = CreateNode(node, parameters, skillsMap);
+
+                    if (n != nullptr)
+                    {
+                        nodes[n->nodeId] = n;
+                    }
+                }
+            }
+
+            // edges
+            if (i.edgesHasValue)
+            {
+                edges.clear(); // create all edges anew
+                for (const manager::dto::FluxioEdge& edge : i.edges)
+                {
+                    const auto& e = FluxioEdge::FromIce(edge, nodes, parameters);
+
+                    if (e.has_value())
+                    {
+                        edges.push_back(*e);
+                    }
+                    else
+                    {
+                        ARMARX_WARNING << "Edge could not be converted";
+                    }
+                }
+            }
+
+            return true;
+        }
+
+        void
+        FluxioSkill::deleteParameter(const std::string& parameterId, const std::string& skillId)
+        {
+            // check wether the parameter belongs to this skill instance or not
+            if (id == skillId)
+            {
+                parameters.erase(parameterId);
+                edges.remove_if(
+                    [parameterId](const FluxioEdge& e) {
+                        return e.fromParameterPtr->id == parameterId ||
+                               e.toParameterPtr->id == parameterId;
+                    });
+            }
+            else
+            {
+                // find relevant subskillnodes
+                std::list<std::string> subSkillNodeIds;
+                for (const auto& [nodeId, nodePtr] : nodes)
+                {
+                    if (nodePtr->nodeType != "SUBSKILL")
+                    {
+                        continue;
+                    }
+
+                    // get subskillnode from pointer
+                    auto* subSkillNode = dynamic_cast<FluxioSubSkillNode*>(nodePtr);
+
+                    if (subSkillNode->skillPtr->id == skillId)
+                    {
+                        subSkillNodeIds.push_back(nodeId);
+                    }
+                }
+
+                // remove relevant edges
+                edges.remove_if(
+                    [parameterId, subSkillNodeIds](const FluxioEdge& e)
+                    {
+                        if (e.fromParameterPtr->id == parameterId &&
+                            std::find(subSkillNodeIds.begin(),
+                                      subSkillNodeIds.end(),
+                                      e.fromNodePtr->nodeId) != subSkillNodeIds.end())
+                        {
+                            return true;
+                        }
+
+                        if (e.toParameterPtr->id == parameterId &&
+                            std::find(subSkillNodeIds.begin(),
+                                      subSkillNodeIds.end(),
+                                      e.toNodePtr->nodeId) != subSkillNodeIds.end())
+                        {
+                            return true;
+                        }
+
+                        return false;
+                    });
+            }
+        }
+
         FluxioSkill*
         FluxioSkill::FromFluxioIdentificatorIce(const manager::dto::FluxioIdentificator& i,
                                                 std::map<std::string, FluxioSkill>& skillsMap)
@@ -92,19 +269,27 @@ namespace armarx
 
             if (skillIt == skillsMap.end())
             {
-                // TODO: throw error or show warning
+                ARMARX_WARNING << "Skill with id " << i.id << " not found";
                 return nullptr;
             }
 
             return &(skillsMap[skillIt->first]);
         }
 
-        FluxioSkill
+        std::optional<FluxioSkill>
         FluxioSkill::FromIce(const manager::dto::FluxioSkill& i,
                              std::map<std::string, FluxioProvider>& providersMap,
                              std::map<std::string, FluxioProfile>& profilesMap,
                              std::map<std::string, FluxioSkill>& skillsMap)
         {
+            const auto& providerPtr =
+                FluxioProvider::FromFluxioIdentificatorIce(i.skillProviderId, providersMap);
+            if (providerPtr == nullptr)
+            {
+                ARMARX_WARNING << "Provider for Skill with id " << i.id << " not found";
+                return std::nullopt;
+            }
+
             FluxioSkill ret;
 
             ret.id = i.id;
@@ -113,54 +298,82 @@ namespace armarx
             ret.lastChanged = i.lastChanged;
             ret.executable = i.executable;
             ret.native = i.native;
-            ret.skillProviderPtr =
-                FluxioProvider::FromFluxioIdentificatorIce(i.skillProviderId, providersMap);
+            ret.skillProviderPtr = providerPtr;
 
             for (const manager::dto::FluxioParameter& parameter : i.parameters)
             {
-                ret.parameters.push_back(FluxioParameter::FromIce(parameter, profilesMap));
+                skills::FluxioParameter param = FluxioParameter::FromIce(parameter, profilesMap);
+                ret.parameters[param.id] = param;
+            }
+
+            ret.nodes = {};
+            ret.edges = {};
+            if (i.native)
+            {
+                return ret; // return early (native skills have no nodes and edges)
             }
 
             if (i.nodesHasValue)
             {
-                ret.nodesPtr = {};
-                for (const auto& node : i.nodes)
+                for (const manager::dto::FluxioNode& node : i.nodes)
                 {
-                    if (node.nodeType == "PARAMETER")
-                    {
-                        ret.nodesPtr->push_back(FluxioParameterNode::FromIce(node, ret.parameters));
-                    }
-                    else if (node.nodeType == "SUBSKILL")
+                    FluxioNode* n = CreateNode(node, ret.parameters, skillsMap);
+
+                    if (n != nullptr)
                     {
-                        ret.nodesPtr->push_back(FluxioSubSkillNode::FromIce(node, skillsMap));
+                        ret.nodes[n->nodeId] = n;
                     }
-                    else
+                }
+            }
+
+            if (i.edgesHasValue)
+            {
+                for (const manager::dto::FluxioEdge& edge : i.edges)
+                {
+                    const auto& e = FluxioEdge::FromIce(edge, ret.nodes, ret.parameters);
+
+                    if (e.has_value())
                     {
-                        // TODO: default case ?
-                        // ignore for now
+                        ret.edges.push_back(*e);
                     }
                 }
             }
-            else
+
+            return ret;
+        }
+
+        skills::FluxioNode*
+        FluxioSkill::CreateNode(const manager::dto::FluxioNode& i,
+                                std::map<std::string, FluxioParameter>& parametersMap,
+                                std::map<std::string, FluxioSkill>& skillsMap)
+        {
+            if (i.nodeType == "PARAMETER")
             {
-                ret.nodesPtr = nullptr;
-            }
+                const auto& n = FluxioParameterNode::FromIce(i, parametersMap);
 
-            if (i.edgesHasValue)
+                if (n.has_value())
+                {
+                    return new FluxioParameterNode(*n);
+                }
+
+                ARMARX_WARNING << "ParameterNode with id " << i.nodeId << "could not be converted";
+            }
+            else if (i.nodeType == "SUBSKILL")
             {
-                ret.edgesPtr = {};
-                for (const manager::dto::FluxioEdge& edge : i.edges)
+                const auto& n = FluxioSubSkillNode::FromIce(i, skillsMap);
+
+                if (n.has_value())
                 {
-                    ret.edgesPtr->push_back(
-                        FluxioEdge::FromIce(edge, *ret.nodesPtr, ret.parameters));
+                    return new FluxioSubSkillNode(*n);
                 }
+                ARMARX_WARNING << "SubSkillNode with id " << i.nodeId << "could not be converted";
             }
             else
             {
-                ret.edgesPtr = nullptr;
+                ARMARX_INFO << "Node type " << i.nodeType << " not supported yet. Ignoring.";
             }
 
-            return ret;
+            return nullptr;
         }
     } // namespace skills
 } // namespace armarx
diff --git a/source/RobotAPI/libraries/skills/core/FluxioSkill.h b/source/RobotAPI/libraries/skills/core/FluxioSkill.h
index 87d890689..6893707ca 100644
--- a/source/RobotAPI/libraries/skills/core/FluxioSkill.h
+++ b/source/RobotAPI/libraries/skills/core/FluxioSkill.h
@@ -1,8 +1,10 @@
 #pragma once
 
+#include <list>
 #include <string>
-#include <vector>
+
 #include <RobotAPI/interface/skills/SkillManagerInterface.h>
+
 #include "FluxioEdge.h"
 #include "FluxioNode.h"
 #include "FluxioParameter.h"
@@ -17,25 +19,35 @@ namespace armarx
             std::string id;
             std::string name;
             std::string description = "";
-            std::string lastChanged; // TODO: chose a format to store in (that works with armarx)
+            std::string lastChanged;
             bool executable =
                 false; // TODO: change from explicit to implicit storing of executable state
             bool native = true;
             FluxioProvider* skillProviderPtr;
-            std::vector<FluxioParameter> parameters = {};
-            std::vector<FluxioNode>* nodesPtr = nullptr;
-            std::vector<FluxioEdge>* edgesPtr = nullptr;
+            std::map<std::string, FluxioParameter> parameters = {};
+            std::map<std::string, FluxioNode*> nodes = {};
+            std::list<FluxioEdge> edges = {};
 
-            manager::dto::FluxioSkill toManagerIce() const;
+            std::optional<manager::dto::FluxioSkill> toManagerIce() const;
             manager::dto::FluxioIdentificator toFluxioIdentificatorIce() const;
+            bool updateFromIce(const manager::dto::FluxioSkill& i,
+                               std::map<std::string, FluxioProvider>& providersMap,
+                               std::map<std::string, FluxioProfile>& profilesMap,
+                               std::map<std::string, FluxioSkill>& skillsMap);
+            void deleteParameter(const std::string& parameterId, const std::string& skillId);
 
             static FluxioSkill*
             FromFluxioIdentificatorIce(const manager::dto::FluxioIdentificator& i,
                                        std::map<std::string, FluxioSkill>& skillsMap);
-            static FluxioSkill FromIce(const manager::dto::FluxioSkill& i,
-                                       std::map<std::string, FluxioProvider>& providersMap,
-                                       std::map<std::string, FluxioProfile>& profilesMap,
-                                       std::map<std::string, FluxioSkill>& skillsMap);
+            static std::optional<FluxioSkill>
+            FromIce(const manager::dto::FluxioSkill& i,
+                    std::map<std::string, FluxioProvider>& providersMap,
+                    std::map<std::string, FluxioProfile>& profilesMap,
+                    std::map<std::string, FluxioSkill>& skillsMap);
+            static skills::FluxioNode*
+            CreateNode(const manager::dto::FluxioNode& i,
+                       std::map<std::string, FluxioParameter>& parametersMap,
+                       std::map<std::string, FluxioSkill>& skillsMap);
         };
     } // namespace skills
 } // namespace armarx
diff --git a/source/RobotAPI/libraries/skills/core/FluxioSubSkillNode.cpp b/source/RobotAPI/libraries/skills/core/FluxioSubSkillNode.cpp
index 20eeb6259..a9df3c92b 100644
--- a/source/RobotAPI/libraries/skills/core/FluxioSubSkillNode.cpp
+++ b/source/RobotAPI/libraries/skills/core/FluxioSubSkillNode.cpp
@@ -1,18 +1,22 @@
 #include "FluxioSubSkillNode.h"
-#include "FluxioSkill.h"
+
+#include <ArmarXCore/core/logging/Logging.h>
+
 #include <RobotAPI/interface/skills/SkillManagerInterface.h>
 
+#include "FluxioSkill.h"
+
 namespace armarx
 {
     namespace skills
     {
-        manager::dto::FluxioNode
+        std::optional<manager::dto::FluxioNode>
         FluxioSubSkillNode::toManagerIce() const
         {
-            if(skillPtr == nullptr)
+            if (skillPtr == nullptr)
             {
-                // TODO: throw error or show warning
-                return {};
+                ARMARX_WARNING << "Skill for SubSkillNode with id " << nodeId << " not set";
+                return std::nullopt;
             }
 
             manager::dto::FluxioNode ret;
@@ -21,7 +25,7 @@ namespace armarx
             ret.name = name;
             ret.xPos = xPos;
             ret.yPos = yPos;
-            
+
             manager::dto::FluxioIdentificator paramId;
             paramId.id = "";
             paramId.hint = "";
@@ -32,17 +36,28 @@ namespace armarx
             return ret;
         }
 
-        FluxioSubSkillNode FluxioSubSkillNode::FromIce(const manager::dto::FluxioNode& i, std::map<std::string, FluxioSkill>& skillsMap)
+        std::optional<FluxioSubSkillNode>
+        FluxioSubSkillNode::FromIce(const manager::dto::FluxioNode& i,
+                                    std::map<std::string, FluxioSkill>& skillsMap)
         {
+            skills::FluxioSkill* skillPtr =
+                FluxioSkill::FromFluxioIdentificatorIce(i.skillId, skillsMap);
+
+            if (skillPtr == nullptr)
+            {
+                ARMARX_WARNING << "Skill for SubSkillNode with id " << i.nodeId << " not found";
+                return std::nullopt;
+            }
+
             FluxioSubSkillNode ret;
             ret.nodeId = i.nodeId;
             ret.nodeType = i.nodeType;
             ret.name = i.name;
             ret.xPos = i.xPos;
             ret.yPos = i.yPos;
-            ret.skillPtr = FluxioSkill::FromFluxioIdentificatorIce(i.skillId, skillsMap);
+            ret.skillPtr = skillPtr;
 
             return ret;
         }
-    }  // namespace skills
-}  // namespace armarx
+    } // namespace skills
+} // namespace armarx
diff --git a/source/RobotAPI/libraries/skills/core/FluxioSubSkillNode.h b/source/RobotAPI/libraries/skills/core/FluxioSubSkillNode.h
index a8a3f834c..ff2cc1ad4 100644
--- a/source/RobotAPI/libraries/skills/core/FluxioSubSkillNode.h
+++ b/source/RobotAPI/libraries/skills/core/FluxioSubSkillNode.h
@@ -1,20 +1,23 @@
 #pragma once
 
+#include <RobotAPI/interface/skills/SkillManagerInterface.h>
+
 #include "FluxioNode.h"
 #include "FluxioSkill.h"
-#include <RobotAPI/interface/skills/SkillManagerInterface.h>
 
 namespace armarx
 {
     namespace skills
     {
-        struct FluxioSubSkillNode : FluxioNode
+        struct FluxioSubSkillNode : public FluxioNode
         {
-            FluxioSkill *skillPtr;
+            FluxioSkill* skillPtr;
 
-            manager::dto::FluxioNode toManagerIce() const override;
+            std::optional<manager::dto::FluxioNode> toManagerIce() const override;
 
-            static FluxioSubSkillNode FromIce(const manager::dto::FluxioNode& i, std::map<std::string, FluxioSkill>& skillsMap);
+            static std::optional<FluxioSubSkillNode>
+            FromIce(const manager::dto::FluxioNode& i,
+                    std::map<std::string, FluxioSkill>& skillsMap);
         };
     } // namespace skills
 } // namespace armarx
diff --git a/source/RobotAPI/libraries/skills/core/FluxioValue.cpp b/source/RobotAPI/libraries/skills/core/FluxioValue.cpp
index b4947c67c..3fa4fdba7 100644
--- a/source/RobotAPI/libraries/skills/core/FluxioValue.cpp
+++ b/source/RobotAPI/libraries/skills/core/FluxioValue.cpp
@@ -1,19 +1,24 @@
 #include "FluxioValue.h"
+
+#include <optional>
+
+#include <ArmarXCore/core/logging/Logging.h>
+
 #include "FluxioProfile.h"
 
 namespace armarx
 {
     namespace skills
     {
-        manager::dto::FluxioValue
+        std::optional<manager::dto::FluxioValue>
         FluxioValue::toManagerIce() const
         {
             manager::dto::FluxioValue ret;
 
             if (profilePtr == nullptr)
             {
-                // TODO: throw exception
-                return {};
+                ARMARX_WARNING << "Profile not set.";
+                return std::nullopt;
             }
 
             ret.profileId = profilePtr->toFluxioIdentificatorIce();
@@ -22,13 +27,21 @@ namespace armarx
             return ret;
         }
 
-        FluxioValue
+        std::optional<FluxioValue>
         FluxioValue::FromIce(const manager::dto::FluxioValue& i,
                              std::map<std::string, FluxioProfile>& profilesMap)
         {
+            FluxioProfile* profilePtr =
+                FluxioProfile::FromFluxioIdentificatorIce(i.profileId, profilesMap);
+
+            if (profilePtr == nullptr)
+            {
+                return std::nullopt;
+            }
+
             FluxioValue ret;
 
-            ret.profilePtr = FluxioProfile::FromFluxioIdentificatorIce(i.profileId, profilesMap);
+            ret.profilePtr = profilePtr;
             ret.content = i.content;
 
             return ret;
diff --git a/source/RobotAPI/libraries/skills/core/FluxioValue.h b/source/RobotAPI/libraries/skills/core/FluxioValue.h
index b644790c0..9c3dea431 100644
--- a/source/RobotAPI/libraries/skills/core/FluxioValue.h
+++ b/source/RobotAPI/libraries/skills/core/FluxioValue.h
@@ -1,7 +1,9 @@
 #pragma once
 
 #include <string>
+
 #include <RobotAPI/interface/skills/SkillManagerInterface.h>
+
 #include "FluxioProfile.h"
 
 namespace armarx
@@ -10,12 +12,14 @@ namespace armarx
     {
         struct FluxioValue
         {
-            FluxioProfile *profilePtr;
+            FluxioProfile* profilePtr;
             std::string content;
 
-            manager::dto::FluxioValue toManagerIce() const;
+            std::optional<manager::dto::FluxioValue> toManagerIce() const;
 
-            static FluxioValue FromIce(const manager::dto::FluxioValue& i, std::map<std::string, FluxioProfile>& profilesMap);
+            static std::optional<FluxioValue>
+            FromIce(const manager::dto::FluxioValue& i,
+                    std::map<std::string, FluxioProfile>& profilesMap);
         };
     } // namespace skills
 } // namespace armarx
diff --git a/source/RobotAPI/libraries/skills/manager/SkillManagerComponentPlugin.cpp b/source/RobotAPI/libraries/skills/manager/SkillManagerComponentPlugin.cpp
index 69d34fda0..492146c74 100644
--- a/source/RobotAPI/libraries/skills/manager/SkillManagerComponentPlugin.cpp
+++ b/source/RobotAPI/libraries/skills/manager/SkillManagerComponentPlugin.cpp
@@ -1,8 +1,8 @@
 #include "SkillManagerComponentPlugin.h"
 
+#include <list>
 #include <optional>
 #include <string>
-#include <list>
 #include <tuple>
 
 #include <IceUtil/UUID.h>
@@ -15,6 +15,8 @@
 
 #include "RobotAPI/libraries/skills/core/FluxioProfile.h"
 #include "RobotAPI/libraries/skills/core/FluxioProvider.h"
+#include "RobotAPI/libraries/skills/core/FluxioValue.h"
+#include <RobotAPI/libraries/aron/converter/json/NLohmannJSONConverter.h>
 #include <RobotAPI/libraries/skills/core/SkillID.h>
 #include <RobotAPI/libraries/skills/core/SkillStatusUpdate.h>
 #include <RobotAPI/libraries/skills/core/error/Exception.h>
@@ -595,28 +597,35 @@ namespace armarx::plugins
         return ret;
     }
 
-    //** Fluxio related methods
+    //****************************//
+    //** Fluxio related methods **//
+    //****************************//
 
     /**
     * @brief converts parametersType or resultType from skilldescription to fluxio parameters
-    * @param ret the vector to insert the fluxio parameters into
+    * @param ret the map to insert the fluxio parameters into
     * @param ptr the aron object ptr to convert (this is either skillDescription.parametersType or skillDescription.resultType)
     * @param isInput whether the parameters are input parameters or output parameters
-    */ 
-    void SkillManagerComponentPlugin::convertAronObjectPtrToFluxioParameters(std::vector<skills::FluxioParameter>& ret, const aron::type::ObjectPtr& ptr, bool isInput)
+    */
+    void
+    SkillManagerComponentPlugin::convertAronObjectPtrToFluxioParameters(
+        std::map<std::string, skills::FluxioParameter>& ret,
+        const aron::type::ObjectPtr& ptr,
+        bool isInput)
     {
-        for (const auto& [name, typeInfo]: ptr->getMemberTypes())
+        for (const auto& [name, typeInfo] : ptr->getMemberTypes())
         {
             skills::FluxioParameter p;
             p.id = name;
             p.name = name;
-            p.description = typeInfo->getFullName(); // Full type name may contain some extra info like dimensions etc. Also, there is no description for parameters yet.
+            // Full type name may contain some extra info like dimensions etc.
+            p.description = typeInfo->getFullName();
             p.type = typeInfo->getShortName();
             p.required = (typeInfo->getMaybe() == 0);
             p.isInput = isInput;
-            p.values = {}; // TODO: insert root profile defaults here
+            p.values = {}; // these are inserted later
 
-            ret.push_back(p);
+            ret[p.id] = p;
         }
     }
 
@@ -664,30 +673,38 @@ namespace armarx::plugins
             if (skillDescription.parametersType != nullptr)
             {
                 // input parameters
-                convertAronObjectPtrToFluxioParameters(s.parameters, skillDescription.parametersType, true);
+                convertAronObjectPtrToFluxioParameters(
+                    s.parameters, skillDescription.parametersType, true);
             }
 
             if (skillDescription.resultType != nullptr)
             {
                 // output parameters
-                convertAronObjectPtrToFluxioParameters(s.parameters, skillDescription.resultType, false);
+                convertAronObjectPtrToFluxioParameters(
+                    s.parameters, skillDescription.resultType, false);
             }
 
-            if(skillDescription.rootProfileDefaults != nullptr){
-                const skills::FluxioProfile& rootProfile = fluxioDC.profiles["root"];
+            if (skillDescription.rootProfileDefaults != nullptr)
+            {
+                skills::FluxioProfile* rootProfilePtr = &fluxioDC.profiles["root"];
 
-                for (auto [k, v]: skillDescription.rootProfileDefaults->getElements())
+                for (auto [k, v] : skillDescription.rootProfileDefaults->getElements())
                 {
-                    // TODO: find a way to read the actual value from v (aron::data::variant)
-                    // TODO: create Fluxio Value with root profile and add it to the corresponding parameter´s values vector
+                    // use NLohmannJSONConverter to generate the json representation of the value
+                    auto json =
+                        aron::data::converter::AronNlohmannJSONConverter::ConvertToNlohmannJSON(v);
 
-                    // aron converter json NLohmannJSONConverter.h
-                    ARMARX_WARNING << "rootProfileDefaults:  (" << k << ", " << v->getFullName() << ")"; 
+                    skills::FluxioValue val;
+                    val.profilePtr = rootProfilePtr;
+                    val.content = json.dump();
+
+                    s.parameters[k].values.push_back(val);
                 }
             }
 
-            s.edgesPtr = nullptr;
-            s.nodesPtr = nullptr;
+            // left empty because native skills have neither nodes nor edges
+            s.edges = {};
+            s.nodes = {};
 
             fluxioDC.skills[s.id] = s;
         }
@@ -707,69 +724,6 @@ namespace armarx::plugins
         return std::nullopt;
     }
 
-    void
-    SkillManagerComponentPlugin::updateSkill(const std::string& userId, const skills::FluxioSkill& skill)
-    {
-        // TODO: check if mutex is held by user
-
-        const auto& skillsEntry = fluxioDC.skills.find(skill.id);
-        if (skillsEntry == fluxioDC.skills.end())
-        {
-            return;
-        }
-        skills::FluxioSkill& oldSkill = skillsEntry->second;
-        oldSkill.name = skill.name;
-        oldSkill.description = skill.description;
-        oldSkill.lastChanged = armarx::DateTime::Now().toDateTimeString();
-        oldSkill.executable = skill.executable;
-        // oldSkill->native = skill.native; Not changeable
-
-        // ---update the parameters lists entirelys
-        std::list<std::string> toRemoveIds;
-        // fill the toRemoveIds vector with all old skills parameters
-        for (const auto& parameter : oldSkill.parameters)
-        {
-            toRemoveIds.push_back(parameter.id);
-        }
-        for (const auto& parameter : skill.parameters)
-        {
-            const auto& oldParameter = std::find_if(oldSkill.parameters.begin(),
-                                                    oldSkill.parameters.end(),
-                                                    [&parameter](const skills::FluxioParameter& p)
-                                                    { return p.id == parameter.id; });
-            if (oldParameter != oldSkill.parameters.end())
-            {
-                oldParameter->name = parameter.name;
-                oldParameter->description = parameter.description;
-                oldParameter->type = parameter.type;
-                oldParameter->required = parameter.required;
-                oldParameter->isInput = parameter.isInput;
-                oldParameter->values = parameter.values; // TODO: Make it right
-                // remove the id from the toRemoveIds vector
-                toRemoveIds.erase(std::remove(toRemoveIds.begin(), toRemoveIds.end(), parameter.id),
-                                  toRemoveIds.end());
-            }
-            else
-            {
-                oldSkill.parameters.push_back(parameter);
-            }
-        }
-        // remove all parameters that are in the toRemoveIds vector
-        for (const auto& id : toRemoveIds)
-        {
-            oldSkill.parameters.erase(std::remove_if(oldSkill.parameters.begin(),
-                                                     oldSkill.parameters.end(),
-                                                     [&id](const skills::FluxioParameter& p)
-                                                     { return p.id == id; }),
-                                      oldSkill.parameters.end());
-        }
-        // --- end of update the parameters lists entirely
-
-        oldSkill.skillProviderPtr = skill.skillProviderPtr;
-        oldSkill.edgesPtr = skill.edgesPtr;
-        oldSkill.nodesPtr = skill.nodesPtr;
-    }
-
     void
     SkillManagerComponentPlugin::removeSkill(const std::string& id)
     {
@@ -777,23 +731,25 @@ namespace armarx::plugins
     }
 
     bool
-    SkillManagerComponentPlugin::getSkillMutex(const std::string& skillId, const std::string& userId)
+    SkillManagerComponentPlugin::getSkillMutex(const std::string& skillId,
+                                               const std::string& userId)
     {
-        return setEditFluxioSkillMutex(true,userId,skillId);
-
+        return setEditFluxioSkillMutex(true, userId, skillId);
     }
 
     void
-    SkillManagerComponentPlugin::deleteSkillMutex(const std::string& skillId, const std::string& userId)
+    SkillManagerComponentPlugin::deleteSkillMutex(const std::string& skillId,
+                                                  const std::string& userId)
     {
-        setEditFluxioSkillMutex(false,userId,skillId); // ignore return value for now
+        setEditFluxioSkillMutex(false, userId, skillId); // ignore return value for now
     }
 
     void
-    SkillManagerComponentPlugin::removeSkillParameter(const std::string& userId, const std::string& skillId, const std::string& parameterId)
+    SkillManagerComponentPlugin::removeSkillParameter(const std::string& userId,
+                                                      const std::string& skillId,
+                                                      const std::string& parameterId)
     {
         // TODO: check if mutex is held by user
-
     }
 
     std::vector<skills::FluxioProfile>
@@ -898,7 +854,9 @@ namespace armarx::plugins
     }
 
     std::optional<skills::FluxioSkill>
-    SkillManagerComponentPlugin::addSkillToProvider(const std::string& userId, const std::string& providerId, const skills::FluxioSkill& skill)
+    SkillManagerComponentPlugin::addSkillToProvider(const std::string& userId,
+                                                    const std::string& providerId,
+                                                    const skills::FluxioSkill& skill)
     {
         // check if the provider exists
         const auto& providerIt = fluxioDC.providers.find(providerId);
@@ -914,17 +872,22 @@ namespace armarx::plugins
         fluxioDC.skills[id].lastChanged = armarx::DateTime::Now().toDateTimeString();
 
         // set mutex
-        setEditFluxioSkillMutex(true,userId,id);
+        setEditFluxioSkillMutex(true, userId, id);
 
         return fluxioDC.skills[id];
     }
 
-    bool SkillManagerComponentPlugin::setEditFluxioSkillMutex(bool aquireMutex,const std::string& userId, const std::string& skillId){
+    bool // TODO: add armarx info messages
+    SkillManagerComponentPlugin::setEditFluxioSkillMutex(bool aquireMutex,
+                                                         const std::string& userId,
+                                                         const std::string& skillId)
+    {
         const auto& skillMutexIt = fluxioDC.skillMutexMap.find(skillId);
-        
+
         // aquire mutex
-        if(aquireMutex){
-            if(skillMutexIt == fluxioDC.skillMutexMap.end())
+        if (aquireMutex)
+        {
+            if (skillMutexIt == fluxioDC.skillMutexMap.end())
             {
                 // skill is free, set mutex
                 fluxioDC.skillMutexMap[skillId] = std::make_tuple(userId, armarx::DateTime::Now());
@@ -932,7 +895,7 @@ namespace armarx::plugins
             }
 
             // skill is already locked
-            if (std::get<0>(fluxioDC.skillMutexMap[skillId]) == userId) 
+            if (std::get<0>(fluxioDC.skillMutexMap[skillId]) == userId)
             {
                 // user already holds the mutex -> reset timestamp / prolong mutex
                 std::get<1>(fluxioDC.skillMutexMap[skillId]) = armarx::DateTime::Now();
@@ -940,20 +903,21 @@ namespace armarx::plugins
             }
 
             // another user holds the mutex -> is it still valid ?
-            armarx::Duration elapsedTime = armarx::DateTime::Now() - std::get<1>(fluxioDC.skillMutexMap[skillId]);
-            if (elapsedTime.toMinutes() > fluxioDC.mutexTimeout) 
+            armarx::Duration elapsedTime =
+                armarx::DateTime::Now() - std::get<1>(fluxioDC.skillMutexMap[skillId]);
+            if (elapsedTime.toMinutes() > fluxioDC.mutexTimeout)
             {
                 // mutex invalid, requesting user gets the mutex instead
                 fluxioDC.skillMutexMap[skillId] = std::make_tuple(userId, armarx::DateTime::Now());
                 return true;
             }
-            
+
             // mutex could not be aquired
             return false;
         }
 
         // remove mutex
-        if( skillMutexIt == fluxioDC.skillMutexMap.end())
+        if (skillMutexIt == fluxioDC.skillMutexMap.end())
         {
             // skill is already free
             return true;
diff --git a/source/RobotAPI/libraries/skills/manager/SkillManagerComponentPlugin.h b/source/RobotAPI/libraries/skills/manager/SkillManagerComponentPlugin.h
index b7e4591a0..02a5f98e2 100644
--- a/source/RobotAPI/libraries/skills/manager/SkillManagerComponentPlugin.h
+++ b/source/RobotAPI/libraries/skills/manager/SkillManagerComponentPlugin.h
@@ -2,6 +2,7 @@
 
 #include <mutex>
 #include <string>
+
 #include <ArmarXCore/core/ComponentPlugin.h>
 #include <ArmarXCore/core/ManagedIceObject.h>
 #include <ArmarXCore/core/time/DateTime.h>
@@ -65,21 +66,23 @@ namespace armarx::plugins
 
         skills::ProviderID getFirstProviderNameThatHasSkill(const skills::SkillID& skillid);
 
-        //** Fluxio related methods
+        //****************************//
+        //** Fluxio related methods **//
+        //****************************//
 
         std::vector<skills::FluxioSkill> getSkillList();
 
         std::optional<skills::FluxioSkill> getSkill(const std::string& id);
 
-        void updateSkill(const std::string& userId, const skills::FluxioSkill& skill);
-
         void removeSkill(const std::string& id);
 
         bool getSkillMutex(const std::string& skillId, const std::string& userId);
 
         void deleteSkillMutex(const std::string& skillId, const std::string& userId);
 
-        void removeSkillParameter(const std::string& userId, const std::string& skillId, const std::string& parameterId);
+        void removeSkillParameter(const std::string& userId,
+                                  const std::string& skillId,
+                                  const std::string& parameterId);
 
         std::vector<skills::FluxioProfile> getProfileList();
 
@@ -95,10 +98,11 @@ namespace armarx::plugins
 
         std::optional<std::vector<skills::FluxioSkill>> getSkillsOfProvider(const std::string& id);
 
-        std::optional<skills::FluxioSkill> addSkillToProvider(const std::string& userId, const std::string& providerId, const skills::FluxioSkill& skill);
+        std::optional<skills::FluxioSkill> addSkillToProvider(const std::string& userId,
+                                                              const std::string& providerId,
+                                                              const skills::FluxioSkill& skill);
 
     private:
-
         struct FluxioDataCollector
         {
             std::map<std::string, skills::FluxioSkill> skills = {};
@@ -116,14 +120,21 @@ namespace armarx::plugins
         std::map<skills::ProviderID, skills::provider::dti::SkillProviderInterfacePrx>
             skillProviderMap;
 
-        template<typename S, typename T> std::vector<T> convertMapValuesToVector(std::map<S, T>& map);
+        template <typename S, typename T>
+        std::vector<T> convertMapValuesToVector(std::map<S, T>& map);
 
-        static void convertAronObjectPtrToFluxioParameters(std::vector<skills::FluxioParameter>& ret, const aron::type::ObjectPtr& ptr, bool isInput);
+        static void
+        convertAronObjectPtrToFluxioParameters(std::map<std::string, skills::FluxioParameter>& ret,
+                                               const aron::type::ObjectPtr& ptr,
+                                               bool isInput);
 
-        template<typename T>
-        void replaceVectorContent(std::vector<T>& originalVector, const std::vector<T>& replacementVector);
+        template <typename T>
+        void replaceVectorContent(std::vector<T>& originalVector,
+                                  const std::vector<T>& replacementVector);
 
-        bool setEditFluxioSkillMutex(bool aquireMutex,const std::string& userId, const std::string& skillId);
+        bool setEditFluxioSkillMutex(bool aquireMutex,
+                                     const std::string& userId,
+                                     const std::string& skillId);
 
         friend class armarx::SkillManagerComponentPluginUser;
     };
diff --git a/source/RobotAPI/libraries/skills/manager/SkillManagerComponentPluginUser.cpp b/source/RobotAPI/libraries/skills/manager/SkillManagerComponentPluginUser.cpp
index b907f5e27..9c8973c88 100644
--- a/source/RobotAPI/libraries/skills/manager/SkillManagerComponentPluginUser.cpp
+++ b/source/RobotAPI/libraries/skills/manager/SkillManagerComponentPluginUser.cpp
@@ -1,6 +1,9 @@
 #include "SkillManagerComponentPluginUser.h"
+
 #include <IceUtil/Optional.h>
 
+#include <ArmarXCore/core/logging/Logging.h>
+
 #include "RobotAPI/libraries/skills/core/FluxioProfile.h"
 #include "RobotAPI/libraries/skills/core/FluxioProvider.h"
 #include <RobotAPI/interface/skills/SkillManagerInterface.h>
@@ -149,7 +152,9 @@ namespace armarx
         return ret;
     }
 
-    //** Fluxio related methods
+    //****************************//
+    //** Fluxio related methods **//
+    //****************************//
 
     skills::manager::dto::FluxioSkillList
     SkillManagerComponentPluginUser::getSkillList(const Ice::Current& current)
@@ -160,7 +165,16 @@ namespace armarx
 
         for (const auto& s : l)
         {
-            ret.push_back(s.toManagerIce());
+            const auto& skill = s.toManagerIce();
+
+            if (!skill.has_value())
+            {
+                ARMARX_WARNING << "SkillManagerComponentPluginUser::getSkillList: Skill with id "
+                               << s.id << " could not be converted";
+                continue;
+            }
+
+            ret.push_back(*skill);
         }
 
         return ret;
@@ -173,13 +187,18 @@ namespace armarx
 
         if (skill.has_value())
         {
-            return skill->toManagerIce();
+            const auto& s = skill->toManagerIce();
+            if (s.has_value())
+            {
+                return *s;
+            }
         }
         return {};
     }
 
-    void
-    SkillManagerComponentPluginUser::updateSkill(const std::string& userId, const skills::manager::dto::FluxioSkill& skill,
+    bool
+    SkillManagerComponentPluginUser::updateSkill(const std::string& userId,
+                                                 const skills::manager::dto::FluxioSkill& skill,
                                                  const Ice::Current& current)
     {
         this->plugin->getSkillList();
@@ -188,8 +207,24 @@ namespace armarx
             this->plugin->fluxioDC.providers;
         std::map<std::string, skills::FluxioProfile>& profilesMap = this->plugin->fluxioDC.profiles;
 
-        this->plugin->updateSkill(userId,
-            skills::FluxioSkill::FromIce(skill, providersMap, profilesMap, skillsMap));
+        // Check if the skill exists
+        const auto& s = skillsMap.find(skill.id);
+        if (s == skillsMap.end())
+        {
+            ARMARX_WARNING << "SkillManagerComponentPluginUser::updateSkill: Skill with id "
+                           << skill.id << " not found";
+            return false;
+        }
+
+        // Check if the user has the mutex for the skill
+        if (!this->plugin->getSkillMutex(skill.id, userId))
+        {
+            ARMARX_WARNING << "SkillManagerComponentPluginUser::updateSkill: User " << userId
+                           << " does not have the mutex for skill with id " << skill.id;
+            return false;
+        }
+
+        return s->second.updateFromIce(skill, providersMap, profilesMap, skillsMap);
     }
 
     void
@@ -215,7 +250,7 @@ namespace armarx
     }
 
     void
-    SkillManagerComponentPluginUser::removeSkillParameter(const std::string& userId, 
+    SkillManagerComponentPluginUser::removeSkillParameter(const std::string& userId,
                                                           const std::string& skillId,
                                                           const std::string& parameterId,
                                                           const Ice::Current& current)
@@ -230,9 +265,9 @@ namespace armarx
 
         auto l = this->plugin->getProfileList();
 
-        for (const auto& s : l)
+        for (const auto& profile : l)
         {
-            ret.push_back(s.toManagerIce());
+            ret.push_back(profile.toManagerIce());
         }
 
         return ret;
@@ -313,7 +348,17 @@ namespace armarx
 
         for (const auto& s : *l)
         {
-            ret.push_back(s.toManagerIce());
+            const auto& skill = s.toManagerIce();
+
+            if (!skill.has_value())
+            {
+                ARMARX_WARNING
+                    << "SkillManagerComponentPluginUser::getSkillsOfProvider: Skill with id "
+                    << s.id << " could not be converted";
+                continue;
+            }
+
+            ret.push_back(*skill);
         }
 
         return ret;
@@ -324,7 +369,7 @@ namespace armarx
         const std::string& userId,
         const std::string& providerId,
         const skills::manager::dto::FluxioSkill& skill,
-        const Ice::Current&  /*current*/)
+        const Ice::Current& current)
     {
         this->plugin->getSkillList();
         std::map<std::string, skills::FluxioSkill>& skillsMap = this->plugin->fluxioDC.skills;
@@ -332,15 +377,26 @@ namespace armarx
             this->plugin->fluxioDC.providers;
         std::map<std::string, skills::FluxioProfile>& profilesMap = this->plugin->fluxioDC.profiles;
 
-        auto s = this->plugin
-            ->addSkillToProvider(
-                userId,
-                providerId,
-                skills::FluxioSkill::FromIce(skill, providersMap, profilesMap, skillsMap));
+        const auto& skillBO =
+            skills::FluxioSkill::FromIce(skill, providersMap, profilesMap, skillsMap);
+        if (!skillBO.has_value())
+        {
+            ARMARX_WARNING << "Skill with id " << skill.id << " could not be converted";
+            return {};
+        }
 
-        if (s.has_value()) {
-            return s->toManagerIce();
+        auto s = this->plugin->addSkillToProvider(userId, providerId, *skillBO);
+        if (s.has_value())
+        {
+            const auto& ret = s->toManagerIce();
+            if (ret.has_value())
+            {
+                return *ret;
+            }
         }
+
+        ARMARX_WARNING << "Skill with id " << skill.id << " could not be added to provider with id "
+                       << providerId;
         return {};
     }
 } // namespace armarx
diff --git a/source/RobotAPI/libraries/skills/manager/SkillManagerComponentPluginUser.h b/source/RobotAPI/libraries/skills/manager/SkillManagerComponentPluginUser.h
index d40d8d563..68a803c7d 100644
--- a/source/RobotAPI/libraries/skills/manager/SkillManagerComponentPluginUser.h
+++ b/source/RobotAPI/libraries/skills/manager/SkillManagerComponentPluginUser.h
@@ -1,8 +1,9 @@
 #pragma once
 
-#include "SkillManagerComponentPlugin.h"
 #include <RobotAPI/interface/skills/SkillManagerInterface.h>
 
+#include "SkillManagerComponentPlugin.h"
+
 namespace armarx::plugins
 {
     class SkillManagerComponentPlugin; // forward declaration
@@ -19,6 +20,7 @@ namespace armarx
 
         void addProvider(const skills::manager::dto::ProviderInfo& providerInfo,
                          const Ice::Current& current) override;
+
         void removeProvider(const skills::manager::dto::ProviderID& provider,
                             const Ice::Current& current) override;
 
@@ -62,46 +64,61 @@ namespace armarx
         skills::manager::dto::SkillStatusUpdateMap
         getSkillExecutionStatuses(const Ice::Current& current) override;
 
-        //** Fluxio related methods
+        //****************************//
+        //** Fluxio related methods **//
+        //****************************//
 
-        skills::manager::dto::FluxioSkillList 
-        getSkillList(const Ice::Current& current) override;
+        skills::manager::dto::FluxioSkillList getSkillList(const Ice::Current& current) override;
 
         IceUtil::Optional<skills::manager::dto::FluxioSkill>
-        getSkill(const std::string& id,const Ice::Current& current) override;
+        getSkill(const std::string& id, const Ice::Current& current) override;
 
-        void updateSkill(const std::string& userId, const skills::manager::dto::FluxioSkill& skill, const Ice::Current& current) override;
+        bool updateSkill(const std::string& userId,
+                         const skills::manager::dto::FluxioSkill& skill,
+                         const Ice::Current& current) override;
+
+        void removeSkill(const std::string& id, const Ice::Current& current) override;
 
-        void removeSkill(const std::string& id,const Ice::Current& current) override;
+        bool getSkillMutex(const std::string& skillId,
+                           const std::string& userId,
+                           const Ice::Current& current) override;
 
-        bool getSkillMutex(const std::string& skillId, const std::string& userId,const Ice::Current& current) override;
-        
-        void deleteSkillMutex(const std::string& skillId, const std::string& userId,const Ice::Current& current) override;
+        void deleteSkillMutex(const std::string& skillId,
+                              const std::string& userId,
+                              const Ice::Current& current) override;
 
-        void removeSkillParameter(const std::string& userId, const std::string& skillId, const std::string& parameterId,const Ice::Current& current) override;
+        void removeSkillParameter(const std::string& userId,
+                                  const std::string& skillId,
+                                  const std::string& parameterId,
+                                  const Ice::Current& current) override;
 
         skills::manager::dto::FluxioProfileList
         getProfileList(const Ice::Current& current) override;
 
-        IceUtil::Optional<skills::manager::dto::FluxioProfile> 
-        getProfile(const std::string& id,const Ice::Current& current) override;
+        IceUtil::Optional<skills::manager::dto::FluxioProfile>
+        getProfile(const std::string& id, const Ice::Current& current) override;
 
         skills::manager::dto::FluxioProfile
-        createProfile(const skills::manager::dto::FluxioProfile& profile,const Ice::Current& current) override;
+        createProfile(const skills::manager::dto::FluxioProfile& profile,
+                      const Ice::Current& current) override;
 
-        void updateProfile(const skills::manager::dto::FluxioProfile& profile,const Ice::Current& current) override;
+        void updateProfile(const skills::manager::dto::FluxioProfile& profile,
+                           const Ice::Current& current) override;
 
         skills::manager::dto::FluxioProviderList
         getProviderList(const Ice::Current& current) override;
 
         IceUtil::Optional<skills::manager::dto::FluxioProvider>
-        getProvider(const std::string& id,const Ice::Current& current) override;
+        getProvider(const std::string& id, const Ice::Current& current) override;
 
         IceUtil::Optional<skills::manager::dto::FluxioSkillList>
-        getSkillsOfProvider(const std::string& id,const Ice::Current& current) override;
+        getSkillsOfProvider(const std::string& id, const Ice::Current& current) override;
 
         IceUtil::Optional<skills::manager::dto::FluxioSkill>
-        addSkillToProvider(const std::string& userId, const std::string& providerId, const skills::manager::dto::FluxioSkill& skill, const Ice::Current& current) override;
+        addSkillToProvider(const std::string& userId,
+                           const std::string& providerId,
+                           const skills::manager::dto::FluxioSkill& skill,
+                           const Ice::Current& current) override;
 
     private:
         armarx::plugins::SkillManagerComponentPlugin* plugin = nullptr;
-- 
GitLab