Skip to content

Commit 006c604

Browse files
facontidavideclaude
andcommitted
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. <Sequence> <AlwaysSuccess name="step"/> <AlwaysSuccess name="step"/> </Sequence> 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 <noreply@anthropic.com>
1 parent 62c4bb4 commit 006c604

3 files changed

Lines changed: 40 additions & 12 deletions

File tree

‎include/behaviortree_cpp/loggers/bt_observer.h‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,8 @@ class TreeObserver : public StatusChangeLogger
5050
Duration last_timestamp = {};
5151
};
5252

53-
// find the statistics of a node, based on its path
53+
// find the statistics of a node, based on its path.
54+
// If more nodes have the same path, this is the first one in depth-first order.
5455
const NodeStatistics& getStatistics(const std::string& path) const;
5556

5657
// find the statistics of a node, based on its TreeNode::UID()

‎src/loggers/bt_observer.cpp‎

Lines changed: 5 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -25,23 +25,17 @@ TreeObserver::TreeObserver(const BT::Tree& tree) : StatusChangeLogger(tree.rootN
2525
}
2626
}
2727

28-
if(_path_to_uid.count(node.fullPath()) != 0)
29-
{
30-
throw LogicError("TreeObserver not built correctly. Report issue");
31-
}
32-
_path_to_uid[node.fullPath()] = node.UID();
28+
// Nodes with the same name in the same subtree share the same path:
29+
// keep the first one, the others can be found by UID.
30+
_path_to_uid.emplace(node.fullPath(), node.UID());
31+
_uid_to_path[node.UID()] = node.fullPath();
32+
_statistics[node.UID()] = {};
3333
};
3434

3535
for(const auto& subtree : tree.subtrees)
3636
{
3737
recursiveStep(*subtree->nodes.front());
3838
}
39-
40-
for(const auto& [path, uid] : _path_to_uid)
41-
{
42-
_statistics[uid] = {};
43-
_uid_to_path[uid] = path;
44-
}
4539
}
4640

4741
TreeObserver::~TreeObserver()

‎tests/gtest_loggers.cpp‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
#include "behaviortree_cpp/loggers/bt_cout_logger.h"
1515
#include "behaviortree_cpp/loggers/bt_file_logger_v2.h"
1616
#include "behaviortree_cpp/loggers/bt_minitrace_logger.h"
17+
#include "behaviortree_cpp/loggers/bt_observer.h"
1718
#include "behaviortree_cpp/loggers/bt_sqlite_logger.h"
1819

1920
#include <array>
@@ -620,3 +621,35 @@ TEST_F(LoggerTest, Logger_DisabledDuringExecution)
620621

621622
ASSERT_TRUE(std::filesystem::exists(filepath));
622623
}
624+
625+
TEST(TreeObserverTest, NodesWithTheSameName)
626+
{
627+
// Two siblings with the same name have the same fullPath()
628+
static const char* xml_text = R"(
629+
<root BTCPP_format="4">
630+
<BehaviorTree ID="MainTree">
631+
<Sequence>
632+
<AlwaysSuccess name="step"/>
633+
<AlwaysSuccess name="step"/>
634+
</Sequence>
635+
</BehaviorTree>
636+
</root>)";
637+
638+
BehaviorTreeFactory factory;
639+
auto tree = factory.createTreeFromText(xml_text);
640+
641+
std::unique_ptr<TreeObserver> observer;
642+
ASSERT_NO_THROW(observer = std::make_unique<TreeObserver>(tree));
643+
ASSERT_EQ(tree.tickWhileRunning(), NodeStatus::SUCCESS);
644+
645+
const auto& nodes = tree.subtrees.front()->nodes;
646+
ASSERT_EQ(nodes.size(), 3u);
647+
EXPECT_EQ(observer->statistics().size(), 3u);
648+
EXPECT_EQ(observer->uidToPath().size(), 3u);
649+
for(const auto& node : nodes)
650+
{
651+
EXPECT_EQ(observer->uidToPath().at(node->UID()), node->fullPath());
652+
EXPECT_EQ(observer->getStatistics(node->UID()).success_count, 1u);
653+
}
654+
EXPECT_EQ(observer->pathToUID().at("step"), nodes[1]->UID());
655+
}

0 commit comments

Comments
 (0)