composition LoadNode does not set parameters for nodes in other namespaces about rclcpp HOT 9 OPEN

If I understand correctly, when components are loaded the parameters/remap rules/ros args are passed into the node via NodeOptions. See here where the NodeOptions are created:

Outside of NodeOptions, the other way to get arguments is via the CLI. These are considered "Global arguments" because they apply to all nodes in the process. There's an option to disable them.

I think global arguments on composable nodes are by default disabled. When loading composable nodes one normally doesn't want the node to get the parameters of the component manager, or of any other node loaded into the component manager. There is an option to allow it though:

This issue is that sometimes a component node is itself a manually composed set of nodes. The other nodes within the component are not getting their parameters forwarded even though they're in the file passed to the component to load with the node.

I would recommend instead making the top level node pass the arguments from its own NodeOptions into the NodeOptions of the nodes it is manually composing. Here's the API to get the options off the node that's doing the composition.

from rclcpp.

are you suggesting that in the scenario that @SteveMacenski outlined, we could call get_node_options on the PlannerServer, and then feed that NodeOptions object when constructing the Costmap2D?

Pretty much. I'm not sure if the whole NodeOptions is desirable since there are a lot of options there, but at the very least I think you'd want to create a new NodeOptions and then set its arguments and parameter overrides to the ones gotten from PlannerServer's NodeOptions before passing them to the contructor of Costmap2DROS.

from rclcpp.

Closing, based on discussion in our weekly issue triage meeting. Feel free to ask for it to be reopen if anyone thinks it needs more discussion.

from rclcpp.

To be concise: This issue is that sometimes a component node is itself a manually composed set of nodes. The other nodes within the component are not getting their parameters forwarded even though they're in the file passed to the component to load with the node. There's clearly a filter happening somewhere in the parameter namespaces, I think that should be relaxed. I don't see any downsides to passing a nodes more than it asked for, but there are downsides in passing less.

The current way we get around this is to pass the param file to the container on construction which contains all nodes, but that's obviously not always a design pattern that can be followed.

from rclcpp.

I grokk what you're saying, but not sure that addresses our problem - but happy to be proven obtuse hah. This is the specific example we had in mind:

• PlannerServer is a composable node which can be used in a component container
• This node contains another node, Costmap2D
• This object is itself its own node to separate it from the planner server's logic, since it itself operates largely independently and is "force composed" into that node for efficiency / safety reasons

As you can see, we don't do anything with parameters / param file fed into the components from the launch file beyond a little bit of remapping of the node's name/namespace. While this is our exact example, it doesn't do anything wonky beyond that remapping node name/namespace, so I feel this is actually a generic problem with composing a node within a component node.

If there's something we can do to get around that like passing in some of the main node's attributes, that would be totally workable -- though seems like something may be wrong with component container's settings since this works fine with "normal" nodes launched via Node() while LoadComponentNode() doesn't (which I logically track to rclcpp_component instead of launch).

from rclcpp.

@sloretz are you suggesting that in the scenario that @SteveMacenski outlined, we could call get_node_options on the PlannerServer, and then feed that NodeOptions object when constructing the Costmap2D?

Since we are using launch_ros, we would also need to verify that it is not also filtering out parameters outside the namespace.

from rclcpp.

@Aposhian do you want to prototype to make sure it solves the issue? Happy to merge something like that in. I could do it, but I wouldn't feel confident that all the items you're looking for are resolved as you're the reporter.

from rclcpp.

I think we're good, thanks

from rclcpp.

I'm attempting to address this in Nav2 (ros-navigation/navigation2#4011 (comment)) but running into the issue that it doesn't appear to work by adding in the parent node's arguments and parameter overrides.

@Aposhian tested by manually adding in a parameter into the LoadNode service and still failed, so is there a reason to believe that the NodeOptions is culling by namespace or node name on loading so that if passing a NodeOption object among other internal nodes with their own node names or namespaces it wouldn't be able to obtain its subset of information?

Using the test case he setup

ros2 run rclcpp_components component_container --ros-args -r __node:=nav2_container
ros2 launch nav2_bringup navigation_launch.py params_file:=./nav2_bringup/params/nav2_params.yaml use_composition:=True

I can see that if I change a PlannerServer parameter in the yaml file that that is being represented by when launched (i.e. mistype a plugin name so it throws an error), but I can't seem to see that it ever ACKs changes of sub-nodes even when passing it in the parent node options as shown in the blurb in the link above

I've even tried setting the NodeOptions for internal nodes to the parent's NodeOptions in entirety and adding its specifics on top of it, and it doesn't get the parameters in the yaml either:

: nav2_util::LifecycleNode(name, "",
    rclcpp::NodeOptions(parent_options).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"),
  })),

I believe then, this is a problem for any setup with manually composed nodes who's highest level is composed dynamically (making up the manually composed components)

from rclcpp.