From e1d6dc086ae47b7b534792af68720f876ce49de0 Mon Sep 17 00:00:00 2001
From: Rainer Kartmann <rainer.kartmann@kit.edu>
Date: Fri, 23 Apr 2021 18:04:01 +0200
Subject: [PATCH] Hookup class segment's remote gui. Add locks

---
 .../server/ObjectMemory/ObjectMemory.cpp      | 11 ++++++--
 .../armem/server/ObjectMemory/ObjectMemory.h  |  1 +
 .../armem_objects/server/class/Segment.cpp    | 25 ++++++-------------
 .../armem_objects/server/class/Segment.h      | 15 ++++-------
 .../armem_objects/server/instance/Segment.cpp | 23 +++++++++++------
 .../armem_objects/server/instance/Segment.h   |  5 ++--
 .../server/instance/SegmentAdapter.cpp        |  2 +-
 7 files changed, 42 insertions(+), 40 deletions(-)

diff --git a/source/RobotAPI/components/armem/server/ObjectMemory/ObjectMemory.cpp b/source/RobotAPI/components/armem/server/ObjectMemory/ObjectMemory.cpp
index 781eed48e..8df7556a9 100644
--- a/source/RobotAPI/components/armem/server/ObjectMemory/ObjectMemory.cpp
+++ b/source/RobotAPI/components/armem/server/ObjectMemory/ObjectMemory.cpp
@@ -58,7 +58,7 @@ namespace armarx::armem::server::obj
     ObjectMemory::ObjectMemory() :
         server::ComponentPluginUser(),
         instance::SegmentAdapter(server::ComponentPluginUser::iceMemory, server::ComponentPluginUser::memoryMutex),
-        classSegment(server::ComponentPluginUser::iceMemory)
+        classSegment(server::ComponentPluginUser::iceMemory, server::ComponentPluginUser::memoryMutex)
     {
     }
 
@@ -128,10 +128,16 @@ namespace armarx::armem::server::obj
         using namespace armarx::RemoteGui::Client;
 
         tab.instance.setup(*this);
+        tab.clazz.setup(classSegment);
 
-        VBoxLayout root =
+        HBoxLayout segments =
         {
             tab.instance.group,
+            tab.clazz.group
+        };
+        VBoxLayout root =
+        {
+            segments,
             VSpacer()
         };
         RemoteGui_createTab(Component::getName(), root, &tab);
@@ -141,6 +147,7 @@ namespace armarx::armem::server::obj
     {
         // Non-atomic variables need to be guarded by a mutex if accessed by multiple threads
         tab.instance.update(*this);
+        tab.clazz.update(classSegment);
     }
 
 }
diff --git a/source/RobotAPI/components/armem/server/ObjectMemory/ObjectMemory.h b/source/RobotAPI/components/armem/server/ObjectMemory/ObjectMemory.h
index 0a53588c1..9a3d65ac7 100644
--- a/source/RobotAPI/components/armem/server/ObjectMemory/ObjectMemory.h
+++ b/source/RobotAPI/components/armem/server/ObjectMemory/ObjectMemory.h
@@ -115,6 +115,7 @@ namespace armarx::armem::server::obj
         struct RemoteGuiTab : armarx::RemoteGui::Client::Tab
         {
             instance::SegmentAdapter::RemoteGui instance;
+            clazz::Segment::RemoteGui clazz;
         };
         RemoteGuiTab tab;
 
diff --git a/source/RobotAPI/libraries/armem_objects/server/class/Segment.cpp b/source/RobotAPI/libraries/armem_objects/server/class/Segment.cpp
index e4d46ae5e..1a42f6968 100644
--- a/source/RobotAPI/libraries/armem_objects/server/class/Segment.cpp
+++ b/source/RobotAPI/libraries/armem_objects/server/class/Segment.cpp
@@ -16,8 +16,9 @@
 namespace armarx::armem::server::obj::clazz
 {
 
-    Segment::Segment(armem::server::MemoryToIceAdapter& memoryToIceAdapter) :
-        iceMemory(memoryToIceAdapter)
+    Segment::Segment(armem::server::MemoryToIceAdapter& memoryToIceAdapter, std::mutex& memoryMutex) :
+        iceMemory(memoryToIceAdapter),
+        memoryMutex(memoryMutex)
     {
         Logging::setTag("ClassSegment");
     }
@@ -46,18 +47,6 @@ namespace armarx::armem::server::obj::clazz
     }
 
 
