From 713163e9befef7f61a542cf7dfb4a2b736151641 Mon Sep 17 00:00:00 2001
From: Peter Albrecht <albrecpe@gmail.com>
Date: Thu, 21 Dec 2023 13:13:59 +0100
Subject: [PATCH] fix: description not showing on selection

---
 .../libraries/skills_gui/SkillMemoryGui.cpp   | 14 +++----
 .../libraries/skills_gui/SkillMemoryGui.h     |  5 +++
 .../skills_gui/memory/SkillManagerWrapper.cpp |  4 +-
 .../skill_details/SkillDetailsGroupBox.cpp    |  9 ++++-
 .../skill_details/SkillDetailsGroupBox.h      |  5 +++
 .../skill_details/SkillDetailsTreeWidget.cpp  | 38 ++++++++++++++++++-
 .../skill_details/SkillDetailsTreeWidget.h    | 19 ++++++----
 .../skills_gui/skills/SkillGroupBox.cpp       |  3 ++
 .../skills_gui/skills/SkillGroupBox.h         |  5 +++
 .../skills_gui/skills/SkillTreeWidget.h       |  4 +-
 10 files changed, 85 insertions(+), 21 deletions(-)

diff --git a/source/RobotAPI/libraries/skills_gui/SkillMemoryGui.cpp b/source/RobotAPI/libraries/skills_gui/SkillMemoryGui.cpp
index 394aab95c..c9676341b 100644
--- a/source/RobotAPI/libraries/skills_gui/SkillMemoryGui.cpp
+++ b/source/RobotAPI/libraries/skills_gui/SkillMemoryGui.cpp
@@ -95,13 +95,13 @@ namespace armarx::skills::gui
                 &PeriodicUpdateWidget::startTimerIfEnabled);
         connect(this, &SkillMemoryGUI::connectGui, skillGroupBox, &SkillGroupBox::connectGui);
 
+        // update cascade
+        connect(this, &SkillMemoryGUI::updateGui, skillGroupBox, &SkillGroupBox::updateGui);
+        connect(
+            this, &SkillMemoryGUI::updateGui, skillDetailGroupBox, &SkillDetailGroupBox::updateGui);
 
-        /*
-        // connect update widget to memory update
-        connect(this->updateWidget,
-                &PeriodicUpdateWidget::update,
-                this->memory.get(),
-                &SkillManagerWrapper::fetchUpdates);
-*/
+        // timer -> update
+        connect(
+            this->updateWidget, &PeriodicUpdateWidget::update, this, &SkillMemoryGUI::updateGui);
     }
 } // namespace armarx::skills::gui
diff --git a/source/RobotAPI/libraries/skills_gui/SkillMemoryGui.h b/source/RobotAPI/libraries/skills_gui/SkillMemoryGui.h
index 076d889d4..537dc4fc6 100644
--- a/source/RobotAPI/libraries/skills_gui/SkillMemoryGui.h
+++ b/source/RobotAPI/libraries/skills_gui/SkillMemoryGui.h
@@ -47,6 +47,11 @@ namespace armarx::skills::gui
          */
         void connectGui(std::string observerName);
 
+        /**
+         * @brief Notify widgets to update themselves
+         */
+        void updateGui();
+
     private:
         void setupUi();
         void connectSignals();
diff --git a/source/RobotAPI/libraries/skills_gui/memory/SkillManagerWrapper.cpp b/source/RobotAPI/libraries/skills_gui/memory/SkillManagerWrapper.cpp
index 1f5fc3ce0..0f99ff132 100644
--- a/source/RobotAPI/libraries/skills_gui/memory/SkillManagerWrapper.cpp
+++ b/source/RobotAPI/libraries/skills_gui/memory/SkillManagerWrapper.cpp
@@ -120,7 +120,7 @@ namespace armarx::skills::gui
 
                 ARMARX_CHECK(skillId.isFullySpecified());
 
-                auto& providedSkillsMap = skills[providerId]; // create new if not existent
+                auto& providedSkillsMap = skills[providerId]; // create new if not existing
                 providedSkillsMap.insert({skillId, description});
             }
 
