From 32589732cfa4e5a1e57c599504ae21a3ec5cbe37 Mon Sep 17 00:00:00 2001
From: Rainer Kartmann <rainer.kartmann@kit.edu>
Date: Mon, 16 Aug 2021 12:50:25 +0200
Subject: [PATCH] Make storing and loading from disk nicer

---
 .../libraries/armem_gui/MemoryViewer.cpp      | 135 ++++--------
 .../libraries/armem_gui/MemoryViewer.h        |   6 +-
 .../armem_gui/disk/ControlWidget.cpp          | 200 ++++++++++++++----
 .../libraries/armem_gui/disk/ControlWidget.h  |  35 +--
 4 files changed, 225 insertions(+), 151 deletions(-)

diff --git a/source/RobotAPI/libraries/armem_gui/MemoryViewer.cpp b/source/RobotAPI/libraries/armem_gui/MemoryViewer.cpp
index 1e4d307d0..d52b8c8fd 100644
--- a/source/RobotAPI/libraries/armem_gui/MemoryViewer.cpp
+++ b/source/RobotAPI/libraries/armem_gui/MemoryViewer.cpp
@@ -3,8 +3,6 @@
 #include <RobotAPI/libraries/armem/core/wm/ice_conversions.h>
 #include <RobotAPI/libraries/armem_gui/gui_utils.h>
 
-#include <RobotAPI/libraries/armem/server/ltm/disk/MemoryManager.h>
-
 #include <RobotAPI/libraries/armem/server/query_proc/wm.h>
 #include <RobotAPI/libraries/armem/server/query_proc/ltm.h>
 
@@ -18,6 +16,7 @@
 
 #include <QBoxLayout>
 #include <QCheckBox>
+#include <QClipboard>
 #include <QDialog>
 #include <QGroupBox>
 #include <QMenu>
@@ -25,7 +24,6 @@
 #include <QLayout>
 #include <QSettings>
 
-#include <filesystem>
 #include <iomanip>
 
 
