From c2e6c0cd3171cb04f30d244727b802af177faedd Mon Sep 17 00:00:00 2001 From: Ivan Santiago Paunovic Date: Fri, 6 Nov 2020 17:47:18 -0300 Subject: [PATCH 1/3] Allow forward slashes within a parameter name rule in argument parsing Signed-off-by: Ivan Santiago Paunovic --- rcl/src/rcl/arguments.c | 43 +++++++++++++++++++++++++++++++++++++++-- 1 file changed, 41 insertions(+), 2 deletions(-) diff --git a/rcl/src/rcl/arguments.c b/rcl/src/rcl/arguments.c index 22306cc53..8c3e7c0c7 100644 --- a/rcl/src/rcl/arguments.c +++ b/rcl/src/rcl/arguments.c @@ -1248,6 +1248,45 @@ _rcl_parse_resource_match( return RCL_RET_OK; } +RCL_LOCAL +rcl_ret_t +_rcl_parse_param_name_token(rcl_lexer_lookahead2_t * lex_lookahead) +{ + rcl_ret_t ret; + rcl_lexeme_t lexeme; + + // Check arguments sanity + assert(NULL != lex_lookahead); + + ret = rcl_lexer_lookahead2_peek(lex_lookahead, &lexeme); + if (RCL_RET_OK != ret) { + return ret; + } + if (RCL_LEXEME_TOKEN != lexeme && RCL_LEXEME_FORWARD_SLASH != lexeme) { + if (RCL_LEXEME_WILD_ONE == lexeme) { + RCL_SET_ERROR_MSG("Wildcard '*' is not implemented"); + return RCL_RET_ERROR; + } else if (RCL_LEXEME_WILD_MULTI == lexeme) { + RCL_SET_ERROR_MSG("Wildcard '**' is not implemented"); + return RCL_RET_ERROR; + } else { + RCL_SET_ERROR_MSG("Expecting token or wildcard"); + return RCL_RET_WRONG_LEXEME; + } + } + do { + ret = rcl_lexer_lookahead2_accept(lex_lookahead, NULL, NULL); + 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_TOKEN == lexeme || RCL_LEXEME_FORWARD_SLASH == lexeme); + return RCL_RET_OK; +} + /// Parse a parameter name in a parameter override rule (ex: `foo.bar`) /** * \sa _rcl_parse_param_rule() @@ -1277,7 +1316,7 @@ _rcl_parse_param_name( } // token ( '.' token )* - ret = _rcl_parse_resource_match_token(lex_lookahead); + ret = _rcl_parse_param_name_token(lex_lookahead); if (RCL_RET_OK != ret) { return ret; } @@ -1290,7 +1329,7 @@ _rcl_parse_param_name( if (RCL_RET_WRONG_LEXEME == ret) { return RCL_RET_INVALID_REMAP_RULE; } - ret = _rcl_parse_resource_match_token(lex_lookahead); + ret = _rcl_parse_param_name_token(lex_lookahead); if (RCL_RET_OK != ret) { return ret; } From 82f7207448876f3358f1dd1b4f7201a81adbf3e2 Mon Sep 17 00:00:00 2001 From: Ivan Santiago Paunovic Date: Wed, 23 Mar 2022 14:42:48 -0300 Subject: [PATCH 2/3] Add test case Signed-off-by: Ivan Santiago Paunovic --- rcl/test/rcl/test_arguments.cpp | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/rcl/test/rcl/test_arguments.cpp b/rcl/test/rcl/test_arguments.cpp index 3f5b462e9..e13fc0c0f 100644 --- a/rcl/test/rcl/test_arguments.cpp +++ b/rcl/test/rcl/test_arguments.cpp @@ -149,10 +149,14 @@ 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"})); + // TODO(ivanpauno): Currently, we're accepting `/`, as they're being accepted by qos overrides. + // We might need to revisit qos overrides parameters names if ROS 2 URIs get + // modified. + EXPECT_TRUE(are_known_ros_args( + {"--ros-args", "-p", "qos_overrides./foo/bar.publisher.history:=keep_last"})); // 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"})); From 012cd8421ed44d08c5d77fb0ac15155202780006 Mon Sep 17 00:00:00 2001 From: Ivan Santiago Paunovic Date: Wed, 23 Mar 2022 14:56:57 -0300 Subject: [PATCH 3/3] please linters Signed-off-by: Ivan Santiago Paunovic --- rcl/test/rcl/test_arguments.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/rcl/test/rcl/test_arguments.cpp b/rcl/test/rcl/test_arguments.cpp index e13fc0c0f..02f8371a7 100644 --- a/rcl/test/rcl/test_arguments.cpp +++ b/rcl/test/rcl/test_arguments.cpp @@ -152,8 +152,9 @@ TEST_F(CLASSNAME(TestArgumentsFixture, RMW_IMPLEMENTATION), check_known_vs_unkno // TODO(ivanpauno): Currently, we're accepting `/`, as they're being accepted by qos overrides. // We might need to revisit qos overrides parameters names if ROS 2 URIs get // modified. - EXPECT_TRUE(are_known_ros_args( - {"--ros-args", "-p", "qos_overrides./foo/bar.publisher.history:=keep_last"})); + EXPECT_TRUE( + are_known_ros_args( + {"--ros-args", "-p", "qos_overrides./foo/bar.publisher.history:=keep_last"})); // 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"}));