From e1549595e98861532e6977bc560612b068a3b33e Mon Sep 17 00:00:00 2001
From: Patrick Hegemann <patrick.hegemann@kit.edu>
Date: Tue, 18 May 2021 13:18:32 +0200
Subject: [PATCH] SkillsMemory Review

---
 .../RobotSkillsMemory/config/SkillsMemory.cfg | 16 ++++++
 .../server/SkillsMemory/SkillsMemory.cpp      | 22 +++----
 .../armem/server/SkillsMemory/SkillsMemory.h  |  9 +--
 .../SkillsMemory/aron/StatechartData.xml      | 57 -------------------
 .../libraries/armem_skills/CMakeLists.txt     |  2 +-
 .../armem_skills/aron_conversions.cpp         | 19 ++++++-
 6 files changed, 50 insertions(+), 75 deletions(-)
 delete mode 100644 source/RobotAPI/components/armem/server/SkillsMemory/aron/StatechartData.xml

diff --git a/scenarios/RobotSkillsMemory/config/SkillsMemory.cfg b/scenarios/RobotSkillsMemory/config/SkillsMemory.cfg
index 33ff12f50..57a92b84c 100644
--- a/scenarios/RobotSkillsMemory/config/SkillsMemory.cfg
+++ b/scenarios/RobotSkillsMemory/config/SkillsMemory.cfg
@@ -151,6 +151,22 @@
 # ArmarX.SkillsMemory.ObjectName = ""
 
 
+# ArmarX.SkillsMemory.StatechartCoreSegmentName:  Name of the core segment for statecharts.
+#  Attributes:
+#  - Default:            Statechart
+#  - Case sensitivity:   yes
+#  - Required:           no
+# ArmarX.SkillsMemory.StatechartCoreSegmentName = Statechart
+
+
+# ArmarX.SkillsMemory.TransitionsProviderSegmentName:  Name of the provider segment for statechart transitions.
+#  Attributes:
+#  - Default:            Transitions
+#  - Case sensitivity:   yes
+#  - Required:           no
+# ArmarX.SkillsMemory.TransitionsProviderSegmentName = Transitions
+
+
 # ArmarX.SkillsMemory.mem.MemoryName:  Name of this memory server.
 #  Attributes:
 #  - Default:            Skills
diff --git a/source/RobotAPI/components/armem/server/SkillsMemory/SkillsMemory.cpp b/source/RobotAPI/components/armem/server/SkillsMemory/SkillsMemory.cpp
index c912f55a1..8a03cc159 100644
--- a/source/RobotAPI/components/armem/server/SkillsMemory/SkillsMemory.cpp
+++ b/source/RobotAPI/components/armem/server/SkillsMemory/SkillsMemory.cpp
@@ -47,7 +47,9 @@ namespace armarx
         // Publish
         defs->topic(debugObserver);
 
-        // Subscribe
+        // Statechart Logging
+        defs->optional(p.statechartCoreSegmentName, "StatechartCoreSegmentName", "Name of the core segment for statecharts.");
+        defs->optional(p.statechartTransitionsProviderSegmentName, "TransitionsProviderSegmentName", "Name of the provider segment for statechart transitions.");
         defs->optional(p.statechartTransitionsTopicName, "tpc.sub.ProfilerListener", "Name of the ProfilerListenerInterface topics to subscribe.");
 
         const std::string prefix = "mem.";
@@ -68,7 +70,7 @@ namespace armarx
         workingMemory.name() = p.memoryName;
 
         {
-            armarx::armem::wm::CoreSegment& c = workingMemory.addCoreSegment(statechartCoreSegmentName, armarx::armem::arondto::Statechart::Transition::toInitialAronType());
+            armarx::armem::wm::CoreSegment& c = workingMemory.addCoreSegment(p.statechartCoreSegmentName, armarx::armem::arondto::Statechart::Transition::toInitialAronType());
             c.addProviderSegment("Transitions", armarx::armem::arondto::Statechart::Transition::toInitialAronType());
         }
     }
@@ -115,13 +117,13 @@ namespace armarx
 
 
     IceInternal::Handle<StatechartListener>
