Get parameter map

[clalancette]

This PR adds the ability to get a std::map set of key-value pairs of parameters. In this case, parameters that form part of a map are all of the form:

param.x
param.y

Assuming param.x has value 0.1 and param.y has value 1.5, then calling this API like:

std::map<std::string, double> params;
node->get_parameter("param", params);

Would fill in the params map with keys of x and y with values of 0.1 and 1.5, respectively. There is also a set_parameter_if_not_set that works much in the same way for setting values. Tests for this new functionality are added in a new test file; I'll move the rest of the "local" parameter tests from system_tests once this is in. This should solve #529 , and will unblock ros2/teleop_twist_joy#8

connects to #529

[clalancette]

CI (up to test_rclcpp, testing only rclcpp and test_rclcpp):

• Linux
• Linux-aarch64
• macOS
• Windows

[dirk-thomas]

Since the methods sets multiple parameters the method name should be plural: set_parameters_if_not_set.

Same for get_parameter() below.

[clalancette]

I don't feel strongly about the name, but I will point out that in ROS1 all of the methods were named the same, regardless of whether they were setting a map, a vector, or a single value. I think there is value to this here as well, since there is only "one" name to remember, regardless of the type you want to set. However, if you insist, I will change the name to be plural.

[dirk-thomas]

Regarding the signature I would rather not pass name here. Each key in the map can already be a "full" name. Otherwise this function prohibits to set two parameters in different "branches" of the "naming tree".

Same below.

[clalancette]

While that is true (and makes some sense for set), it doesn't really make much sense for get. If we just make it so that there is no "grouping" with this API, then there is little reason to add this API; we can do the same thing by calling get_parameters with a vector of names. The explicit purpose of the map-based one is to have a group of variables that we can get into a single map. This is similar to the map-based getParam overload in ROS1.

[dirk-thomas]

Instead of calling get_parameter N times I would suggest to call get_parameters() once.

[clalancette]

I can make this work using get_parameters(), but I think it is much uglier and less efficient:

template<typename MapValueT>
void
Node::set_parameter_if_not_set(
  const std::string & name,
  const std::map<std::string, MapValueT> & values)
{
  std::vector<std::string> names;

  for (const auto & val : values) {
    std::string param_name = name + "." + val.first;
    names.push_back(param_name);
  }

  std::vector<rclcpp::Parameter> params = node_parameters_->get_parameters(names);
  std::vector<rclcpp::Parameter> params_to_set;

  for (const auto & val : values) {
    std::string param_name = name + "." + val.first;
    if (std::find_if(params.begin(), params.end(),
                     [&param_name](const rclcpp::Parameter& param) {
                       return param.get_name() == param_name;
                     }) == params.end()) {
      params_to_set.push_back(rclcpp::Parameter(param_name, val.second));
    }
  }

  this->set_parameters(params_to_set);
}

This is due to the fact that get_parameters() returns only what is currently in the parameters, and silently ignores the ones that aren't there. Unless you can think of a way to reduce the number of loops and finds here, I'd prefer to stick to the current implementation.

[clalancette]

@ros2/team There are two open questions on this PR:

1. Whether the APIs that modify multiple values should be plural. On the one hand, this makes sense from a grammatical sense. On the other hand, all of the APIs in ROS 1 were named the same, regardless of how many items they touched (I'm thinking of getParam() and friends here). Opinions?
2. Should we have a map-based overload at all? I think it makes it much more convenient to get a group of similar values, but it is simple syntactic sugar and all of the values can just be fetched using a vector of names. If the answer to this is "no", then I'll just close out this PR and do something different in Add in the ability to control via parameters teleop_twist_joy#8

[clalancette]

(also, I'm going to close and re-open this PR to retrigger the build, since I think the failure was unrelated to this PR)

[tfoote]

1. Regarding plural vs singular I'd suggest that we go with plural we're already using the plural in most other situations like describe parameters and get_parameter_types

2. I think that the std::map based interface is a significant improvement for searching and querying compared to a vector that exposing it will be a significant benefit to the user. It might make sense to implement it as the syntactic sugar, where the API just vectorizes the requests before interacting with the underlying implementation.

[jacobperron]

1. The vector based interface is already plural, so I think making the map based interface plural is consistent.
2. +1 for std:map based interface.

[clalancette]

Updated to use plurals. Here's CI up to rclcpp:

• Linux
• Linux-aarch64
• macOS
• Windows

[wjwwood]

lgtm, only comments about documentation