Allow forward slashes within a parameter name rule in argument parsing (

#860) * Allow forward slashes within a parameter name rule in argument parsing Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
ros2 · Mar 24, 2022 · ab4f1d6 · ab4f1d6
1 parent d5baf85
commit ab4f1d6
Show file tree

Hide file tree

Showing 2 changed files with 47 additions and 3 deletions.
diff --git 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;
     }

diff --git a/rcl/test/rcl/test_arguments.cpp b/rcl/test/rcl/test_arguments.cpp
@@ -149,10 +149,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"}));
+  // 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"}));