Using composition doesn't load costmap parameters from yaml file

[Aposhian]

Bug report

Required Info:

• Operating System:
  • Ubuntu 22.04
• ROS2 Version:
  • humble
• Version or commit hash:
  • 1.1.12-1jammy.20231117.222305
• DDS implementation:
  • CycloneDDS

Steps to reproduce issue

Modify ./nav2_bringup/params/nav2_params.yaml:

global_costmap:
  global_costmap:
    resolution: 0.12345

run a component container:

ros2 run rclcpp_components component_container --ros-args -r __node:=nav2_container

launch with composition enabled:

ros2 launch nav2_bringup navigation_launch.py params_file:=./nav2_bringup/params/nav2_params.yaml use_composition:=True

check param

ros2 param get /global_costmap/global_costmap resolution

Expected behavior

0.12345

Actual behavior

0.1

the default

Additional information

This seems to be because the service used to load composable nodes composition_interfaces/srv/LoadNode only passes the params for that node, not other nodes, since global costmap is a node created inside of the planner server.

Are people working around this some way? It seems like the long-term solution is to refactor out all the nested nodes in Nav2 into their own nodes, and utilize more callback groups and intraprocess communication for efficiencies.

[SteveMacenski]

@Aposhian please check: I believe component containers (wrongly; but maybe unreported?) use parameter files given to it, rather than parameters given to specifically loaded nodes. If you give the params file to the container and the nodes, does that work?

the separation of the container without params file is what points me there as a memory that that’s a thing

[SteveMacenski]

@Aposhian any update?

[Aposhian]

It does look like passing the params file to the composable node container works.

I'm guessing the reason why the composition version of the launch doesn't actually launch the component container, but just loads them into an existing container is that if someone wants to load nav2 into a container that they spawn in a separate (or parent) launch file? That seems like that should be the secondary use-case. Because if someone wants to do that then they may want to write their own launch file anyway (that is what we do) and not necessarily use the pre-built launch files for Nav2. I would expect the pre-built launch files to just "work" regardless of whether composition is enabled or not. But maybe others disagree, since I only use the pre-built launch files for bug reproductions :grin:.

If we want to keep the launch file as is, then it may be worth adding a doc comment or even a launch LogInfo when composition is enabled explaining that the need to launch a separate container, and load the params file into that container.

[SteveMacenski]

I don't disagree that this isn't ideal. I think this ticket should be filed with launch_ros https://github.com/ros2/launch_ros/tree/rolling that the ComposableNode() object doesn't respect its given parameters when loading into a container. Which, if you look at ComposableNode uses, is 100% of its uses. Looking at the implementation of ComposableNodeContainer, I think even if you did give it the list of nodes when the container was constructed, you'd have the exact same issue if you didn't still give the parameters file to the container itself.

I don't understand the separate container comment, you can just provide your param file to the launch file and it should work fine as long as all the nodes' parameters that the container will be loaded with are contained. Not saying that there isn't still a bug here as described above, but that seems like an easy work around (even if having one big params file isn't). If you're owning the container construction that you're passing into Nav2, you can actually pass multiple param file paths to the parameters= in Launch, so you actually wouldn't need them all in a single file. But, you would need to give the container all the params one way or another it appears.

[Aposhian]

the ComposableNode() object doesn't respect its given parameters when loading into a container.

It does respect the given parameters, but it doesn't try to load parameters for different nodes. So it will load parameters for planner_server, but just not for global_costmap. So this is breaking specifically as a result of Nav2 creating nodes inside of nodes.

Don't understand the separate container comment

Could you explain the reasoning why the navigation_launch.py does not launch its own ComposableNodeContainer, and requires the composable node container to be launched elsewhere?

[SteveMacenski]

Because you could want to have a container that contains more than just Nav2. Having a parent container to navigation allows you to load application sp. nodes, localization nodes, and others into the same container.

This seems like a ticket to find with launch_ros. I don't see how anything in Nav2 is broken or misused that are actionable for us here, unfortunately. Launch_ros's composable nodes need to handle parameters not given to the parent container, IMO.

[Aposhian]

Because you could want to have a container that contains more than just Nav2. Having a parent container to navigation allows you to load application sp. nodes, localization nodes, and others into the same container.

Yes, this is true. However, this means that you can't do a one-line launch for Nav2 when running in composition mode, so it is a change in behavior from if you have composition disabled. It threw me for a loop.

Launch_ros's composable nodes need to handle parameters not given to the parent container, IMO.

I'm concerned that pushing for upstream support and maintenance of creating nodes within nodes in rclcpp will not be received well. To me, it seems like the preferred solution to nested nodes should be callback groups, or separate nodes using intraprocess communication where zero-copy is important. Since that is my opinion, I would probably not be a good candidate to try to persuade launch_ros that they need to support this :smile:. But someone else is welcome to ask them.

[SteveMacenski]

I suppose I don't understand - why can't it be a 1-line launch? We do that for the TB3 demos, there aren't multiple commands. The parent launch file launching Nav2 (or even another launch file somewhere else, as long as its executed before that one) just needs to make a container. It then has the option to populate it with whatever other stuff you like for your application or sensors to make it all in a single process.

But someone else is welcome to ask them.

I think you should file a ticket and make sure its documented, if nothing else. Keep in mind that manually composed nodes intrinsically have multiple Nodes in a single process -- that in theory you could make into a composition node to compose with other parts of the system. There are reasons to want to have manually-forced composition of a few modules together. Manual composition is something that is officially supported. The way that the costmaps in the planner/controller server are structured is slightly different from that simple case, but its the same technology problem that if the 'simple' case was solved, would solve this as well. You can propose it more abstractly :-)

