Parse CLI parameters and YAML files (#508)

* Enable incremental parameter yaml file parsing.

Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com>

* Parse CLI parameter files and overrides in rcl.

Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com>

* Address peer review comments.

Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com>

rcl/include/rcl/arguments.h

@@ -283,8 +283,11 @@ rcl_arguments_get_param_files(
   rcl_allocator_t allocator,
   char *** parameter_files);
 
-/// Return all parameter overrides specified on the command line.
+/// Return all parameter overrides parsed from the command line.
 /**
+ * Parameter overrides are parsed directly from command line arguments and
+ * parameter files provided in the command line.
+ *
  * <hr>
  * Attribute          | Adherence
  * ------------------ | -------------
@@ -294,8 +297,9 @@ rcl_arguments_get_param_files(
  * Lock-Free          | Yes
  *
  * \param[in] arguments An arguments structure that has been parsed.
- * \param[out] parameter_overrides Zero or more parameter overrides.
+ * \param[out] parameter_overrides Parameter overrides as parsed from command line arguments.
  *   This structure must be finalized by the caller.
+ *   The output is NULL if no parameter overrides were parsed.
  * \return `RCL_RET_OK` if everything goes correctly, or
  * \return `RCL_RET_INVALID_ARGUMENT` if any function arguments are invalid, or
  * \return `RCL_RET_BAD_ALLOC` if allocating memory failed, or

rcl/package.xml

@@ -25,6 +25,7 @@
   <test_depend>ament_cmake_pytest</test_depend>
   <test_depend>ament_lint_auto</test_depend>
   <test_depend>ament_lint_common</test_depend>
+  <test_depend>rcpputils</test_depend>
   <test_depend>rmw</test_depend>
   <test_depend>rmw_implementation_cmake</test_depend>
   <test_depend>launch</test_depend>

rcl/src/rcl/arguments.c

@@ -124,13 +124,17 @@ rcl_arguments_get_param_overrides(
   RCL_CHECK_ARGUMENT_FOR_NULL(arguments, RCL_RET_INVALID_ARGUMENT);
   RCL_CHECK_ARGUMENT_FOR_NULL(arguments->impl, RCL_RET_INVALID_ARGUMENT);
   RCL_CHECK_ARGUMENT_FOR_NULL(parameter_overrides, RCL_RET_INVALID_ARGUMENT);
+
   if (NULL != *parameter_overrides) {
     RCL_SET_ERROR_MSG("Output parameter override pointer is not null. May leak memory.");
     return RCL_RET_INVALID_ARGUMENT;
   }
-  *parameter_overrides = rcl_yaml_node_struct_copy(arguments->impl->parameter_overrides);
-  if (NULL == *parameter_overrides) {
-    return RCL_RET_BAD_ALLOC;
+  *parameter_overrides = NULL;
+  if (NULL != arguments->impl->parameter_overrides) {
+    *parameter_overrides = rcl_yaml_node_struct_copy(arguments->impl->parameter_overrides);
+    if (NULL == *parameter_overrides) {
+      return RCL_RET_BAD_ALLOC;
+    }
   }
   return RCL_RET_OK;
 }
@@ -218,6 +222,7 @@ rcl_ret_t
 _rcl_parse_param_file(
   const char * arg,
   rcl_allocator_t allocator,
+  rcl_params_t * params,
   char ** param_file);
 
 /// Parse an argument that may or may not be a parameter file rule.
@@ -237,6 +242,7 @@ rcl_ret_t
 _rcl_parse_param_file_rule(
   const char * arg,
   rcl_allocator_t allocator,
+  rcl_params_t * params,
   char ** param_file);
 
 #define RCL_ENABLE_FLAG_PREFIX "--enable-"
@@ -443,11 +449,11 @@ rcl_parse_arguments(
       // Attempt to parse argument as parameter file rule
       if (strcmp(RCL_PARAM_FILE_FLAG, argv[i]) == 0) {
         if (i + 1 < argc) {
-          // Attempt to parse next argument as remap rule
+          // Attempt to parse next argument as parameter file rule
           args_impl->parameter_files[args_impl->num_param_files_args] = NULL;
           if (
             RCL_RET_OK == _rcl_parse_param_file(
-              argv[i + 1], allocator,
+              argv[i + 1], allocator, args_impl->parameter_overrides,
               &args_impl->parameter_files[args_impl->num_param_files_args]))
           {
             ++(args_impl->num_param_files_args);
@@ -606,7 +612,8 @@ rcl_parse_arguments(
       args_impl->parameter_files[args_impl->num_param_files_args] = NULL;
       if (
         RCL_RET_OK == _rcl_parse_param_file_rule(
-          argv[i], allocator, &(args_impl->parameter_files[args_impl->num_param_files_args])))
+          argv[i], allocator, args_impl->parameter_overrides,
+          &args_impl->parameter_files[args_impl->num_param_files_args]))
       {
         ++(args_impl->num_param_files_args);
         RCUTILS_LOG_WARN_NAMED(ROS_PACKAGE_NAME,
@@ -743,6 +750,12 @@ rcl_parse_arguments(
     }
   }
 
+  // Drop parameter overrides if none was found.
+  if (0U == args_impl->parameter_overrides->num_nodes) {
+    rcl_yaml_node_struct_fini(args_impl->parameter_overrides);
+    args_impl->parameter_overrides = NULL;
+  }
+
   // Shrink unparsed_ros_args
   if (0 == args_impl->num_unparsed_ros_args) {
     // No unparsed ros args
@@ -1811,26 +1824,34 @@ rcl_ret_t
 _rcl_parse_param_file(
   const char * arg,
   rcl_allocator_t allocator,
+  rcl_params_t * params,
   char ** param_file)
 {
   RCL_CHECK_ARGUMENT_FOR_NULL(arg, RCL_RET_INVALID_ARGUMENT);
+  RCL_CHECK_ARGUMENT_FOR_NULL(params, RCL_RET_INVALID_ARGUMENT);
   RCL_CHECK_ARGUMENT_FOR_NULL(param_file, RCL_RET_INVALID_ARGUMENT);
-
   *param_file = rcutils_strdup(arg, allocator);
   if (NULL == *param_file) {
     RCL_SET_ERROR_MSG("Failed to allocate memory for parameters file path");
     return RCL_RET_BAD_ALLOC;
   }
+  if (!rcl_parse_yaml_file(*param_file, params)) {
+    // Error message already set.
+    return RCL_RET_ERROR;
+  }
   return RCL_RET_OK;
 }
 
 rcl_ret_t
 _rcl_parse_param_file_rule(
   const char * arg,
   rcl_allocator_t allocator,
+  rcl_params_t * params,
   char ** param_file)
 {
   RCL_CHECK_ARGUMENT_FOR_NULL(arg, RCL_RET_INVALID_ARGUMENT);
+  RCL_CHECK_ARGUMENT_FOR_NULL(params, RCL_RET_INVALID_ARGUMENT);
+  RCL_CHECK_ARGUMENT_FOR_NULL(param_file, RCL_RET_INVALID_ARGUMENT);
 
   const size_t param_prefix_len = strlen(RCL_PARAM_FILE_ARG_RULE);
   if (strncmp(RCL_PARAM_FILE_ARG_RULE, arg, param_prefix_len) == 0) {
@@ -1841,6 +1862,10 @@ _rcl_parse_param_file_rule(
       return RCL_RET_BAD_ALLOC;
     }
     snprintf(*param_file, outlen + 1, "%s", arg + param_prefix_len);
+    if (!rcl_parse_yaml_file(*param_file, params)) {
+      // Error message already set.
+      return RCL_RET_ERROR;
+    }
     return RCL_RET_OK;
   }
   RCL_SET_ERROR_MSG("Argument does not start with '" RCL_PARAM_FILE_ARG_RULE "'");

rcl/src/rcl/arguments_impl.h

@@ -39,6 +39,7 @@ typedef struct rcl_arguments_impl_t
 
   /// Parameter override rules parsed from arguments.
   rcl_params_t * parameter_overrides;
+
   /// Array of yaml parameter file paths
   char ** parameter_files;
   /// Length of parameter_files.

rcl/test/CMakeLists.txt

@@ -3,6 +3,7 @@ find_package(launch_testing_ament_cmake REQUIRED)
 
 find_package(test_msgs REQUIRED)
 
+find_package(rcpputils REQUIRED)
 find_package(rmw_implementation_cmake REQUIRED)
 
 find_package(osrf_testing_tools_cpp REQUIRED)
@@ -148,7 +149,7 @@ function(test_target_function)
     ENV ${rmw_implementation_env_var}
     APPEND_LIBRARY_DIRS ${extra_lib_dirs}
     LIBRARIES ${PROJECT_NAME} osrf_testing_tools_cpp::memory_tools
-    AMENT_DEPENDENCIES "osrf_testing_tools_cpp"
+    AMENT_DEPENDENCIES "osrf_testing_tools_cpp" "rcpputils"
   )
 
   rcl_add_custom_gtest(test_remap${target_suffix}