Skip to content

Pass node options to costmap node #5202

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Conversation

MarcoMatteoBassa
Copy link
Contributor

Basic Info

Info Please fill out this column
Primary OS tested on Ubuntu
Robotic platform tested on Simulation
Does this PR contain AI generated software? No

Description of contribution in a few bullet points

Currently it is impossible to remap the publishers and subscribers of the costmap objects, because the node options of the controller server are not passed to the child node.
The change allows the child costmap node to inherit the node options of the parent, while still using customized options for the name, namespace and use_sim_time.

Description of how this change was tested

  • verified that the costmap nodes retain the expected name and namespace
  • verified that the costmap topics retain the expected name and namespace
  • verified that the costmap services retain the expected name and namespace
  • verified that it's possible to remap topic names for the costmap subscribers.

Future work that may be required in bullet points

The free functions might be moved to utils and used where other child nodes are created if the use-case arises.

For Maintainers:

  • Check that any new parameters added are updated in docs.nav2.org
  • Check that any significant change is added to the migration guide
  • Check that any new features OR changes to existing behaviors are reflected in the tuning guide
  • Check that any new functions have Doxygen added
  • Check that any new features have test coverage
  • Check that any new plugins is added to the plugins page
  • If BT Node, Additionally: add to BT's XML index of nodes for groot, BT package's readme table, and BT library lists

@SteveMacenski
Copy link
Member

@MarcoMatteoBassa please fix the failing CI jobs (DCO and linting it looks like). Also, did you test this using a namespace? You can do so with nav2_bringup with just the launch arg of namespace:=robotA for example just to make sure that all namespaces right still.

@SteveMacenski
Copy link
Member

SteveMacenski commented May 28, 2025

I fixed the linting but I can't do DCO for you. You can ignore the precommit failure, that's on a github workflow file that I already fixed as part of the Kilted release process

Copy link

codecov bot commented May 28, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Files with missing lines Coverage Δ
nav2_controller/src/controller_server.cpp 85.39% <100.00%> (ø)
...tmap_2d/include/nav2_costmap_2d/costmap_2d_ros.hpp 100.00% <ø> (ø)
nav2_costmap_2d/src/costmap_2d_ros.cpp 87.95% <100.00%> (+0.35%) ⬆️
nav2_planner/src/planner_server.cpp 92.70% <100.00%> (ø)

... and 5 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@MarcoMatteoBassa MarcoMatteoBassa force-pushed the pass_node_options_to_costmap_node branch from 0ce6d0d to 7c8222d Compare May 30, 2025 07:41
@MarcoMatteoBassa
Copy link
Contributor Author

Thanks! It would look like the namespace is correctly passed to the child nodes if added, yes Ex:

/hello_namespace/local_costmap/costmap
/hello_namespace/local_costmap/costmap_raw
/hello_namespace/local_costmap/costmap_raw_updates
/hello_namespace/local_costmap/costmap_updates
/hello_namespace/local_costmap/footprint
...

I passed the options to the global costmap from the planner server too for completeness.

@SteveMacenski SteveMacenski merged commit 4cade8d into ros-navigation:main May 30, 2025
15 checks passed
tonynajjar pushed a commit to angsa-robotics/navigation2 that referenced this pull request Jun 2, 2025
* Passing parent node options from controller_server to costmap node

Signed-off-by: Marco Bassa <[email protected]>

* Passing parent node options to global costmap

Signed-off-by: Marco Bassa <[email protected]>

---------

Signed-off-by: Marco Bassa <[email protected]>
roncapat added a commit to roncapat/navigation2 that referenced this pull request Jun 27, 2025
Closes ros-navigation#5242 
Inspired by: ros-navigation#5202 

- `bt_navigator_***_rclcpp_node` now gets the same namespace of  `bt_navigator`
- `bt_navigator_***_rclcpp_node` now gets the same node options of  `bt_navigator` (apart from node name, that is still forced to respect the pattern `bt_navigator_***_rclcpp_node` )

Signed-off-by: Patrick Roncagliolo <[email protected]>
SteveMacenski pushed a commit that referenced this pull request Jul 2, 2025
…#5310)

* `bt_navigator_***_rclcpp_node`  - Namespacing + NodeOptions forwarding

Closes #5242 
Inspired by: #5202 

- `bt_navigator_***_rclcpp_node` now gets the same namespace of  `bt_navigator`
- `bt_navigator_***_rclcpp_node` now gets the same node options of  `bt_navigator` (apart from node name, that is still forced to respect the pattern `bt_navigator_***_rclcpp_node` )

Signed-off-by: Patrick Roncagliolo <[email protected]>

* Deduplicate `replaceOrAddArgument` implementation

Moved to `nav2_ros_common/node_utils.hpp`, namespace `nav2`

Signed-off-by: Patrick Roncagliolo <[email protected]>

---------

Signed-off-by: Patrick Roncagliolo <[email protected]>
SakshayMahna pushed a commit to SakshayMahna/navigation2 that referenced this pull request Jul 6, 2025
…ros-navigation#5310)

* `bt_navigator_***_rclcpp_node`  - Namespacing + NodeOptions forwarding

Closes ros-navigation#5242 
Inspired by: ros-navigation#5202 

- `bt_navigator_***_rclcpp_node` now gets the same namespace of  `bt_navigator`
- `bt_navigator_***_rclcpp_node` now gets the same node options of  `bt_navigator` (apart from node name, that is still forced to respect the pattern `bt_navigator_***_rclcpp_node` )

Signed-off-by: Patrick Roncagliolo <[email protected]>

* Deduplicate `replaceOrAddArgument` implementation

Moved to `nav2_ros_common/node_utils.hpp`, namespace `nav2`

Signed-off-by: Patrick Roncagliolo <[email protected]>

---------

Signed-off-by: Patrick Roncagliolo <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants