Skip to content

Commit 3e8c292

Browse files
facontidavidefacontidavide
authored
Fullpath fix (#1053)
* add claude file * ignore generated files * detect duplicated instance names
1 parent 93f0f1a commit 3e8c292

File tree

4 files changed

+311
-0
lines changed

4 files changed

+311
-0
lines changed

‎.gitignore‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,3 +22,5 @@ tags
2222
/clang_tidy_output.log
2323
/.clang-tidy-venv/*
2424
/llvm.sh
25+
t11_groot_howto.btlog
26+
minitrace.json

‎CLAUDE.md‎

Lines changed: 131 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,131 @@
1+
# CLAUDE.md
2+
3+
This file provides guidance to Claude Code (claude.ai/code) when working with code in this repository.
4+
5+
## Build Commands
6+
7+
```bash
8+
# Plain CMake (recommended for development)
9+
mkdir build && cmake -S . -B build && cmake --build build --parallel
10+
11+
# With Conan (CMake 3.23+ required)
12+
conan install . -s build_type=Release --build=missing
13+
cmake --preset conan-release
14+
cmake --build --preset conan-release
15+
16+
# Pixi/Conda
17+
pixi run build
18+
```
19+
20+
Requires CMake 3.16.3+ and C++17 compiler. The project exports `CMAKE_EXPORT_COMPILE_COMMANDS=ON` by default.
21+
22+
## Testing
23+
24+
```bash
25+
# Run all tests via CTest
26+
ctest --test-dir build
27+
28+
# Run test executable directly
29+
./build/tests/behaviortree_cpp_test
30+
31+
# Run specific test
32+
./build/tests/behaviortree_cpp_test --gtest_filter="TestName*"
33+
34+
# Pixi
35+
pixi run test
36+
```
37+
38+
Test files are in `tests/` using Google Test. Key test categories: `gtest_blackboard.cpp`, `gtest_factory.cpp`, `gtest_tree.cpp`, `gtest_sequence.cpp`, `gtest_fallback.cpp`, `gtest_parallel.cpp`, `gtest_decorator.cpp`, `gtest_reactive.cpp`, `gtest_ports.cpp`, `gtest_port_type_rules.cpp`.
39+
40+
## Linting and Formatting
41+
42+
```bash
43+
# Pre-commit hooks (clang-format, codespell)
44+
pre-commit install
45+
pre-commit run -a
46+
47+
# Clang-tidy (requires clangd-21, build must exist for compile_commands.json)
48+
./run_clang_tidy.sh [build_path]
49+
```
50+
51+
Install clang-tidy-21:
52+
```bash
53+
wget https://apt.llvm.org/llvm.sh && chmod +x llvm.sh
54+
sudo ./llvm.sh 21
55+
sudo apt install clangd-21 clang-tidy-21
56+
```
57+
58+
Code style: Google C++ with 90-char line limit, 2-space indent. See `.clang-format` and `.clang-tidy` for details.
59+
60+
## Architecture
61+
62+
**Namespace:** `BT::`
63+
**Library:** `behaviortree_cpp`
64+
65+
### Node Hierarchy
66+
67+
All behavior tree nodes inherit from `TreeNode` (`include/behaviortree_cpp/tree_node.h`):
68+
69+
- **LeafNode**: `ActionNode`, `ConditionNode` - user-defined tasks and checks
70+
- **ControlNode**: `SequenceNode`, `FallbackNode`, `ParallelNode`, `ReactiveSequence`, `ReactiveFallback`, `SwitchNode`, `IfThenElseNode`, `WhileDoElseNode`
71+
- **DecoratorNode**: `InverterNode`, `RetryNode`, `RepeatNode`, `TimeoutNode`, `DelayNode`, `SubtreeNode`
72+
73+
### Node Status
74+
75+
`TreeNodeStatus`: `IDLE`, `RUNNING`, `SUCCESS`, `FAILURE`, `SKIPPED`
76+
77+
### Key Components
78+
79+
| Component | Location | Purpose |
80+
|-----------|----------|---------|
81+
| `BehaviorTreeFactory` | `bt_factory.h` | Node registration, XML parsing, tree creation |
82+
| `Blackboard` | `blackboard.h` | Shared typed key-value storage between nodes |
83+
| Port System | `basic_types.h` | Type-safe dataflow: `InputPort<T>`, `OutputPort<T>`, `BidirectionalPort<T>` |
84+
| XML Parser | `xml_parsing.cpp` | Loads trees from XML with type validation |
85+
| Script Parser | `scripting/` | Embedded expression language for conditions |
86+
87+
### Port System Rules
88+
89+
Ports enable type-safe data passing between nodes via the Blackboard:
90+
- Same-typed ports always connect
91+
- Generic ports (`AnyTypeAllowed`, `BT::Any`) accept any type
92+
- `std::string` output acts as "universal donor" (converts via `convertFromString<T>`)
93+
- Type locks after first strongly-typed write
94+
- Reserved names: `name`, `ID`, names starting with `_`
95+
96+
See `docs/PORT_CONNECTION_RULES.md` for detailed rules.
97+
98+
### Source Layout
99+
100+
```
101+
src/
102+
├── *.cpp # Core: tree_node, blackboard, xml_parsing, bt_factory
103+
├── actions/ # Built-in action nodes
104+
├── controls/ # Control flow nodes (sequence, fallback, parallel, etc.)
105+
├── decorators/ # Decorator nodes (retry, repeat, timeout, etc.)
106+
└── loggers/ # Logging infrastructure (Groot2, SQLite, file)
107+
108+
include/behaviortree_cpp/
109+
├── *.h # Public API headers
110+
├── controls/ # Control node headers
111+
├── decorators/ # Decorator node headers
112+
├── loggers/ # Logger headers
113+
├── scripting/ # Script parser (lexy-based)
114+
└── contrib/ # Third-party contributions
115+
```
116+
117+
### Integration Points
118+
119+
- **Groot2**: Visual editor integration via ZeroMQ (`BTCPP_GROOT_INTERFACE` option)
120+
- **ROS2**: Auto-detected via `ament_cmake`, uses colcon build
121+
- **Conan**: Package manager support for non-ROS builds
122+
123+
### Vendored Dependencies
124+
125+
All in `3rdparty/`: TinyXML2, cppzmq, flatbuffers, lexy, minicoro, minitrace. Controlled via `USE_VENDORED_*` CMake options.
126+
127+
## Contributing
128+
129+
- Run `pre-commit run -a` and `./run_clang_tidy.sh` before PRs
130+
- Bug fixes should include a failing test that passes after the fix
131+
- Consider API/ABI compatibility implications

‎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)