use regex for wildcard matching

Co-authored-by: Aaron Lipinski <aaron.lipinski@roboticsplus.co.nz>
Signed-off-by: Chen Lihui <lihui.chen@sony.com>

rclcpp/src/rclcpp/detail/resolve_parameter_overrides.cpp

@@ -16,9 +16,11 @@
 
 #include <string>
 #include <map>
+#include <regex>
 #include <vector>
 
 #include "rcl_yaml_param_parser/parser.h"
+#include "rcpputils/find_and_replace.hpp"
 #include "rcpputils/scope_exit.hpp"
 
 #include "rclcpp/parameter_map.hpp"
@@ -51,20 +53,30 @@ rclcpp::detail::resolve_parameter_overrides(
         [params]() {
           rcl_yaml_node_struct_fini(params);
         });
+      // TODO(iuhilnehc-ynos) use map instead of unordered_map for ParameterMap
+      // to set param with contents of parameter file by order
       rclcpp::ParameterMap initial_map = rclcpp::parameter_map_from(params);
 
-      // Enforce wildcard matching precedence
-      // TODO(cottsay) implement further wildcard matching
-      const std::array<std::string, 2> node_matching_names{"/**", node_fqn};
-      for (const auto & node_name : node_matching_names) {
-        if (initial_map.count(node_name) > 0) {
-          // Combine parameter yaml files, overwriting values in older ones
-          for (const rclcpp::Parameter & param : initial_map.at(node_name)) {
+      for (auto & item : initial_map) {
+        if (item.first == node_fqn) {
+          continue;
+        }
+        // Update the regular expression ["/*" -> "(/\\w+)" and "/**" -> "(/\\w+)*"]
+        std::string regex = rcpputils::find_and_replace(item.first, "/*", "(/\\w+)");
+        if (std::regex_match(node_fqn, std::regex(regex))) {
+          for (const rclcpp::Parameter & param : item.second) {
             result[param.get_name()] =
               rclcpp::ParameterValue(param.get_value_message());
           }
         }
       }
+      if (initial_map.count(node_fqn) > 0) {
+        // Combine parameter yaml files, overwriting values in older ones
+        for (const rclcpp::Parameter & param : initial_map.at(node_fqn)) {
+          result[param.get_name()] =
+            rclcpp::ParameterValue(param.get_value_message());
+        }
+      }
     }
   }
 

rclcpp/test/rclcpp/node_interfaces/test_node_parameters.cpp

@@ -31,6 +31,8 @@
 #include "../../mocking_utils/patch.hpp"
 #include "../../utils/rclcpp_gtest_macros.hpp"
 
+#include "rcpputils/filesystem_helper.hpp"
+
 class TestNodeParameters : public ::testing::Test
 {
 public:
@@ -47,6 +49,7 @@ class TestNodeParameters : public ::testing::Test
       dynamic_cast<rclcpp::node_interfaces::NodeParameters *>(
       node->get_node_parameters_interface().get());
     ASSERT_NE(nullptr, node_parameters);
+    test_resources_path /= "test_node_parameters";
   }
 
   void TearDown()
@@ -57,6 +60,8 @@ class TestNodeParameters : public ::testing::Test
 protected:
   std::shared_ptr<rclcpp::Node> node;
   rclcpp::node_interfaces::NodeParameters * node_parameters;
+
+  rcpputils::fs::path test_resources_path{TEST_RESOURCES_DIRECTORY};
 };
 
 TEST_F(TestNodeParameters, construct_destruct_rcl_errors) {
@@ -199,3 +204,67 @@ TEST_F(TestNodeParameters, add_remove_parameters_callback) {
     node_parameters->remove_on_set_parameters_callback(handle.get()),
     std::runtime_error("Callback doesn't exist"));
 }
