diff --git a/nav2_behavior_tree/include/nav2_behavior_tree/bt_action_server_impl.hpp b/nav2_behavior_tree/include/nav2_behavior_tree/bt_action_server_impl.hpp index c03b9b94737..693b94df294 100644 --- a/nav2_behavior_tree/include/nav2_behavior_tree/bt_action_server_impl.hpp +++ b/nav2_behavior_tree/include/nav2_behavior_tree/bt_action_server_impl.hpp @@ -261,6 +261,7 @@ bool BtActionServer::loadBehaviorTree(const std::string & bt_xml } std::set registered_ids; + std::vector conflicting_files; std::string main_id; auto register_all_bt_files = [&](const std::string & skip_file = "") { for (const auto & directory : search_directories_) { @@ -278,9 +279,7 @@ bool BtActionServer::loadBehaviorTree(const std::string & bt_xml continue; } if (registered_ids.count(id)) { - RCLCPP_WARN( - logger_, "Skipping conflicting BT file %s (duplicate ID %s)", - entry.path().c_str(), id.c_str()); + conflicting_files.push_back(entry.path().string()); continue; } @@ -318,6 +317,23 @@ bool BtActionServer::loadBehaviorTree(const std::string & bt_xml main_id = file_or_id; register_all_bt_files(); } + + // Log all conflicting files once at the end + if (!conflicting_files.empty()) { + std::string files_list; + for (const auto & file : conflicting_files) { + if (!files_list.empty()) { + files_list += ", "; + } + files_list += file; + } + RCLCPP_WARN( + logger_, + "Skipping conflicting BT XML files, multiple files have the same ID. " + "Please set unique behavior tree IDs. This may affect loading of subtrees. " + "Files not loaded: %s", + files_list.c_str()); + } } catch (const std::exception & e) { setInternalError( ActionT::Result::FAILED_TO_LOAD_BEHAVIOR_TREE, diff --git a/nav2_system_tests/src/behavior_tree/test_behavior_tree_node.cpp b/nav2_system_tests/src/behavior_tree/test_behavior_tree_node.cpp index 6c190b39a42..e26e01f5aa7 100644 --- a/nav2_system_tests/src/behavior_tree/test_behavior_tree_node.cpp +++ b/nav2_system_tests/src/behavior_tree/test_behavior_tree_node.cpp @@ -119,6 +119,7 @@ class BehaviorTreeHandler kXmlExtension.size(), kXmlExtension) != 0); std::set registered_ids; + std::vector conflicting_files; std::string main_id; auto register_all_bt_files = [&](const std::string & skip_file = "") { @@ -137,8 +138,7 @@ class BehaviorTreeHandler continue; } if (registered_ids.count(id)) { - std::cerr << "Skipping conflicting BT file " << entry.path() << " (duplicate ID " << - id << ")" << "\n"; + conflicting_files.push_back(entry.path().string()); continue; } std::cout << "Registering Tree from File: " << entry.path().string() << "\n"; @@ -169,6 +169,20 @@ class BehaviorTreeHandler register_all_bt_files(); } + // Log all conflicting files once at the end + if (!conflicting_files.empty()) { + std::string files_list; + for (const auto & file : conflicting_files) { + if (!files_list.empty()) { + files_list += ", "; + } + files_list += file; + } + std::cerr << "Skipping conflicting BT XML files, multiple files have the same ID. " + << "Please set unique behavior tree IDs. This may affect loading of subtrees. " + << "Files not loaded: " << files_list << "\n"; + } + // Create the tree with the specified ID blackboard = setBlackboardVariables(); try { @@ -437,15 +451,18 @@ TEST_F(BehaviorTreeTestFixture, TestDuplicateIDsWithFileSpecified) { EXPECT_TRUE(result); - bool found_conflict = - log_output.find( - "Skipping conflicting BT file \"" + dup2_file + - "\" (duplicate ID DuplicateTree)") != std::string::npos; - EXPECT_TRUE(found_conflict); + // Verify the new message format + EXPECT_NE( + log_output.find("Skipping conflicting BT XML files, multiple files have the same ID."), + std::string::npos) << "Should warn about duplicate IDs"; + EXPECT_NE(log_output.find("Please set unique behavior tree IDs"), std::string::npos) + << "Should provide guidance about unique IDs"; + EXPECT_NE(log_output.find("Files not loaded:"), std::string::npos) + << "Should list files not loaded"; + EXPECT_NE(log_output.find(dup2_file), std::string::npos) + << "Should mention the skipped file"; EXPECT_NE(log_output.find("Registering Tree from File"), std::string::npos); - EXPECT_NE(log_output.find("Skipping conflicting BT file"), std::string::npos) - << "Should warn about duplicate ID"; EXPECT_NE(log_output.find("Created BT from ID: DuplicateTree"), std::string::npos); std::filesystem::remove_all(tmp_dir); @@ -590,7 +607,7 @@ TEST_F(BehaviorTreeTestFixture, TestDuplicateIDsWithIDSpecified) { EXPECT_NE(log_output.find("Registering Tree from File"), std::string::npos) << "Should have registered at least one BT file"; - EXPECT_NE(log_output.find("Skipping conflicting BT file"), std::string::npos) + EXPECT_NE(log_output.find("Skipping conflicting BT XML files"), std::string::npos) << "Should warn about duplicate IDs"; EXPECT_NE(log_output.find("Created BT from ID: DuplicateTree"), std::string::npos) << "Should have created BT from the given ID"; @@ -604,7 +621,7 @@ TEST_F(BehaviorTreeTestFixture, TestDuplicateIDsWithIDSpecified) { << "At least one duplicate file should have been registered"; EXPECT_FALSE(registered_dup1 && registered_dup2) << "Only one of the duplicate files should be registered as the main tree"; - EXPECT_NE(log_output.find("Skipping conflicting BT file"), std::string::npos); + EXPECT_NE(log_output.find("Skipping conflicting BT XML files"), std::string::npos); EXPECT_NE(log_output.find("Created BT from ID: DuplicateTree"), std::string::npos); diff --git a/tools/underlay.repos b/tools/underlay.repos index 1c5fe52ed22..7dcb5ae9b69 100644 --- a/tools/underlay.repos +++ b/tools/underlay.repos @@ -4,9 +4,9 @@ repositories: # url: https://github.com/BehaviorTree/BehaviorTree.CPP.git # version: 4.8.2 # ros2/rviz: - # type: git - # url: https://github.com/ros2/rviz.git - # version: rolling + # type: git + # url: https://github.com/ros2/rviz.git + # version: rolling # ros/angles: # type: git # url: https://github.com/ros/angles.git