From 006c60410c8f8bd02d9650e8e6599ef4810dd78a Mon Sep 17 00:00:00 2001 From: Davide Faconti Date: Sat, 10 Oct 2026 10:44:05 +0200 Subject: [PATCH 1/2] TreeObserver: don't throw when two nodes have the same path fullPath() is the subtree prefix plus the node name, so two nodes with the same explicit name in the same subtree, e.g. share a path, and the TreeObserver constructor threw "TreeObserver not built correctly. Report issue" for a valid tree. Register every node by UID, and keep the first node in depth-first order for a duplicated path in pathToUID() / getStatistics(path). No API or ABI change. Co-Authored-By: Claude Opus 5.5 --- .../behaviortree_cpp/loggers/bt_observer.h | 3 +- src/loggers/bt_observer.cpp | 16 +++------ tests/gtest_loggers.cpp | 33 +++++++++++++++++++ 3 files changed, 40 insertions(+), 12 deletions(-) diff --git a/include/behaviortree_cpp/loggers/bt_observer.h b/include/behaviortree_cpp/loggers/bt_observer.h index 6f99798ce..b5fe8e02c 100644 --- a/include/behaviortree_cpp/loggers/bt_observer.h +++ b/include/behaviortree_cpp/loggers/bt_observer.h @@ -50,7 +50,8 @@ class TreeObserver : public StatusChangeLogger Duration last_timestamp = {}; }; - // find the statistics of a node, based on its path + // find the statistics of a node, based on its path. + // If more nodes have the same path, this is the first one in depth-first order. const NodeStatistics& getStatistics(const std::string& path) const; // find the statistics of a node, based on its TreeNode::UID() diff --git a/src/loggers/bt_observer.cpp b/src/loggers/bt_observer.cpp index 76aaeebea..30a5426ae 100644 --- a/src/loggers/bt_observer.cpp +++ b/src/loggers/bt_observer.cpp @@ -25,23 +25,17 @@ TreeObserver::TreeObserver(const BT::Tree& tree) : StatusChangeLogger(tree.rootN } } - if(_path_to_uid.count(node.fullPath()) != 0) - { - throw LogicError("TreeObserver not built correctly. Report issue"); - } - _path_to_uid[node.fullPath()] = node.UID(); + // Nodes with the same name in the same subtree share the same path: + // keep the first one, the others can be found by UID. + _path_to_uid.emplace(node.fullPath(), node.UID()); + _uid_to_path[node.UID()] = node.fullPath(); + _statistics[node.UID()] = {}; }; for(const auto& subtree : tree.subtrees) { recursiveStep(*subtree->nodes.front()); } - - for(const auto& [path, uid] : _path_to_uid) - { - _statistics[uid] = {}; - _uid_to_path[uid] = path; - } } TreeObserver::~TreeObserver() diff --git a/tests/gtest_loggers.cpp b/tests/gtest_loggers.cpp index 20b078ace..75e77e005 100644 --- a/tests/gtest_loggers.cpp +++ b/tests/gtest_loggers.cpp @@ -14,6 +14,7 @@ #include "behaviortree_cpp/loggers/bt_cout_logger.h" #include "behaviortree_cpp/loggers/bt_file_logger_v2.h" #include "behaviortree_cpp/loggers/bt_minitrace_logger.h" +#include "behaviortree_cpp/loggers/bt_observer.h" #include "behaviortree_cpp/loggers/bt_sqlite_logger.h" #include @@ -620,3 +621,35 @@ TEST_F(LoggerTest, Logger_DisabledDuringExecution) ASSERT_TRUE(std::filesystem::exists(filepath)); } + +TEST(TreeObserverTest, NodesWithTheSameName) +{ + // Two siblings with the same name have the same fullPath() + static const char* xml_text = R"( + + + + + + + + )"; + + BehaviorTreeFactory factory; + auto tree = factory.createTreeFromText(xml_text); + + std::unique_ptr observer; + ASSERT_NO_THROW(observer = std::make_unique(tree)); + ASSERT_EQ(tree.tickWhileRunning(), NodeStatus::SUCCESS); + + const auto& nodes = tree.subtrees.front()->nodes; + ASSERT_EQ(nodes.size(), 3u); + EXPECT_EQ(observer->statistics().size(), 3u); + EXPECT_EQ(observer->uidToPath().size(), 3u); + for(const auto& node : nodes) + { + EXPECT_EQ(observer->uidToPath().at(node->UID()), node->fullPath()); + EXPECT_EQ(observer->getStatistics(node->UID()).success_count, 1u); + } + EXPECT_EQ(observer->pathToUID().at("step"), nodes[1]->UID()); +} From c179852d0803cf728ccc0ab5701b78b618ead259 Mon Sep 17 00:00:00 2001 From: Davide Faconti Date: Sat, 10 Oct 2026 10:54:08 +0200 Subject: [PATCH 2/2] use try_emplace for the path map (Sonar cpp:S6030) Co-Authored-By: Claude Opus 5.5 --- src/loggers/bt_observer.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/loggers/bt_observer.cpp b/src/loggers/bt_observer.cpp index 30a5426ae..8405cce4c 100644 --- a/src/loggers/bt_observer.cpp +++ b/src/loggers/bt_observer.cpp @@ -27,7 +27,7 @@ TreeObserver::TreeObserver(const BT::Tree& tree) : StatusChangeLogger(tree.rootN // Nodes with the same name in the same subtree share the same path: // keep the first one, the others can be found by UID. - _path_to_uid.emplace(node.fullPath(), node.UID()); + _path_to_uid.try_emplace(node.fullPath(), node.UID()); _uid_to_path[node.UID()] = node.fullPath(); _statistics[node.UID()] = {}; };