From 375552834e936314fb31f8cb508a3caa0b043316 Mon Sep 17 00:00:00 2001
From: phesch <ulila@student.kit.edu>
Date: Thu, 28 Apr 2022 22:07:42 +0200
Subject: [PATCH] Implement improvements to prediction interface

---
 .../server/ExampleMemory/ExampleMemory.cpp    | 22 +++---
 .../RobotAPI/interface/armem/prediction.ice   | 12 ++-
 .../libraries/armem/client/Prediction.cpp     | 37 +++++++--
 .../libraries/armem/client/Prediction.h       | 21 ++++-
 .../libraries/armem/client/Reader.cpp         | 79 ++++++++++---------
 .../RobotAPI/libraries/armem/client/Reader.h  |  2 +-
 .../libraries/armem/core/error/ArMemError.cpp | 17 ++++
 .../libraries/armem/core/error/ArMemError.h   | 12 ++-
 .../server/plugins/ReadWritePluginUser.cpp    |  7 +-
 9 files changed, 143 insertions(+), 66 deletions(-)

diff --git a/source/RobotAPI/components/armem/server/ExampleMemory/ExampleMemory.cpp b/source/RobotAPI/components/armem/server/ExampleMemory/ExampleMemory.cpp
index 15efc34f0..1e8f29802 100644
--- a/source/RobotAPI/components/armem/server/ExampleMemory/ExampleMemory.cpp
+++ b/source/RobotAPI/components/armem/server/ExampleMemory/ExampleMemory.cpp
@@ -26,6 +26,7 @@
 
 #include <SimoxUtility/algorithm/string.h>
 
+#include <RobotAPI/libraries/armem/client/Prediction.h>
 #include <RobotAPI/libraries/armem/client/query/Builder.h>
 #include <RobotAPI/libraries/armem/core/error.h>
 #include <RobotAPI/libraries/armem/core/ice_conversions_templates.h>
@@ -226,12 +227,12 @@ namespace armarx
     armem::prediction::data::PredictionResult
     ExampleMemory::predictSingle(const armem::prediction::data::PredictionRequest& request)
     {
-        armem::prediction::data::PredictionResult result;
+        armem::client::PredictionResult result;
 
-        std::string engine = request.settings.predictionEngine;
+        std::string engine = request.settings.predictionEngineID;
         if (engine.empty() || engine == "Latest")
         {
-            auto boID = fromIce<armem::MemoryID>(request.memoryID);
+            auto boID = fromIce<armem::MemoryID>(request.snapshotID);
             armem::client::QueryBuilder builder;
             builder.latestEntitySnapshot(boID);
             auto queryResult =
@@ -246,34 +247,35 @@ namespace armarx
                                         ? latest->getInstance(boID)
                                         : latest->getInstance(latest->getInstanceIndices().at(0));
                     result.success = true;
-                    result.prediction = instance.data()->toAronDictDTO();
+                    result.snapshotID = boID;
+                    result.prediction = instance.data();
                 }
                 else
                 {
                     result.success = false;
-                    result.errorMessage =
-                        "Could not find entity referenced by MemoryID " + boID.str();
+                    result.errorMessage <<
+                        "Could not find entity referenced by MemoryID " << boID;
                 }
             }
             else
             {
                 result.success = false;
-                result.errorMessage = "Could not find entity referenced by MemoryID " + boID.str();
+                result.errorMessage << "Could not find entity referenced by MemoryID " << boID;
             }
         }
         else
         {
             result.success = false;
-            result.errorMessage = "This memory does not support the prediction engine " + engine;
+            result.errorMessage << "This memory does not support the prediction engine \"" << engine << "\"";
         }
 
-        return result;
+        return result.toIce();
     }
 
     armem::prediction::data::PredictionEngineSeq
     ExampleMemory::getAvailableEngines()
     {
-        return {"Latest"};
+        return {{"Latest"}};
     }
 
     // REMOTE GUI