[Aposhian]

The parent launch file launching Nav2 (or even another launch file somewhere else, as long as its executed before that one) just needs to make a container.

Exactly. The non-composition launch makes no assumptions about what other launch files are doing: it runs on its own. So it's an unintuitive change of behavior. If I want to run nav2 by itself, I can simply run ros2 launch nav2_bringup navigation_launch.py. But if I add use_composition:=True, now it does not work because:

• it requires a container to be running somewhere else
• I am required to load the nav2 params into that container

Keep in mind that manually composed nodes intrinsically have multiple Nodes in a single process -- that in theory you could make into a composition node to compose with other parts of the system. There are reasons to want to have manually-forced composition of a few modules together. Manual composition is something that is officially supported. The way that the costmaps in the planner/controller server are structured is slightly different from that simple case, but its the same technology problem that if the 'simple' case was solved, would solve this as well. You can propose it more abstractly :-)

Composing multiple nodes into the same process is not the problem. The problem is that the composition API is designed to load one node at a time, and breaks if a node is loaded that then creates another node in a completely different namespace.

I'll file the ticket to document the limitation in launch_ros.

[Aposhian]

Actually, I just looked a little deeper: this is not a launch_ros problem at all: this is an rclcpp problem.

Even if I manually call composition_interfaces/srv/LoadNode with parameters like global_costmap.global_costmap.resolution, they do not get propagated to the costmap.

Example

ros2 run tf2_ros static_transform_publisher --frame-id map --child-frame-id base_link

ros2 run rclcpp_components component_container

ros2 service call /ComponentManager/_container/load_node composition_interfaces/srv/LoadNode "{ package_name: "nav2_planner", plugin_name: "nav2_planner::PlannerServer", parameters: [ {name: "global_costmap.global_costmap.resolution", value: { type: 3, double_value: "1.23"}}]}"
ros2 lifecycle set /planner_server configure
ros2 lifecycle set /planner_server activate
ros2 param get /global_costmap/global_costmap resolution

and you still get the default 0.1.

So I think that fundamentally if you want to provide nodes for "sibling" nodes in rclcpp_components, you have to load the parameters in the container.

[SteveMacenski]

But if I add use_composition:=True, now it does not work because: ...

I think that's a worthwhile trade-off. Navigation should be launched in conjunction with other files (i.e. simulator, hardware, sensor drivers, etc) so there are other files around it which could be feeding it the container. If you have different launch files in different terminals, then that would be an issue.

I don't know if there's a way to detect if a container exists before trying to load, then this launch file could just create its own if not. Otherwise, perhaps have the container name default to empty string. If one is passed, then we use that. Else, we make our own. That I know with python expressions should be possible.

That would be a nice contrib to help with your need that I would merge :smile:

---

The rclcpp problems are good debugging and a slightly different topic, I'll follow up in that ticket if I have anything to add

[Aposhian]

Otherwise, perhaps have the container name default to empty string. If one is passed, then we use that. Else, we make our own. That I know with python expressions should be possible.

Yes I think that would be a good solution.

[SteveMacenski]

@Aposhian did the suggestion in https://github.com/ros2/rclcpp/issues/2404 solve the problem? If so, can you submit a PR to add it, that should be an easy fix + backport

[rayferric]

Hello! Is it possible to load the YAML into an existing component container that was already created in another launch file? The name of the container is known and I have tried using ros2 param load via ExecuteProcess, which did not work. It appears to me that the only way is to load the parameters in the Node definition of the container:

Node(
    package="rclcpp_components",
    executable="component_container",
    name="nav2_container",
    parameters=[nav2_params_path],
),
IncludeLaunchDescription(
    PythonLaunchDescriptionSource("navigation_launch.launch.py"),
    launch_arguments={
        "params_file": nav2_params_path,
        "use_composition": "True",
        "container_name": "nav2_container",
    }.items(),
),

And I do not want to introduce a dependency on Nav2 to the launch file that instantiated the container.

[Aposhian]

Is it possible to load the YAML into an existing component container that was already created in another launch file?

Not that I know of.

[SteveMacenski]

@Aposhian did you end up looking into this with a solution? I tried adding the parent options' parameter overrides & arguments but that continues to fail. I expect that's because its in another namespace or node name. I see that parameter changes for the main node (i.e. PlannerServer) all are made.

https://github.com/ros-planning/navigation2/tree/4011

Costmap2DROS::Costmap2DROS(
  const std::string & name,
  const std::string & parent_namespace,
  const std::string & local_namespace,
  const bool & use_sim_time,
  const rclcpp::NodeOptions & parent_options)
: nav2_util::LifecycleNode(name, "",
    // NodeOption arguments take precedence over the ones provided on the command line
    // use this to make sure the node is placed on the provided namespace
    // TODO(orduno) Pass a sub-node instead of creating a new node for better handling
    //              of the namespaces
    rclcpp::NodeOptions().arguments(parent_options.arguments()).arguments({
    "--ros-args", "-r", std::string("__ns:=") +
    nav2_util::add_namespaces(parent_namespace, local_namespace),
    "--ros-args", "-r", name + ":" + std::string("__node:=") + name,
    "--ros-args", "-p", "use_sim_time:=" + std::string(use_sim_time ? "true" : "false"),
  }).parameter_overrides(parent_options.parameter_overrides())

...

[Aposhian]

No I started to implement what you had there but didn't get very far.

[SteveMacenski]

Got it - I reopened the rclcpp ticket and I'll keep pushing that forward over there since this appears to be an rclcpp bug that is manifesting for us in this way. There doesn't appear to be anything in Nav2's control to work around this besides an rclcpp update.

Closing and moving the discussion over there since this is firmly in that camp now.