diff --git a/.gitignore b/.gitignore index 0b25bb78b..99a502680 100644 --- a/.gitignore +++ b/.gitignore @@ -22,3 +22,5 @@ tags /clang_tidy_output.log /.clang-tidy-venv/* /llvm.sh +t11_groot_howto.btlog +minitrace.json diff --git a/CLAUDE.md b/CLAUDE.md new file mode 100644 index 000000000..4459bce1e --- /dev/null +++ b/CLAUDE.md @@ -0,0 +1,131 @@ +# CLAUDE.md + +This file provides guidance to Claude Code (claude.ai/code) when working with code in this repository. + +## Build Commands + +```bash +# Plain CMake (recommended for development) +mkdir build && cmake -S . -B build && cmake --build build --parallel + +# With Conan (CMake 3.23+ required) +conan install . -s build_type=Release --build=missing +cmake --preset conan-release +cmake --build --preset conan-release + +# Pixi/Conda +pixi run build +``` + +Requires CMake 3.16.3+ and C++17 compiler. The project exports `CMAKE_EXPORT_COMPILE_COMMANDS=ON` by default. + +## Testing + +```bash +# Run all tests via CTest +ctest --test-dir build + +# Run test executable directly +./build/tests/behaviortree_cpp_test + +# Run specific test +./build/tests/behaviortree_cpp_test --gtest_filter="TestName*" + +# Pixi +pixi run test +``` + +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`. + +## Linting and Formatting + +```bash +# Pre-commit hooks (clang-format, codespell) +pre-commit install +pre-commit run -a + +# Clang-tidy (requires clangd-21, build must exist for compile_commands.json) +./run_clang_tidy.sh [build_path] +``` + +Install clang-tidy-21: +```bash +wget https://apt.llvm.org/llvm.sh && chmod +x llvm.sh +sudo ./llvm.sh 21 +sudo apt install clangd-21 clang-tidy-21 +``` + +Code style: Google C++ with 90-char line limit, 2-space indent. See `.clang-format` and `.clang-tidy` for details. + +## Architecture + +**Namespace:** `BT::` +**Library:** `behaviortree_cpp` + +### Node Hierarchy + +All behavior tree nodes inherit from `TreeNode` (`include/behaviortree_cpp/tree_node.h`): + +- **LeafNode**: `ActionNode`, `ConditionNode` - user-defined tasks and checks +- **ControlNode**: `SequenceNode`, `FallbackNode`, `ParallelNode`, `ReactiveSequence`, `ReactiveFallback`, `SwitchNode`, `IfThenElseNode`, `WhileDoElseNode` +- **DecoratorNode**: `InverterNode`, `RetryNode`, `RepeatNode`, `TimeoutNode`, `DelayNode`, `SubtreeNode` + +### Node Status + +`TreeNodeStatus`: `IDLE`, `RUNNING`, `SUCCESS`, `FAILURE`, `SKIPPED` + +### Key Components + +| Component | Location | Purpose | +|-----------|----------|---------| +| `BehaviorTreeFactory` | `bt_factory.h` | Node registration, XML parsing, tree creation | +| `Blackboard` | `blackboard.h` | Shared typed key-value storage between nodes | +| Port System | `basic_types.h` | Type-safe dataflow: `InputPort`, `OutputPort`, `BidirectionalPort` | +| XML Parser | `xml_parsing.cpp` | Loads trees from XML with type validation | +| Script Parser | `scripting/` | Embedded expression language for conditions | + +### Port System Rules + +Ports enable type-safe data passing between nodes via the Blackboard: +- Same-typed ports always connect +- Generic ports (`AnyTypeAllowed`, `BT::Any`) accept any type +- `std::string` output acts as "universal donor" (converts via `convertFromString`) +- Type locks after first strongly-typed write +- Reserved names: `name`, `ID`, names starting with `_` + +See `docs/PORT_CONNECTION_RULES.md` for detailed rules. + +### Source Layout + +``` +src/ +├── *.cpp # Core: tree_node, blackboard, xml_parsing, bt_factory +├── actions/ # Built-in action nodes +├── controls/ # Control flow nodes (sequence, fallback, parallel, etc.) +├── decorators/ # Decorator nodes (retry, repeat, timeout, etc.) +└── loggers/ # Logging infrastructure (Groot2, SQLite, file) + +include/behaviortree_cpp/ +├── *.h # Public API headers +├── controls/ # Control node headers +├── decorators/ # Decorator node headers +├── loggers/ # Logger headers +├── scripting/ # Script parser (lexy-based) +└── contrib/ # Third-party contributions +``` + +### Integration Points + +- **Groot2**: Visual editor integration via ZeroMQ (`BTCPP_GROOT_INTERFACE` option) +- **ROS2**: Auto-detected via `ament_cmake`, uses colcon build +- **Conan**: Package manager support for non-ROS builds + +### Vendored Dependencies + +All in `3rdparty/`: TinyXML2, cppzmq, flatbuffers, lexy, minicoro, minitrace. Controlled via `USE_VENDORED_*` CMake options. + +## Contributing + +- Run `pre-commit run -a` and `./run_clang_tidy.sh` before PRs +- Bug fixes should include a failing test that passes after the fix +- Consider API/ABI compatibility implications diff --git a/src/xml_parsing.cpp b/src/xml_parsing.cpp index a094fea22..5c3e07ee1 100644 --- a/src/xml_parsing.cpp +++ b/src/xml_parsing.cpp @@ -1010,6 +1010,20 @@ void BT::XMLParser::PImpl::recursivelyCreateSubtree(const std::string& tree_ID, subtree_path += subtree_ID + "::" + std::to_string(node->UID()); } + // Check if the path already exists - duplicate paths cause issues in Groot2 + // and TreeObserver (see Groot2 issue #56) + for(const auto& sub : output_tree.subtrees) + { + if(sub->instance_name == subtree_path) + { + throw RuntimeError("Duplicate SubTree path detected: '", subtree_path, + "'. Multiple SubTree nodes with the same 'name' attribute " + "under the same parent are not allowed. " + "Please use unique names or omit the 'name' attribute " + "to auto-generate unique paths."); + } + } + recursivelyCreateSubtree(subtree_ID, subtree_path, // name subtree_path + "/", //prefix diff --git a/tests/gtest_subtree.cpp b/tests/gtest_subtree.cpp index 6402bafba..f8810091d 100644 --- a/tests/gtest_subtree.cpp +++ b/tests/gtest_subtree.cpp @@ -1,4 +1,5 @@ #include +#include #include "behaviortree_cpp/bt_factory.h" #include "../sample_nodes/dummy_nodes.h" #include "../sample_nodes/movebase_node.h" @@ -726,3 +727,166 @@ TEST(SubTree, SubtreeNameNotRegistered) ASSERT_ANY_THROW(auto tree = factory.createTreeFromText(xml_text)); ASSERT_ANY_THROW(factory.registerBehaviorTreeFromText(xml_text)); } + +// Test for Groot2 issue #56: duplicate _fullpath when multiple subtrees have the same name +// https://github.com/BehaviorTree/Groot2/issues/56 +// +// When two SubTree nodes under the same parent have the same "name" attribute, +// tree creation should fail with a clear error message. +TEST(SubTree, DuplicateSubTreeName_Groot2Issue56) +{ + // clang-format off + static const char* xml_text = R"( + + + + + + + + + + + + +)"; + // clang-format on + + BehaviorTreeFactory factory; + + // Should throw RuntimeError because of duplicate SubTree names + ASSERT_THROW(factory.createTreeFromText(xml_text), RuntimeError); +} + +// Additional test to verify the error message content +TEST(SubTree, DuplicateSubTreeName_ErrorMessage) +{ + // clang-format off + static const char* xml_text = R"( + + + + + + + + + + + + +)"; + // clang-format on + + BehaviorTreeFactory factory; + + try + { + factory.createTreeFromText(xml_text); + FAIL() << "Expected RuntimeError to be thrown"; + } + catch(const RuntimeError& e) + { + std::string msg = e.what(); + EXPECT_TRUE(msg.find("Duplicate SubTree path") != std::string::npos) + << "Error message should mention 'Duplicate SubTree path'. Got: " << msg; + EXPECT_TRUE(msg.find("my_task") != std::string::npos) + << "Error message should mention the duplicate path 'my_task'. Got: " << msg; + } +} + +// Test that unique names under the same parent work correctly +TEST(SubTree, UniqueSubTreeNames_WorksCorrectly) +{ + // clang-format off + static const char* xml_text = R"( + + + + + + + + + + + + +)"; + // clang-format on + + BehaviorTreeFactory factory; + Tree tree = factory.createTreeFromText(xml_text); + + // Verify paths are unique + std::set all_paths; + tree.applyVisitor([&](TreeNode* node) { + EXPECT_EQ(all_paths.count(node->fullPath()), 0); + all_paths.insert(node->fullPath()); + }); + + ASSERT_EQ(tree.subtrees.size(), 3); + auto status = tree.tickWhileRunning(); + ASSERT_EQ(status, NodeStatus::SUCCESS); +} + +// Test that omitting name attribute auto-generates unique paths +TEST(SubTree, NoNameAttribute_AutoGeneratesUniquePaths) +{ + // clang-format off + static const char* xml_text = R"( + + + + + + + + + + + + +)"; + // clang-format on + + BehaviorTreeFactory factory; + Tree tree = factory.createTreeFromText(xml_text); + + // Verify paths are unique (auto-generated with UID) + std::set all_paths; + tree.applyVisitor([&](TreeNode* node) { + EXPECT_EQ(all_paths.count(node->fullPath()), 0); + all_paths.insert(node->fullPath()); + }); + + ASSERT_EQ(tree.subtrees.size(), 3); + auto status = tree.tickWhileRunning(); + ASSERT_EQ(status, NodeStatus::SUCCESS); +} + +// Test nested subtrees - duplicate names at the same level should fail +TEST(SubTree, NestedDuplicateNames_ShouldFail) +{ + // clang-format off + static const char* xml_text = R"( + + + + + + + + + + + + +)"; + // clang-format on + + BehaviorTreeFactory factory; + + // Should throw RuntimeError because of duplicate SubTree names + ASSERT_THROW(factory.createTreeFromText(xml_text), RuntimeError); +}