-    SkillsMemory::createStatechartListener(const std::string& topicName, const std::string& _name)
+    SkillsMemory::createStatechartListener(const std::string& topicName, const std::string& name)
     {
-        const std::string name = _name.empty() ? topicName + ".Listener" : _name;
-        ARMARX_DEBUG << "Registering StatechartListener '" << name << "' listening to topic '" << topicName << "'.";
+        const std::string name_ = name.empty() ? topicName + ".Listener" : name;
+        ARMARX_DEBUG << "Registering StatechartListener '" << name_ << "' listening to topic '" << topicName << "'.";
 
         IceInternal::Handle<armarx::StatechartListener> listener = Component::create<armarx::StatechartListener>();
-        listener->setName(name);
+        listener->setName(name_);
         listener->setTopicName(topicName);
 
         // Callback for the transition listener
@@ -138,9 +140,9 @@ namespace armarx
         }
         catch (const Ice::AlreadyRegisteredException& e)
         {
-            ARMARX_ERROR << "The name '" << name << "' is already used. Please choose another one.\n"
+            ARMARX_ERROR << "The name '" << name_ << "' is already used. Please choose another one.\n"
                          << "Reason: " << e.what();
-            getArmarXManager()->removeObjectBlocking(name);
+            getArmarXManager()->removeObjectBlocking(name_);
         }
         listener->getObjectScheduler()->waitForObjectState(armarx::ManagedIceObjectState::eManagedIceObjectStarted);
 
@@ -158,8 +160,8 @@ namespace armarx
             armem::EntityUpdate update;
             update.entityID = armem::MemoryID()
                               .withMemoryName(p.memoryName)
-                              .withCoreSegmentName(statechartCoreSegmentName)
-                              .withProviderSegmentName(statechartTransitionsProviderSegmentName)
+                              .withCoreSegmentName(p.statechartCoreSegmentName)
+                              .withProviderSegmentName(p.statechartTransitionsProviderSegmentName)
                               .withEntityName(entityName);
 
             update.timeCreated = transitionTime;
diff --git a/source/RobotAPI/components/armem/server/SkillsMemory/SkillsMemory.h b/source/RobotAPI/components/armem/server/SkillsMemory/SkillsMemory.h
index 266806187..ebf0b02c4 100644
--- a/source/RobotAPI/components/armem/server/SkillsMemory/SkillsMemory.h
+++ b/source/RobotAPI/components/armem/server/SkillsMemory/SkillsMemory.h
@@ -88,6 +88,9 @@ namespace armarx
         {
             std::string memoryName = "Skills";
 
+            // Statechart transition logging
+            std::string statechartCoreSegmentName = "Statechart";
+            std::string statechartTransitionsProviderSegmentName = "Transitions";
             std::string statechartTransitionsTopicName = "StateReportingTopic";
 
             struct CoreSegments
@@ -98,12 +101,10 @@ namespace armarx
         };
         Properties p;
 
-        // Statechart transition logging
-        const std::string statechartCoreSegmentName = "Statechart";
-        const std::string statechartTransitionsProviderSegmentName = "Transitions";
+
 
         IceInternal::Handle<StatechartListener> createStatechartListener(const std::string& topicName,
-                const std::string& _name = "");
+                const std::string& name = "");
         IceInternal::Handle<armarx::StatechartListener> statechartListener;
         void reportTransitions(const std::vector<StatechartListener::Transition>& transitions);
 
