From 62ddeb169ed5f7ec4a2f5a11d686f4705475d411 Mon Sep 17 00:00:00 2001
From: Fabian Peller <fabian.peller-konrad@kit.edu>
Date: Fri, 20 Oct 2023 08:56:49 +0200
Subject: [PATCH] fix race condition when using callbacks
fix hello world skill parameters type
---
.../SkillProviderExample/HelloWorld.cpp | 6 +--
.../SkillProviderExample/Incomplete.cpp | 1 -
.../RobotAPI/libraries/skills/core/Skill.cpp | 1 -
.../skills/provider/SimpleSpecializedSkill.h | 41 ++++++++-----------
.../provider/SkillProviderComponentPlugin.cpp | 10 ++---
.../detail/SkillImplementationWrapper.cpp | 2 +-
6 files changed, 23 insertions(+), 38 deletions(-)
diff --git a/source/RobotAPI/components/skills/SkillProviderExample/HelloWorld.cpp b/source/RobotAPI/components/skills/SkillProviderExample/HelloWorld.cpp
index 6779f1d6a..d57f66c3a 100644
--- a/source/RobotAPI/components/skills/SkillProviderExample/HelloWorld.cpp
+++ b/source/RobotAPI/components/skills/SkillProviderExample/HelloWorld.cpp
@@ -27,7 +27,7 @@ namespace armarx::skills::provider
.description = "This skill logs a message on ARMARX_IMPORTANT",
.rootProfileDefaults = root_profile_params.toAron(),
.timeout = armarx::core::time::Duration::MilliSeconds(1000),
- .resultType =
+ .parametersType =
armarx::skills::Example::HelloWorldAcceptedType::ToAronType()};
}
@@ -40,10 +40,6 @@ namespace armarx::skills::provider
in.parameters.toAron())
.dump(2)
<< "\n"
- << "Type fulfilled? "
- << parameters->fullfillsType(
- armarx::skills::Example::HelloWorldAcceptedType::ToAronType())
- << "\n"
<< "(executed at: " << IceUtil::Time::now() << ")";
return {TerminatedSkillStatus::Succeeded, nullptr};
}
diff --git a/source/RobotAPI/components/skills/SkillProviderExample/Incomplete.cpp b/source/RobotAPI/components/skills/SkillProviderExample/Incomplete.cpp
index 264876a7e..8d6b4f277 100644
--- a/source/RobotAPI/components/skills/SkillProviderExample/Incomplete.cpp
+++ b/source/RobotAPI/components/skills/SkillProviderExample/Incomplete.cpp
@@ -48,7 +48,6 @@ namespace armarx::skills::provider
IncompleteSkill::main(const MainInput& in)
{
auto s = HelloWorldSkill();
- s.setParameters(in.parameters);
return s.mainOfSkill();
}
} // namespace armarx::skills::provider
diff --git a/source/RobotAPI/libraries/skills/core/Skill.cpp b/source/RobotAPI/libraries/skills/core/Skill.cpp
index 34895c906..907881580 100644
--- a/source/RobotAPI/libraries/skills/core/Skill.cpp
+++ b/source/RobotAPI/libraries/skills/core/Skill.cpp
@@ -70,7 +70,6 @@ namespace armarx
aron::data::DictPtr
Skill::getParameters() const
{
- std::scoped_lock l(this->parametersMutex);
return this->parameters;
}
diff --git a/source/RobotAPI/libraries/skills/provider/SimpleSpecializedSkill.h b/source/RobotAPI/libraries/skills/provider/SimpleSpecializedSkill.h
index b14531195..babdb0189 100644
--- a/source/RobotAPI/libraries/skills/provider/SimpleSpecializedSkill.h
+++ b/source/RobotAPI/libraries/skills/provider/SimpleSpecializedSkill.h
@@ -1,16 +1,16 @@
#pragma once
-#include <RobotAPI/libraries/skills/core/Skill.h>
+#include <RobotAPI/libraries/skills/provider/SpecializedSkill.h>
namespace armarx
{
namespace skills
{
template <class AronT>
- class SimpleSpecializedSkill : public Skill
+ class SimpleSpecializedSkill : public SpecializedSkill<AronT>
{
public:
- using Base = Skill;
+ using Base = SpecializedSkill<AronT>;
using ParamType = AronT;
using Base::Base;
@@ -25,7 +25,7 @@ namespace armarx
{
std::string executorName;
AronT parameters;
- CallbackT callback;
+ Skill::CallbackT callback;
};
struct SpecializedExitInput
@@ -37,61 +37,52 @@ namespace armarx
protected:
// legacy methods
- virtual InitResult
+ virtual Skill::InitResult
init(const SpecializedInitInput& in)
{
- return InitResult{.status = TerminatedSkillStatus::Succeeded};
+ return Skill::InitResult{.status = TerminatedSkillStatus::Succeeded};
}
- virtual MainResult
+ virtual Skill::MainResult
main(const SpecializedMainInput& in)
{
- ARMARX_IMPORTANT << "Dummy executing skill '" << description.skillId
+ ARMARX_IMPORTANT << "Dummy executing skill '" << this->description.skillId
<< "'. Please overwrite this method.";
return Skill::MainResult{.status = TerminatedSkillStatus::Succeeded,
.data = nullptr};
}
- virtual ExitResult
+ virtual Skill::ExitResult
exit(const SpecializedExitInput& in)
{
- return ExitResult{.status = TerminatedSkillStatus::Succeeded};
+ return Skill::ExitResult{.status = TerminatedSkillStatus::Succeeded};
}
- InitResult
+ Skill::InitResult
init() final
{
- AronT p;
- p.fromAron(this->parameters);
-
SpecializedInitInput i;
i.executorName = this->executorName;
- i.parameters = p;
+ i.parameters = this->parameters;
return this->init(i);
}
- MainResult
+ Skill::MainResult
main() final
{
- AronT p;
- p.fromAron(this->parameters);
-
SpecializedMainInput i;
i.executorName = this->executorName;
i.callback = this->callback;
- i.parameters = p;
+ i.parameters = this->parameters;
return this->main(i);
}
- ExitResult
+ Skill::ExitResult
exit() final
{
- AronT p;
- p.fromAron(this->parameters);
-
SpecializedExitInput i;
i.executorName = this->executorName;
- i.parameters = p;
+ i.parameters = this->parameters;
return this->exit(i);
}
};
diff --git a/source/RobotAPI/libraries/skills/provider/SkillProviderComponentPlugin.cpp b/source/RobotAPI/libraries/skills/provider/SkillProviderComponentPlugin.cpp
index e9e86b054..a42d78082 100644
--- a/source/RobotAPI/libraries/skills/provider/SkillProviderComponentPlugin.cpp
+++ b/source/RobotAPI/libraries/skills/provider/SkillProviderComponentPlugin.cpp
@@ -233,11 +233,11 @@ namespace armarx::plugins
wrapper->execution.join();
// tidy up map of executions
- const std::unique_lock l2{skillExecutionsMutex};
- if (auto it = skillExecutions.find(executionId); it != skillExecutions.end())
- {
- skillExecutions.erase(it);
- }
+ // const std::unique_lock l2{skillExecutionsMutex};
+ // if (auto it = skillExecutions.find(executionId); it != skillExecutions.end())
+ // {
+ // skillExecutions.erase(it);
+ // }
}
return ret;
}
diff --git a/source/RobotAPI/libraries/skills/provider/detail/SkillImplementationWrapper.cpp b/source/RobotAPI/libraries/skills/provider/detail/SkillImplementationWrapper.cpp
index b9529ee7f..60db0fa91 100644
--- a/source/RobotAPI/libraries/skills/provider/detail/SkillImplementationWrapper.cpp
+++ b/source/RobotAPI/libraries/skills/provider/detail/SkillImplementationWrapper.cpp
@@ -127,7 +127,7 @@ namespace armarx
{ updateStatus(s, d); });
// set initial parameters that were attached to the execution request (only add as we are not sure whether some updates already arrived)
- skill->updateParameters(initial_aron_params);
+ this->updateSkillParameters(initial_aron_params);
auto makeAbortedResult = [&](const std::string& message)
{
--
GitLab