diff --git a/source/RobotAPI/components/skills/SkillProviderExample/BusyWaiting.cpp b/source/RobotAPI/components/skills/SkillProviderExample/BusyWaiting.cpp new file mode 100644 index 0000000000000000000000000000000000000000..209c73778ec31686d50b9c32d3bb244b13e1862d --- /dev/null +++ b/source/RobotAPI/components/skills/SkillProviderExample/BusyWaiting.cpp @@ -0,0 +1,32 @@ +#include "BusyWaiting.h" + +#include <chrono> +#include <thread> + +namespace armarx::skills::provider +{ + + BusyWaitingSkill::BusyWaitingSkill() : SimpleSkill(GetSkillDescription()) + { + } + + SkillDescription + BusyWaitingSkill::GetSkillDescription() + { + return SkillDescription{.skillId = SkillID{.skillName = "BusyWaiting"}, + .description = "This skill takes two seconds", + .timeout = armarx::core::time::Duration::MilliSeconds(10000)}; + } + + Skill::MainResult + BusyWaitingSkill::main(const MainInput& in) + { + for (unsigned int i = 0; i < 10; i++) + { + throwIfSkillShouldTerminate(); + std::this_thread::sleep_for(std::chrono::milliseconds(200)); + } + + return {TerminatedSkillStatus::Succeeded, nullptr}; + } +} // namespace armarx::skills::provider diff --git a/source/RobotAPI/components/skills/SkillProviderExample/BusyWaiting.h b/source/RobotAPI/components/skills/SkillProviderExample/BusyWaiting.h new file mode 100644 index 0000000000000000000000000000000000000000..87d00c69fb557749e8129c174c630a3bfc2957cb --- /dev/null +++ b/source/RobotAPI/components/skills/SkillProviderExample/BusyWaiting.h @@ -0,0 +1,42 @@ + +/* + * This file is part of ArmarX. + * + * ArmarX is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + * + * ArmarX is distributed in the hope that it will be useful, but + * WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see <http://www.gnu.org/licenses/>. + * + * @author Andre Meixner ( andre dot meixner at kit dot edu ) + * @date 2024 + * @copyright http://www.gnu.org/licenses/gpl-2.0.txt + * GNU General Public License + */ + +#pragma once + + +// RobotAPI +#include <RobotAPI/libraries/skills/provider/SimpleSkill.h> + +namespace armarx::skills::provider +{ + + class BusyWaitingSkill : public SimpleSkill + { + public: + BusyWaitingSkill(); + + static SkillDescription GetSkillDescription(); + + private: + Skill::MainResult main(const MainInput&) final; + }; +} // namespace armarx::skills::provider diff --git a/source/RobotAPI/components/skills/SkillProviderExample/CMakeLists.txt b/source/RobotAPI/components/skills/SkillProviderExample/CMakeLists.txt index 56bfd4d8af2866f732345b2e65f8ab6ce6463fff..fbebe63f4288a73b4a445d98769020f68830225d 100644 --- a/source/RobotAPI/components/skills/SkillProviderExample/CMakeLists.txt +++ b/source/RobotAPI/components/skills/SkillProviderExample/CMakeLists.txt @@ -19,6 +19,10 @@ set(SOURCES Callback.cpp Timeout.cpp Segfault.cpp + RandomChaining.cpp + InstantKill.cpp + BusyWaiting.cpp + Recursive.cpp ) set(HEADERS @@ -29,6 +33,10 @@ set(HEADERS Callback.h Timeout.h Segfault.h + RandomChaining.h + InstantKill.h + BusyWaiting.h + Recursive.h ) armarx_add_component("${SOURCES}" "${HEADERS}") @@ -38,6 +46,7 @@ armarx_enable_aron_file_generation_for_target( ${ARMARX_COMPONENT_NAME} ARON_FILES aron/HelloWorldAcceptedType.xml + aron/RecursiveSkillParams.xml ) #generate the application diff --git a/source/RobotAPI/components/skills/SkillProviderExample/Chaining.cpp b/source/RobotAPI/components/skills/SkillProviderExample/Chaining.cpp index 983902512c03bc28c8d4429fd22c4690aeb14833..843f1a84dbf224a7af14fb354e0141e2c93fb2df 100644 --- a/source/RobotAPI/components/skills/SkillProviderExample/Chaining.cpp +++ b/source/RobotAPI/components/skills/SkillProviderExample/Chaining.cpp @@ -12,7 +12,7 @@ namespace armarx::skills::provider SkillDescription ChainingSkill::GetSkillDescription() { - return SkillDescription{.skillId = armarx::skills::SkillID{.skillName = "ChainingSkill"}, + return SkillDescription{.skillId = armarx::skills::SkillID{.skillName = "Chaining"}, .description = "This skill calls the Timeout skill three times. The last " "execution is aborted due to a timeout of this skill.", diff --git a/source/RobotAPI/components/skills/SkillProviderExample/Incomplete.cpp b/source/RobotAPI/components/skills/SkillProviderExample/Incomplete.cpp index 8d6b4f277dcb36adc2b73bfe94b45b9adb500b22..b5afe130d00aed97e443a1263d6db97d313bab39 100644 --- a/source/RobotAPI/components/skills/SkillProviderExample/Incomplete.cpp +++ b/source/RobotAPI/components/skills/SkillProviderExample/Incomplete.cpp @@ -17,7 +17,7 @@ namespace armarx::skills::provider IncompleteSkill::GetSkillDescription() { auto d = HelloWorldSkill::GetSkillDescription(); - return SkillDescription{.skillId = {.skillName = "IncompleteSkill"}, + return SkillDescription{.skillId = {.skillName = "Incomplete"}, .description = d.description, .timeout = d.timeout + armarx::core::time::Duration::Seconds(2), .parametersType = d.parametersType}; diff --git a/source/RobotAPI/components/skills/SkillProviderExample/InstantKill.cpp b/source/RobotAPI/components/skills/SkillProviderExample/InstantKill.cpp new file mode 100644 index 0000000000000000000000000000000000000000..fa81224c1a659de619a13b5c20133e7d6e24e365 --- /dev/null +++ b/source/RobotAPI/components/skills/SkillProviderExample/InstantKill.cpp @@ -0,0 +1,41 @@ + +#include <cstdlib> +#include "InstantKill.h" + +#include <thread> +#include <chrono> + +namespace armarx::skills::provider +{ + + InstantKillSkill::InstantKillSkill() : SimpleSkill(GetSkillDescription()) + { + } + + SkillDescription + InstantKillSkill::GetSkillDescription() + { + return SkillDescription{.skillId = armarx::skills::SkillID{.skillName = "InstantKill"}, + .description = + "This skill calls Timeout and instantly aboirts it.", + .timeout = armarx::core::time::Duration::MilliSeconds(50000)}; + } + + Skill::MainResult + InstantKillSkill::main(const MainInput& in) + { + + this->throwIfSkillShouldTerminate(); + + SkillProxy prx( + manager, + skills::SkillID{.providerId = *getSkillId().providerId, .skillName = "Chaining"}); + + for (unsigned int i = 0; i < 10; ++i) + { + auto id = callSubskillAsync(prx); + prx.abortSkillAsync(id); + } + return {TerminatedSkillStatus::Succeeded, nullptr}; + } +} // namespace armarx::skills::provider diff --git a/source/RobotAPI/components/skills/SkillProviderExample/InstantKill.h b/source/RobotAPI/components/skills/SkillProviderExample/InstantKill.h new file mode 100644 index 0000000000000000000000000000000000000000..d5487d2fca63995554e877cef03c5df1f18ea2e6 --- /dev/null +++ b/source/RobotAPI/components/skills/SkillProviderExample/InstantKill.h @@ -0,0 +1,40 @@ + +/* + * This file is part of ArmarX. + * + * ArmarX is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + * + * ArmarX is distributed in the hope that it will be useful, but + * WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see <http://www.gnu.org/licenses/>. + * + * @author Fabian Ternava ( fabian dot ternava at kit dot edu ) + * @date 2024 + * @copyright http://www.gnu.org/licenses/gpl-2.0.txt + * GNU General Public License + */ + +#pragma once + +// RobotAPI +#include <RobotAPI/libraries/skills/provider/SimpleSkill.h> + +namespace armarx::skills::provider +{ + class InstantKillSkill : public SimpleSkill + { + public: + InstantKillSkill(); + + static SkillDescription GetSkillDescription(); + + private: + Skill::MainResult main(const MainInput&) final; + }; +} // namespace armarx::skills::provider diff --git a/source/RobotAPI/components/skills/SkillProviderExample/RandomChaining.cpp b/source/RobotAPI/components/skills/SkillProviderExample/RandomChaining.cpp new file mode 100644 index 0000000000000000000000000000000000000000..4ca2e3f337bb3bfb41109a525373d2df19d814dd --- /dev/null +++ b/source/RobotAPI/components/skills/SkillProviderExample/RandomChaining.cpp @@ -0,0 +1,72 @@ + +#include <cstdlib> +#include "RandomChaining.h" + +#include <thread> +#include <chrono> + +namespace armarx::skills::provider +{ + namespace util + { + int randomgen(int max, int min) //Pass in range + { + srand(time(NULL)); //Changed from rand(). srand() seeds rand for you. + int random = rand() % max + min; + return random; + } + } + + RandomChainingSkill::RandomChainingSkill() : SimpleSkill(GetSkillDescription()) + { + } + + SkillDescription + RandomChainingSkill::GetSkillDescription() + { + return SkillDescription{.skillId = armarx::skills::SkillID{.skillName = "RandomChaining"}, + .description = + "This skill calls 100 random subskills from the skillProviderExample excluding the segfault skill.", + .timeout = armarx::core::time::Duration::MilliSeconds(120000)}; + } + + Skill::MainResult + RandomChainingSkill::main(const MainInput& in) + { + std::vector<std::string> subskillNames = { + "Timeout", "Chaining", "Foo", "HelloWorld", + "Incomplete", "ShowMeCallbacks", "BusyWaiting", "Recursive" + }; + + ARMARX_CHECK(subskillNames.size() > 0); + + for (unsigned int i = 0; i < 100; ++i) + { + this->throwIfSkillShouldTerminate(); + + auto index = util::randomgen(subskillNames.size() -1, 0); + + auto subskillName = subskillNames[index]; + + SkillProxy prx( + manager, + skills::SkillID{.providerId = *getSkillId().providerId, .skillName = subskillName}); + + if (util::randomgen(10, 0) < 2) + { + callSubskill(prx); + } + else + { + callSubskillAsync(prx); + + auto sleep_milliseconds = util::randomgen(1000, 0); + + ARMARX_INFO << "SLEEP FOR " << sleep_milliseconds << "ms"; + std::this_thread::sleep_for(std::chrono::milliseconds(sleep_milliseconds)); + } + } + + return {TerminatedSkillStatus::Succeeded, nullptr}; + } +} // namespace armarx::skills::provider diff --git a/source/RobotAPI/components/skills/SkillProviderExample/RandomChaining.h b/source/RobotAPI/components/skills/SkillProviderExample/RandomChaining.h new file mode 100644 index 0000000000000000000000000000000000000000..375a8daf710c42037c5daf1bd0af1a32dbefdf3c --- /dev/null +++ b/source/RobotAPI/components/skills/SkillProviderExample/RandomChaining.h @@ -0,0 +1,40 @@ + +/* + * This file is part of ArmarX. + * + * ArmarX is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + * + * ArmarX is distributed in the hope that it will be useful, but + * WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see <http://www.gnu.org/licenses/>. + * + * @author Fabian Reister ( fabian dot reister at kit dot edu ) + * @date 2021 + * @copyright http://www.gnu.org/licenses/gpl-2.0.txt + * GNU General Public License + */ + +#pragma once + +// RobotAPI +#include <RobotAPI/libraries/skills/provider/SimpleSkill.h> + +namespace armarx::skills::provider +{ + class RandomChainingSkill : public SimpleSkill + { + public: + RandomChainingSkill(); + + static SkillDescription GetSkillDescription(); + + private: + Skill::MainResult main(const MainInput&) final; + }; +} // namespace armarx::skills::provider diff --git a/source/RobotAPI/components/skills/SkillProviderExample/Recursive.cpp b/source/RobotAPI/components/skills/SkillProviderExample/Recursive.cpp new file mode 100644 index 0000000000000000000000000000000000000000..7ad3c457ac45d3cde2daf94fde6e89d11ee04b54 --- /dev/null +++ b/source/RobotAPI/components/skills/SkillProviderExample/Recursive.cpp @@ -0,0 +1,49 @@ +#include "Recursive.h" + +#include "RobotAPI/libraries/skills/core/SkillDescription.h" + +namespace armarx::skills::provider +{ + + RecursiveSkill::RecursiveSkill() : SimpleSpecializedSkill<skills::Example::RecursiveSkillParams>(GetSkillDescription()) + { + } + + SkillDescription + RecursiveSkill::GetSkillDescription() + { + armarx::skills::Example::RecursiveSkillParams root_profile_params; + root_profile_params.n = 10; + + return SkillDescription{.skillId = skills::SkillID{.skillName = "Recursive"}, + .description = "This skill calls itself recursively {n} times", + .rootProfileDefaults = root_profile_params.toAron(), + .timeout = armarx::core::time::Duration::MilliSeconds(10000), + .parametersType = + armarx::skills::Example::RecursiveSkillParams::ToAronType(), + .resultType = armarx::skills::Example::RecursiveSkillParams::ToAronType()}; + } + + Skill::MainResult + RecursiveSkill::main(const SpecializedMainInput& in) + { + const int n = in.parameters.n; + + if (n > 0) + { + SkillProxy prx(manager, + skills::SkillID{.providerId = *getSkillId().providerId, .skillName = "Recursive"}); + + armarx::skills::Example::RecursiveSkillParams params; + params.n = n - 1; + + std::this_thread::sleep_for(std::chrono::milliseconds(100)); + + callSubskill(prx, params.toAron()); + } + + throwIfSkillShouldTerminate(); + + return {TerminatedSkillStatus::Succeeded, nullptr}; + } +} // namespace armarx::skills::provider diff --git a/source/RobotAPI/components/skills/SkillProviderExample/Recursive.h b/source/RobotAPI/components/skills/SkillProviderExample/Recursive.h new file mode 100644 index 0000000000000000000000000000000000000000..ac96cce58a07cea59f6b118f6b410a46d7e09210 --- /dev/null +++ b/source/RobotAPI/components/skills/SkillProviderExample/Recursive.h @@ -0,0 +1,42 @@ + +/* + * This file is part of ArmarX. + * + * ArmarX is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + * + * ArmarX is distributed in the hope that it will be useful, but + * WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see <http://www.gnu.org/licenses/>. + * + * @author Andre Meixner ( andre dot meixner at kit dot edu ) + * @date 2024 + * @copyright http://www.gnu.org/licenses/gpl-2.0.txt + * GNU General Public License + */ + +#pragma once + +// RobotAPI +#include <RobotAPI/libraries/skills/provider/SimpleSpecializedSkill.h> + +#include <RobotAPI/components/skills/SkillProviderExample/aron/RecursiveSkillParams.aron.generated.h> + +namespace armarx::skills::provider +{ + class RecursiveSkill : public SimpleSpecializedSkill<skills::Example::RecursiveSkillParams> + { + public: + RecursiveSkill(); + + static SkillDescription GetSkillDescription(); + + private: + Skill::MainResult main(const SpecializedMainInput&) final; + }; +} // namespace armarx::skills::provider diff --git a/source/RobotAPI/components/skills/SkillProviderExample/SkillProviderExample.cpp b/source/RobotAPI/components/skills/SkillProviderExample/SkillProviderExample.cpp index d292d7465bd8df09806fbe77b235478425f3442b..fe98dc157eb556fb51991d48f5f4428155117950 100644 --- a/source/RobotAPI/components/skills/SkillProviderExample/SkillProviderExample.cpp +++ b/source/RobotAPI/components/skills/SkillProviderExample/SkillProviderExample.cpp @@ -61,6 +61,20 @@ namespace armarx::skills::provider // segfault ARMARX_INFO << "Adding skill SegfaultSkill"; addSkillFactory<SegfaultSkill>(); + + // random + ARMARX_INFO << "Adding skill RandomChainingSkill"; + addSkillFactory<RandomChainingSkill>(); + + // insta kill + ARMARX_INFO << "Adding skill InstaKill"; + addSkillFactory<InstantKillSkill>(); + + ARMARX_INFO << "Adding skill BusyWaiting"; + addSkillFactory<BusyWaitingSkill>(); + + ARMARX_INFO << "Adding skill Recursive"; + addSkillFactory<RecursiveSkill>(); } void diff --git a/source/RobotAPI/components/skills/SkillProviderExample/SkillProviderExample.h b/source/RobotAPI/components/skills/SkillProviderExample/SkillProviderExample.h index be0c6b3704daafcbec412fd0b9dfd9690485e8f1..63168c11da2be2a36eb6a0a911dab85d9cf1ae01 100644 --- a/source/RobotAPI/components/skills/SkillProviderExample/SkillProviderExample.h +++ b/source/RobotAPI/components/skills/SkillProviderExample/SkillProviderExample.h @@ -35,6 +35,10 @@ #include "Incomplete.h" #include "Segfault.h" #include "Timeout.h" +#include "RandomChaining.h" +#include "InstantKill.h" +#include "BusyWaiting.h" +#include "Recursive.h" namespace armarx::skills::provider { diff --git a/source/RobotAPI/components/skills/SkillProviderExample/aron/RecursiveSkillParams.xml b/source/RobotAPI/components/skills/SkillProviderExample/aron/RecursiveSkillParams.xml new file mode 100644 index 0000000000000000000000000000000000000000..47d69a1b7f090a8ac43adf8ebc3df266a56adf8b --- /dev/null +++ b/source/RobotAPI/components/skills/SkillProviderExample/aron/RecursiveSkillParams.xml @@ -0,0 +1,11 @@ +<?xml version="1.0" encoding="UTF-8" ?> +<AronTypeDefinition> + <GenerateTypes> + <Object name='armarx::skills::Example::RecursiveSkillParams'> + <ObjectChild key='n'> + <int32 /> + </ObjectChild> + </Object> + </GenerateTypes> + +</AronTypeDefinition> \ No newline at end of file diff --git a/source/RobotAPI/libraries/armem_skills/server/segment/ExecutableSkillLibrarySegment.cpp b/source/RobotAPI/libraries/armem_skills/server/segment/ExecutableSkillLibrarySegment.cpp index 2f4c14e82d1c701363fbc81e3d974b47a0a43cd1..bde88afce40524ef09bb216e31a1e856311b1724 100644 --- a/source/RobotAPI/libraries/armem_skills/server/segment/ExecutableSkillLibrarySegment.cpp +++ b/source/RobotAPI/libraries/armem_skills/server/segment/ExecutableSkillLibrarySegment.cpp @@ -54,7 +54,7 @@ namespace armarx::skills::segment entityUpdate.entityID = provId.withEntityName(d.skillId.skillName); // Commit data to memory and notify - iceMemory.commit(commit); + iceMemory.commitLocking(commit); } } diff --git a/source/RobotAPI/libraries/armem_skills/server/segment/SkillEventSegment.cpp b/source/RobotAPI/libraries/armem_skills/server/segment/SkillEventSegment.cpp index ba3d3b6aa9ddd54c24622d8163d575c11cb502c8..057c7573743497ff8f518bd0ab2d1d0d90153b8a 100644 --- a/source/RobotAPI/libraries/armem_skills/server/segment/SkillEventSegment.cpp +++ b/source/RobotAPI/libraries/armem_skills/server/segment/SkillEventSegment.cpp @@ -20,7 +20,7 @@ namespace armarx::skills::segment void SkillEventCoreSegment::defineProperties(PropertyDefinitionsPtr defs, const std::string& prefix) { - this->setDefaultMaxHistorySize(10); + this->setDefaultMaxHistorySize(300); Base::defineProperties(defs, prefix); } @@ -60,7 +60,8 @@ namespace armarx::skills::segment auto coreSegment = this->segmentPtr; ARMARX_CHECK(coreSegment); - coreSegment->forEachInstance( + coreSegment->doLocked([&](){ + coreSegment->forEachInstance( [&](const armem::wm::EntityInstance& i) { auto event = i.dataAs<armarx::skills::arondto::SkillStatusUpdate>(); @@ -75,6 +76,7 @@ namespace armarx::skills::segment // set or replace ret[up.executionId] = up; }); + }); // for (const auto& [k, v] : ret) // { @@ -91,7 +93,8 @@ namespace armarx::skills::segment auto coreSegment = this->segmentPtr; ARMARX_CHECK(coreSegment); - coreSegment->forEachInstance( + coreSegment->doLocked([&](){ + coreSegment->forEachInstance( [&](const armem::wm::EntityInstance& i) { auto event = i.dataAs<armarx::skills::arondto::SkillStatusUpdate>(); @@ -106,6 +109,7 @@ namespace armarx::skills::segment } } }); + }); return ret; } } // namespace armarx::skills::segment diff --git a/source/RobotAPI/libraries/armem_skills/server/segment/SkillExecutionRequestSegment.cpp b/source/RobotAPI/libraries/armem_skills/server/segment/SkillExecutionRequestSegment.cpp index a19463b9d5a2f58ba0713d422ad7575f3077b785..1fd19f7ff174f430c799b0e7c8fa88c7696541a0 100644 --- a/source/RobotAPI/libraries/armem_skills/server/segment/SkillExecutionRequestSegment.cpp +++ b/source/RobotAPI/libraries/armem_skills/server/segment/SkillExecutionRequestSegment.cpp @@ -20,7 +20,7 @@ namespace armarx::skills::segment SkillExecutionRequestCoreSegment::defineProperties(PropertyDefinitionsPtr defs, const std::string& prefix) { - this->setDefaultMaxHistorySize(10); + this->setDefaultMaxHistorySize(50); Base::defineProperties(defs, prefix); } @@ -86,6 +86,6 @@ namespace armarx::skills::segment } - iceMemory.commit(comm); + iceMemory.commitLocking(comm); } } // namespace armarx::skills::segment diff --git a/source/RobotAPI/libraries/skills/core/Skill.cpp b/source/RobotAPI/libraries/skills/core/Skill.cpp index a01eb4908baed4e5f6dc18c76f5493053d4bb886..62fee239e4bf83fdfe6cb3d9a09428484280d4aa 100644 --- a/source/RobotAPI/libraries/skills/core/Skill.cpp +++ b/source/RobotAPI/libraries/skills/core/Skill.cpp @@ -38,13 +38,23 @@ namespace armarx return ret; } + skills::SkillExecutionID - Skill::callSubskillAsync(const skills::SkillProxy& prx, const aron::data::DictPtr& params) + Skill::callSubskillAsync(const skills::SkillProxy& prx) { - std::unique_lock l(subskillsMutex); + return callSubskillAsync(prx, prx.getRootProfileParameters()); + } + skills::SkillExecutionID + Skill::callSubskillAsync(const skills::SkillProxy& prx, const aron::data::DictPtr& params) + { std::string executorHistory = this->executorName + "->" + getSkillId().toString(); + + throwIfSkillShouldTerminate(); auto eid = prx.executeSkillAsync(executorHistory, params); + + std::unique_lock l(subskillsMutex); + throwIfSkillShouldTerminate([&](){prx.abortSkillAsync(eid);}); // also notify newly added skill as it was not added to subskills list yet this->subskills.push_back(eid); return eid; } @@ -331,6 +341,11 @@ namespace armarx void Skill::notifySkillToStop() { + if (stopped) + { + // skill already got stopped. Ignore + return; + } std::scoped_lock l(subskillsMutex); stopped = true; _onStopRequested(); @@ -340,6 +355,12 @@ namespace armarx void Skill::notifyTimeoutReached() { + if (stopped || timeoutReached) + { + // skill already got timeoutReached. Ignore + return; + } + std::scoped_lock l(subskillsMutex); timeoutReached = true; _onTimeoutReached(); @@ -356,10 +377,13 @@ namespace armarx void Skill::_onTimeoutReached() { + // WE ASSUME THAT THE LOCK IS ALREADY TAKEN + if (!manager) { return; } + for (const auto& execId : subskills) { manager->abortSkillAsync(execId.toManagerIce()); @@ -369,10 +393,13 @@ namespace armarx void Skill::_onStopRequested() { + // WE ASSUME THAT THE LOCK IS ALREADY TAKEN + if (!manager) { return; } + for (const auto& execId : subskills) { manager->abortSkillAsync(execId.toManagerIce()); diff --git a/source/RobotAPI/libraries/skills/manager/SkillManagerComponentPlugin.cpp b/source/RobotAPI/libraries/skills/manager/SkillManagerComponentPlugin.cpp index 14e4e64e8a0ba1af5d8a22a824bc76543cc91fe2..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 @@ -207,17 +201,11 @@ namespace armarx::plugins executionId.skillId.providerId = provderId; return executionId; } - 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 @@ -233,6 +221,8 @@ namespace armarx::plugins SkillManagerComponentPlugin::updateSkillParameters(const skills::SkillExecutionID& executionId, const aron::data::DictPtr& data) { + ARMARX_DEBUG << "updateSkillParameters for skill " << executionId.skillId; + ARMARX_CHECK(executionId.skillId.isFullySpecified()) << "Got: " << executionId.skillId.toString(); @@ -255,14 +245,16 @@ 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; } - catch (...) + catch (const std::exception &e) { - ARMARX_WARNING << __PRETTY_FUNCTION__ << ": Found buggy skill provider '" - << providerId << "'. Removing it from skills on next execute."; + l.lock(); + + handleExceptionNonLocking(__PRETTY_FUNCTION__, e, providerId); + return false; } } @@ -275,6 +267,8 @@ namespace armarx::plugins bool SkillManagerComponentPlugin::abortSkill(const skills::SkillExecutionID& executionId) { + ARMARX_DEBUG << "abortSkill for skill " << executionId.skillId; + ARMARX_CHECK(executionId.skillId.isFullySpecified()) << "Got: " << executionId.skillId.toString(); @@ -296,14 +290,16 @@ 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; } - catch (...) + catch (const std::exception &e) { - ARMARX_WARNING << __PRETTY_FUNCTION__ << ": Found buggy skill provider '" - << providerId << "'. Removing it from skills on next execute."; + l.lock(); + + handleExceptionNonLocking(__PRETTY_FUNCTION__, e, providerId); + return false; } } @@ -339,10 +335,10 @@ namespace armarx::plugins auto async = provider->begin_abortSkill(executionId.toProviderIce()); return true; } - catch (...) + catch (const std::exception &e) { - ARMARX_WARNING << __PRETTY_FUNCTION__ << ": Found buggy skill provider '" - << providerId << "'. Removing it from skills on next execute."; + handleExceptionNonLocking(__PRETTY_FUNCTION__, e, providerId); + return false; } } @@ -355,6 +351,8 @@ namespace armarx::plugins std::optional<skills::SkillDescription> SkillManagerComponentPlugin::getSkillDescription(const skills::SkillID& skillId) { + ARMARX_DEBUG << "getSkillDescription for skill " << skillId; + ARMARX_CHECK(skillId.isFullySpecified()) << "Got: " << skillId.toString(); std::unique_lock l(skillProviderMapMutex); @@ -379,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) @@ -392,18 +390,9 @@ namespace armarx::plugins skills::SkillDescription::FromIce(provider_desc_ice.value(), providerId); return desc; } - catch (...) + catch (const std::exception &e) { - ARMARX_WARNING << __PRETTY_FUNCTION__ - << ": Found disconnected or buggy skill provider '" << providerId - << "' during execution. Removing it from skills."; - skillProviderMap.erase(it); - - 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 @@ -444,11 +433,12 @@ 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; @@ -484,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) @@ -497,18 +487,11 @@ namespace armarx::plugins provider_statusUpdate_ice.value(), providerId); return statusUpdate; } - catch (...) + catch (const std::exception &e) { - ARMARX_WARNING << __PRETTY_FUNCTION__ - << ": Found disconnected or buggy skill provider '" << providerId - << "' during execution. Removing it from skills."; - skillProviderMap.erase(it); + l.lock(); - 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 @@ -549,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 8aeea6f1ab7d387ce6c46e4212e1455c1c2a9435..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> @@ -53,9 +56,17 @@ namespace armarx::plugins std::map<skills::SkillExecutionID, skills::SkillStatusUpdate> getSkillExecutionStatuses(); + protected: 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; diff --git a/source/RobotAPI/libraries/skills/provider/SkillProviderComponentPlugin.cpp b/source/RobotAPI/libraries/skills/provider/SkillProviderComponentPlugin.cpp index 730ce95a67671fad8464f4b886431a544a6410e3..09b9d248be794aa673eea7db7a051d405004d739 100644 --- a/source/RobotAPI/libraries/skills/provider/SkillProviderComponentPlugin.cpp +++ b/source/RobotAPI/libraries/skills/provider/SkillProviderComponentPlugin.cpp @@ -129,6 +129,7 @@ namespace armarx::plugins return std::nullopt; } + std::scoped_lock l2{skillExecutions.at(execId).skillStatusesMutex}; return skillExecutions.at(execId).statusUpdate; } @@ -223,9 +224,16 @@ namespace armarx::plugins [&]() { // execute waits until the previous execution finishes. - auto x = wrapper->executeSkill(); - ret.result = x.result; - ret.status = armarx::skills::toSkillStatus(x.status); + try + { + auto x = wrapper->executeSkill(); + ret.result = x.result; + ret.status = armarx::skills::toSkillStatus(x.status); + } + catch(std::exception& e) + { + ARMARX_WARNING << "Got an uncatched Exception when executing a skill. Exception was: " << e.what(); + } }); } // release lock. We don't know how long the skill needs to finish and we have to release the lock for being able to abort the execution @@ -249,19 +257,28 @@ namespace armarx::plugins { ARMARX_CHECK(executionRequest.skillId.isFullySpecified()); - skills::SkillExecutionID executionId{executionRequest.skillId, - executionRequest.executorName, - armarx::core::time::DateTime::Now()}; + skills::SkillExecutionID executionId; + skills::detail::SkillRuntime* wrapper; { auto l1 = std::unique_lock{skillFactoriesMutex}; - const auto& fac = getSkillFactory(executionId.skillId); - ARMARX_CHECK(fac) << "Could not find a factory for skill " << executionId.skillId; + const auto& fac = getSkillFactory(executionRequest.skillId); + ARMARX_CHECK(fac) << "Could not find a factory for skill " << executionRequest.skillId; { const std::unique_lock l2{skillExecutionsMutex}; + + executionId = skills::SkillExecutionID{executionRequest.skillId, + executionRequest.executorName, + armarx::core::time::DateTime::Now()}; + + if (skillExecutions.count(executionId) > 0) + { + ARMARX_ERROR << "SkillsExecutionID already exists! This is undefined behaviour and should not occur!"; + } + auto it = skillExecutions.emplace(std::piecewise_construct, std::make_tuple(executionId), @@ -276,8 +293,15 @@ namespace armarx::plugins wrapper->execution = std::thread( [&]() { - // execute waits until the previous execution finishes. - auto x = wrapper->executeSkill(); + try + { + // execute waits until the previous execution finishes. + auto x = wrapper->executeSkill(); + } + catch(std::exception& e) + { + ARMARX_WARNING << "Got an uncatched Exception when executing a skill. Exception was: " << e.what(); + } }); } diff --git a/source/RobotAPI/libraries/skills_gui/memory/SkillManagerWrapper.cpp b/source/RobotAPI/libraries/skills_gui/memory/SkillManagerWrapper.cpp index 1b7b492b5c24dcc87f536eccb2dffae29a94ca89..d19f45df54da7f914889b7243f72df840e183f42 100644 --- a/source/RobotAPI/libraries/skills_gui/memory/SkillManagerWrapper.cpp +++ b/source/RobotAPI/libraries/skills_gui/memory/SkillManagerWrapper.cpp @@ -26,7 +26,7 @@ namespace armarx::skills::gui // we return this map StatusMap statusMap; - auto currentManagerStatuses = memory->getSkillExecutionStatuses(); + auto currentManagerStatuses = memory->ice_invocationTimeout(10000)->getSkillExecutionStatuses(); // iterate over raw data and convert to common types for (const auto& [k, v] : currentManagerStatuses) @@ -136,7 +136,7 @@ namespace armarx::skills::gui SkillMap skills; - auto managerSkills = memory->getSkillDescriptions(); + auto managerSkills = memory->ice_invocationTimeout(5000)->getSkillDescriptions(); for (const auto& [sid, desc] : managerSkills) { @@ -295,7 +295,7 @@ namespace armarx::skills::gui { ARMARX_INFO << "Aborting skill '" << executionId.skillId.skillName << "'..."; std::scoped_lock l(mutex_memory); - this->memory->abortSkillAsync(executionId.toManagerIce()); + this->memory->ice_invocationTimeout(5000)->abortSkillAsync(executionId.toManagerIce()); } catch (Ice::Exception const& e) { @@ -381,7 +381,7 @@ namespace armarx::skills::gui try { std::scoped_lock l(mutex_memory); - memory->executeSkillAsync(req.toManagerIce()); + memory->ice_invocationTimeout(5000)->executeSkillAsync(req.toManagerIce()); } catch (Ice::Exception const& e) {