diff --git a/source/RobotAPI/components/armem/server/SkillsMemory/aron/StatechartData.xml b/source/RobotAPI/components/armem/server/SkillsMemory/aron/StatechartData.xml
deleted file mode 100644
index 45fe63006..000000000
--- a/source/RobotAPI/components/armem/server/SkillsMemory/aron/StatechartData.xml
+++ /dev/null
@@ -1,57 +0,0 @@
-<?xml version="1.0" encoding="UTF-8" ?>
-<AronTypeDefinition>
-    <CodeIncludes>
-    </CodeIncludes>
-
-    <GenerateTypes>
-        <IntEnum name="armarx::statechart::aron::StateType">
-            <EnumValue key="NORMAL" value="0" />
-            <EnumValue key="FINAL" value="1" />
-            <EnumValue key="REMOTE" value="2" />
-            <EnumValue key="DYNAMIC_REMOTE" value="3" />
-            <EnumValue key="UNDEFINED" value="4" />
-        </IntEnum>
-
-        <Object name='armarx::statechart::aron::ParameterMap'>
-            <ObjectChild key='parameters'>
-                <dict>
-                    <String />
-                </dict>
-            </ObjectChild>
-        </Object>
-
-        <Object name='armarx::statechart::aron::Transition'>
-            <ObjectChild key='processId'>
-                <int />
-            </ObjectChild>
-
-            <ObjectChild key="sourceStateIdentifier">
-                <String />
-            </ObjectChild>
-
-            <ObjectChild key="targetStateIdentifier">
-                <String />
-            </ObjectChild>
-
-            <ObjectChild key="eventName">
-                <String />
-            </ObjectChild>
-
-            <ObjectChild key="targetStateType">
-                <armarx::statechart::aron::StateType />
-            </ObjectChild>
-
-            <ObjectChild key="inputParameters">
-                <armarx::statechart::aron::ParameterMap />
-            </ObjectChild>
-
-            <ObjectChild key="localParameters">
-                <armarx::statechart::aron::ParameterMap />
-            </ObjectChild>
-
-            <ObjectChild key="outputParameters">
-                <armarx::statechart::aron::ParameterMap />
-            </ObjectChild>
-        </Object>
-    </GenerateTypes>
-</AronTypeDefinition>
diff --git a/source/RobotAPI/libraries/armem_skills/CMakeLists.txt b/source/RobotAPI/libraries/armem_skills/CMakeLists.txt
index 0e18610cd..bb6968ecb 100644
--- a/source/RobotAPI/libraries/armem_skills/CMakeLists.txt
+++ b/source/RobotAPI/libraries/armem_skills/CMakeLists.txt
@@ -10,7 +10,7 @@ armarx_add_library(
         ArmarXCoreObservers
 
         RobotAPI::Core
-        RobotAPI::libraries::armem
+        RobotAPI::armem
     SOURCES  
         ./aron_conversions.cpp
         ./StatechartListener.cpp
diff --git a/source/RobotAPI/libraries/armem_skills/aron_conversions.cpp b/source/RobotAPI/libraries/armem_skills/aron_conversions.cpp
index ca1292031..0b6d18a61 100644
--- a/source/RobotAPI/libraries/armem_skills/aron_conversions.cpp
+++ b/source/RobotAPI/libraries/armem_skills/aron_conversions.cpp
@@ -5,12 +5,26 @@ namespace armarx::armem
 
     void fromAron(const arondto::Statechart::StateType& dto, eStateType& bo)
     {
-        bo = fromAronStateTypeMap[dto];
+        if (fromAronStateTypeMap.find(dto) != fromAronStateTypeMap.end())
+        {
+            bo = fromAronStateTypeMap[dto];
+        }
+        else
+        {
+            bo = eStateType::eUndefined;
+        }
     }
 
     void toAron(arondto::Statechart::StateType& dto, const eStateType& bo)
     {
-        dto.value = toAronStateTypeMap[bo].value;
+        if (toAronStateTypeMap.find(bo) != toAronStateTypeMap.end())
+        {
+            dto.value = toAronStateTypeMap[bo].value;
+        }
+        else
+        {
+            dto.value = arondto::Statechart::StateType::UNDEFINED;
+        }
     }
 
     void fromAron(const arondto::Statechart::ParameterMap& dto, StateParameterMap& bo)
@@ -49,7 +63,6 @@ namespace armarx::armem
         dto.sourceStateIdentifier = bo.sourceStateIdentifier;
         dto.targetStateIdentifier = bo.targetStateIdentifier;
         dto.eventName = bo.eventName;
-        dto.targetStateType.initialize();
         toAron(dto.targetStateType, bo.targetStateType);
         toAron(dto.inputParameters, bo.inputParameters);
         toAron(dto.localParameters, bo.localParameters);
-- 
GitLab