@@ -281,7 +281,7 @@ namespace armarx::skills::gui
 
         try
         {
-            memory->executeSkill(req.toManagerIce());
+            memory->begin_executeSkill(req.toManagerIce());
         }
         catch (Ice::Exception const& e)
         {
diff --git a/source/RobotAPI/libraries/skills_gui/skill_details/SkillDetailsGroupBox.cpp b/source/RobotAPI/libraries/skills_gui/skill_details/SkillDetailsGroupBox.cpp
index 00e5944e7..fdba9547e 100644
--- a/source/RobotAPI/libraries/skills_gui/skill_details/SkillDetailsGroupBox.cpp
+++ b/source/RobotAPI/libraries/skills_gui/skill_details/SkillDetailsGroupBox.cpp
@@ -44,6 +44,7 @@ namespace armarx::skills::gui
 
         // setup groupBox
         this->setTitle(QString::fromStdString(skillId.toString()));
+        setDisabled(false);
 
         // setup table view
         skillDetailsTreeWidget->updateContents(skillId, descr);
@@ -59,8 +60,6 @@ namespace armarx::skills::gui
 
         // add new profiles for this skill
         // TODO: Where stored?
-
-        setDisabled(false);
     }
 
     void
@@ -119,6 +118,12 @@ namespace armarx::skills::gui
                 &SkillDetailGroupBox::disconnectGui,
                 skillDetailsTreeWidget,
                 &SkillDetailsTreeWidget::disconnectGui);
+
+        // update cascade
+        connect(this,
+                &SkillDetailGroupBox::updateGui,
+                skillDetailsTreeWidget,
+                &SkillDetailsTreeWidget::updateGui);
     }
 
 
diff --git a/source/RobotAPI/libraries/skills_gui/skill_details/SkillDetailsGroupBox.h b/source/RobotAPI/libraries/skills_gui/skill_details/SkillDetailsGroupBox.h
index 9326e86f4..63c32dfba 100644
--- a/source/RobotAPI/libraries/skills_gui/skill_details/SkillDetailsGroupBox.h
+++ b/source/RobotAPI/libraries/skills_gui/skill_details/SkillDetailsGroupBox.h
@@ -31,6 +31,11 @@ namespace armarx::skills::gui
     signals:
         void disconnectGui();
 