+
+TEST_F(TestNodeParameters, wildcard_with_namespace)
+{
+  rclcpp::NodeOptions opts;
+  opts.arguments(
+  {
+    "--ros-args",
+    "--params-file", (test_resources_path / "wildcards.yaml").string()
+  });
+
+  std::shared_ptr<rclcpp::Node> node = std::make_shared<rclcpp::Node>("node", "ns", opts);
+
+  auto * node_parameters =
+    dynamic_cast<rclcpp::node_interfaces::NodeParameters *>(
+    node->get_node_parameters_interface().get());
+  ASSERT_NE(nullptr, node_parameters);
+
+  const auto & parameter_overrides = node_parameters->get_parameter_overrides();
+  EXPECT_EQ(7u, parameter_overrides.size());
+  EXPECT_EQ(parameter_overrides.at("full_wild").get<std::string>(), "full_wild");
+  EXPECT_EQ(parameter_overrides.at("namespace_wild").get<std::string>(), "namespace_wild");
+  EXPECT_EQ(
+    parameter_overrides.at("namespace_wild_another").get<std::string>(),
+    "namespace_wild_another");
+  EXPECT_EQ(
+    parameter_overrides.at("namespace_wild_one_star").get<std::string>(),
+    "namespace_wild_one_star");
+  EXPECT_EQ(parameter_overrides.at("node_wild_in_ns").get<std::string>(), "node_wild_in_ns");
+  EXPECT_EQ(
+    parameter_overrides.at("node_wild_in_ns_another").get<std::string>(),
+    "node_wild_in_ns_another");
+  EXPECT_EQ(parameter_overrides.at("explicit_in_ns").get<std::string>(), "explicit_in_ns");
+  EXPECT_EQ(parameter_overrides.count("should_not_appear"), 0u);
+}
+
+TEST_F(TestNodeParameters, wildcard_no_namespace)
+{
+  rclcpp::NodeOptions opts;
+  opts.arguments(
+  {
+    "--ros-args",
+    "--params-file", (test_resources_path / "wildcards.yaml").string()
+  });
+
+  std::shared_ptr<rclcpp::Node> node = std::make_shared<rclcpp::Node>("node", opts);
+
+  auto * node_parameters =
+    dynamic_cast<rclcpp::node_interfaces::NodeParameters *>(
+    node->get_node_parameters_interface().get());
+  ASSERT_NE(nullptr, node_parameters);
+
+  const auto & parameter_overrides = node_parameters->get_parameter_overrides();
+  EXPECT_EQ(5u, parameter_overrides.size());
+  EXPECT_EQ(parameter_overrides.at("full_wild").get<std::string>(), "full_wild");
+  EXPECT_EQ(parameter_overrides.at("namespace_wild").get<std::string>(), "namespace_wild");
+  EXPECT_EQ(
+    parameter_overrides.at("namespace_wild_another").get<std::string>(),
+    "namespace_wild_another");
+  EXPECT_EQ(parameter_overrides.at("node_wild_no_ns").get<std::string>(), "node_wild_no_ns");
+  EXPECT_EQ(parameter_overrides.at("explicit_no_ns").get<std::string>(), "explicit_no_ns");
+  EXPECT_EQ(parameter_overrides.count("should_not_appear"), 0u);
+  // "/*" match exactly one token, not expect to get `namespace_wild_one_star`
+  EXPECT_EQ(parameter_overrides.count("namespace_wild_one_star"), 0u);
+}

rclcpp/test/resources/test_node_parameters/wildcards.yaml

@@ -0,0 +1,57 @@
+/**:
+  ros__parameters:
+    full_wild: "full_wild"
+
+/**:
+  node:
+    ros__parameters:
+      namespace_wild: "namespace_wild"
+
+/**/node:
+  ros__parameters:
+    namespace_wild_another: "namespace_wild_another"
+
+/*:
+  node:
+    ros__parameters:
+      namespace_wild_one_star: "namespace_wild_one_star"
+
+ns:
+  "*":
+    ros__parameters:
+      node_wild_in_ns: "node_wild_in_ns"
+
+/ns/*:
+  ros__parameters:
+    node_wild_in_ns_another: "node_wild_in_ns_another"
+
+ns:
+  node:
+    ros__parameters:
+      explicit_in_ns: "explicit_in_ns"
+
+"*":
+  ros__parameters:
+    node_wild_no_ns: "node_wild_no_ns"
+
+node:
+  ros__parameters:
+    explicit_no_ns: "explicit_no_ns"
+
+ns:
+  nodeX:
+    ros__parameters:
+      should_not_appear: "incorrect_node_name"
+
+/**/nodeX:
+  ros__parameters:
+    should_not_appear: "incorrect_node_name"
+
+nsX:
+  node:
+    ros__parameters:
+      should_not_appear: "incorrect_namespace"
+
+/nsX/*:
+  ros__parameters:
+    should_not_appear: "incorrect_namespace"