@@ -48,6 +46,10 @@ namespace armarx::armem::gui
         connect(statusLabel, &QLabel::customContextMenuRequested, [statusLabel](const QPoint & pos)
         {
             QMenu menu(statusLabel);
+            menu.addAction("Copy to clipboard", [statusLabel]()
+            {
+                QApplication::clipboard()->setText(statusLabel->text());
+            });
             menu.addAction("Clear status", [statusLabel]()
             {
                 statusLabel->clear();
@@ -61,14 +63,6 @@ namespace armarx::armem::gui
         updateWidget = new armem::gui::PeriodicUpdateWidget(2.0, 60);
         updateWidgetLayout->insertWidget(0, updateWidget);
 
-        // LTM Control
-        if (diskControlWidgetLayout)
-        {
-            this->diskControlLayout = diskControlWidgetLayout;
-            diskControl = new armem::gui::disk::ControlWidget();
-            diskControlWidgetLayout->addWidget(diskControl);
-        }
-
         // Memory View
         memoryGroup = new armem::gui::MemoryGroupBox();
         armarx::gui::replaceWidget(memoryGroupBox, memoryGroup, memoryGroupBoxParentLayout);
@@ -80,11 +74,19 @@ namespace armarx::armem::gui
         this->instanceGroup->setStatusLabel(statusLabel);
         ARMARX_CHECK_NULL(instanceGroupBox);
 
+        // Disk Control
+        if (diskControlWidgetLayout)
+        {
+            this->diskControlLayout = diskControlWidgetLayout;
+            diskControl = new armem::gui::disk::ControlWidget();
+            diskControlWidgetLayout->addWidget(diskControl);
+        }
+
         // Connections
         //connect(this, &This::connected, this, &This::updateMemory);
 
-        connect(diskControl, &armem::gui::disk::ControlWidget::storeOnDisk, this, &This::storeOnDisk);
-        connect(diskControl, &armem::gui::disk::ControlWidget::loadFromDisk, this, &This::loadFromDisk);
+        connect(diskControl, &armem::gui::disk::ControlWidget::requestedStoreOnDisk, this, &This::storeOnDisk);
+        connect(diskControl, &armem::gui::disk::ControlWidget::requestedLoadFromDisk, this, &This::loadFromDisk);
 
         connect(this, &This::connected, this, &This::updateMemories);
         connect(updateWidget, &armem::gui::PeriodicUpdateWidget::update, this, &This::updateMemories);
@@ -99,9 +101,9 @@ namespace armarx::armem::gui
         connect(instanceGroup->view, &armem::gui::InstanceView::memoryIdResolutionRequested, this, &This::resolveMemoryID);
     }
 
-    void MemoryViewer::setLogTag(const std::string& tag)
+    void MemoryViewer::setLogTag(const std::string& _tag)  // Leading _ silences a warning
     {
-        Logging::setTag(tag);
+        Logging::setTag(_tag);
     }
 
     void MemoryViewer::onInit(ManagedIceObject& component)
@@ -170,7 +172,7 @@ namespace armarx::armem::gui
 
     void MemoryViewer::storeInLTM()
     {
-        TIMING_START(MemoryStore);
+        TIMING_START(MemoryStore)
 
         for (auto& [name, reader] : memoryReaders)
         {
@@ -179,87 +181,34 @@ namespace armarx::armem::gui
             reader.readAndStore(input);
         }
 
-        TIMING_END_STREAM(MemoryStore, ARMARX_VERBOSE);
+        TIMING_END_STREAM(MemoryStore, ARMARX_VERBOSE)
     }
 
-    void MemoryViewer::storeOnDisk()
+    void MemoryViewer::storeOnDisk(QString directory)
     {
-        TIMING_START(MemoryExport);
-        QString qs = diskControl->getEnteredPath();
-        std::string utf8_text = qs.toUtf8().constData();
-
-        if (not utf8_text.empty())
-        {
-            ARMARX_IMPORTANT << "Exporting all memories at '" << utf8_text << "'.";
+        TIMING_START(MemoryExport)
 
-            std::filesystem::path p(utf8_text);
-            if (std::filesystem::is_regular_file(p))
-            {
-                ARMARX_WARNING << "Could not export a memory at '" << utf8_text << "'. Skipping export.";
-                return;
-            }
+        std::string status;
+        diskControl->storeOnDisk(directory, memoryData, &status);
 
-            std::filesystem::create_directories(p);
-            for (auto& [name, reader] : memoryReaders)
-            {
-                armem::client::QueryInput input = memoryGroup->queryWidget()->queryInput();
-                armem::client::QueryResult result = reader.query(input);
-
-                // create folder
-                std::filesystem::create_directories(p / name);
-
-                armem::server::ltm::disk::MemoryManager manager;
-                manager.setName(name);
-                manager.setBasePath(p / name);
-                manager.reload();
-                manager.append(result.memory);
-            }
-        }
-        else
-        {
-            ARMARX_WARNING << "The path is empty. Could not export the memory in nirvana.";
-        }
-
-        TIMING_END_STREAM(MemoryExport, ARMARX_VERBOSE);
+        statusLabel->setText(QString::fromStdString(status));
+        TIMING_END_STREAM(MemoryExport, ARMARX_VERBOSE)
     }
 
 
-    void MemoryViewer::loadFromDisk()
+    void MemoryViewer::loadFromDisk(QString directory)
     {
-        armem::client::QueryInput input = memoryGroup->queryWidget()->queryInput();
+        std::string status;
+        std::map<std::string, wm::Memory> data =
+            diskControl->loadFromDisk(directory, memoryGroup->queryWidget()->queryInput(), &status);
 
-        QString qs = diskControl->getEnteredPath();
-        std::string utf8_text = qs.toUtf8().constData();
-        std::filesystem::path p(utf8_text);
-
-        // first check if the local file system should be queried
-        if (std::filesystem::is_directory(p))
+        for (auto& [name, memory] : data)
         {
-            for (const auto& d : std::filesystem::directory_iterator(p))
-            {
-                if (d.is_directory())
-                {
-                    std::string k = d.path().filename();
-                    armem::server::ltm::disk::MemoryManager manager;
-                    manager.setName(k);
-                    manager.setBasePath(p / k);
-
-                    manager.reload();
-
-                    input.addQueryTargetToAll(armem::query::data::QueryTarget::LTM); // We use LTM as query target for the disk
-
-                    armem::server::query_proc::ltm::MemoryQueryProcessor ltm_processor;
-                    armem::wm::Memory query_res = ltm_processor.process(input.toIce(), manager.getCacheAndLutNotConverted());
-
-                    manager.convert(query_res);
-                    memoryData[k] = std::move(query_res);
-                }
-            }
-        }
-        else
-        {
-            ARMARX_WARNING << "Could not import a memory from '" << utf8_text << "'. Skipping import.";
+            this->memoryData[name] = std::move(memory);
         }
+        statusLabel->setText(QString::fromStdString(status));
+
+        emit memoryDataChanged();
     }
 
 
@@ -274,7 +223,7 @@ namespace armarx::armem::gui
 
         for (auto& [name, reader] : memoryReaders)
         {
-            TIMING_START(MemoryQuery);
+            TIMING_START(MemoryQuery)
             {
                 armem::client::QueryResult result = reader.query(input);
                 if (result.success)
@@ -287,7 +236,7 @@ namespace armarx::armem::gui
                     errorCount++;
                 }
             }
-            TIMING_END_STREAM(MemoryQuery, ARMARX_VERBOSE);
+            TIMING_END_STREAM(MemoryQuery, ARMARX_VERBOSE)
 
             if (debugObserver)
             {
@@ -385,8 +334,7 @@ namespace armarx::armem::gui
         std::optional<wm::EntityInstance> instance;
         try
         {
-            const wm::Memory* data = getSingleMemoryData(id.memoryName);
-            if (data)
+            if (const wm::Memory* data = getSingleMemoryData(id.memoryName))
             {
                 segmentType = data->getProviderSegment(id).aronType();
 
@@ -404,13 +352,12 @@ namespace armarx::armem::gui
                 }
             }
         }
-        catch (const armem::error::ArMemError& e)
+        catch (const armem::error::ArMemError&)
         {
             // May be handled by remote lookup
-            (void) e;
         }
 
-        if (!instance)
+        if (not instance)
         {
             try
             {
@@ -444,9 +391,9 @@ namespace armarx::armem::gui
 
         // if convMap.empty(), we still need to update to remove existing entries.
 
-        TIMING_START(GuiUpdate);
+        TIMING_START(GuiUpdate)
         memoryGroup->tree()->update(convMap);
-        TIMING_END_STREAM(GuiUpdate, ARMARX_VERBOSE);
+        TIMING_END_STREAM(GuiUpdate, ARMARX_VERBOSE)
 
         if (debugObserver)
         {
diff --git a/source/RobotAPI/libraries/armem_gui/MemoryViewer.h b/source/RobotAPI/libraries/armem_gui/MemoryViewer.h
index d46dc8fe9..f2fba3438 100644
--- a/source/RobotAPI/libraries/armem_gui/MemoryViewer.h
+++ b/source/RobotAPI/libraries/armem_gui/MemoryViewer.h
@@ -70,9 +70,9 @@ namespace armarx::armem::gui
 
         void resolveMemoryID(const MemoryID& id);
 
-        // ControlWidget
-        void storeOnDisk();
-        void loadFromDisk();
+        // Disk Control
+        void storeOnDisk(QString directory);
+        void loadFromDisk(QString directory);
 
         void storeInLTM();
 
diff --git a/source/RobotAPI/libraries/armem_gui/disk/ControlWidget.cpp b/source/RobotAPI/libraries/armem_gui/disk/ControlWidget.cpp
index b7d4a163f..e36143809 100644
--- a/source/RobotAPI/libraries/armem_gui/disk/ControlWidget.cpp
+++ b/source/RobotAPI/libraries/armem_gui/disk/ControlWidget.cpp
@@ -1,10 +1,16 @@
 #include "ControlWidget.h"
 
-#include <QPushButton>
-#include <QLineEdit>
-#include <QTimer>
+#include <RobotAPI/libraries/armem/server/ltm/disk/MemoryManager.h>
+#include <RobotAPI/libraries/armem/server/query_proc/ltm.h>
+
+#include <ArmarXCore/core/exceptions/local/ExpressionException.h>
+
 #include <QHBoxLayout>
+#include <QSpacerItem>
 #include <QFileDialog>
+#include <QPushButton>
+
+#include <filesystem>
 
 #include <cmath>
 
@@ -14,60 +20,176 @@ namespace armarx::armem::gui::disk
 
     ControlWidget::ControlWidget()
     {
-        setSizePolicy(QSizePolicy::Policy::Minimum, QSizePolicy::Policy::Fixed);
+        _latestDirectory = QString::fromStdString("/tmp/MemoryExport");
 
-        auto vlayout = new QVBoxLayout();
-        auto hlayout1 = new QHBoxLayout();
-        auto hlayout2 = new QHBoxLayout();
+        _loadFromDiskButton = new QPushButton(" Load from Disk", this);
+        _loadFromDiskButton->setIcon(QIcon(":/icons/document-open.svg"));
+        _storeOnDiskButton = new QPushButton(" Store on Disk", this);
+        _storeOnDiskButton->setIcon(QIcon(":/icons/document-save.svg"));
 
-        hlayout1->setContentsMargins(10, 2, 10, 2);
-        hlayout2->setContentsMargins(10, 2, 10, 2);
+        // Allow horizontal shrinking of buttons
+        std::vector<QPushButton*> buttons { _storeOnDiskButton, _loadFromDiskButton };
+        for (QPushButton* button : buttons)
+        {
+            button->setSizePolicy(QSizePolicy::Maximum, QSizePolicy::Fixed);
+        }
 
-        const int margin = 0;
-        vlayout->setContentsMargins(margin, margin, margin, margin);
+        this->setSizePolicy(QSizePolicy::Policy::Preferred, QSizePolicy::Policy::Fixed);
 
-        _lineEdit = new QLineEdit("/tmp/MemoryExport", this);
-        //_lineEdit->setSizePolicy(QSizePolicy::Ignored, QSizePolicy::Preferred);
+        QHBoxLayout* layout = new QHBoxLayout();
+        this->setLayout(layout);
 
-        _openFileBrowserButton = new QPushButton("Search files", this);
+        const int margin = 0;
+        layout->setContentsMargins(margin, margin, margin, margin);
+
+        layout->addWidget(_loadFromDiskButton);
+        layout->addWidget(_storeOnDiskButton);
+
+
+        // Connections
+
+        connect(_loadFromDiskButton, &QPushButton::pressed, [this]()
+        {
+            QString directory = chooseDirectoryDialog();
+            if (directory.size() > 0)
+            {
+                emit requestedLoadFromDisk(directory);
+            }
+        });
+        connect(_storeOnDiskButton, &QPushButton::pressed, [this]()
+        {
+            QString directory = chooseDirectoryDialog();
+            if (directory.size() > 0)
+            {
+                emit requestedStoreOnDisk(directory);
+            }
+        });
+    }
 
-        _storeOnDiskButton = new QPushButton("Store", this);
-        _loadFromDiskButton = new QPushButton("Load", this);
 
-        hlayout1->addWidget(_lineEdit);
-        hlayout1->addWidget(_openFileBrowserButton);
+    static
+    const std::string&
+    handleSingular(int num, const std::string& singular, const std::string& plural)
+    {
+        return num == 1 ? singular : plural;
+    }
 
-        hlayout2->addWidget(_storeOnDiskButton);
-        hlayout2->addWidget(_loadFromDiskButton);
 
-        vlayout->addItem(hlayout1);
-        vlayout->addItem(hlayout2);
+    void
+    ControlWidget::storeOnDisk(
+        QString directory,
+        const std::map<std::string, wm::Memory> memoryData,
+        std::string* outStatus)
+    {
+        std::filesystem::path path(directory.toUtf8().constData());
+        ARMARX_CHECK_POSITIVE(path.string().size());  // An empty path indicates an error.
+
+        std::stringstream status;
+        if (std::filesystem::is_regular_file(path))
+        {
+            status << "Could not export memories contents to " << path << ": Cannot overwrite existing file.";
+        }
+        else
+        {
+            int numStored = 0;
+            for (const auto& [name, data] : memoryData)
+            {
+                if (std::filesystem::is_regular_file(path / name))
+                {
+                    status << "Could not export memory '" << name << "' to " << path << ": Cannot overwrite existing file.\n";
+                }
+                else
+                {
+                    std::filesystem::create_directories(path / name);
+
+                    armem::server::ltm::disk::MemoryManager manager;
+                    manager.setName(name);
+                    manager.setBasePath(path / name);
+                    manager.reload();
+                    manager.append(data);
+
+                    numStored++;
+                }
+            }
+            status << "Exported " << numStored << " " << handleSingular(numStored, "memory", "memories") << " to " << path << ".";
+        }
+
+        if (outStatus)
+        {
+            *outStatus = status.str();
+        }
+    }
 
-        this->setLayout(vlayout);
-        // Private connections.
 
-        // Public connections.
-        connect(_openFileBrowserButton, &QPushButton::pressed, this, &This::openFileBrowser);
-        connect(_loadFromDiskButton, &QPushButton::pressed, this, &This::loadFromDisk);
-        connect(_storeOnDiskButton, &QPushButton::pressed, this, &This::storeOnDisk);
+    std::map<std::string, wm::Memory>
+    ControlWidget::loadFromDisk(
+        QString directory,
+        const armem::client::QueryInput& _queryInput,
+        std::string* outStatus)
+    {
+        std::filesystem::path path(directory.toUtf8().constData());
+
+        std::map<std::string, wm::Memory> memoryData;
+        std::stringstream status;
+
+        if (not std::filesystem::is_directory(path))
+        {
+            status << "Could not import a memory from " << path << ". Skipping import.";
+        }
+        else
+        {
+            // We use LTM as query target for the disk
+            armem::client::QueryInput queryInput = _queryInput;
+            queryInput.addQueryTargetToAll(armem::query::data::QueryTarget::LTM);
+            const query::data::Input queryIce = queryInput.toIce();
+
+            int numLoaded = 0;
+            for (const auto& dir : std::filesystem::directory_iterator(path))
+            {
+                if (dir.is_directory())
+                {
+                    const std::string key = dir.path().filename();
+                    armem::server::ltm::disk::MemoryManager manager;
+                    manager.setName(key);
+                    manager.setBasePath(path / key);
+
+                    manager.reload();
+
+                    armem::server::query_proc::ltm::MemoryQueryProcessor ltm_processor;
+                    armem::wm::Memory query_res = ltm_processor.process(queryIce, manager.getCacheAndLutNotConverted());
+
+                    manager.convert(query_res);
+                    memoryData[key] = std::move(query_res);
+
+                    numLoaded++;
+                }
+            }
+            status << "Loaded " << numLoaded << " " << handleSingular(numLoaded, "memory", "memories") << " from " << path << ".";
+        }
+
+        if (outStatus)
+        {
+            *outStatus = status.str();
+        }
+        return memoryData;
     }
 
 
-    void ControlWidget::openFileBrowser()
+    QString ControlWidget::chooseDirectoryDialog()
     {
         QFileDialog dialog;
         dialog.setFileMode(QFileDialog::DirectoryOnly);
-        //dialog.setOption(QFileDialog::DontUseNativeDialog, true);
         dialog.setOption(QFileDialog::ShowDirsOnly, false);
-        dialog.exec();
-        QString open = dialog.directory().path();
-        _lineEdit->setText(open);
-    }
-
-
-    QString ControlWidget::getEnteredPath()
-    {
-        return _lineEdit->text();
+        dialog.setDirectory(_latestDirectory);
+        if (dialog.exec())
+        {
+            _latestDirectory = dialog.directory().path();
+            return _latestDirectory;
+        }
+        else
+        {
+            return QString::fromStdString("");
+        }
     }
 
 }
diff --git a/source/RobotAPI/libraries/armem_gui/disk/ControlWidget.h b/source/RobotAPI/libraries/armem_gui/disk/ControlWidget.h
index 4ba8f91d5..b14c9b0a0 100644
--- a/source/RobotAPI/libraries/armem_gui/disk/ControlWidget.h
+++ b/source/RobotAPI/libraries/armem_gui/disk/ControlWidget.h
@@ -1,11 +1,13 @@
 #pragma once
 
+#include <RobotAPI/libraries/armem/core/forward_declarations.h>
+#include <RobotAPI/libraries/armem/client/Query.h>
+
 #include <QWidget>
 #include <QString>
 
+
 class QPushButton;
-class QLineEdit;
-class QFileDialog;
 
 
 namespace armarx::armem::gui::disk
@@ -21,36 +23,39 @@ namespace armarx::armem::gui::disk
         ControlWidget();
 
 
-        QString getEnteredPath();
-
+        void
+        storeOnDisk(
+            QString directory,
+            const std::map<std::string, wm::Memory> memoryData,
+            std::string* outStatus = nullptr);
 
-    public slots:
-
-        void openFileBrowser();
+        std::map<std::string, wm::Memory>
+        loadFromDisk(
+            QString directory,
+            const armem::client::QueryInput& queryInput,
+            std::string* outStatus = nullptr);
 
 
     signals:
 
-        void loadFromDisk();
-        void storeOnDisk();
+        void requestedLoadFromDisk(QString directory);
+        void requestedStoreOnDisk(QString directory);
 
 
     private slots:
 
+        QString chooseDirectoryDialog();
+
 
     signals:
 
 
     private:
 
-        QLineEdit* _lineEdit;
-
-        QPushButton* _openFileBrowserButton;
-
-        QPushButton* _storeInLTMButton;
-        QPushButton* _storeOnDiskButton;
         QPushButton* _loadFromDiskButton;
+        QPushButton* _storeOnDiskButton;
 
+        QString _latestDirectory;
 
     };
 
-- 
GitLab