From c4c0eeb3d6300ffd39bddd0cf3d6c7c3f825c81a Mon Sep 17 00:00:00 2001
From: Julian Tusch <urhrf@student.kit.edu>
Date: Tue, 7 May 2024 14:23:31 +0200
Subject: [PATCH] renamed method, added basic implementation

---
 .../skills/SkillManagerInterface.ice          | 10 +--
 .../manager/SkillManagerComponentPlugin.cpp   | 70 ++++++++++++++++++-
 .../manager/SkillManagerComponentPlugin.h     | 14 ++--
 .../SkillManagerComponentPluginUser.cpp       |  4 +-
 .../manager/SkillManagerComponentPluginUser.h |  2 +-
 5 files changed, 82 insertions(+), 18 deletions(-)

diff --git a/source/RobotAPI/interface/skills/SkillManagerInterface.ice b/source/RobotAPI/interface/skills/SkillManagerInterface.ice
index f8049eee5..245cb1a90 100644
--- a/source/RobotAPI/interface/skills/SkillManagerInterface.ice
+++ b/source/RobotAPI/interface/skills/SkillManagerInterface.ice
@@ -248,10 +248,6 @@ module armarx
                     bool 
                     updateSkill(string userId, dto::FluxioSkill skill); // updates an existing skill
 
-                    // TODO: implement dry-run
-                    void 
-                    removeSkill(string id); // removes a skill
-
                     bool 
                     getSkillMutex(string skillId, string userId); // request a mutex for a skill
 
@@ -260,7 +256,11 @@ module armarx
 
                     // TODO: implement dry-run
                     void 
-                    removeSkillParameter(string userId, string skillId, string parameterId); // removes a parameter from a skill
+                    deleteSkill(string skillId, string userId); // removes a skill
+
+                    // TODO: implement dry-run
+                    void 
+                    removeSkillParameter(string skillId, string parameterId, string userId); // removes a parameter from a skill
 
                     dto::FluxioProfileList
                     getProfileList(); // returns all profiles
diff --git a/source/RobotAPI/libraries/skills/manager/SkillManagerComponentPlugin.cpp b/source/RobotAPI/libraries/skills/manager/SkillManagerComponentPlugin.cpp
index 9a788ad0f..2101defa9 100644
--- a/source/RobotAPI/libraries/skills/manager/SkillManagerComponentPlugin.cpp
+++ b/source/RobotAPI/libraries/skills/manager/SkillManagerComponentPlugin.cpp
@@ -1,6 +1,5 @@
 #include "SkillManagerComponentPlugin.h"
 
-#include <cstddef>
 #include <list>
 #include <optional>
 #include <string>
@@ -15,10 +14,12 @@
 #include <ArmarXCore/core/time/Duration.h>
 #include <ArmarXCore/core/time/ice_conversions.h>
 
+#include "RobotAPI/libraries/skills/core/FluxioNode.h"
 #include "RobotAPI/libraries/skills/core/FluxioParameter.h"
 #include "RobotAPI/libraries/skills/core/FluxioProfile.h"
 #include "RobotAPI/libraries/skills/core/FluxioProvider.h"
 #include "RobotAPI/libraries/skills/core/FluxioSkill.h"
+#include "RobotAPI/libraries/skills/core/FluxioSubSkillNode.h"
 #include "RobotAPI/libraries/skills/core/FluxioValue.h"
 #include "RobotAPI/libraries/skills/manager/FluxioCompositeExecutor.h"
 #include "RobotAPI/libraries/skills/manager/FluxioExecutor.h"
@@ -936,9 +937,72 @@ namespace armarx::plugins
     }
 
     void
-    SkillManagerComponentPlugin::removeSkill(const std::string& id)
+    SkillManagerComponentPlugin::deleteSkill(const std::string& skillId, const std::string& userId)
     {
-        // TODO: does the user need to have the mutex here ?
+        // check if the skill exists
+        const auto& skill = getSkill(skillId);
+        if (!skill.has_value())
+        {
+            ARMARX_WARNING << "Skill with id '" << skillId << "' not found.";
+            return;
+        }
+
+        // check if the skill is a native skill
+        if (skill->native)
+        {
+            ARMARX_WARNING << "Skill with id '" << skillId
+                           << "' is a native skill and cannot be deleted.";
+            return;
+        }
+
+        // check mutex
+        if (!getSkillMutex(skillId, userId))
+        {
+            ARMARX_WARNING << "User '" << userId << "' needs to acquire the mutex for skill '"
+                           << skillId << "' in order to delete it.";
+            return;
+        }
+
+        // check which skills are affected
+        std::vector<std::string> affectedSkills;
+        for (const auto& [id, s] : fluxioDC.skills)
+        {
+            if(s.native || s.id == skillId)
+            {
+                continue; // ignore native skills and the skill to be deleted
+            }
+
+            for (const auto& [nodeId, node] : s.nodes)
+            {
+                if (node->nodeType == "Subskill")
+                {
+                    // cast to the right node type
+                    const auto& subSkillNodeptr = dynamic_cast<skills::FluxioSubSkillNode*>(node);
+                    if (subSkillNodeptr == nullptr)
+                    {
+                        ARMARX_WARNING << "Error while casting node to FluxioSubSkillNode.";
+                        continue;
+                    }
+
+                    if (subSkillNodeptr->skillPtr->id == skillId)
+                    {
+                        affectedSkills.push_back(id);
+                        break;
+                    }
+                }
+            }
+        }
+
+        // TODO: remove the relevant subskill nodes and all edges connected to them from the affected skills
+
+        // FIXME: for now, only allow deletion if there are no affected skills
+        if (affectedSkills.empty())
+        {
+            fluxioDC.skills.erase(skillId);
+        }
+        else {
+            ARMARX_WARNING << "Skill with id " << skillId << " cannot be deleted because it is used in one or more skills";
+        }
     }
 
     bool
