Merge branch 'skills/fixes' into 'master'

Minor fixes and improvements in skills

See merge request !398

source/RobotAPI/libraries/armem/client/util/SimpleReaderBase.cpp

@@ -17,21 +17,25 @@ namespace armarx::armem::client::util
 
         const std::string prefix = propertyPrefix();
 
-        props = defaultProperties();
+        if (not props.has_value())
+        {
+            props = defaultProperties();
+        }
 
-        def->optional(props.memoryName, prefix + "Memory");
-        def->optional(props.coreSegmentName, prefix + "CoreSegment");
+        def->optional(props->memoryName, prefix + "Memory");
+        def->optional(props->coreSegmentName, prefix + "CoreSegment");
     }
 
     void
     SimpleReaderBase::connect(armarx::armem::client::MemoryNameSystem& mns)
     {
         // Wait for the memory to become available and add it as dependency.
-        ARMARX_IMPORTANT << "SimpleReaderBase: Waiting for memory '" << props.memoryName << "' ...";
+        ARMARX_IMPORTANT << "SimpleReaderBase: Waiting for memory '" << properties().memoryName
+                         << "' ...";
         try
         {
-            memoryReaderClient = mns.useReader(MemoryID().withMemoryName(props.memoryName));
-            ARMARX_IMPORTANT << "SimpleReaderBase: Connected to memory '" << props.memoryName
+            memoryReaderClient = mns.useReader(MemoryID().withMemoryName(properties().memoryName));
+            ARMARX_IMPORTANT << "SimpleReaderBase: Connected to memory '" << properties().memoryName
                              << "'";
         }
         catch (const armem::error::CouldNotResolveMemoryServer& e)
@@ -41,12 +45,6 @@ namespace armarx::armem::client::util
         }
     }
 
-    std::mutex&
-    SimpleReaderBase::memoryReaderMutex()
-    {
-        return memoryMutex;
-    }
-
     const armem::client::Reader&
     SimpleReaderBase::memoryReader() const
     {
@@ -56,7 +54,17 @@ namespace armarx::armem::client::util
     const SimpleReaderBase::Properties&
     SimpleReaderBase::properties() const
     {
-        return props;
+        if (not props.has_value())
+        {
+            const_cast<std::optional<Properties>&>(props) = defaultProperties();
+        }
+        return props.value();
+    }
+
+    void
+    SimpleReaderBase::setProperties(const Properties& p)
+    {
+        props = p;
     }
 
 } // namespace armarx::armem::client::util

source/RobotAPI/libraries/armem/client/util/SimpleReaderBase.h

@@ -21,7 +21,7 @@
 
 #pragma once
 
-#include <mutex>
+#include <optional>
 
 #include <ArmarXCore/core/application/properties/PropertyDefinitionContainer.h>
 
@@ -53,25 +53,18 @@ namespace armarx::armem::client::util
         virtual void connect(armarx::armem::client::MemoryNameSystem& mns);
 
         const Properties& properties() const;
-
-        void
-        setProperties(const Properties& p)
-        {
-            props = p;
-        }
+        void setProperties(const Properties& p);
 
     protected:
         virtual std::string propertyPrefix() const = 0;
         virtual Properties defaultProperties() const = 0;
 
-        std::mutex& memoryReaderMutex();
         const armem::client::Reader& memoryReader() const;
 
     private:
-        Properties props;
+        std::optional<Properties> props;
 
         armem::client::Reader memoryReaderClient;
-        std::mutex memoryMutex;
     };
 
 } // namespace armarx::armem::client::util

source/RobotAPI/libraries/aron/core/data/variant/container/Dict.cpp

@@ -261,9 +261,8 @@ namespace armarx::aron::data
             const auto& p = data->getPath();
             if (not p.hasDirectPrefix(this->getPath()))
             {
-                ARMARX_WARNING
-                    << "An element added to a dict does not have a correct path set. This "
-                       "may cause errors. Please use setElemetCopy() instead.";
+                ARMARX_DEBUG << "An element added to a dict does not have a correct path set. This "
+                                "may cause errors. Please use setElemetCopy() instead.";
             }
         }
 

source/RobotAPI/libraries/skills/core/Skill.cpp

@@ -245,7 +245,7 @@ namespace armarx
                     std::string("The skill '" + getSkillId().toString() + "' was asked to stop.");
                 message += abortedMessage.empty() ? "" : " Additional message: " + abortedMessage;
 
-                throw error::SkillAbortedException(message);
+                throw error::SkillAbortedException(__PRETTY_FUNCTION__, message);
                 return;
             }
 
@@ -256,7 +256,7 @@ namespace armarx
                 message += abortedMessage.empty() ? "" : " Additional message: " + abortedMessage;
 
                 ARMARX_WARNING << message;
-                throw error::SkillFailedException(message);
+                throw error::SkillFailedException(__PRETTY_FUNCTION__, message);
             }
         }
 

source/RobotAPI/libraries/skills/core/SkillProxy.cpp

@@ -12,9 +12,19 @@ namespace armarx
             manager(manager)
         {
             ARMARX_CHECK_NOT_NULL(manager);
-            skillDescription = SkillDescription::FromIce(
-                manager->getSkillDescription(skillId.toManagerIce()).value());
-            ARMARX_CHECK(skillDescription.skillId.isFullySpecified());
+            IceUtil::Optional<manager::dto::SkillDescription> description =
+                manager->getSkillDescription(skillId.toManagerIce());
+            if (description)
+            {
+                skillDescription = SkillDescription::FromIce(description.value());
+                ARMARX_CHECK(skillDescription.skillId.isFullySpecified());
+            }
+            else
+            {
+                std::stringstream reason;
+                reason << "No skill with ID " << skillId << " found";
+                throw error::SkillNotFoundException(__PRETTY_FUNCTION__, reason.str());
+            }
         }
 
         SkillProxy::SkillProxy(const manager::dti::SkillManagerInterfacePrx& manager,

source/RobotAPI/libraries/skills/core/error/Exception.h

@@ -48,32 +48,38 @@ namespace armarx::skills::error
         }
     };
 
-    class SkillAbortedException : public armarx::LocalException
+    /**
+     * @brief Indicates that a skill was not found, e.g., by the skill manager.
+     */
+    class SkillNotFoundException : public SkillException
     {
     public:
-        SkillAbortedException() = delete;
+        SkillNotFoundException() = delete;
 
-        SkillAbortedException(const std::string& reason) : LocalException(reason)
+        SkillNotFoundException(const std::string& prettymethod, const std::string& reason) :
+            SkillException(prettymethod, reason)
         {
         }
+    };
+
+    class SkillAbortedException : public SkillException
+    {
+    public:
+        SkillAbortedException() = delete;
 
         SkillAbortedException(const std::string& prettymethod, const std::string& reason) :
-            LocalException(prettymethod + ": " + reason + ".")
+            SkillException(prettymethod, reason)
         {
         }
     };
 
-    class SkillFailedException : public armarx::LocalException
+    class SkillFailedException : public SkillException
     {
     public:
         SkillFailedException() = delete;
 
-        SkillFailedException(const std::string& reason) : LocalException(reason)
-        {
-        }
-
         SkillFailedException(const std::string& prettymethod, const std::string& reason) :
-            LocalException(prettymethod + ": " + reason + ".")
+            SkillException(prettymethod, reason)
         {
         }
     };