Skip to content

Commit 00dfd67

Browse files
Bt warning fix (#5594)
* fix the loadbehaviortree impl and its test Signed-off-by: Jad El Hajj <[email protected]> * pr refactoring and added tests Signed-off-by: Jad El Hajj <[email protected]> * simplify logic Signed-off-by: Jad El Hajj <[email protected]> * simplify loadbehaviortree Signed-off-by: Jad El Hajj <[email protected]> * unused Signed-off-by: Jad El Hajj <[email protected]> * removed st bb Signed-off-by: Jad El Hajj <[email protected]> * PR fix Signed-off-by: Jad El Hajj <[email protected]> * readd old code Signed-off-by: Jad El Hajj <[email protected]> * Update nav2_behavior_tree/include/nav2_behavior_tree/bt_action_server_impl.hpp Signed-off-by: Steve Macenski <[email protected]> * added try catch and comments Signed-off-by: Jad El Hajj <[email protected]> * lint Signed-off-by: Jad El Hajj <[email protected]> * PR fix Signed-off-by: Jad El Hajj <[email protected]> * Update nav2_behavior_tree/include/nav2_behavior_tree/bt_action_server_impl.hpp Signed-off-by: Steve Macenski <[email protected]> * Update nav2_behavior_tree/include/nav2_behavior_tree/bt_action_server_impl.hpp Signed-off-by: Steve Macenski <[email protected]> * Fix indentation Signed-off-by: Jad El Hajj <[email protected]> * Added test Signed-off-by: Jad El Hajj <[email protected]> --------- Signed-off-by: Jad El Hajj <[email protected]> Signed-off-by: Steve Macenski <[email protected]> Co-authored-by: Steve Macenski <[email protected]>
1 parent feb3418 commit 00dfd67

File tree

2 files changed

+328
-104
lines changed

2 files changed

+328
-104
lines changed

nav2_behavior_tree/include/nav2_behavior_tree/bt_action_server_impl.hpp

Lines changed: 60 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@
2121
#include <limits>
2222
#include <memory>
2323
#include <set>
24-
#include <unordered_set>
24+
#include <utility>
2525
#include <string>
2626
#include <vector>
2727

@@ -253,41 +253,73 @@ bool BtActionServer<ActionT, NodeT>::loadBehaviorTree(const std::string & bt_xml
253253
is_bt_id = true;
254254
}
255255

256-
std::unordered_set<std::string> used_bt_id;
257-
for (const auto & directory : search_directories_) {
258-
try {
259-
for (const auto & entry : fs::directory_iterator(directory)) {
260-
if (entry.path().extension() == ".xml") {
261-
auto current_bt_id = bt_->extractBehaviorTreeID(entry.path().string());
262-
if (current_bt_id.empty()) {
263-
RCLCPP_ERROR(logger_, "Skipping BT file %s (missing ID)",
264-
entry.path().string().c_str());
256+
std::set<std::string> registered_ids;
257+
std::string main_id;
258+
auto register_all_bt_files = [&](const std::string & skip_file = "") {
259+
for (const auto & directory : search_directories_) {
260+
for (const auto & entry : fs::directory_iterator(directory)) {
261+
if (entry.path().extension() != ".xml") {
262+
continue;
263+
}
264+
if (!skip_file.empty() && entry.path().string() == skip_file) {
265+
continue;
266+
}
267+
268+
auto id = bt_->extractBehaviorTreeID(entry.path().string());
269+
if (id.empty()) {
270+
RCLCPP_ERROR(logger_, "Skipping BT file %s (missing ID)", entry.path().c_str());
265271
continue;
266272
}
267-
auto [it, inserted] = used_bt_id.insert(current_bt_id);
268-
if (!inserted) {
269-
RCLCPP_WARN(
270-
logger_,
271-
"Warning: Duplicate BT IDs found. Make sure to have all BT IDs unique! "
272-
"ID: %s File: %s",
273-
current_bt_id.c_str(), entry.path().string().c_str());
273+
if (registered_ids.count(id)) {
274+
RCLCPP_WARN(logger_, "Skipping conflicting BT file %s (duplicate ID %s)",
275+
entry.path().c_str(), id.c_str());
276+
continue;
274277
}
278+
279+
RCLCPP_DEBUG(logger_, "Registering Tree from File: %s", entry.path().string().c_str());
275280
bt_->registerTreeFromFile(entry.path().string());
281+
registered_ids.insert(id);
276282
}
277283
}
278-
} catch (const std::exception & e) {
279-
setInternalError(ActionT::Result::FAILED_TO_LOAD_BEHAVIOR_TREE,
280-
"Exception reading behavior tree directory: " + std::string(e.what()));
281-
return false;
282-
}
283-
}
284-
// Try to load the main BT tree (by ID)
284+
};
285+
285286
try {
286-
if(!is_bt_id) {
287-
tree_ = bt_->createTreeFromFile(file_or_id, blackboard_);
287+
if (!is_bt_id) {
288+
// file_or_id is a filename: register it first
289+
std::string main_file = file_or_id;
290+
main_id = bt_->extractBehaviorTreeID(main_file);
291+
if (main_id.empty()) {
292+
RCLCPP_ERROR(logger_, "Failed to extract ID from %s", main_file.c_str());
293+
return false;
294+
}
295+
RCLCPP_DEBUG(logger_, "Registering Tree from File: %s", main_file.c_str());
296+
bt_->registerTreeFromFile(main_file);
297+
registered_ids.insert(main_id);
298+
299+
// When a filename is specified, it must be register first
300+
// and treat it as the "main" tree to execute.
301+
// This ensures the requested tree is always available
302+
// and prioritized, even if other files in the directory have duplicate IDs.
303+
// The lambda then skips this main file to avoid
304+
// re-registering it or logging a duplicate warning.
305+
// In contrast, when an ID is specified, it's unknown which file is "main"
306+
// so all files are registered and conflicts are handled in the lambda.
307+
register_all_bt_files(main_file);
288308
} else {
289-
tree_ = bt_->createTree(file_or_id, blackboard_);
309+
// file_or_id is an ID: register all files, skipping conflicts
310+
main_id = file_or_id;
311+
register_all_bt_files();
290312
}
313+
} catch (const std::exception & e) {
314+
setInternalError(ActionT::Result::FAILED_TO_LOAD_BEHAVIOR_TREE,
315+
"Exception registering behavior trees: " + std::string(e.what()));
316+
return false;
317+
}
318+
319+
// Create the tree with the specified ID
320+
try {
321+
tree_ = bt_->createTree(main_id, blackboard_);
322+
RCLCPP_INFO(logger_, "Created BT from ID: %s", main_id.c_str());
291323

292324
for (auto & subtree : tree_.subtrees) {
293325
auto & blackboard = subtree->blackboard;
@@ -299,7 +331,7 @@ bool BtActionServer<ActionT, NodeT>::loadBehaviorTree(const std::string & bt_xml
299331
}
300332
} catch (const std::exception & e) {
301333
setInternalError(ActionT::Result::FAILED_TO_LOAD_BEHAVIOR_TREE,
302-
std::string("Exception when creating BT tree from file: ") + e.what());
334+
std::string("Exception when creating BT tree from ID: ") + e.what());
303335
return false;
304336
}
305337

0 commit comments

Comments
 (0)