From 5f45a2c26392f581f663cb648f19583b0f64335b Mon Sep 17 00:00:00 2001 From: AndyZe Date: Wed, 20 Nov 2024 14:18:39 -0600 Subject: [PATCH 1/5] Additional XML verification for Control nodes --- src/xml_parsing.cpp | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/src/xml_parsing.cpp b/src/xml_parsing.cpp index 32a1e42ab..50af9be2a 100644 --- a/src/xml_parsing.cpp +++ b/src/xml_parsing.cpp @@ -541,6 +541,18 @@ void VerifyXML(const std::string& xml_text, ThrowError(line_number, std::string("The node <") + name + "> must have 1 or more children"); } + if (name == "ReactiveSequence") + { + const std::string child_name = node->FirstChildElement()->Name(); + const auto child_search = registered_nodes.find(child_name); + auto child_type = child_search->second; + // TODO: what this really needs to check is if the child is async + if (child_type != NodeType::CONDITION) + { + ThrowError(line_number, + std::string("The first child of a ReactiveSequence cannot be asynchronous")); + } + } } } //recursion From ff14d914513b1f69751957909809ad26d9ed8caf Mon Sep 17 00:00:00 2001 From: AndyZe Date: Wed, 20 Nov 2024 15:10:30 -0600 Subject: [PATCH 2/5] Parse for async nodes based on node name --- src/xml_parsing.cpp | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/xml_parsing.cpp b/src/xml_parsing.cpp index 50af9be2a..060b10d1f 100644 --- a/src/xml_parsing.cpp +++ b/src/xml_parsing.cpp @@ -541,16 +541,17 @@ void VerifyXML(const std::string& xml_text, ThrowError(line_number, std::string("The node <") + name + "> must have 1 or more children"); } - if (name == "ReactiveSequence") + if(name == "ReactiveSequence") { const std::string child_name = node->FirstChildElement()->Name(); const auto child_search = registered_nodes.find(child_name); auto child_type = child_search->second; - // TODO: what this really needs to check is if the child is async - if (child_type != NodeType::CONDITION) + if(child_type == NodeType::CONTROL && + ((child_name == "ThreadedAction") || (child_name == "StatefulActionNode") || + (child_name == "CoroActionNode"))) { - ThrowError(line_number, - std::string("The first child of a ReactiveSequence cannot be asynchronous")); + ThrowError(line_number, std::string("The first child of a ReactiveSequence " + "cannot be asynchronous")); } } } From ff029cc821501766224c63e292a19cfebf0d6b82 Mon Sep 17 00:00:00 2001 From: AndyZe Date: Thu, 28 Nov 2024 19:18:32 -0600 Subject: [PATCH 3/5] Add a unit test --- src/xml_parsing.cpp | 2 +- tests/gtest_reactive.cpp | 28 ++++++++++++++++++++++++++++ 2 files changed, 29 insertions(+), 1 deletion(-) diff --git a/src/xml_parsing.cpp b/src/xml_parsing.cpp index 060b10d1f..1632d02b2 100644 --- a/src/xml_parsing.cpp +++ b/src/xml_parsing.cpp @@ -548,7 +548,7 @@ void VerifyXML(const std::string& xml_text, auto child_type = child_search->second; if(child_type == NodeType::CONTROL && ((child_name == "ThreadedAction") || (child_name == "StatefulActionNode") || - (child_name == "CoroActionNode"))) + (child_name == "CoroActionNode") || (child_name == "AsyncSequence"))) { ThrowError(line_number, std::string("The first child of a ReactiveSequence " "cannot be asynchronous")); diff --git a/tests/gtest_reactive.cpp b/tests/gtest_reactive.cpp index b393c03e8..47e4435ae 100644 --- a/tests/gtest_reactive.cpp +++ b/tests/gtest_reactive.cpp @@ -156,3 +156,31 @@ TEST(Reactive, TestLogging) ASSERT_EQ(observer.getStatistics("testA").success_count, num_ticks); ASSERT_EQ(observer.getStatistics("success").success_count, num_ticks); } + +TEST(Reactive, TwoAsyncNodesInReactiveSequence) +{ + static const char* reactive_xml_text = R"( + + + + + + + + + + + + + + + + +)"; + + BT::BehaviorTreeFactory factory; + std::array counters; + RegisterTestTick(factory, "Test", counters); + + EXPECT_ANY_THROW(auto tree = factory.createTreeFromText(reactive_xml_text)); +} From e87c7127b0e93c1ff5787b6df195d0b3f9148daa Mon Sep 17 00:00:00 2001 From: AndyZe Date: Thu, 28 Nov 2024 19:49:09 -0600 Subject: [PATCH 4/5] Improve the check by counting num async children --- src/xml_parsing.cpp | 26 ++++++++++++++++++-------- 1 file changed, 18 insertions(+), 8 deletions(-) diff --git a/src/xml_parsing.cpp b/src/xml_parsing.cpp index 1632d02b2..26d9cb7b3 100644 --- a/src/xml_parsing.cpp +++ b/src/xml_parsing.cpp @@ -543,15 +543,25 @@ void VerifyXML(const std::string& xml_text, } if(name == "ReactiveSequence") { - const std::string child_name = node->FirstChildElement()->Name(); - const auto child_search = registered_nodes.find(child_name); - auto child_type = child_search->second; - if(child_type == NodeType::CONTROL && - ((child_name == "ThreadedAction") || (child_name == "StatefulActionNode") || - (child_name == "CoroActionNode") || (child_name == "AsyncSequence"))) + size_t async_count = 0; + for(auto child = node->FirstChildElement(); child != nullptr; + child = child->NextSiblingElement()) { - ThrowError(line_number, std::string("The first child of a ReactiveSequence " - "cannot be asynchronous")); + const std::string child_name = node->FirstChildElement()->Name(); + const auto child_search = registered_nodes.find(child_name); + auto child_type = child_search->second; + if(child_type == NodeType::CONTROL && + ((child_name == "ThreadedAction") || + (child_name == "StatefulActionNode") || + (child_name == "CoroActionNode") || (child_name == "AsyncSequence"))) + { + ++async_count; + if(async_count > 1) + { + ThrowError(line_number, std::string("A ReactiveSequence cannot have more " + "than one async child.")); + } + } } } } From b31ae5fd5c5cacc124933b1616421a665d44e0ff Mon Sep 17 00:00:00 2001 From: AndyZe Date: Mon, 2 Dec 2024 11:11:42 -0600 Subject: [PATCH 5/5] Minor update (const) --- src/xml_parsing.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/xml_parsing.cpp b/src/xml_parsing.cpp index 26d9cb7b3..31e2f28f7 100644 --- a/src/xml_parsing.cpp +++ b/src/xml_parsing.cpp @@ -549,7 +549,7 @@ void VerifyXML(const std::string& xml_text, { const std::string child_name = node->FirstChildElement()->Name(); const auto child_search = registered_nodes.find(child_name); - auto child_type = child_search->second; + const auto child_type = child_search->second; if(child_type == NodeType::CONTROL && ((child_name == "ThreadedAction") || (child_name == "StatefulActionNode") ||