add use_global_arguments for node options of component nodes (#1776)

* add use_global_arguments for node options of component nodes Signed-off-by: zhenpeng ge <zhenpeng.ge@qq.com> keep use_global_arguments false default Signed-off-by: zhenpeng ge <zhenpeng.ge@qq.com> update warning message Signed-off-by: zhenpeng ge <zhenpeng.ge@qq.com> update warnning message Signed-off-by: zhenpeng ge <zhenpeng.ge@qq.com> add test Signed-off-by: zhenpeng ge <zhenpeng.ge@qq.com> * use forward_global_arguments instead of use_global_arguments Signed-off-by: zhenpeng ge <zhenpeng.ge@qq.com>
ros2 · Jan 19, 2022 · bca3fd7 · bca3fd7
1 parent 8afef51
commit bca3fd7
Show file tree

Hide file tree

Showing 2 changed files with 63 additions and 4 deletions.
diff --git a/rclcpp_components/src/component_manager.cpp b/rclcpp_components/src/component_manager.cpp
@@ -172,6 +172,18 @@ ComponentManager::create_node_options(const std::shared_ptr<LoadNode::Request> r
                 "Extra component argument 'use_intra_process_comms' must be a boolean");
       }
       options.use_intra_process_comms(extra_argument.get_value<bool>());
+    } else if (extra_argument.get_name() == "forward_global_arguments") {
+      if (extra_argument.get_type() != rclcpp::ParameterType::PARAMETER_BOOL) {
+        throw ComponentManagerException(
+                "Extra component argument 'forward_global_arguments' must be a boolean");
+      }
+      options.use_global_arguments(extra_argument.get_value<bool>());
+      if (extra_argument.get_value<bool>()) {
+        RCLCPP_WARN(
+          get_logger(), "forward_global_arguments is true by default in nodes, but is not "
+          "recommended in a component manager. If true, this will cause this node's behavior "
+          "to be influenced by global arguments, not only those targeted at this node.");
+      }
     }
   }
 

diff --git a/rclcpp_components/test/test_component_manager_api.cpp b/rclcpp_components/test/test_component_manager_api.cpp
@@ -219,6 +219,49 @@ void test_components_api(bool use_dedicated_executor)
     EXPECT_EQ(result->unique_id, 0u);
   }
 
+  {
+    // forward_global_arguments
+    auto request = std::make_shared<composition_interfaces::srv::LoadNode::Request>();
+    request->package_name = "rclcpp_components";
+    request->plugin_name = "test_rclcpp_components::TestComponentFoo";
+    request->node_name = "test_component_global_arguments";
+    rclcpp::Parameter forward_global_arguments("forward_global_arguments",
+      rclcpp::ParameterValue(true));
+    request->extra_arguments.push_back(forward_global_arguments.to_parameter_msg());
+
+    auto future = composition_client->async_send_request(request);
+    auto ret = exec->spin_until_future_complete(future, 5s);  // Wait for the result.
+    auto result = future.get();
+    EXPECT_EQ(ret, rclcpp::FutureReturnCode::SUCCESS);
+    EXPECT_EQ(result->success, true);
+    EXPECT_EQ(result->error_message, "");
+    EXPECT_EQ(result->full_node_name, "/test_component_global_arguments");
+    EXPECT_EQ(result->unique_id, 7u);
+  }
+
+  {
+    // forward_global_arguments is not a bool type parameter
+    auto request = std::make_shared<composition_interfaces::srv::LoadNode::Request>();
+    request->package_name = "rclcpp_components";
+    request->plugin_name = "test_rclcpp_components::TestComponentFoo";
+    request->node_name = "test_component_global_arguments_str";
+
+    rclcpp::Parameter forward_global_arguments("forward_global_arguments",
+      rclcpp::ParameterValue("hello"));
+    request->extra_arguments.push_back(forward_global_arguments.to_parameter_msg());
+
+    auto future = composition_client->async_send_request(request);
+    auto ret = exec->spin_until_future_complete(future, 5s);  // Wait for the result.
+    auto result = future.get();
+    EXPECT_EQ(ret, rclcpp::FutureReturnCode::SUCCESS);
+    EXPECT_EQ(result->success, false);
+    EXPECT_EQ(
+      result->error_message,
+      "Extra component argument 'forward_global_arguments' must be a boolean");
+    EXPECT_EQ(result->full_node_name, "");
+    EXPECT_EQ(result->unique_id, 0u);
+  }
+
   auto node_names = node->get_node_names();
 
   auto find_in_nodes = [node_names](std::string name) {
@@ -247,20 +290,22 @@ void test_components_api(bool use_dedicated_executor)
       auto result_node_names = result->full_node_names;
       auto result_unique_ids = result->unique_ids;
 
-      EXPECT_EQ(result_node_names.size(), 6u);
+      EXPECT_EQ(result_node_names.size(), 7u);
       EXPECT_EQ(result_node_names[0], "/test_component_foo");
       EXPECT_EQ(result_node_names[1], "/test_component_bar");
       EXPECT_EQ(result_node_names[2], "/test_component_baz");
       EXPECT_EQ(result_node_names[3], "/ns/test_component_bing");
       EXPECT_EQ(result_node_names[4], "/test_component_remap");
       EXPECT_EQ(result_node_names[5], "/test_component_intra_process");
-      EXPECT_EQ(result_unique_ids.size(), 6u);
+      EXPECT_EQ(result_node_names[6], "/test_component_global_arguments");
+      EXPECT_EQ(result_unique_ids.size(), 7u);
       EXPECT_EQ(result_unique_ids[0], 1u);
       EXPECT_EQ(result_unique_ids[1], 2u);
       EXPECT_EQ(result_unique_ids[2], 3u);
       EXPECT_EQ(result_unique_ids[3], 4u);
       EXPECT_EQ(result_unique_ids[4], 5u);
       EXPECT_EQ(result_unique_ids[5], 6u);
+      EXPECT_EQ(result_unique_ids[6], 7u);
     }
   }
 
@@ -314,18 +359,20 @@ void test_components_api(bool use_dedicated_executor)
       auto result_node_names = result->full_node_names;
       auto result_unique_ids = result->unique_ids;
 
-      EXPECT_EQ(result_node_names.size(), 5u);
+      EXPECT_EQ(result_node_names.size(), 6u);
       EXPECT_EQ(result_node_names[0], "/test_component_bar");
       EXPECT_EQ(result_node_names[1], "/test_component_baz");
       EXPECT_EQ(result_node_names[2], "/ns/test_component_bing");
       EXPECT_EQ(result_node_names[3], "/test_component_remap");
       EXPECT_EQ(result_node_names[4], "/test_component_intra_process");
-      EXPECT_EQ(result_unique_ids.size(), 5u);
+      EXPECT_EQ(result_node_names[5], "/test_component_global_arguments");
+      EXPECT_EQ(result_unique_ids.size(), 6u);
       EXPECT_EQ(result_unique_ids[0], 2u);
       EXPECT_EQ(result_unique_ids[1], 3u);
       EXPECT_EQ(result_unique_ids[2], 4u);
       EXPECT_EQ(result_unique_ids[3], 5u);
       EXPECT_EQ(result_unique_ids[4], 6u);
+      EXPECT_EQ(result_unique_ids[5], 7u);
     }
   }
 }