diff --git a/source/RobotAPI/libraries/skills/manager/SkillManagerComponentPlugin.h b/source/RobotAPI/libraries/skills/manager/SkillManagerComponentPlugin.h
index fd27f9a26..a531c4938 100644
--- a/source/RobotAPI/libraries/skills/manager/SkillManagerComponentPlugin.h
+++ b/source/RobotAPI/libraries/skills/manager/SkillManagerComponentPlugin.h
@@ -75,7 +75,7 @@ namespace armarx::plugins
         //****************************//
 
         FluxioExecutor* executeFluxioSkill(std::string skillId,
-                                                      std::string executorName = "Fluxio");
+                                           std::string executorName = "Fluxio");
 
         void abortFluxioSkill(const std::string& executionId);
 
@@ -86,7 +86,7 @@ namespace armarx::plugins
 
         std::optional<skills::FluxioSkill> getSkill(const std::string& id);
 
-        void removeSkill(const std::string& id);
+        void deleteSkill(const std::string& id, const std::string& userId);
 
         bool getSkillMutex(const std::string& skillId, const std::string& userId);
 
@@ -117,12 +117,12 @@ namespace armarx::plugins
     private:
         struct FluxioDataCollector
         {
-            std::map<std::string, skills::FluxioSkill> skills = {};
-            std::map<std::string, skills::FluxioProfile> profiles = {};
-            std::map<std::string, skills::FluxioProvider> providers = {};
-            std::map<std::string, std::tuple<std::string, armarx::DateTime>> skillMutexMap = {};
+            std::map<std::string, skills::FluxioSkill> skills;
+            std::map<std::string, skills::FluxioProfile> profiles;
+            std::map<std::string, skills::FluxioProvider> providers;
+            std::map<std::string, std::tuple<std::string, armarx::DateTime>> skillMutexMap;
             const std::int64_t mutexTimeout = 30; // minutes
-            std::map<std::string, FluxioExecutor*> fluxioExecutors = {};
+            std::map<std::string, FluxioExecutor*> fluxioExecutors;
         };
 
         FluxioDataCollector fluxioDC = {};
diff --git a/source/RobotAPI/libraries/skills/manager/SkillManagerComponentPluginUser.cpp b/source/RobotAPI/libraries/skills/manager/SkillManagerComponentPluginUser.cpp
index a559ad78e..09305a49e 100644
--- a/source/RobotAPI/libraries/skills/manager/SkillManagerComponentPluginUser.cpp
+++ b/source/RobotAPI/libraries/skills/manager/SkillManagerComponentPluginUser.cpp
@@ -268,9 +268,9 @@ namespace armarx
     }
 
     void
-    SkillManagerComponentPluginUser::removeSkill(const std::string& id, const Ice::Current& current)
+    SkillManagerComponentPluginUser::deleteSkill(const std::string& skillId, const std::string& userId, const Ice::Current& current)
     {
-        this->plugin->removeSkill(id);
+        this->plugin->deleteSkill(skillId, userId);
     }
 
     bool
diff --git a/source/RobotAPI/libraries/skills/manager/SkillManagerComponentPluginUser.h b/source/RobotAPI/libraries/skills/manager/SkillManagerComponentPluginUser.h
index b297ef22d..d9d592bb5 100644
--- a/source/RobotAPI/libraries/skills/manager/SkillManagerComponentPluginUser.h
+++ b/source/RobotAPI/libraries/skills/manager/SkillManagerComponentPluginUser.h
@@ -85,7 +85,7 @@ namespace armarx
                          const skills::manager::dto::FluxioSkill& skill,
                          const Ice::Current& current) override;
 
-        void removeSkill(const std::string& id, const Ice::Current& current) override;
+        void deleteSkill(const std::string& skillId, const std::string& userId, const Ice::Current& current) override;
 
         bool getSkillMutex(const std::string& skillId,
                            const std::string& userId,
-- 
GitLab