From 9d4d2f274a6e9a32809686a7359bfe55e1b80de0 Mon Sep 17 00:00:00 2001 From: albrecpe <albrecpe@gmail.com> Date: Fri, 8 Mar 2024 17:35:36 +0100 Subject: [PATCH] Change memory wrapper to use and update snapshot --- .../executions/SkillExecutionTreeWidget.cpp | 8 +-- .../skills_gui/memory/SkillManagerWrapper.cpp | 57 ++++++++++++++++--- .../skills_gui/memory/SkillManagerWrapper.h | 30 +++++++++- .../skill_details/SkillDetailsGroupBox.cpp | 2 +- .../skill_details/SkillDetailsTreeWidget.cpp | 6 +- .../skills_gui/skills/SkillTreeWidget.cpp | 2 +- 6 files changed, 85 insertions(+), 20 deletions(-) diff --git a/source/RobotAPI/libraries/skills_gui/executions/SkillExecutionTreeWidget.cpp b/source/RobotAPI/libraries/skills_gui/executions/SkillExecutionTreeWidget.cpp index aae241ae4..2699e41f2 100644 --- a/source/RobotAPI/libraries/skills_gui/executions/SkillExecutionTreeWidget.cpp +++ b/source/RobotAPI/libraries/skills_gui/executions/SkillExecutionTreeWidget.cpp @@ -20,11 +20,11 @@ namespace armarx::skills::gui // Stop skill QAction* stopSkillAction = new QAction("Stop execution", this); - const auto& executions = memory->fetchExecutions(); + const auto& executions = memory->getExecutions(); if (executions.count(selectedExecution.skillExecutionId) == 0) return; skills::SkillStatus currentStatus = - memory->fetchExecutions().at(selectedExecution.skillExecutionId).status; + memory->getExecutions().at(selectedExecution.skillExecutionId).status; stopSkillAction->setDisabled(currentStatus == skills::SkillStatus::Aborted || currentStatus == skills::SkillStatus::Failed || currentStatus == skills::SkillStatus::Succeeded); @@ -60,7 +60,7 @@ namespace armarx::skills::gui return; // we don't want to hold state in the gui, so we need to get the parameters from memory: skills::SkillExecutionID currentExecutionId = this->selectedExecution.skillExecutionId; - auto executions = memory->fetchExecutions(); + auto executions = memory->getExecutions(); if (executions.empty()) return; @@ -142,7 +142,7 @@ namespace armarx::skills::gui void SkillExecutionTreeWidget::updateExecutions() { - auto currentManagerStatuses = memory->fetchExecutions(); + auto currentManagerStatuses = memory->getExecutions(); if (currentManagerStatuses.empty()) return; diff --git a/source/RobotAPI/libraries/skills_gui/memory/SkillManagerWrapper.cpp b/source/RobotAPI/libraries/skills_gui/memory/SkillManagerWrapper.cpp index bf3db4738..25119c9a1 100644 --- a/source/RobotAPI/libraries/skills_gui/memory/SkillManagerWrapper.cpp +++ b/source/RobotAPI/libraries/skills_gui/memory/SkillManagerWrapper.cpp @@ -11,7 +11,7 @@ namespace armarx::skills::gui std::map<skills::ProviderID, std::map<skills::SkillID, skills::SkillDescription>>; StatusMap - SkillManagerWrapper::fetchExecutions() + SkillManagerWrapper::fetchExecutionsFromMemory() { if (!memory) { @@ -109,8 +109,8 @@ namespace armarx::skills::gui } } - const SkillMap - SkillManagerWrapper::fetchSkills() + SkillMap + SkillManagerWrapper::fetchSkillsFromMemory() { if (!memory) { @@ -185,7 +185,22 @@ namespace armarx::skills::gui SkillManagerWrapper::stopAllExecutions() { ARMARX_IMPORTANT << "Stopping all running executions."; - const auto& executions = this->fetchExecutions(); + + StatusMap executions; + + // we ALWAYS want the newest information when stopping all! + // e.g. there is some new skill not known to the GUI which we explicitely want to stop too. + // the stop-all function is often used in an emergency, so we'll live with the extra call... + try + { + executions = this->fetchExecutionsFromMemory(); + } + catch (...) // if any error occurs, we use the snapshot as backup. better to miss a skill + // than to not do anything. + { + executions = this->getExecutions(); + } + for (auto& [executionId, status] : executions) { // select all running executions... @@ -197,10 +212,17 @@ namespace armarx::skills::gui } } - void SkillManagerWrapper::updateFromMemory() + void + SkillManagerWrapper::updateFromMemory() { - snapshot.statuses = fetchExecutions(); - snapshot.skills = fetchSkills(); + std::scoped_lock l(mutex_snapshot); + + snapshot.statuses = fetchExecutionsFromMemory(); + snapshot.skills = fetchSkillsFromMemory(); + + // notify registered widgets of update + emit skillUpdate(snapshot.skills); + emit statusUpdate(snapshot.statuses); } const std::optional<ProviderID> @@ -227,6 +249,20 @@ namespace armarx::skills::gui return std::nullopt; } + SkillMap + SkillManagerWrapper::getSkills() + { + std::scoped_lock l(mutex_snapshot); + return snapshot.skills; + } + + StatusMap + SkillManagerWrapper::getExecutions() + { + std::scoped_lock l(mutex_snapshot); + return snapshot.statuses; + } + void SkillManagerWrapper::stopExecution(skills::SkillExecutionID const& executionId, const unsigned int max_retries) @@ -301,8 +337,13 @@ namespace armarx::skills::gui } std::map<skills::SkillID, skills::SkillDescription> skillDescriptions; + if (this->UPDATE_ON_EXECUTION_REQUEST) + { + skillDescriptions = this->fetchSkillsFromMemory().at(providerId.value()); + } + else { - skillDescriptions = this->fetchSkills().at(providerId.value()); + skillDescriptions = this->getSkills().at(providerId.value()); } if (skillDescriptions.find(skillId) == skillDescriptions.end()) diff --git a/source/RobotAPI/libraries/skills_gui/memory/SkillManagerWrapper.h b/source/RobotAPI/libraries/skills_gui/memory/SkillManagerWrapper.h index ea902a723..f6f797c8e 100644 --- a/source/RobotAPI/libraries/skills_gui/memory/SkillManagerWrapper.h +++ b/source/RobotAPI/libraries/skills_gui/memory/SkillManagerWrapper.h @@ -55,8 +55,22 @@ namespace armarx::skills::gui static const std::optional<skills::ProviderID> findFirstProvider(SkillMap const& map, SkillID const& skillId); + /** + * @brief Returns the latest skills snapshot. + * @return The map representing all currently known skills. + */ + SkillMap getSkills(); + + /** + * @brief Returns the latest status snapshot. + * @return The map containing status updates for all execution ids. + */ + StatusMap getExecutions(); + signals: void connectionUpdate(std::string const& message, std::string const& error); + void skillUpdate(SkillMap update); + void statusUpdate(StatusMap update); public slots: /** @@ -82,11 +96,13 @@ namespace armarx::skills::gui private: mutable std::mutex mutex_memory; + mutable std::mutex mutex_snapshot; armarx::skills::manager::dti::SkillManagerInterfacePrx memory; std::string currentSkillSearch = ""; - struct Snapshot { + struct Snapshot + { StatusMap statuses; SkillMap skills; @@ -102,13 +118,13 @@ namespace armarx::skills::gui * @brief Fetches and returns the latest skills update from memory. * @return The map representing all skills in memory. Empty, if error occurred. */ - SkillMap fetchSkills(); + SkillMap fetchSkillsFromMemory(); /** * @brief Fetches and returns the latest status update from memory. * @return The map containing status updates for all execution ids. Empty, if error occurred. */ - StatusMap fetchExecutions(); + StatusMap fetchExecutionsFromMemory(); /** * @brief Modifies the given map to match the search. @@ -116,5 +132,13 @@ namespace armarx::skills::gui */ void filterSkillUpdate(std::map<skills::manager::dto::SkillID, skills::manager::dto::SkillDescription>& update); + + // config + // TODO: perhaps use proper config file for the gui + + // should we do an ice call to use the latest information when requesting an execution? + static constexpr bool UPDATE_ON_EXECUTION_REQUEST = true; + + // --- }; } // namespace armarx::skills::gui diff --git a/source/RobotAPI/libraries/skills_gui/skill_details/SkillDetailsGroupBox.cpp b/source/RobotAPI/libraries/skills_gui/skill_details/SkillDetailsGroupBox.cpp index 137140d25..8eaa3633f 100644 --- a/source/RobotAPI/libraries/skills_gui/skill_details/SkillDetailsGroupBox.cpp +++ b/source/RobotAPI/libraries/skills_gui/skill_details/SkillDetailsGroupBox.cpp @@ -8,7 +8,7 @@ namespace armarx::skills::gui if (_skillId.skillName == SkillID::UNKNOWN) return; - auto const& skills = memory->fetchSkills(); + auto const& skills = memory->getSkills(); std::optional<skills::ProviderID> provider_opt; // check skillId diff --git a/source/RobotAPI/libraries/skills_gui/skill_details/SkillDetailsTreeWidget.cpp b/source/RobotAPI/libraries/skills_gui/skill_details/SkillDetailsTreeWidget.cpp index 0287c5468..57b26011b 100644 --- a/source/RobotAPI/libraries/skills_gui/skill_details/SkillDetailsTreeWidget.cpp +++ b/source/RobotAPI/libraries/skills_gui/skill_details/SkillDetailsTreeWidget.cpp @@ -91,7 +91,7 @@ namespace armarx::skills::gui ARMARX_CHECK(sid.isFullySpecified()); // maybe the search is empty? - auto skillsMap = memory->fetchSkills(); + auto skillsMap = memory->getSkills(); if (skillsMap.count(sid.providerId.value()) == 0 || skillsMap.at(sid.providerId.value()).count(sid) == 0) { @@ -99,7 +99,7 @@ namespace armarx::skills::gui return; } - auto descr = memory->fetchSkills().at(sid.providerId.value()).at(sid); + auto descr = memory->getSkills().at(sid.providerId.value()).at(sid); this->updateContents(sid, descr); } @@ -196,7 +196,7 @@ namespace armarx::skills::gui return; } skills::SkillID& skillId = shownSkill.value().skillId; - auto& skills = memory->fetchSkills(); + const auto skills = memory->getSkills(); ARMARX_CHECK(skillId.isProviderSpecified()); // find description diff --git a/source/RobotAPI/libraries/skills_gui/skills/SkillTreeWidget.cpp b/source/RobotAPI/libraries/skills_gui/skills/SkillTreeWidget.cpp index c1b100130..142a387cc 100644 --- a/source/RobotAPI/libraries/skills_gui/skills/SkillTreeWidget.cpp +++ b/source/RobotAPI/libraries/skills_gui/skills/SkillTreeWidget.cpp @@ -28,7 +28,7 @@ namespace armarx::skills::gui SkillTreeWidget::updateSkills() { setSortingEnabled(false); - const auto skills = memory->fetchSkills(); + const auto skills = memory->getSkills(); // update tree view. Remove non-existing elements int i = 0; -- GitLab