diff --git a/source/RobotAPI/interface/armem/prediction.ice b/source/RobotAPI/interface/armem/prediction.ice
index b4f0efabe..f42535928 100644
--- a/source/RobotAPI/interface/armem/prediction.ice
+++ b/source/RobotAPI/interface/armem/prediction.ice
@@ -11,19 +11,24 @@ module armarx
         {
             module data
             {
-                sequence<string> PredictionEngineSeq;
+                // Holds properties of a prediction engine identified by its engineID.
+                struct PredictionEngine
+                {
+                    string engineID;
+                }
+                sequence<PredictionEngine> PredictionEngineSeq;
 
                 // Settings to use for a given prediction (e.g. which engine to use)
                 struct PredictionSettings
                 {
-                    string predictionEngine = "";
+                    string predictionEngineID = "";
                 };
 
                 // A single request for a prediction from a memory.
                 // The timestamp to request the prediction for is in the MemoryID.
                 struct PredictionRequest
                 {
-                    armem::data::MemoryID memoryID;
+                    armem::data::MemoryID snapshotID;
                     PredictionSettings settings;
                 }
                 sequence<PredictionRequest> PredictionRequestSeq;
@@ -35,6 +40,7 @@ module armarx
                     bool success = false;
                     string errorMessage;
 
+                    armem::data::MemoryID snapshotID;
                     aron::data::dto::Dict prediction;
                 }
                 sequence<PredictionResult> PredictionResultSeq;
diff --git a/source/RobotAPI/libraries/armem/client/Prediction.cpp b/source/RobotAPI/libraries/armem/client/Prediction.cpp
index fa7429f2c..f58649a45 100644
--- a/source/RobotAPI/libraries/armem/client/Prediction.cpp
+++ b/source/RobotAPI/libraries/armem/client/Prediction.cpp
@@ -28,6 +28,17 @@
 
 namespace armarx::armem::client
 {
+    PredictionEngine
+    PredictionEngine::fromIce(const armem::prediction::data::PredictionEngine& ice)
+    {
+        return armarx::fromIce<PredictionEngine>(ice);
+    }
+
+    armem::prediction::data::PredictionEngine
+    PredictionEngine::toIce() const
+    {
+        return armarx::toIce<armem::prediction::data::PredictionEngine>(*this);
+    }
 
     PredictionSettings
     PredictionSettings::fromIce(const armem::prediction::data::PredictionSettings& ice)
@@ -65,29 +76,41 @@ namespace armarx::armem::client
         return armarx::toIce<armem::prediction::data::PredictionResult>(*this);
     }
 
+    void
+    toIce(armem::prediction::data::PredictionEngine& ice, const PredictionEngine& engine)
+    {
+        ice.engineID = engine.engineID;
+    }
+
+    void
+    fromIce(const armem::prediction::data::PredictionEngine& ice, PredictionEngine& engine)
+    {
+        engine.engineID = ice.engineID;
+    }
+
     void
     toIce(armem::prediction::data::PredictionSettings& ice, const PredictionSettings& settings)
     {
-        ice.predictionEngine = settings.predictionEngine;
+        ice.predictionEngineID = settings.predictionEngineID;
     }
 
     void
     fromIce(const armem::prediction::data::PredictionSettings& ice, PredictionSettings& settings)
     {
-        settings.predictionEngine = ice.predictionEngine;
+        settings.predictionEngineID = ice.predictionEngineID;
     }
 
     void
     toIce(armem::prediction::data::PredictionRequest& ice, const PredictionRequest& request)
     {
-        toIce(ice.memoryID, request.memoryID);
+        toIce(ice.snapshotID, request.snapshotID);
         toIce(ice.settings, request.predictionSettings);
     }
 
     void
     fromIce(const armem::prediction::data::PredictionRequest& ice, PredictionRequest& request)
     {
-        fromIce(ice.memoryID, request.memoryID);
+        fromIce(ice.snapshotID, request.snapshotID);
         fromIce(ice.settings, request.predictionSettings);
     }
 
@@ -95,7 +118,8 @@ namespace armarx::armem::client
     toIce(armem::prediction::data::PredictionResult& ice, const PredictionResult& result)
     {
         ice.success = result.success;
-        ice.errorMessage = result.errorMessage;
+        ice.errorMessage = result.errorMessage.str();
+        toIce(ice.snapshotID, result.snapshotID);
         ice.prediction = result.prediction->toAronDictDTO();
     }
 
@@ -103,7 +127,8 @@ namespace armarx::armem::client
     fromIce(const armem::prediction::data::PredictionResult& ice, PredictionResult& result)
     {
         result.success = ice.success;
-        result.errorMessage = ice.errorMessage;
+        result.errorMessage.str(ice.errorMessage);
+        fromIce(ice.snapshotID, result.snapshotID);
         result.prediction = aron::data::Dict::FromAronDictDTO(ice.prediction);
     }
 
diff --git a/source/RobotAPI/libraries/armem/client/Prediction.h b/source/RobotAPI/libraries/armem/client/Prediction.h
index 840891141..a15387585 100644
--- a/source/RobotAPI/libraries/armem/client/Prediction.h
+++ b/source/RobotAPI/libraries/armem/client/Prediction.h
@@ -28,9 +28,18 @@
 
 namespace armarx::armem::client
 {
+
+    struct PredictionEngine
+    {
+        std::string engineID;
+
+        static PredictionEngine fromIce(const armem::prediction::data::PredictionEngine& ice);
+        armem::prediction::data::PredictionEngine toIce() const;
+    };
+
     struct PredictionSettings
     {
-        std::string predictionEngine;
+        std::string predictionEngineID;
 
         static PredictionSettings fromIce(const armem::prediction::data::PredictionSettings& ice);
         armem::prediction::data::PredictionSettings toIce() const;
@@ -38,7 +47,7 @@ namespace armarx::armem::client
 
     struct PredictionRequest
     {
-        armem::MemoryID memoryID;
+        armem::MemoryID snapshotID;
         PredictionSettings predictionSettings;
 
         static PredictionRequest fromIce(const armem::prediction::data::PredictionRequest& ice);
@@ -48,14 +57,20 @@ namespace armarx::armem::client
     struct PredictionResult
     {
         bool success;
-        std::string errorMessage;
+        std::stringstream errorMessage;
 
+        armem::MemoryID snapshotID;
         aron::data::DictPtr prediction;
 
         static PredictionResult fromIce(const armem::prediction::data::PredictionResult& ice);
         armem::prediction::data::PredictionResult toIce() const;
     };
 
+    void toIce(armem::prediction::data::PredictionEngine& ice,
+               const PredictionEngine& engine);
+    void fromIce(const armem::prediction::data::PredictionEngine& ice,
+                 PredictionEngine& engine);
+
     void toIce(armem::prediction::data::PredictionSettings& ice,
                const PredictionSettings& settings);
     void fromIce(const armem::prediction::data::PredictionSettings& ice,
diff --git a/source/RobotAPI/libraries/armem/client/Reader.cpp b/source/RobotAPI/libraries/armem/client/Reader.cpp
index 0cd573bd4..52be34eb4 100644
--- a/source/RobotAPI/libraries/armem/client/Reader.cpp
+++ b/source/RobotAPI/libraries/armem/client/Reader.cpp
@@ -3,11 +3,11 @@
 #include <sstream>
 
 #include <ArmarXCore/core/logging/Logging.h>
+#include <ArmarXCore/core/ice_conversions/ice_conversions_templates.h>
 
 #include <RobotAPI/libraries/armem/aron/MemoryLink.aron.generated.h>
 #include <RobotAPI/libraries/armem/core/MemoryID_operators.h>
 #include <RobotAPI/libraries/armem/core/aron_conversions.h>
-#include <RobotAPI/libraries/armem/core/wm/ice_conversions.h>
 #include <RobotAPI/libraries/armem/core/wm/memory_definitions.h>
 #include <RobotAPI/libraries/armem/util/util.h>
 #include <RobotAPI/libraries/aron/common/util/object_finders.h>
@@ -37,30 +37,22 @@ namespace armarx::armem::client
     armem::query::data::Result
     Reader::query(const armem::query::data::Input& input) const
     {
-        auto makeErrorMsg = [](auto e)
+        armem::query::data::Result result;
+        if (!readingPrx)
         {
-            std::stringstream ss;
-            ss << "Memory query failed.\nReason: " << e.what();
-            return ss.str();
-        };
+            throw error::ProxyNotSet("ReadingMemoryInterfacePrx",
+                                     "Reading interface proxy must be set to perform a query.");
+        }
 
-        armem::query::data::Result result;
-        ARMARX_CHECK_NOT_NULL(readingPrx);
         try
         {
             result = readingPrx->query(input);
         }
-        catch (const Ice::ConnectionRefusedException& e)
-        {
-            result.errorMessage = makeErrorMsg(e);
-        }
-        catch (const Ice::ConnectionLostException& e)
-        {
-            result.errorMessage = makeErrorMsg(e);
-        }
-        catch (const Ice::NotRegisteredException& e)
+        catch (const Ice::LocalException& e)
         {
-            result.errorMessage = makeErrorMsg(e);
+            std::stringstream sstream;
+            sstream << "Memory query failed.\nReason: " << e.what();
+            result.errorMessage = sstream.str();
         }
         return result;
     }
@@ -99,15 +91,13 @@ namespace armarx::armem::client
             armem::client::MemoryNameSystem& mns,
             int recursionDepth) const
     {
-        auto makeErrorMsg = [](auto error)
+        if (!readingPrx)
         {
-            std::stringstream sstream;
-            sstream << "Memory query failed.\nReason: " << error.what();
-            return sstream.str();
-        };
+            throw error::ProxyNotSet("ReadingMemoryInterfacePrx",
+                                     "Reading interface proxy must be set to perform a query.");
+        }
 
         armem::query::data::Result result;
-        ARMARX_CHECK_NOT_NULL(readingPrx);
         try
         {
             result = readingPrx->query(input);
@@ -117,17 +107,11 @@ namespace armarx::armem::client
                 { resolveMemoryLinks(instance, bObj.memory, mns, recursionDepth); });
             result = bObj.toIce();
         }
-        catch (const Ice::ConnectionRefusedException& e)
-        {
-            result.errorMessage = makeErrorMsg(e);
-        }
-        catch (const Ice::ConnectionLostException& e)
-        {
-            result.errorMessage = makeErrorMsg(e);
-        }
-        catch (const Ice::NotRegisteredException& e)
+        catch (const Ice::LocalException& e)
         {
-            result.errorMessage = makeErrorMsg(e);
+            std::stringstream sstream;
+            sstream << "Memory query failed.\nReason: " << e.what();
+            result.errorMessage = sstream.str();
         }
         catch (const exceptions::local::ExpressionException& e)
         {
@@ -454,7 +438,12 @@ namespace armarx::armem::client
     std::vector<PredictionResult>
     Reader::predict(const std::vector<PredictionRequest>& requests) const
     {
-        ARMARX_CHECK_NOT_NULL(predictionPrx);
+        if (!predictionPrx)
+        {
+            throw error::ProxyNotSet(
+                "PredictingMemoryInterfacePrx",
+                "Prediction interface proxy must be set to request a prediction.");
+        }
 
         armem::prediction::data::PredictionRequestSeq iceRequests;
         for (const auto& request : requests)
@@ -490,12 +479,17 @@ namespace armarx::armem::client
         return boResults;
     }
 
-    std::vector<std::string>
+    std::vector<PredictionEngine>
     Reader::getAvailablePredictionEngines() const
     {
-        ARMARX_CHECK_NOT_NULL(predictionPrx);
+        if (!predictionPrx)
+        {
+            throw error::ProxyNotSet(
+                "PredictingMemoryInterfacePrx",
+                "Prediction interface proxy must be set to request a prediction.");
+        }
 
-        std::vector<std::string> engines;
+        std::vector<armem::prediction::data::PredictionEngine> engines;
         try
         {
             engines = predictionPrx->getAvailableEngines();
@@ -505,6 +499,13 @@ namespace armarx::armem::client
             // Just leave engines empty in this case.
         }
 
-        return engines;
+        std::vector<PredictionEngine> boEngines;
+        boEngines.reserve(engines.size());
+        for (const auto& engine : engines)
+        {
+            boEngines.push_back(armarx::fromIce<PredictionEngine>(engine));
+        }
+
+        return boEngines;
     }
 } // namespace armarx::armem::client
diff --git a/source/RobotAPI/libraries/armem/client/Reader.h b/source/RobotAPI/libraries/armem/client/Reader.h
index 2e5825a5e..beb0e53f5 100644
--- a/source/RobotAPI/libraries/armem/client/Reader.h
+++ b/source/RobotAPI/libraries/armem/client/Reader.h
@@ -185,7 +185,7 @@ namespace armarx::armem::client
          *
          * @return the vector of supported prediction engines
          */
-        std::vector<std::string> getAvailablePredictionEngines() const;
+        std::vector<PredictionEngine> getAvailablePredictionEngines() const;
 
         inline operator bool() const
         {
diff --git a/source/RobotAPI/libraries/armem/core/error/ArMemError.cpp b/source/RobotAPI/libraries/armem/core/error/ArMemError.cpp
index ac8be17b5..87b687447 100644
--- a/source/RobotAPI/libraries/armem/core/error/ArMemError.cpp
+++ b/source/RobotAPI/libraries/armem/core/error/ArMemError.cpp
@@ -220,4 +220,21 @@ namespace armarx::armem::error
         return ss.str();
     }
 
+    ProxyNotSet::ProxyNotSet(const std::string& proxyName, const std::string& message) :
+        ArMemError(makeMsg(proxyName, message))
+    {
+    }
+
+    std::string
+    ProxyNotSet::makeMsg(const std::string& proxyName, const std::string& message)
+    {
+        std::stringstream sstream;
+        sstream << "Proxy \"" << proxyName << "\" not set.";
+        if (!message.empty())
+        {
+            sstream << "\n" << message;
+        }
+        return sstream.str();
+    }
+
 } // namespace armarx::armem::error
diff --git a/source/RobotAPI/libraries/armem/core/error/ArMemError.h b/source/RobotAPI/libraries/armem/core/error/ArMemError.h
index 5678ee1a9..6f5be02a2 100644
--- a/source/RobotAPI/libraries/armem/core/error/ArMemError.h
+++ b/source/RobotAPI/libraries/armem/core/error/ArMemError.h
@@ -203,5 +203,15 @@ namespace armarx::armem::error
 
     };
 
