ros2 / design

Design documentation for ROS 2.0 effort
http://design.ros2.org/
Apache License 2.0
215 stars 194 forks source link

Clarify ros2 arguments precedence #289

Open ivanpauno opened 4 years ago

ivanpauno commented 4 years ago

Adds a section to the command line arguments article, clarifying how precedence currently works. Adds a discussion section exposing inconsistencies and weird interaction with launch files.

The goal of this PR is to state how precedence currently works to kick off a discussion on how it should work. After that discussion takes place, I will update the document with the conclusion.

Follow up of https://github.com/ros2/rclcpp/issues/953.

ivanpauno commented 4 years ago

@hidmic can you make a first pass?

clalancette commented 4 years ago

As the starting point of what rules are currently in place, this is great. Thanks @ivanpauno .

Where we want to go is harder to say. I would say that there are 2 main problems pointed out here:

  1. The rules for parameter overrides and remapping overrides are different.
  2. The rules for launch are inconsistent with the command-line.

I'm going to concentrate on the second one for now (the first one is unfortunate, but at least documented now). The problem as I understand it is that launch does not (and cannot) know the fully-qualified node name. That's because the node may have hard-coded node names and even namespaces internally to itself. Is that a good summary of the problem?

If that's the case, I'm wondering what would happen if we end up making launch a bit dumber. That is, launch would just pass along the arguments exactly as it had them. Then the precedence rules would be figured out by the rcl/rclcpp layer inside of the node, and the parameters applied. This takes launch completely out of the equation when doing overrides, and concentrates all of the logic of overrides in one place (it needs to be in rcl/rclcpp for the command-line case anyway).

I'm sure there are complications to the above, so I'm looking for feedback on why that would or wouldn't work.

ivanpauno commented 4 years ago

If that's the case, I'm wondering what would happen if we end up making launch a bit dumber. That is, launch would just pass along the arguments exactly as it had them. Then the precedence rules would be figured out by the rcl/rclcpp layer inside of the node, and the parameters applied. This takes launch completely out of the equation when doing overrides, and concentrates all of the logic of overrides in one place (it needs to be in rcl/rclcpp for the command-line case anyway).

The current implementation tries to be as dumb as possible. The problem is that Node action targets a single process with a single node, and rcl doesn't have any assumption about the number of nodes in a process. We could definitely have in launch a MultiNodeExecutable action, which would allow to set up a process with manually composed nodes (trying to use Node action for this doesn't work at all).

When launch generates a command from:

Node(
  package='my_pkg',
  executable='my_exec',
  name = 'my_node',
  parameters=['path/to/file', {'my_param': 'my_value}]
)

the result can be:

(op A)

/path/to/executable/in/package --ros-args -r __node:=my_node --params-file path/to/file -p my_param:=my_value

or:

(op B)

/path/to/executable/in/package --ros-args -r __node:=my_node --params-file path/to/file --params-file path/to/tmp/param/file

where:

# path/to/tmp/param/file contents
/**:
  ros__parameters:
    my_param: my_value

Currently, launch is doing op B, but op A would result in the same problem.

I'm not sure of what modifications in rcl and launch_ros could solve the problem without the user explicitly passing the namespace in the Node action. i.e. I'm not sure if we can make launch more dumb of what currently is (I have tried to think of a solution, and I haven't come up with anything).


Support to wildcards in parameter files was added to solve the problem of not knowing the fully qualified node name at launch time (see https://github.com/ros2/launch_ros/pull/35, https://github.com/ros2/launch_ros/issues/4).

Considering that that was the main goal of wildcards, maybe we could make them have the same precedence than parameter assignments targeting a specific node. The problem is that that is a bit counter intuitive (I find it super counter intuitive, but it would solve the launch file use case).

jacobperron commented 3 years ago

Reviewing proposed changes for additional wildcard support (https://github.com/ros2/rclcpp/issues/1265) brought me here. I realized that wildcard support in parameter YAML files was not documented anywhere and so it would be nice to update this design doc.

I've opened https://github.com/ros2/design/pull/303 to document how wildcards can be used in parameter files. I have not addressed precedence (like this PR aims to do).