diff --git a/rcl/include/rcl/arguments.h b/rcl/include/rcl/arguments.h index 80cee984d..0e301a207 100644 --- a/rcl/include/rcl/arguments.h +++ b/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. + * *
* 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 diff --git a/rcl/package.xml b/rcl/package.xml index 36f36eab2..fcbd74ebe 100644 --- a/rcl/package.xml +++ b/rcl/package.xml @@ -25,6 +25,7 @@ ament_cmake_pytest ament_lint_auto ament_lint_common + rcpputils rmw rmw_implementation_cmake launch diff --git a/rcl/src/rcl/arguments.c b/rcl/src/rcl/arguments.c index d25620fdd..f4806c8f1 100644 --- a/rcl/src/rcl/arguments.c +++ b/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,16 +1824,21 @@ 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; } @@ -1828,9 +1846,12 @@ 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 "'"); diff --git a/rcl/src/rcl/arguments_impl.h b/rcl/src/rcl/arguments_impl.h index 1aa804d29..9065164c9 100644 --- a/rcl/src/rcl/arguments_impl.h +++ b/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. diff --git a/rcl/test/CMakeLists.txt b/rcl/test/CMakeLists.txt index 58468e60e..f000ce509 100644 --- a/rcl/test/CMakeLists.txt +++ b/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} diff --git a/rcl/test/rcl/test_arguments.cpp b/rcl/test/rcl/test_arguments.cpp index 6cd5b7881..a600c78ba 100644 --- a/rcl/test/rcl/test_arguments.cpp +++ b/rcl/test/rcl/test_arguments.cpp @@ -14,9 +14,11 @@ #include #include +#include #include #include "osrf_testing_tools_cpp/scope_exit.hpp" +#include "rcpputils/filesystem_helper.hpp" #include "rcl/rcl.h" #include "rcl/arguments.h" @@ -36,11 +38,14 @@ class CLASSNAME (TestArgumentsFixture, RMW_IMPLEMENTATION) : public ::testing::T public: void SetUp() { + test_path /= "test_arguments"; } void TearDown() { } + + rcpputils::fs::path test_path{TEST_RESOURCES_DIRECTORY}; }; #define EXPECT_UNPARSED(parsed_args, ...) \ @@ -143,7 +148,9 @@ TEST_F(CLASSNAME(TestArgumentsFixture, RMW_IMPLEMENTATION), check_known_vs_unkno EXPECT_TRUE(are_known_ros_args({"--ros-args", "-p", "/foo/bar:=bar"})); EXPECT_TRUE(are_known_ros_args({"--ros-args", "-p", "foo:=/bar"})); EXPECT_TRUE(are_known_ros_args({"--ros-args", "-p", "/foo123:=/bar123"})); - EXPECT_TRUE(are_known_ros_args({"--ros-args", "--params-file", "file_name.yaml"})); + + const std::string parameters_filepath = (test_path / "test_parameters.1.yaml").string(); + EXPECT_TRUE(are_known_ros_args({"--ros-args", "--params-file", parameters_filepath.c_str()})); EXPECT_FALSE(are_known_ros_args({"--ros-args", "--custom-ros-arg"})); EXPECT_FALSE(are_known_ros_args({"--ros-args", "__node:=node_name"})); @@ -204,7 +211,9 @@ TEST_F(CLASSNAME(TestArgumentsFixture, RMW_IMPLEMENTATION), check_known_deprecat EXPECT_TRUE(are_known_ros_args({"rostopic:///foo/bar:=baz"})); // Setting params file - EXPECT_TRUE(are_known_ros_args({"__params:=file_name.yaml"})); + const std::string parameters_filepath = (test_path / "test_parameters.1.yaml").string(); + const std::string parameter_rule = "__params:=" + parameters_filepath; + EXPECT_TRUE(are_known_ros_args({parameter_rule.c_str()})); // Setting config logging file EXPECT_TRUE(are_known_ros_args({"__log_config_file:=file.config"})); @@ -245,9 +254,10 @@ are_valid_ros_args(std::vector argv) } TEST_F(CLASSNAME(TestArgumentsFixture, RMW_IMPLEMENTATION), check_valid_vs_invalid_args) { + const std::string parameters_filepath = (test_path / "test_parameters.1.yaml").string(); EXPECT_TRUE(are_valid_ros_args({ "--ros-args", "-p", "foo:=bar", "-r", "__node:=node_name", - "--params-file", "file_name.yaml", "--log-level", "INFO", + "--params-file", parameters_filepath.c_str(), "--log-level", "INFO", "--log-config-file", "file.config" })); @@ -530,10 +540,12 @@ TEST_F(CLASSNAME(TestArgumentsFixture, RMW_IMPLEMENTATION), test_double_parse) { rcl_arguments_t parsed_args = rcl_get_zero_initialized_arguments(); ASSERT_EQ(RCL_RET_OK, rcl_parse_arguments(argc, argv, rcl_get_default_allocator(), &parsed_args)); + OSRF_TESTING_TOOLS_CPP_SCOPE_EXIT({ + EXPECT_EQ(RCL_RET_OK, rcl_arguments_fini(&parsed_args)); + }); ASSERT_EQ(RCL_RET_INVALID_ARGUMENT, rcl_parse_arguments(argc, argv, rcl_get_default_allocator(), &parsed_args)); rcl_reset_error(); - EXPECT_EQ(RCL_RET_OK, rcl_arguments_fini(&parsed_args)); } TEST_F(CLASSNAME(TestArgumentsFixture, RMW_IMPLEMENTATION), test_fini_null) { @@ -669,16 +681,19 @@ TEST_F(CLASSNAME(TestArgumentsFixture, RMW_IMPLEMENTATION), test_param_argument_ ret = rcl_parse_arguments(argc, argv, alloc, &parsed_args); ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + OSRF_TESTING_TOOLS_CPP_SCOPE_EXIT({ + EXPECT_EQ(RCL_RET_OK, rcl_arguments_fini(&parsed_args)); + }); int parameter_filecount = rcl_arguments_get_param_files_count(&parsed_args); EXPECT_EQ(0, parameter_filecount); - EXPECT_EQ(RCL_RET_OK, rcl_arguments_fini(&parsed_args)); } TEST_F(CLASSNAME(TestArgumentsFixture, RMW_IMPLEMENTATION), test_param_argument_single) { + const std::string parameters_filepath = (test_path / "test_parameters.1.yaml").string(); const char * argv[] = { "process_name", "--ros-args", "-r", "__ns:=/namespace", "random:=arg", - "--params-file", "parameter_filepath" + "--params-file", parameters_filepath.c_str() }; int argc = sizeof(argv) / sizeof(const char *); rcl_ret_t ret; @@ -688,26 +703,49 @@ TEST_F(CLASSNAME(TestArgumentsFixture, RMW_IMPLEMENTATION), test_param_argument_ ret = rcl_parse_arguments(argc, argv, alloc, &parsed_args); ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + OSRF_TESTING_TOOLS_CPP_SCOPE_EXIT({ + EXPECT_EQ(RCL_RET_OK, rcl_arguments_fini(&parsed_args)); + }); int parameter_filecount = rcl_arguments_get_param_files_count(&parsed_args); EXPECT_EQ(1, parameter_filecount); char ** parameter_files = NULL; ret = rcl_arguments_get_param_files(&parsed_args, alloc, ¶meter_files); ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; - EXPECT_STREQ("parameter_filepath", parameter_files[0]); + EXPECT_STREQ(parameters_filepath.c_str(), parameter_files[0]); for (int i = 0; i < parameter_filecount; ++i) { alloc.deallocate(parameter_files[i], alloc.state); } alloc.deallocate(parameter_files, alloc.state); - EXPECT_EQ(RCL_RET_OK, rcl_arguments_fini(&parsed_args)); + + rcl_params_t * params = NULL; + ret = rcl_arguments_get_param_overrides(&parsed_args, ¶ms); + EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + OSRF_TESTING_TOOLS_CPP_SCOPE_EXIT({ + rcl_yaml_node_struct_fini(params); + }); + EXPECT_EQ(1U, params->num_nodes); + + rcl_variant_t * param_value = + rcl_yaml_node_struct_get("some_node", "param_group.string_param", params); + ASSERT_TRUE(NULL != param_value); + ASSERT_TRUE(NULL != param_value->string_value); + EXPECT_STREQ("foo", param_value->string_value); + + param_value = rcl_yaml_node_struct_get("some_node", "int_param", params); + ASSERT_TRUE(NULL != param_value); + ASSERT_TRUE(NULL != param_value->integer_value); + EXPECT_EQ(1, *(param_value->integer_value)); } // \deprecated to be removed in F-Turtle TEST_F(CLASSNAME(TestArgumentsFixture, RMW_IMPLEMENTATION), test_deprecated_param_argument_single) { + const std::string parameters_filepath = (test_path / "test_parameters.1.yaml").string(); + const std::string parameter_rule = "__params:=" + parameters_filepath; const char * argv[] = { "process_name", "--ros-args", "-r", "__ns:=/namespace", "random:=arg", "--", - "__params:=parameter_filepath" + parameter_rule.c_str() }; int argc = sizeof(argv) / sizeof(const char *); rcl_ret_t ret; @@ -717,25 +755,48 @@ TEST_F(CLASSNAME(TestArgumentsFixture, RMW_IMPLEMENTATION), test_deprecated_para ret = rcl_parse_arguments(argc, argv, alloc, &parsed_args); ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + OSRF_TESTING_TOOLS_CPP_SCOPE_EXIT({ + EXPECT_EQ(RCL_RET_OK, rcl_arguments_fini(&parsed_args)); + }); int parameter_filecount = rcl_arguments_get_param_files_count(&parsed_args); EXPECT_EQ(1, parameter_filecount); char ** parameter_files = NULL; ret = rcl_arguments_get_param_files(&parsed_args, alloc, ¶meter_files); ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; - EXPECT_STREQ("parameter_filepath", parameter_files[0]); + EXPECT_STREQ(parameters_filepath.c_str(), parameter_files[0]); for (int i = 0; i < parameter_filecount; ++i) { alloc.deallocate(parameter_files[i], alloc.state); } alloc.deallocate(parameter_files, alloc.state); - EXPECT_EQ(RCL_RET_OK, rcl_arguments_fini(&parsed_args)); + + rcl_params_t * params = NULL; + ret = rcl_arguments_get_param_overrides(&parsed_args, ¶ms); + EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + OSRF_TESTING_TOOLS_CPP_SCOPE_EXIT({ + rcl_yaml_node_struct_fini(params); + }); + EXPECT_EQ(1U, params->num_nodes); + + rcl_variant_t * param_value = + rcl_yaml_node_struct_get("some_node", "param_group.string_param", params); + ASSERT_TRUE(NULL != param_value); + ASSERT_TRUE(NULL != param_value->string_value); + EXPECT_STREQ("foo", param_value->string_value); + + param_value = rcl_yaml_node_struct_get("some_node", "int_param", params); + ASSERT_TRUE(NULL != param_value); + ASSERT_TRUE(NULL != param_value->integer_value); + EXPECT_EQ(1, *(param_value->integer_value)); } TEST_F(CLASSNAME(TestArgumentsFixture, RMW_IMPLEMENTATION), test_param_argument_multiple) { + const std::string parameters_filepath1 = (test_path / "test_parameters.1.yaml").string(); + const std::string parameters_filepath2 = (test_path / "test_parameters.2.yaml").string(); const char * argv[] = { - "process_name", "--ros-args", "--params-file", "parameter_filepath1", - "-r", "__ns:=/namespace", "random:=arg", "--params-file", "parameter_filepath2" + "process_name", "--ros-args", "--params-file", parameters_filepath1.c_str(), + "-r", "__ns:=/namespace", "random:=arg", "--params-file", parameters_filepath2.c_str() }; int argc = sizeof(argv) / sizeof(const char *); rcl_ret_t ret; @@ -745,19 +806,55 @@ TEST_F(CLASSNAME(TestArgumentsFixture, RMW_IMPLEMENTATION), test_param_argument_ ret = rcl_parse_arguments(argc, argv, alloc, &parsed_args); ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + OSRF_TESTING_TOOLS_CPP_SCOPE_EXIT({ + EXPECT_EQ(RCL_RET_OK, rcl_arguments_fini(&parsed_args)); + }); int parameter_filecount = rcl_arguments_get_param_files_count(&parsed_args); EXPECT_EQ(2, parameter_filecount); char ** parameter_files = NULL; ret = rcl_arguments_get_param_files(&parsed_args, alloc, ¶meter_files); ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; - EXPECT_STREQ("parameter_filepath1", parameter_files[0]); - EXPECT_STREQ("parameter_filepath2", parameter_files[1]); + EXPECT_STREQ(parameters_filepath1.c_str(), parameter_files[0]); + EXPECT_STREQ(parameters_filepath2.c_str(), parameter_files[1]); for (int i = 0; i < parameter_filecount; ++i) { alloc.deallocate(parameter_files[i], alloc.state); } + alloc.deallocate(parameter_files, alloc.state); - EXPECT_EQ(RCL_RET_OK, rcl_arguments_fini(&parsed_args)); + + rcl_params_t * params = NULL; + ret = rcl_arguments_get_param_overrides(&parsed_args, ¶ms); + EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + OSRF_TESTING_TOOLS_CPP_SCOPE_EXIT({ + rcl_yaml_node_struct_fini(params); + }); + EXPECT_EQ(2U, params->num_nodes); + + rcl_variant_t * param_value = + rcl_yaml_node_struct_get("some_node", "int_param", params); + ASSERT_TRUE(NULL != param_value); + ASSERT_TRUE(NULL != param_value->integer_value); + EXPECT_EQ(3, *(param_value->integer_value)); + + param_value = rcl_yaml_node_struct_get("some_node", "param_group.string_param", params); + ASSERT_TRUE(NULL != param_value); + ASSERT_TRUE(NULL != param_value->string_value); + EXPECT_STREQ("foo", param_value->string_value); + + param_value = rcl_yaml_node_struct_get("another_node", "double_param", params); + ASSERT_TRUE(NULL != param_value); + ASSERT_TRUE(NULL != param_value->double_value); + EXPECT_DOUBLE_EQ(1.0, *(param_value->double_value)); + + param_value = rcl_yaml_node_struct_get("another_node", "param_group.bool_array_param", params); + ASSERT_TRUE(NULL != param_value); + ASSERT_TRUE(NULL != param_value->bool_array_value); + ASSERT_TRUE(NULL != param_value->bool_array_value->values); + ASSERT_EQ(3U, param_value->bool_array_value->size); + EXPECT_TRUE(param_value->bool_array_value->values[0]); + EXPECT_FALSE(param_value->bool_array_value->values[1]); + EXPECT_FALSE(param_value->bool_array_value->values[2]); } TEST_F(CLASSNAME(TestArgumentsFixture, RMW_IMPLEMENTATION), test_no_param_overrides) { @@ -769,6 +866,9 @@ TEST_F(CLASSNAME(TestArgumentsFixture, RMW_IMPLEMENTATION), test_no_param_overri rcl_ret_t ret = rcl_parse_arguments(argc, argv, alloc, &parsed_args); ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + OSRF_TESTING_TOOLS_CPP_SCOPE_EXIT({ + EXPECT_EQ(RCL_RET_OK, rcl_arguments_fini(&parsed_args)); + }); ret = rcl_arguments_get_param_overrides(&parsed_args, NULL); EXPECT_EQ(RCL_RET_INVALID_ARGUMENT, ret) << rcl_get_error_string().str; @@ -788,16 +888,15 @@ TEST_F(CLASSNAME(TestArgumentsFixture, RMW_IMPLEMENTATION), test_no_param_overri params = NULL; ret = rcl_arguments_get_param_overrides(&parsed_args, ¶ms); EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; - EXPECT_EQ(0U, params->num_nodes); - rcl_yaml_node_struct_fini(params); - - EXPECT_EQ(RCL_RET_OK, rcl_arguments_fini(&parsed_args)); + EXPECT_TRUE(NULL == params); } -TEST_F(CLASSNAME(TestArgumentsFixture, RMW_IMPLEMENTATION), test_two_param_overrides) { +TEST_F(CLASSNAME(TestArgumentsFixture, RMW_IMPLEMENTATION), test_param_overrides) { + const std::string parameters_filepath = (test_path / "test_parameters.1.yaml").string(); const char * argv[] = { "process_name", "--ros-args", - "--param", "string_param:=test_string", + "--params-file", parameters_filepath.c_str(), + "--param", "string_param:=bar", "-p", "some_node:int_param:=4" }; int argc = sizeof(argv) / sizeof(const char *); @@ -823,10 +922,15 @@ TEST_F(CLASSNAME(TestArgumentsFixture, RMW_IMPLEMENTATION), test_two_param_overr rcl_yaml_node_struct_get("/**", "string_param", params); ASSERT_TRUE(NULL != param_value); ASSERT_TRUE(NULL != param_value->string_value); - EXPECT_STREQ("test_string", param_value->string_value); + EXPECT_STREQ("bar", param_value->string_value); param_value = rcl_yaml_node_struct_get("some_node", "int_param", params); ASSERT_TRUE(NULL != param_value); ASSERT_TRUE(NULL != param_value->integer_value); EXPECT_EQ(4, *(param_value->integer_value)); + + param_value = rcl_yaml_node_struct_get("some_node", "param_group.string_param", params); + ASSERT_TRUE(NULL != param_value); + ASSERT_TRUE(NULL != param_value->string_value); + EXPECT_STREQ("foo", param_value->string_value); } diff --git a/rcl/test/resources/test_arguments/test_parameters.1.yaml b/rcl/test/resources/test_arguments/test_parameters.1.yaml new file mode 100644 index 000000000..8c8faed32 --- /dev/null +++ b/rcl/test/resources/test_arguments/test_parameters.1.yaml @@ -0,0 +1,5 @@ +some_node: + ros__parameters: + int_param: 1 + param_group: + string_param: foo diff --git a/rcl/test/resources/test_arguments/test_parameters.2.yaml b/rcl/test/resources/test_arguments/test_parameters.2.yaml new file mode 100644 index 000000000..e1e3c1627 --- /dev/null +++ b/rcl/test/resources/test_arguments/test_parameters.2.yaml @@ -0,0 +1,8 @@ +some_node: + ros__parameters: + int_param: 3 +another_node: + ros__parameters: + double_param: 1.0 + param_group: + bool_array_param: [true, false, false] diff --git a/rcl_yaml_param_parser/src/parser.c b/rcl_yaml_param_parser/src/parser.c index 4fdbe7110..379d3b75b 100644 --- a/rcl_yaml_param_parser/src/parser.c +++ b/rcl_yaml_param_parser/src/parser.c @@ -137,6 +137,8 @@ static rcutils_ret_t parse_key( const yaml_event_t event, uint32_t * map_level, bool * is_new_map, + size_t * node_idx, + size_t * parameter_idx, namespace_tracker_t * ns_tracker, rcl_params_t * params_st); @@ -1295,6 +1297,8 @@ static rcutils_ret_t parse_key( const yaml_event_t event, uint32_t * map_level, bool * is_new_map, + size_t * node_idx, + size_t * parameter_idx, namespace_tracker_t * ns_tracker, rcl_params_t * params_st) { @@ -1323,11 +1327,6 @@ static rcutils_ret_t parse_key( } } - size_t node_idx = 0U; - size_t num_nodes = params_st->num_nodes; - if (num_nodes > 0U) { - node_idx = num_nodes - 1U; - } rcutils_ret_t ret = RCUTILS_RET_OK; switch (*map_level) { case MAP_UNINIT_LVL: @@ -1359,21 +1358,18 @@ static rcutils_ret_t parse_key( ret = RCUTILS_RET_BAD_ALLOC; break; } - params_st->node_names[num_nodes] = node_name_ns; - ret = rem_name_from_ns(ns_tracker, NS_TYPE_NODE, allocator); + ret = find_node(node_name_ns, params_st, node_idx); if (RCUTILS_RET_OK != ret) { - RCUTILS_SET_ERROR_MSG_WITH_FORMAT_STRING( - "Internal error adding node namespace at line %d", line_num); break; } - ret = node_params_init(&(params_st->params[num_nodes]), allocator); + + ret = rem_name_from_ns(ns_tracker, NS_TYPE_NODE, allocator); if (RCUTILS_RET_OK != ret) { RCUTILS_SET_ERROR_MSG_WITH_FORMAT_STRING( - "Error creating node parameter at line %d", line_num); + "Internal error adding node namespace at line %d", line_num); break; } - params_st->num_nodes++; /// Bump the map level to PARAMS (*map_level)++; } @@ -1381,15 +1377,12 @@ static rcutils_ret_t parse_key( break; case MAP_PARAMS_LVL: { - size_t parameter_idx; char * parameter_ns; char * param_name; /// If it is a new map, the previous key is param namespace if (true == *is_new_map) { - params_st->params[node_idx].num_params--; - parameter_idx = params_st->params[node_idx].num_params; - parameter_ns = params_st->params[node_idx].parameter_names[parameter_idx]; + parameter_ns = params_st->params[*node_idx].parameter_names[*parameter_idx]; if (NULL == parameter_ns) { RCUTILS_SET_ERROR_MSG_WITH_FORMAT_STRING( "Internal error creating param namespace at line %d", line_num); @@ -1408,7 +1401,7 @@ static rcutils_ret_t parse_key( } // Guard against adding more than the maximum allowed parameters - if (params_st->params[node_idx].num_params >= MAX_NUM_PARAMS_PER_NODE) { + if (params_st->params[*node_idx].num_params >= MAX_NUM_PARAMS_PER_NODE) { RCUTILS_SET_ERROR_MSG_WITH_FORMAT_STRING( "Exceeded maximum allowed number of parameters for a node (%d)", MAX_NUM_PARAMS_PER_NODE); @@ -1417,15 +1410,18 @@ static rcutils_ret_t parse_key( } /// Add a parameter name into the node parameters - parameter_idx = params_st->params[node_idx].num_params; parameter_ns = ns_tracker->parameter_ns; if (NULL == parameter_ns) { - param_name = rcutils_strdup(value, allocator); - if (NULL == param_name) { - ret = RCUTILS_RET_BAD_ALLOC; + ret = find_parameter(*node_idx, value, params_st, parameter_idx); + if (ret != RCUTILS_RET_OK) { break; } } else { + ret = find_parameter(*node_idx, parameter_ns, params_st, parameter_idx); + if (ret != RCUTILS_RET_OK) { + break; + } + const size_t params_ns_len = strlen(parameter_ns); const size_t param_name_len = strlen(value); const size_t tot_len = (params_ns_len + param_name_len + 2U); @@ -1447,9 +1443,9 @@ static rcutils_ret_t parse_key( param_name[params_ns_len] = '.'; memmove((param_name + params_ns_len + 1U), value, param_name_len); param_name[tot_len - 1U] = '\0'; + + params_st->params[*node_idx].parameter_names[*parameter_idx] = param_name; } - params_st->params[node_idx].parameter_names[parameter_idx] = param_name; - params_st->params[node_idx].num_params++; } break; default: @@ -1484,6 +1480,8 @@ static rcutils_ret_t parse_file_events( &allocator, "invalid allocator", return RCUTILS_RET_INVALID_ARGUMENT); yaml_event_t event; + size_t node_idx = 0; + size_t parameter_idx = 0; rcutils_ret_t ret = RCUTILS_RET_OK; while (0 == done_parsing) { if (RCUTILS_RET_OK != ret) { @@ -1506,7 +1504,8 @@ static rcutils_ret_t parse_file_events( { /// Need to toggle between key and value at params level if (true == is_key) { - ret = parse_key(event, &map_level, &is_new_map, ns_tracker, params_st); + ret = parse_key( + event, &map_level, &is_new_map, &node_idx, ¶meter_idx, ns_tracker, params_st); if (RCUTILS_RET_OK != ret) { break; } @@ -1525,14 +1524,12 @@ static rcutils_ret_t parse_file_events( yaml_event_delete(&event); return RCUTILS_RET_ERROR; } - const size_t node_idx = (params_st->num_nodes - 1U); if (0U == params_st->params[node_idx].num_params) { RCUTILS_SET_ERROR_MSG_WITH_FORMAT_STRING( "Cannot have a value before %s at line %d", PARAMS_KEY, line_num); yaml_event_delete(&event); return RCUTILS_RET_ERROR; } - const size_t parameter_idx = (params_st->params[node_idx].num_params - 1U); ret = parse_value(event, is_seq, node_idx, parameter_idx, &seq_data_type, params_st); if (RCUTILS_RET_OK != ret) { break; diff --git a/rcl_yaml_param_parser/test/overlay.yaml b/rcl_yaml_param_parser/test/overlay.yaml new file mode 100644 index 000000000..fa822ed12 --- /dev/null +++ b/rcl_yaml_param_parser/test/overlay.yaml @@ -0,0 +1,13 @@ +# config/test_yaml +--- + +lidar_ns: + lidar_2: + ros__parameters: + is_back: true +camera: + ros__parameters: + loc: back +intel: + ros__parameters: + num_cores: 12 diff --git a/rcl_yaml_param_parser/test/test_parse_yaml.cpp b/rcl_yaml_param_parser/test/test_parse_yaml.cpp index 25706a75c..6d593c27d 100644 --- a/rcl_yaml_param_parser/test/test_parse_yaml.cpp +++ b/rcl_yaml_param_parser/test/test_parse_yaml.cpp @@ -48,6 +48,15 @@ TEST(test_parser, correct_syntax) { bool res = rcl_parse_yaml_file(path, params_hdl); ASSERT_TRUE(res) << rcutils_get_error_string().str; + char * another_path = rcutils_join_path(test_path, "overlay.yaml", allocator); + ASSERT_TRUE(NULL != another_path) << rcutils_get_error_string().str; + OSRF_TESTING_TOOLS_CPP_SCOPE_EXIT({ + allocator.deallocate(another_path, allocator.state); + }); + ASSERT_TRUE(rcutils_exists(another_path)) << "No test YAML file found at " << another_path; + res = rcl_parse_yaml_file(another_path, params_hdl); + ASSERT_TRUE(res) << rcutils_get_error_string().str; + rcl_params_t * copy_of_params_hdl = rcl_yaml_node_struct_copy(params_hdl); ASSERT_TRUE(NULL != copy_of_params_hdl) << rcutils_get_error_string().str; OSRF_TESTING_TOOLS_CPP_SCOPE_EXIT({ @@ -59,11 +68,11 @@ TEST(test_parser, correct_syntax) { rcl_variant_t * param_value = rcl_yaml_node_struct_get("lidar_ns/lidar_2", "is_back", params); ASSERT_TRUE(NULL != param_value) << rcutils_get_error_string().str; ASSERT_TRUE(NULL != param_value->bool_value); - EXPECT_FALSE(*param_value->bool_value); - res = rcl_parse_yaml_value("lidar_ns/lidar_2", "is_back", "true", params); + EXPECT_TRUE(*param_value->bool_value); + res = rcl_parse_yaml_value("lidar_ns/lidar_2", "is_back", "false", params); EXPECT_TRUE(res) << rcutils_get_error_string().str; ASSERT_TRUE(NULL != param_value->bool_value); - EXPECT_TRUE(*param_value->bool_value); + EXPECT_FALSE(*param_value->bool_value); param_value = rcl_yaml_node_struct_get("lidar_ns/lidar_2", "id", params); ASSERT_TRUE(NULL != param_value) << rcutils_get_error_string().str; @@ -74,6 +83,15 @@ TEST(test_parser, correct_syntax) { ASSERT_TRUE(NULL != param_value->integer_value); EXPECT_EQ(12, *param_value->integer_value); + param_value = rcl_yaml_node_struct_get("camera", "loc", params); + ASSERT_TRUE(NULL != param_value) << rcutils_get_error_string().str; + ASSERT_TRUE(NULL != param_value->string_value); + EXPECT_STREQ("back", param_value->string_value); + res = rcl_parse_yaml_value("camera", "loc", "front", params); + EXPECT_TRUE(res) << rcutils_get_error_string().str; + ASSERT_TRUE(NULL != param_value->string_value); + EXPECT_STREQ("front", param_value->string_value); + param_value = rcl_yaml_node_struct_get("camera", "cam_spec.angle", params); ASSERT_TRUE(NULL != param_value) << rcutils_get_error_string().str; ASSERT_TRUE(NULL != param_value->double_value); @@ -83,6 +101,15 @@ TEST(test_parser, correct_syntax) { ASSERT_TRUE(NULL != param_value->double_value); EXPECT_DOUBLE_EQ(2.2, *param_value->double_value); + param_value = rcl_yaml_node_struct_get("intel", "num_cores", params); + ASSERT_TRUE(NULL != param_value) << rcutils_get_error_string().str; + ASSERT_TRUE(NULL != param_value->integer_value); + EXPECT_EQ(12, *param_value->integer_value); + res = rcl_parse_yaml_value("intel", "num_cores", "8", params); + EXPECT_TRUE(res) << rcutils_get_error_string().str; + ASSERT_TRUE(NULL != param_value->integer_value); + EXPECT_EQ(8, *param_value->integer_value); + param_value = rcl_yaml_node_struct_get("intel", "arch", params); ASSERT_TRUE(NULL != param_value) << rcutils_get_error_string().str; ASSERT_TRUE(NULL != param_value->string_value);