Set multiple parameters

[ihasdapie]

Support setting multiple parameters at once using ros2 param set

See: https://github.com/ros2/ros2cli/pull/727#issuecomment-1164515330

This PR uses features from https://github.com/ros2/ros2cli/pull/716 (Diff: https://github.com/ros2/ros2cli/compare/brianc/parameter_client...brianc/multiple_param_set)

$ ros2 param set -h

usage: ros2 param set [-h] [--spin-time SPIN_TIME] [-s] [--no-daemon] [--include-hidden-nodes] node_name parameter_name value

Set parameter

positional arguments:
  node_name             Name of the ROS node
  parameters            List of parameter name and value pairs i.e. "int_param 1 str_param hello_world"

options:
  -h, --help            show this help message and exit
  --spin-time SPIN_TIME
                        Spin time in seconds to wait for discovery (only applies when not using an already running daemon)
  -s, --use-sim-time    Enable ROS simulation time
  --no-daemon           Do not spawn nor use an already running daemon
  --include-hidden-nodes
                        Consider hidden nodes as well

Example usage:

ros2 param set /test_node int_param 12 str_param hello_world

[ihasdapie]

@clalancette Friendly ping -- I've made the requested changes, can you take a look?

[ihasdapie]

I think test needs to be added in ros2cli/ros2param/test to set and get the parameters w/o error.

Done in https://github.com/ros2/ros2cli/pull/728/commits/5547f87a6efc1a3b7894a07bee021e343977cf74. There weren't any tests for the set verb before.

[clalancette]

I just rebased this and made some small fixes to it. With this, it is looking good to me but I'd like another opinion before I merge it. I'll go ahead and run CI, though.

[clalancette]

CI:

• Linux
• Linux-aarch64
• Windows

[scpeters]

For backwards compatibility with ros2 param set node param_name value, I think the sequence of ros2 param set node param_name1 value1 param_name2 value2 makes sense, but it gets tricky to follow visually. I think the launch syntax of param:=value is a little easier to read when there are multiple parameters being set, but it's not clear how to do that in backwards compatible way.

Just an observation, not blocking

[clalancette]

For backwards compatibility with ros2 param set node param_name value, I think the sequence of ros2 param set node param_name1 value1 param_name2 value2 makes sense, but it gets tricky to follow visually. I think the launch syntax of param:=value is a little easier to read when there are multiple parameters being set, but it's not clear how to do that in backwards compatible way.

Yeah, I agree completely. The thing is I think the backwards compatibility is important.

We could do a thing where the first argument is name val, and all subsequent ones are name:=val, but I think that inconsistency is worse. We could also do a thing where we accept name val for a single argument with a warning, and then accept name:=val for arbitrary numbers of values without a warning. That would be more work, but would be more like launch and ros2 run as well.

I'm honestly not sure. @ros2/team thoughts?

[nuclearsandwich]

@ros2/team thoughts?

Perhaps it's possible to maintain the old behavior or at least emit a warning if a sequence of arguments is given and none of the arguments contain the := delimiter, thus indicating that it's a param value param value sequence rather than a param:=value param:=value sequence.

The thing is I think the backwards compatibility is important.

Breaking compatibility to improve usability is worthwhile. I'm not really sure that tick-tock cycles help for command-line tools so I would be in favor of:

• Target Iron for this change (i.e. don't backport it)
• Update the Iron release docs with this as a Notable Change since last release
• Make an extra announcement on Discourse about this when it lands in Rolling

[ihasdapie]

Another option could be to have param value for setting single values and then param1:=value1 param2:=value2... for multiple parameters. That way we keep backwards compatability for single parameters and introduce the more readable syntax for multiple parameters.

EDIT: I realized this is exactly what @clalancette suggested, sorry. I think it'd be better to emit a warning for param value syntax and not support param1 value1 param2 value2 in lieu of bringing users onto param1:=value1... sequences