-    armem::CoreSegment& Segment::getCoreSegment()
-    {
-        ARMARX_CHECK_NOT_NULL(coreSegment);
-        return *coreSegment;
-    }
-
-    const armem::CoreSegment& Segment::getCoreSegment() const
-    {
-        ARMARX_CHECK_NOT_NULL(coreSegment);
-        return *coreSegment;
-    }
-
     void Segment::loadByObjectFinder(const std::string& objectsPackage)
     {
         loadByObjectFinder(ObjectFinder(objectsPackage));
@@ -101,7 +90,7 @@ namespace armarx::armem::server::obj::clazz
         iceMemory.commit(commit);
     }
 
-    arondto::ObjectClass Segment::objectClassFromInfo(const ObjectInfo& info) const
+    arondto::ObjectClass Segment::objectClassFromInfo(const ObjectInfo& info)
     {
         namespace fs = std::filesystem;
 
@@ -155,14 +144,16 @@ namespace armarx::armem::server::obj::clazz
         grid.add(Label("Infinite History Size"), {row, 0}).add(infiniteHistory, {row, 1});
         row++;
 
-        group.setLabel("Data");
-        group.addChild(grid);
+        group = {};
+        group.setLabel("Class");
+        group.addChildren({grid, VSpacer()});
     }
 
     void Segment::RemoteGui::update(Segment& data)
     {
         if (infiniteHistory.hasValueChanged() || maxHistorySize.hasValueChanged())
         {
+            std::scoped_lock lock(data.memoryMutex);
             data.p.maxHistorySize = infiniteHistory.getValue() ? -1 : maxHistorySize.getValue();
             if (data.coreSegment)
             {
diff --git a/source/RobotAPI/libraries/armem_objects/server/class/Segment.h b/source/RobotAPI/libraries/armem_objects/server/class/Segment.h
index 851148e13..6ba5409cd 100644
--- a/source/RobotAPI/libraries/armem_objects/server/class/Segment.h
+++ b/source/RobotAPI/libraries/armem_objects/server/class/Segment.h
@@ -1,5 +1,6 @@
 #pragma once
 
+#include <mutex>
 #include <string>
 
 #include <ArmarXCore/core/logging/Logging.h>
@@ -22,10 +23,8 @@ namespace armarx::armem::server::obj::clazz
     {
     public:
 
-
-    public:
-
-        Segment(armem::server::MemoryToIceAdapter& iceMemory);
+        Segment(armem::server::MemoryToIceAdapter& iceMemory,
+                std::mutex& memoryMutex);
 
 
         void defineProperties(armarx::PropertyDefinitionsPtr defs, const std::string& prefix = "");
@@ -35,18 +34,14 @@ namespace armarx::armem::server::obj::clazz
         void loadByObjectFinder(const ObjectFinder& finder);
         void loadByObjectFinder();
 
-
-        armem::CoreSegment& getCoreSegment();
-        const armem::CoreSegment& getCoreSegment() const;
-
-
-        arondto::ObjectClass objectClassFromInfo(const ObjectInfo& info) const;
+        static arondto::ObjectClass objectClassFromInfo(const ObjectInfo& info);
 
 
     private:
 
         armem::server::MemoryToIceAdapter& iceMemory;
         armem::CoreSegment* coreSegment = nullptr;
+        std::mutex& memoryMutex;
 
         ObjectFinder objectFinder;
 
diff --git a/source/RobotAPI/libraries/armem_objects/server/instance/Segment.cpp b/source/RobotAPI/libraries/armem_objects/server/instance/Segment.cpp
index d831d5dee..6283f6aa0 100644
--- a/source/RobotAPI/libraries/armem_objects/server/instance/Segment.cpp
+++ b/source/RobotAPI/libraries/armem_objects/server/instance/Segment.cpp
@@ -24,8 +24,9 @@
 namespace armarx::armem::server::obj::instance
 {
 
-    Segment::Segment(armem::server::MemoryToIceAdapter& memoryToIceAdapter) :
-        iceMemory(memoryToIceAdapter)
+    Segment::Segment(armem::server::MemoryToIceAdapter& memoryToIceAdapter, std::mutex& memoryMutex) :
+        iceMemory(memoryToIceAdapter),
+        memoryMutex(memoryMutex)
     {
         Logging::setTag("InstanceSegment");
 
@@ -691,16 +692,22 @@ namespace armarx::armem::server::obj::instance
 
     void Segment::RemoteGui::update(Segment& data)
     {
-        if (infiniteHistory.hasValueChanged() || maxHistorySize.hasValueChanged())
+        if (infiniteHistory.hasValueChanged() || maxHistorySize.hasValueChanged()
+            || discardSnapshotsWhileAttached.hasValueChanged())
         {
-            data.p.maxHistorySize = infiniteHistory.getValue() ? -1 : maxHistorySize.getValue();
-            if (data.coreSegment)
+            std::scoped_lock lock(data.memoryMutex);
+
+            if (infiniteHistory.hasValueChanged() || maxHistorySize.hasValueChanged())
             {
-                data.coreSegment->setMaxHistorySize(long(data.p.maxHistorySize));
+                data.p.maxHistorySize = infiniteHistory.getValue() ? -1 : maxHistorySize.getValue();
+                if (data.coreSegment)
+                {
+                    data.coreSegment->setMaxHistorySize(long(data.p.maxHistorySize));
+                }
             }
-        }
 
-        data.p.discardSnapshotsWhileAttached = discardSnapshotsWhileAttached.getValue();
+            data.p.discardSnapshotsWhileAttached = discardSnapshotsWhileAttached.getValue();
+        }
     }
 
 }
diff --git a/source/RobotAPI/libraries/armem_objects/server/instance/Segment.h b/source/RobotAPI/libraries/armem_objects/server/instance/Segment.h
index 4a1e69742..5e82e2ac4 100644
--- a/source/RobotAPI/libraries/armem_objects/server/instance/Segment.h
+++ b/source/RobotAPI/libraries/armem_objects/server/instance/Segment.h
@@ -40,7 +40,8 @@ namespace armarx::armem::server::obj::instance
 
     public:
 
-        Segment(armem::server::MemoryToIceAdapter& iceMemory);
+        Segment(armem::server::MemoryToIceAdapter& iceMemory,
+                std::mutex& memoryMutex);
 
 
         void defineProperties(armarx::PropertyDefinitionsPtr defs, const std::string& prefix = "");
@@ -151,8 +152,8 @@ namespace armarx::armem::server::obj::instance
     private:
 
         armem::server::MemoryToIceAdapter& iceMemory;
-
         armem::CoreSegment* coreSegment = nullptr;
+        std::mutex& memoryMutex;
 
 
         struct Properties
diff --git a/source/RobotAPI/libraries/armem_objects/server/instance/SegmentAdapter.cpp b/source/RobotAPI/libraries/armem_objects/server/instance/SegmentAdapter.cpp
index d44f5dfa0..f570ca9b4 100644
--- a/source/RobotAPI/libraries/armem_objects/server/instance/SegmentAdapter.cpp
+++ b/source/RobotAPI/libraries/armem_objects/server/instance/SegmentAdapter.cpp
@@ -38,7 +38,7 @@ namespace armarx::armem::server::obj::instance
 {
 
     SegmentAdapter::SegmentAdapter(MemoryToIceAdapter& iceMemory, std::mutex& memoryMutex) :
-        segment(iceMemory),
+        segment(iceMemory, memoryMutex),
         memoryMutex(memoryMutex)
     {
     }
-- 
GitLab