-}
+    /**
+     * @brief Indicates that a proxy required for an operation wasn't usable.
+     */
+    class ProxyNotSet : public ArMemError
+    {
+    public:
+        ProxyNotSet(const std::string& proxyName, const std::string& message = "");
+
+        static std::string makeMsg(const std::string& proxyName, const std::string& message = "");
+    };
 
+} // namespace armarx::armem::error
diff --git a/source/RobotAPI/libraries/armem/server/plugins/ReadWritePluginUser.cpp b/source/RobotAPI/libraries/armem/server/plugins/ReadWritePluginUser.cpp
index 45b08dfd3..115e38311 100644
--- a/source/RobotAPI/libraries/armem/server/plugins/ReadWritePluginUser.cpp
+++ b/source/RobotAPI/libraries/armem/server/plugins/ReadWritePluginUser.cpp
@@ -1,6 +1,7 @@
 #include "ReadWritePluginUser.h"
 #include "Plugin.h"
 
+#include <RobotAPI/libraries/armem/client/Prediction.h>
 #include <RobotAPI/libraries/armem/core/error.h>
 #include <RobotAPI/libraries/armem/server/MemoryToIceAdapter.h>
 
@@ -124,11 +125,11 @@ namespace armarx::armem::server::plugins
         armem::prediction::data::PredictionResultSeq result;
         for (auto request : requests)
         {
-            armem::prediction::data::PredictionResult singleResult;
+            armem::client::PredictionResult singleResult;
             singleResult.success = false;
-            singleResult.errorMessage = "This memory does not implement predictions.";
+            singleResult.errorMessage << "This memory does not implement predictions.";
             singleResult.prediction = nullptr;
-            result.push_back(singleResult);
+            result.push_back(singleResult.toIce());
         }
         return result;
     }
-- 
GitLab