Changing BT loading warning to be more clear (#5938)

* Changing BT loading warning to be more clear

Signed-off-by: SteveMacenski <stevenmacenski@gmail.com>

* Comment out ros2/rviz repository

Comment out the ros2/rviz repository entry in underlay.repos

Signed-off-by: Steve Macenski <stevenmacenski@gmail.com>

---------

Signed-off-by: SteveMacenski <stevenmacenski@gmail.com>
Signed-off-by: Steve Macenski <stevenmacenski@gmail.com>

Author: SteveMacenski

nav2_behavior_tree/include/nav2_behavior_tree/bt_action_server_impl.hpp

@@ -261,6 +261,7 @@ bool BtActionServer<ActionT, NodeT>::loadBehaviorTree(const std::string & bt_xml
   }
 
   std::set<std::string> registered_ids;
+  std::vector<std::string> 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<ActionT, NodeT>::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<ActionT, NodeT>::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,

nav2_system_tests/src/behavior_tree/test_behavior_tree_node.cpp

@@ -119,6 +119,7 @@ class BehaviorTreeHandler
         kXmlExtension.size(), kXmlExtension) != 0);
 
     std::set<std::string> registered_ids;
+    std::vector<std::string> 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);
 
 

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