+        /**
+         * @brief Notify widgets to update themselves
+         */
+        void updateGui();
+
     public slots:
         /**
          * @brief Update subwidgets of an updated skill selection.
diff --git a/source/RobotAPI/libraries/skills_gui/skill_details/SkillDetailsTreeWidget.cpp b/source/RobotAPI/libraries/skills_gui/skill_details/SkillDetailsTreeWidget.cpp
index 3defb59e7..a9df69ad0 100644
--- a/source/RobotAPI/libraries/skills_gui/skill_details/SkillDetailsTreeWidget.cpp
+++ b/source/RobotAPI/libraries/skills_gui/skill_details/SkillDetailsTreeWidget.cpp
@@ -8,15 +8,32 @@
 
 namespace armarx::skills::gui
 {
-    std::optional<skills::SkillID>
+    SkillDetailsTreeWidget::SkillDetailsTreeWidget(std::shared_ptr<SkillManagerWrapper> _memory,
+                                                   QWidget* parent) :
+        QTreeWidget(parent), MemoryCommunicatorBase(_memory)
+    {
+        setupUi();
+    }
+
+    std::optional<SkillID>
     SkillDetailsTreeWidget::getShownId()
     {
-        return shownSkillId;
+        if (shownSkill.has_value())
+        {
+            return shownSkill.value().skillId;
+        }
+        else
+        {
+            return std::nullopt;
+        }
     }
 
     void
     SkillDetailsTreeWidget::updateContents(const SkillID& skillId, const SkillDescription& descr)
     {
+        // dont touch the widget if the skill id didn't change
+        if (shownSkill.has_value() && skillId == shownSkill.value().skillId)
+            return;
         this->clear();
         aronTreeWidgetController = nullptr;
         skillsArgumentsTreeWidgetItem = nullptr;
@@ -52,6 +69,9 @@ namespace armarx::skills::gui
 
 
         skillsArgumentsTreeWidgetItem->setExpanded(true);
+
+        // update the ShownSkill
+        shownSkill = {skillId, descr};
     }
 
     void
@@ -61,6 +81,20 @@ namespace armarx::skills::gui
         this->aronTreeWidgetController = nullptr;
     }
 
+    void
+    SkillDetailsTreeWidget::updateGui()
+    {
+        if (!shownSkill.has_value())
+            return;
+        skills::SkillID sid = shownSkill.value().skillId;
+
+        // we assume the id to be fully specified, as it is checked while constructing
+
+        auto descr = memory->fetchSkills().at(sid.providerId.value()).at(sid);
+
+        this->updateContents(sid, descr);
+    }
+
     void
     SkillDetailsTreeWidget::setupUi()
     {
diff --git a/source/RobotAPI/libraries/skills_gui/skill_details/SkillDetailsTreeWidget.h b/source/RobotAPI/libraries/skills_gui/skill_details/SkillDetailsTreeWidget.h
index f13cca581..f6116a938 100644
--- a/source/RobotAPI/libraries/skills_gui/skill_details/SkillDetailsTreeWidget.h
+++ b/source/RobotAPI/libraries/skills_gui/skill_details/SkillDetailsTreeWidget.h
@@ -14,12 +14,7 @@ namespace armarx::skills::gui
         Q_OBJECT
     public:
         SkillDetailsTreeWidget(std::shared_ptr<SkillManagerWrapper> _memory,
-                               QWidget* parent = nullptr,
-                               std::optional<skills::SkillID> skillId = std::nullopt) :
-            QTreeWidget(parent), MemoryCommunicatorBase(_memory), shownSkillId(skillId)
-        {
-            setupUi();
-        }
+                               QWidget* parent = nullptr);
 
         std::optional<skills::SkillID> getShownId();
         void updateContents(skills::SkillID const& skillId, skills::SkillDescription const& descr);
@@ -31,9 +26,19 @@ namespace armarx::skills::gui
 
     public slots:
         void disconnectGui();
+        void updateGui();
 
     private:
-        std::optional<skills::SkillID> shownSkillId;
+        struct ShownSkill
+        {
+            skills::SkillID skillId;
+            skills::SkillDescription descr;
+        };
+
+        std::optional<ShownSkill> shownSkill;
+
+        // check for update
+        skills::SkillDescription shownDescription;
         QTreeWidgetItem* skillsArgumentsTreeWidgetItem = nullptr;
         AronTreeWidgetControllerPtr aronTreeWidgetController = nullptr;
         void setupUi();
diff --git a/source/RobotAPI/libraries/skills_gui/skills/SkillGroupBox.cpp b/source/RobotAPI/libraries/skills_gui/skills/SkillGroupBox.cpp
index 71c2702e8..91fb11c02 100644
--- a/source/RobotAPI/libraries/skills_gui/skills/SkillGroupBox.cpp
+++ b/source/RobotAPI/libraries/skills_gui/skills/SkillGroupBox.cpp
@@ -67,5 +67,8 @@ namespace armarx::skills::gui
         // disconnect
         connect(
             this, &SkillGroupBox::disconnectGui, skillTreeWidget, &SkillTreeWidget::disconnectGui);
+
+        // update cascade
+        connect(this, &SkillGroupBox::updateGui, skillTreeWidget, &SkillTreeWidget::updateSkills);
     }
 } // namespace armarx::skills::gui
diff --git a/source/RobotAPI/libraries/skills_gui/skills/SkillGroupBox.h b/source/RobotAPI/libraries/skills_gui/skills/SkillGroupBox.h
index f7f581a32..2ac6044ba 100644
--- a/source/RobotAPI/libraries/skills_gui/skills/SkillGroupBox.h
+++ b/source/RobotAPI/libraries/skills_gui/skills/SkillGroupBox.h
@@ -36,6 +36,11 @@ namespace armarx::skills::gui
 
         void disconnectGui();
 
+        /**
+         * @brief Notify widgets to update themselves
+         */
+        void updateGui();
+
     public slots:
         void connectGui(std::string observerName);
 
diff --git a/source/RobotAPI/libraries/skills_gui/skills/SkillTreeWidget.h b/source/RobotAPI/libraries/skills_gui/skills/SkillTreeWidget.h
index 43fc9a741..46500d0d1 100644
--- a/source/RobotAPI/libraries/skills_gui/skills/SkillTreeWidget.h
+++ b/source/RobotAPI/libraries/skills_gui/skills/SkillTreeWidget.h
@@ -9,6 +9,7 @@
 
 namespace armarx::skills::gui
 {
+
     class SkillTreeWidget : public QTreeWidget, public MemoryCommunicatorBase
     {
         Q_OBJECT
@@ -41,10 +42,11 @@ namespace armarx::skills::gui
 
     public slots:
         void disconnectGui();
+        void updateSkills();
 
     private slots:
         void skillSelectionChanged(QTreeWidgetItem* current, QTreeWidgetItem* previous);
-        void updateSkills();
+
 
     private:
         SelectedSkill selectedSkill;
-- 
GitLab