diff --git a/source/RobotAPI/libraries/skills/manager/SkillManagerComponentPlugin.cpp b/source/RobotAPI/libraries/skills/manager/SkillManagerComponentPlugin.cpp
index 4b0b2aeb0c70eefec9daa3b55590f93e4cfd4126..c0d4757462c142c3792a323bc44c09407c2431a5 100644
--- a/source/RobotAPI/libraries/skills/manager/SkillManagerComponentPlugin.cpp
+++ b/source/RobotAPI/libraries/skills/manager/SkillManagerComponentPlugin.cpp
@@ -124,7 +124,7 @@ namespace armarx::plugins
auto async =
provider->begin_executeSkill(provider_executionRequest.toProviderIce());
- l.unlock(); // allow parallel e.g. stopping
+ l.unlock(); // allow parallel e.g. stopping. Otherwise the manager would lock himself in nested calls
auto provider_statusUpdate_ice = provider->end_executeSkill(async);
// convert to manager view
@@ -132,17 +132,11 @@ namespace armarx::plugins
skills::SkillStatusUpdate::FromIce(provider_statusUpdate_ice, provderId);
return statusUpdate;
}
- catch (...)
+ catch (const std::exception &e)
{
- ARMARX_WARNING << __PRETTY_FUNCTION__
- << ": Found disconnected or buggy skill provider '" << provderId
- << "' during execution. Removing it from skills.";
- skillProviderMap.erase(it);
+ l.lock();
- throw skills::error::SkillException(
- __PRETTY_FUNCTION__,
- "Skill execution failed. Could not execute a skill of provider '" +
- provderId.toString() + "' because the provider does not exist.");
+ handleExceptionNonLockingThrow(__PRETTY_FUNCTION__, e, provderId);
}
}
else
@@ -198,7 +192,7 @@ namespace armarx::plugins
auto async =
provider->begin_executeSkillAsync(provider_executionRequest.toProviderIce());
- l.unlock(); // allow parallel e.g. stopping
+ l.unlock(); // allow parallel e.g. stopping. Otherwise the manager would lock himself in nested calls
auto provider_executionID_ice = provider->end_executeSkillAsync(async);
// convert to manager view
@@ -211,26 +205,7 @@ namespace armarx::plugins
{
l.lock();
- if (it = skillProviderMap.find(provderId); it != skillProviderMap.end())
- {
- ARMARX_WARNING << __PRETTY_FUNCTION__
- << ": Found disconnected or buggy skill provider '" << provderId
- << "' during execution. Removing it from skills. "
- << "Error: " << e.what();
- skillProviderMap.erase(it);
- }
- else
- {
- ARMARX_WARNING << __PRETTY_FUNCTION__
- << ": Found disconnected or buggy skill provider '" << provderId
- << "' during execution. However, it already got removed... "
- << "Error: " << e.what();
- }
-
- throw skills::error::SkillException(
- __PRETTY_FUNCTION__,
- "Skill execution failed. Could not execute a skill of provider '" +
- provderId.toString() + "' because the provider does not exist.");
+ handleExceptionNonLockingThrow(__PRETTY_FUNCTION__, e, provderId);
}
}
else
@@ -270,7 +245,7 @@ namespace armarx::plugins
{
auto async = provider->begin_updateSkillParameters(executionId.toProviderIce(),
data->toAronDictDTO());
- l.unlock(); // allow parallel e.g. stopping
+ l.unlock(); // allow parallel e.g. stopping. Otherwise the manager would lock himself in nested calls
auto r = provider->end_updateSkillParameters(async);
return r.success;
}
@@ -278,21 +253,7 @@ namespace armarx::plugins
{
l.lock();
- if (it = skillProviderMap.find(providerId); it != skillProviderMap.end())
- {
- ARMARX_WARNING << __PRETTY_FUNCTION__
- << ": Found disconnected or buggy skill provider '" << providerId
- << "' during execution. Removing it from skills. "
- << "Error: " << e.what();
- skillProviderMap.erase(it);
- }
- else
- {
- ARMARX_WARNING << __PRETTY_FUNCTION__
- << ": Found disconnected or buggy skill provider '" << providerId
- << "' during execution. However, it already got removed... "
- << "Error: " << e.what();
- }
+ handleExceptionNonLocking(__PRETTY_FUNCTION__, e, providerId);
return false;
}
@@ -329,7 +290,7 @@ namespace armarx::plugins
try
{
auto async = provider->begin_abortSkill(executionId.toProviderIce());
- l.unlock(); // allow parallel e.g. stopping
+ l.unlock(); // allow parallel e.g. stopping. Otherwise the manager would lock himself in nested calls
auto r = provider->end_abortSkill(async);
return r.success;
}
@@ -337,21 +298,7 @@ namespace armarx::plugins
{
l.lock();
- if (it = skillProviderMap.find(providerId); it != skillProviderMap.end())
- {
- ARMARX_WARNING << __PRETTY_FUNCTION__
- << ": Found disconnected or buggy skill provider '" << providerId
- << "' during execution. Removing it from skills. "
- << "Error: " << e.what();
- skillProviderMap.erase(it);
- }
- else
- {
- ARMARX_WARNING << __PRETTY_FUNCTION__
- << ": Found disconnected or buggy skill provider '" << providerId
- << "' during execution. However, it already got removed... "
- << "Error: " << e.what();
- }
+ handleExceptionNonLocking(__PRETTY_FUNCTION__, e, providerId);
return false;
}
@@ -390,21 +337,7 @@ namespace armarx::plugins
}
catch (const std::exception &e)
{
- if (it = skillProviderMap.find(providerId); it != skillProviderMap.end())
- {
- ARMARX_WARNING << __PRETTY_FUNCTION__
- << ": Found disconnected or buggy skill provider '" << providerId
- << "' during execution. Removing it from skills. "
- << "Error: " << e.what();
- skillProviderMap.erase(it);
- }
- else
- {
- ARMARX_WARNING << __PRETTY_FUNCTION__
- << ": Found disconnected or buggy skill provider '" << providerId
- << "' during execution. However, it already got removed... "
- << "Error: " << e.what();
- }
+ handleExceptionNonLocking(__PRETTY_FUNCTION__, e, providerId);
return false;
}
@@ -444,7 +377,7 @@ namespace armarx::plugins
try
{
auto async = provider->begin_getSkillDescription(skillId.toProviderIce());
- l.unlock(); // allow parallel e.g. stopping
+ l.unlock(); // allow parallel e.g. stopping. Otherwise the manager would lock himself in nested calls
auto provider_desc_ice = provider->end_getSkillDescription(async);
if (not provider_desc_ice)
@@ -459,28 +392,7 @@ namespace armarx::plugins
}
catch (const std::exception &e)
{
- if (it = skillProviderMap.find(providerId); it != skillProviderMap.end())
- {
- ARMARX_WARNING << __PRETTY_FUNCTION__
- << ": Found disconnected or buggy skill provider '" << providerId
- << "' during execution. Removing it from skills. "
- << "Error: " << e.what();
- skillProviderMap.erase(it);
- }
- else
- {
- ARMARX_WARNING << __PRETTY_FUNCTION__
- << ": Found disconnected or buggy skill provider '" << providerId
- << "' during execution. However, it already got removed... "
- << "Error: " << e.what();
- }
-
-
- throw skills::error::SkillException(__PRETTY_FUNCTION__,
- "Skill execution failed. Could not query a "
- "status update of a skill of provider '" +
- providerId.toString() +
- "' because the provider does not exist.");
+ handleExceptionNonLockingThrow(__PRETTY_FUNCTION__, e, providerId);
}
}
else
@@ -523,20 +435,9 @@ namespace armarx::plugins
}
catch (const std::exception &e)
{
- if (it = skillProviderMap.find(providerId); it != skillProviderMap.end())
- {
- ARMARX_WARNING << __PRETTY_FUNCTION__
- << ": Found disconnected or buggy skill provider '" << providerId
- << "' during execution. Removing it from skills. "
- << "Error: " << e.what();
- skillProviderMap.erase(it);
- }
- else
+ if (auto _it = handleExceptionNonLocking(__PRETTY_FUNCTION__, e, providerId))
{
- ARMARX_WARNING << __PRETTY_FUNCTION__
- << ": Found disconnected or buggy skill provider '" << providerId
- << "' during execution. However, it already got removed... "
- << "Error: " << e.what();
+ it = _it.value(); // next element
}
}
}
@@ -573,7 +474,7 @@ namespace armarx::plugins
try
{
auto async = provider->begin_getSkillExecutionStatus(executionId.toProviderIce());
- l.unlock(); // allow parallel e.g. stopping
+ l.unlock(); // allow parallel e.g. stopping. Otherwise the manager would lock himself in nested calls
auto provider_statusUpdate_ice = provider->end_getSkillExecutionStatus(async);
if (not provider_statusUpdate_ice)
@@ -590,27 +491,7 @@ namespace armarx::plugins
{
l.lock();
- if (it = skillProviderMap.find(providerId); it != skillProviderMap.end())
- {
- ARMARX_WARNING << __PRETTY_FUNCTION__
- << ": Found disconnected or buggy skill provider '" << providerId
- << "' during execution. Removing it from skills. "
- << "Error: " << e.what();
- skillProviderMap.erase(it);
- }
- else
- {
- ARMARX_WARNING << __PRETTY_FUNCTION__
- << ": Found disconnected or buggy skill provider '" << providerId
- << "' during execution. However, it already got removed... "
- << "Error: " << e.what();
- }
-
- throw skills::error::SkillException(__PRETTY_FUNCTION__,
- "Skill execution failed. Could not query a "
- "status update of a skill of provider '" +
- providerId.toString() +
- "' because the provider does not exist.");
+ handleExceptionNonLockingThrow(__PRETTY_FUNCTION__, e, providerId);
}
}
else
@@ -651,16 +532,53 @@ namespace armarx::plugins
}
it++;
}
- catch (...)
+ catch (const std::exception &e)
{
- ARMARX_WARNING << __PRETTY_FUNCTION__ << ": Found buggy skill provider '"
- << providerId << "'. Removing it from skills.";
- it = skillProviderMap.erase(it);
+ if (auto _it = handleExceptionNonLocking(__PRETTY_FUNCTION__, e, providerId))
+ {
+ it = _it.value(); // next element
+ }
}
}
return ret;
}
+ std::optional<std::map<skills::ProviderID, skills::provider::dti::SkillProviderInterfacePrx>::iterator>
+ SkillManagerComponentPlugin::handleExceptionNonLocking(
+ const char* funcName, const std::exception& e, skills::ProviderID providerId, bool eraseSkillProvider)
+ {
+ // NON LOCKING! WE ASSERT THAT THE CALLER HOLDS LOCK
+ if (auto it = skillProviderMap.find(providerId); eraseSkillProvider and it != skillProviderMap.end())
+ {
+ ARMARX_WARNING << funcName
+ << ": Found disconnected or buggy skill provider '" << providerId
+ << "' during execution. Removing it from skills. "
+ << "Error: " << e.what();
+ return skillProviderMap.erase(it);
+ }
+ else
+ {
+ ARMARX_WARNING << funcName
+ << ": Found disconnected or buggy skill provider '" << providerId
+ << "' during execution. However, it already got removed... "
+ << "Error: " << e.what();
+ return std::nullopt;
+ }
+ }
+
+ void
+ SkillManagerComponentPlugin::handleExceptionNonLockingThrow(
+ const char* funcName, const std::exception& e, skills::ProviderID providerId, bool eraseSkillProvider)
+ {
+ // NON LOCKING! WE ASSERT THAT THE CALLER HOLDS LOCK
+ handleExceptionNonLocking(funcName, e, providerId, eraseSkillProvider);
+
+ throw skills::error::SkillException(
+ funcName,
+ "Skill execution failed. Could not execute a skill of provider '" +
+ providerId.toString() + "' because the provider does not exist.");
+ }
+
} // namespace armarx::plugins
namespace armarx
diff --git a/source/RobotAPI/libraries/skills/manager/SkillManagerComponentPlugin.h b/source/RobotAPI/libraries/skills/manager/SkillManagerComponentPlugin.h
index b672ee082ddbb8193592eafb00dee0c89df3a623..261037f624c2821d15d79560d24f127125740f8b 100644
--- a/source/RobotAPI/libraries/skills/manager/SkillManagerComponentPlugin.h
+++ b/source/RobotAPI/libraries/skills/manager/SkillManagerComponentPlugin.h
@@ -1,11 +1,14 @@
#pragma once
+#include <exception>
#include <mutex>
+#include <optional>
#include <ArmarXCore/core/ComponentPlugin.h>
#include <ArmarXCore/core/ManagedIceObject.h>
#include <RobotAPI/interface/skills/SkillManagerInterface.h>
+#include <RobotAPI/libraries/skills/core/error/Exception.h>
#include <RobotAPI/libraries/skills/core/ProviderID.h>
#include <RobotAPI/libraries/skills/core/ProviderInfo.h>
#include <RobotAPI/libraries/skills/core/SkillExecutionRequest.h>
@@ -57,6 +60,13 @@ namespace armarx::plugins
skills::ProviderID getFirstProviderNameThatHasSkill(const skills::SkillID& skillid);
private:
+ [[ noreturn ]] void handleExceptionNonLockingThrow(const char* funcName, const std::exception& e, skills::ProviderID providerId,
+ bool eraseSkillProvider = true);
+
+ std::optional<std::map<skills::ProviderID, skills::provider::dti::SkillProviderInterfacePrx>::iterator>
+ handleExceptionNonLocking(const char* funcName, const std::exception& e, skills::ProviderID providerId,
+ bool eraseSkillProvider = true);
+
skills::manager::dti::SkillManagerInterfacePrx myPrx;
std::mutex skillProviderMapMutex;