From 513a185d1b8ad182f74519194682f10f132e5dc9 Mon Sep 17 00:00:00 2001 From: Michel Hidalgo Date: Thu, 24 Oct 2019 13:02:46 -0300 Subject: [PATCH] Support CLI parameter overrides using dots instead of slashes. (#530) Signed-off-by: Michel Hidalgo --- rcl/include/rcl/lexer.h | 6 ++- rcl/src/rcl/arguments.c | 68 ++++++++++++++++++++++++++++++++- rcl/src/rcl/lexer.c | 8 +++- rcl/test/rcl/test_arguments.cpp | 19 +++++++-- 4 files changed, 94 insertions(+), 7 deletions(-) diff --git a/rcl/include/rcl/lexer.h b/rcl/include/rcl/lexer.h index d87a85acd..2ad41608b 100644 --- a/rcl/include/rcl/lexer.h +++ b/rcl/include/rcl/lexer.h @@ -73,7 +73,11 @@ typedef enum rcl_lexeme_t /// * RCL_LEXEME_WILD_ONE = 20, /// ** - RCL_LEXEME_WILD_MULTI = 21 + RCL_LEXEME_WILD_MULTI = 21, + // TODO(hidmic): remove when parameter names are standardized to + // use slashes in lieu of dots + /// . + RCL_LEXEME_DOT = 22, } rcl_lexeme_t; diff --git a/rcl/src/rcl/arguments.c b/rcl/src/rcl/arguments.c index a64fe3541..cf62670bb 100644 --- a/rcl/src/rcl/arguments.c +++ b/rcl/src/rcl/arguments.c @@ -1351,6 +1351,70 @@ _rcl_parse_resource_match( return RCL_RET_OK; } +/// Parse a parameter name in a parameter override rule (ex: `foo.bar`) +/** + * \sa _rcl_parse_param_rule() + */ +// TODO(hidmic): remove when parameter names are standardized to use slashes +// in lieu of dots. +RCL_LOCAL +rcl_ret_t +_rcl_parse_param_name( + rcl_lexer_lookahead2_t * lex_lookahead, + rcl_allocator_t allocator, + char ** param_name) +{ + rcl_ret_t ret; + rcl_lexeme_t lexeme; + + // Check arguments sanity + assert(NULL != lex_lookahead); + assert(rcutils_allocator_is_valid(&allocator)); + assert(NULL != param_name); + assert(NULL == *param_name); + + const char * name_start = rcl_lexer_lookahead2_get_text(lex_lookahead); + if (NULL == name_start) { + RCL_SET_ERROR_MSG("failed to get start of param name"); + return RCL_RET_ERROR; + } + + // token ( '.' token )* + ret = _rcl_parse_resource_match_token(lex_lookahead); + if (RCL_RET_OK != ret) { + return ret; + } + ret = rcl_lexer_lookahead2_peek(lex_lookahead, &lexeme); + if (RCL_RET_OK != ret) { + return ret; + } + while (RCL_LEXEME_SEPARATOR != lexeme) { + ret = rcl_lexer_lookahead2_expect(lex_lookahead, RCL_LEXEME_DOT, NULL, NULL); + if (RCL_RET_WRONG_LEXEME == ret) { + return RCL_RET_INVALID_REMAP_RULE; + } + ret = _rcl_parse_resource_match_token(lex_lookahead); + if (RCL_RET_OK != ret) { + return ret; + } + ret = rcl_lexer_lookahead2_peek(lex_lookahead, &lexeme); + if (RCL_RET_OK != ret) { + return ret; + } + } + + // Copy param name + const char * name_end = rcl_lexer_lookahead2_get_text(lex_lookahead); + const size_t length = (size_t)(name_end - name_start); + *param_name = rcutils_strndup(name_start, length, allocator); + if (NULL == *param_name) { + RCL_SET_ERROR_MSG("failed to copy param name"); + return RCL_RET_BAD_ALLOC; + } + + return RCL_RET_OK; +} + /// Parse the match side of a name remapping rule (ex: `rostopic://foo`) /** @@ -1796,7 +1860,9 @@ _rcl_parse_param_rule( } } - ret = _rcl_parse_resource_match(&lex_lookahead, params->allocator, ¶m_name); + // TODO(hidmic): switch to _rcl_parse_resource_match() when parameter names + // are standardized to use slashes in lieu of dots. + ret = _rcl_parse_param_name(&lex_lookahead, params->allocator, ¶m_name); if (RCL_RET_OK != ret) { if (RCL_RET_WRONG_LEXEME == ret) { ret = RCL_RET_INVALID_PARAM_RULE; diff --git a/rcl/src/rcl/lexer.c b/rcl/src/rcl/lexer.c index dd4445361..f6c7d2289 100644 --- a/rcl/src/rcl/lexer.c +++ b/rcl/src/rcl/lexer.c @@ -57,8 +57,10 @@ digraph remapping_lexer { T_WILD_MULTI T_EOF T_NONE + T_DOT node [shape = circle]; S0 -> T_FORWARD_SLASH [ label = "/"]; + S0 -> T_DOT [ label = "."]; S0 -> S1 [ label = "\\"]; S0 -> S2 [ label = "~"]; S0 -> S3 [ label = "_" ]; @@ -155,7 +157,7 @@ typedef struct rcl_lexer_state_t /// Movement associated with taking else state const unsigned char else_movement; /// Transitions in the state machine (NULL value at end of array) - const rcl_lexer_transition_t transitions[11]; + const rcl_lexer_transition_t transitions[12]; } rcl_lexer_state_t; #define S0 0u @@ -213,6 +215,7 @@ typedef struct rcl_lexer_state_t #define T_WILD_MULTI 50u #define T_EOF 51u #define T_NONE 52u +#define T_DOT 53u // used to figure out if a state is terminal or not #define FIRST_TERMINAL T_TILDE_SLASH @@ -229,6 +232,7 @@ static const rcl_lexer_state_t g_states[LAST_STATE + 1] = 0u, { {T_FORWARD_SLASH, '/', '/'}, + {T_DOT, '.', '.'}, {S1, '\\', '\\'}, {S2, '~', '~'}, {S3, '_', '_'}, @@ -573,6 +577,8 @@ static const rcl_lexeme_t g_terminals[LAST_TERMINAL + 1] = { RCL_LEXEME_EOF, // 21 RCL_LEXEME_NONE, + // 22 + RCL_LEXEME_DOT }; rcl_ret_t diff --git a/rcl/test/rcl/test_arguments.cpp b/rcl/test/rcl/test_arguments.cpp index 2dc9a9758..0f8964069 100644 --- a/rcl/test/rcl/test_arguments.cpp +++ b/rcl/test/rcl/test_arguments.cpp @@ -144,10 +144,15 @@ TEST_F(CLASSNAME(TestArgumentsFixture, RMW_IMPLEMENTATION), check_known_vs_unkno EXPECT_TRUE(are_known_ros_args({"--ros-args", "-r", "rostopic:///rosservice:=rostopic"})); EXPECT_TRUE(are_known_ros_args({"--ros-args", "-r", "rostopic:///foo/bar:=baz"})); EXPECT_TRUE(are_known_ros_args({"--ros-args", "-p", "foo:=bar"})); - EXPECT_TRUE(are_known_ros_args({"--ros-args", "-p", "~/foo:=~/bar"})); - 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"})); + // TODO(hidmic): restore tests (and drop the following ones) when parameter names + // are standardized to use slashes in lieu of dots. + // EXPECT_TRUE(are_known_ros_args({"--ros-args", "-p", "~/foo:=~/bar"})); + // 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", "-p", "foo.bar:=bar"})); + EXPECT_TRUE(are_known_ros_args({"--ros-args", "-p", "node:foo:=bar"})); + EXPECT_TRUE(are_known_ros_args({"--ros-args", "-p", "fizz123:=buzz456"})); 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()})); @@ -998,6 +1003,7 @@ TEST_F(CLASSNAME(TestArgumentsFixture, RMW_IMPLEMENTATION), test_param_overrides "process_name", "--ros-args", "--params-file", parameters_filepath.c_str(), "--param", "string_param:=bar", + "-p", "some.bool_param:=false", "-p", "some_node:int_param:=4" }; int argc = sizeof(argv) / sizeof(const char *); @@ -1025,6 +1031,11 @@ TEST_F(CLASSNAME(TestArgumentsFixture, RMW_IMPLEMENTATION), test_param_overrides ASSERT_TRUE(NULL != param_value->string_value); EXPECT_STREQ("bar", param_value->string_value); + param_value = rcl_yaml_node_struct_get("/**", "some.bool_param", params); + ASSERT_TRUE(NULL != param_value); + ASSERT_TRUE(NULL != param_value->bool_value); + EXPECT_FALSE(*(param_value->bool_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);