Node names of costmap_2d cannot be modified in launch file anymore

[karl-schulz]

Bug report

The node name and namespace of a costmap_2d node cannot be changed in a launch file. This makes it really difficult to run two parallel costmaps.

Required Info:

• Operating System:
  • Ubuntu 20.04
• ROS2 Version:
  • Foxy binaries
• Version or commit hash:
  • ros-foxy-navigation2 0.4.5-1focal.20210106.033048
• DDS implementation:
  • Default

Steps to reproduce issue

Launch file like this:

def generate_launch_description():
    return LaunchDescription([
        Node(
            package="nav2_costmap_2d",
            executable='nav2_costmap_2d',
            name="special_name",
            namespace="special_ns",
        )

Expected behavior

A node called special_ns/special_name is spawned.

Actual behavior

A node called costmap/costmap is spawned.

Additional information

This seems to be hardcoded in src/costmap_2d_node.cpp. However, in Dashing, this was working (e.g. creating local_costmap and a global_costmap). Why was this feature dropped?

[karl-schulz]

I could fix this behavior by omitting the custom NodeOptions src/costmap_2d_ros.cpp in line 64. The section becomes:

....
: nav2_util::LifecycleNode(name, "", true
    //,
    // 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({
    //"--ros-args", "-r", std::string("__ns:=") +
    //nav2_util::add_namespaces(parent_namespace, local_namespace),
    //"--ros-args", "-r", name + ":" + std::string("__node:=") + name
    // })
    ),
  name_(name),
...

This makes the node appear with the name specified in the launch file.

The comment also apparently says this non-dynamic behavior is intended:

    // 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

So does this change break any other behavior? Is this a workaround of some kind?

If not, I would suggest a pull request.

Thanks!

[SteveMacenski]

Can you point to a time when this was working properly? That would help figuring out what caused this.

This makes the node appear with the name specified in the launch file.

https://github.com/ros-planning/navigation2/blob/main/nav2_planner/src/planner_server.cpp#L59 This is where its set as the constructor in the task servers. https://github.com/ros-planning/navigation2/blob/main/nav2_costmap_2d/src/costmap_2d_node.cpp perhaps we could add the parameterizations for name / namespace here? We need the separate NodeOptions for typical Nav2 use for the task servers since the node's namespaces may not be the same as the costmap's namespaces.

Dashing was so long ago in this project, I can't tell you specifically why that was changed. Massive changes happened in both Eloquent and Foxy - Dashing was basically MVP time.

[karl-schulz]

Hi Steve,

I think I found the commit which caused this: https://github.com/ros-planning/navigation2/commit/ea25a016f5805762304ec281f121ec8e80e8daaa

To me it seems that this behavior was actually intentionally in some way.

As I said, the changes I described above are fixing the issue. However, I am not absolutely certain what break when the node name and namespace are not overridden in this way. Maybe @bpwilcox can help?

Knowing that other nodes are also able to change their name as specified in a launch file, I think it is really important that the costmap node doesn't do it differently.

Right now, the only way to change the costmap's name to something else than "costmap" is to invoke it via C++ (like the planner server does), which is not ideal when you just want to use the package without building it.

[SteveMacenski]

I don't think we plan on changing this anytime, keeping the current behavior in the use of costmaps in the task servers is more important to us than use of costmaps as stand alone nodes - if I have to prioritize one over another. What you suggest would break the use of costmaps that Nav2 is primarily relying on.

Adding additional params to the costmap node main to set the namespace / name would be a reasonable compromise that doesn't break current behavior and gets what you're looking for (changing names of costmaps). Due to the design of multi-node servers in rclcpp right now, there's not another way around this.

[bpwilcox]

Knowing that other nodes are also able to change their name as specified in a launch file, I think it is really important that the costmap node doesn't do it differently.

I believe the costmap was a unique case due to the limitations and conflicts of scoped node/namespace name parsing during the parameter remapping (the rewritten yaml params) when launching. There was some discussion on this topic in the relevant PR: https://github.com/ros-planning/navigation2/pull/1211

[karl-schulz]

Okay, thanks guys. I don't fully understand the conflicts on the cpp side, as I don't have enough experience with rclcpp. If you are aware of this behavior, I will find a workaround for me locally, however maybe if would be good to let people know that they can't use launch files with the costmap node like with other nodes? I spent quite some time troubleshooting this behavior and it would be good to prevent other people from doing the same. However, I don't know the best way of doing so.

Thanks again for your help - and for general effort in the Nav2 stack!

[alanxuefei]

Would it be clearer if we rename the global name to the name of task server, as illustrated below? This modification can provide better clarity since the costmap is part of controller_server. Additionally, The ros2 param list command can display parameters related to controller_server together.

Original code:

  // The costmap node is used in the implementation of the controller
  costmap_ros_ = std::make_shared<nav2_costmap_2d::Costmap2DROS>(
    "local_costmap", std::string{get_namespace()}, "local_costmap",
    get_parameter("use_sim_time").as_bool());

Suggested change:

  // The costmap node is used in the implementation of the controller
  costmap_ros_ = std::make_shared<nav2_costmap_2d::Costmap2DROS>(
    "controller_server", std::string{get_namespace()}, "local_costmap",
    get_parameter("use_sim_time").as_bool());

Without this change, the repetitive namespace might still lead to ambiguity for some users:

local_costmap:
  local_costmap: