Skip to content

Commit 5d53e04

Browse files
facontidavidefacontidavide
committed
detect duplicated instance names
1 parent 4c6946d commit 5d53e04

File tree

2 files changed

+178
-0
lines changed

2 files changed

+178
-0
lines changed

‎src/xml_parsing.cpp‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1010,6 +1010,20 @@ void BT::XMLParser::PImpl::recursivelyCreateSubtree(const std::string& tree_ID,
10101010
subtree_path += subtree_ID + "::" + std::to_string(node->UID());
10111011
}
10121012

1013+
// Check if the path already exists - duplicate paths cause issues in Groot2
1014+
// and TreeObserver (see Groot2 issue #56)
1015+
for(const auto& sub : output_tree.subtrees)
1016+
{
1017+
if(sub->instance_name == subtree_path)
1018+
{
1019+
throw RuntimeError("Duplicate SubTree path detected: '", subtree_path,
1020+
"'. Multiple SubTree nodes with the same 'name' attribute "
1021+
"under the same parent are not allowed. "
1022+
"Please use unique names or omit the 'name' attribute "
1023+
"to auto-generate unique paths.");
1024+
}
1025+
}
1026+
10131027
recursivelyCreateSubtree(subtree_ID,
10141028
subtree_path, // name
10151029
subtree_path + "/", //prefix

‎tests/gtest_subtree.cpp‎

Lines changed: 164 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
#include <gtest/gtest.h>
2+
#include <set>
23
#include "behaviortree_cpp/bt_factory.h"
34
#include "../sample_nodes/dummy_nodes.h"
45
#include "../sample_nodes/movebase_node.h"
@@ -726,3 +727,166 @@ TEST(SubTree, SubtreeNameNotRegistered)
726727
ASSERT_ANY_THROW(auto tree = factory.createTreeFromText(xml_text));
727728
ASSERT_ANY_THROW(factory.registerBehaviorTreeFromText(xml_text));
728729
}
730+
731+
// Test for Groot2 issue #56: duplicate _fullpath when multiple subtrees have the same name
732+
// https://github.com/BehaviorTree/Groot2/issues/56
733+
//
734+
// When two SubTree nodes under the same parent have the same "name" attribute,
735+
// tree creation should fail with a clear error message.
736+
TEST(SubTree, DuplicateSubTreeName_Groot2Issue56)
737+
{
738+
// clang-format off
739+
static const char* xml_text = R"(
740+
<root BTCPP_format="4" main_tree_to_execute="MainTree">
741+
<BehaviorTree ID="MainTree">
742+
<ParallelAll>
743+
<SubTree ID="Worker" name="my_worker"/>
744+
<SubTree ID="Worker" name="my_worker"/>
745+
</ParallelAll>
746+
</BehaviorTree>
747+
748+
<BehaviorTree ID="Worker">
749+
<AlwaysSuccess name="do_work"/>
750+
</BehaviorTree>
751+
</root>
752+
)";
753+
// clang-format on
754+
755+
BehaviorTreeFactory factory;
756+
757+
// Should throw RuntimeError because of duplicate SubTree names
758+
ASSERT_THROW(factory.createTreeFromText(xml_text), RuntimeError);
759+
}
760+
761+
// Additional test to verify the error message content
762+
TEST(SubTree, DuplicateSubTreeName_ErrorMessage)
763+
{
764+
// clang-format off
765+
static const char* xml_text = R"(
766+
<root BTCPP_format="4" main_tree_to_execute="MainTree">
767+
<BehaviorTree ID="MainTree">
768+
<Sequence>
769+
<SubTree ID="Task" name="my_task"/>
770+
<SubTree ID="Task" name="my_task"/>
771+
</Sequence>
772+
</BehaviorTree>
773+
774+
<BehaviorTree ID="Task">
775+
<AlwaysSuccess/>
776+
</BehaviorTree>
777+
</root>
778+
)";
779+
// clang-format on
780+
781+
BehaviorTreeFactory factory;
782+
783+
try
784+
{
785+
factory.createTreeFromText(xml_text);
786+
FAIL() << "Expected RuntimeError to be thrown";
787+
}
788+
catch(const RuntimeError& e)
789+
{
790+
std::string msg = e.what();
791+
EXPECT_TRUE(msg.find("Duplicate SubTree path") != std::string::npos)
792+
<< "Error message should mention 'Duplicate SubTree path'. Got: " << msg;
793+
EXPECT_TRUE(msg.find("my_task") != std::string::npos)
794+
<< "Error message should mention the duplicate path 'my_task'. Got: " << msg;
795+
}
796+
}
797+
798+
// Test that unique names under the same parent work correctly
799+
TEST(SubTree, UniqueSubTreeNames_WorksCorrectly)
800+
{
801+
// clang-format off
802+
static const char* xml_text = R"(
803+
<root BTCPP_format="4" main_tree_to_execute="MainTree">
804+
<BehaviorTree ID="MainTree">
805+
<ParallelAll>
806+
<SubTree ID="Worker" name="worker_1"/>
807+
<SubTree ID="Worker" name="worker_2"/>
808+
</ParallelAll>
809+
</BehaviorTree>
810+
811+
<BehaviorTree ID="Worker">
812+
<AlwaysSuccess name="do_work"/>
813+
</BehaviorTree>
814+
</root>
815+
)";
816+
// clang-format on
817+
818+
BehaviorTreeFactory factory;
819+
Tree tree = factory.createTreeFromText(xml_text);
820+
821+
// Verify paths are unique
822+
std::set<std::string> all_paths;
823+
tree.applyVisitor([&](TreeNode* node) {
824+
EXPECT_EQ(all_paths.count(node->fullPath()), 0);
825+
all_paths.insert(node->fullPath());
826+
});
827+
828+
ASSERT_EQ(tree.subtrees.size(), 3);
829+
auto status = tree.tickWhileRunning();
830+
ASSERT_EQ(status, NodeStatus::SUCCESS);
831+
}
832+
833+
// Test that omitting name attribute auto-generates unique paths
834+
TEST(SubTree, NoNameAttribute_AutoGeneratesUniquePaths)
835+
{
836+
// clang-format off
837+
static const char* xml_text = R"(
838+
<root BTCPP_format="4" main_tree_to_execute="MainTree">
839+
<BehaviorTree ID="MainTree">
840+
<ParallelAll>
841+
<SubTree ID="Worker"/>
842+
<SubTree ID="Worker"/>
843+
</ParallelAll>
844+
</BehaviorTree>
845+
846+
<BehaviorTree ID="Worker">
847+
<AlwaysSuccess name="do_work"/>
848+
</BehaviorTree>
849+
</root>
850+
)";
851+
// clang-format on
852+
853+
BehaviorTreeFactory factory;
854+
Tree tree = factory.createTreeFromText(xml_text);
855+
856+
// Verify paths are unique (auto-generated with UID)
857+
std::set<std::string> all_paths;
858+
tree.applyVisitor([&](TreeNode* node) {
859+
EXPECT_EQ(all_paths.count(node->fullPath()), 0);
860+
all_paths.insert(node->fullPath());
861+
});
862+
863+
ASSERT_EQ(tree.subtrees.size(), 3);
864+
auto status = tree.tickWhileRunning();
865+
ASSERT_EQ(status, NodeStatus::SUCCESS);
866+
}
867+
868+
// Test nested subtrees - duplicate names at the same level should fail
869+
TEST(SubTree, NestedDuplicateNames_ShouldFail)
870+
{
871+
// clang-format off
872+
static const char* xml_text = R"(
873+
<root BTCPP_format="4" main_tree_to_execute="MainTree">
874+
<BehaviorTree ID="MainTree">
875+
<Sequence>
876+
<SubTree ID="Level1" name="task"/>
877+
<SubTree ID="Level1" name="task"/>
878+
</Sequence>
879+
</BehaviorTree>
880+
881+
<BehaviorTree ID="Level1">
882+
<AlwaysSuccess name="work"/>
883+
</BehaviorTree>
884+
</root>
885+
)";
886+
// clang-format on
887+
888+
BehaviorTreeFactory factory;
889+
890+
// Should throw RuntimeError because of duplicate SubTree names
891+
ASSERT_THROW(factory.createTreeFromText(xml_text), RuntimeError);
892+
}

0 commit comments

Comments
 (0)