Support CLI parameter overrides using dots instead of slashes. (#530)

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

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;
 
 

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, &param_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, &param_name);
   if (RCL_RET_OK != ret) {
     if (RCL_RET_WRONG_LEXEME == ret) {
       ret = RCL_RET_INVALID_PARAM_RULE;

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

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);