`bt_navigator_***_rclcpp_node` - Namespacing + NodeOptions forwarding by roncapat · Pull Request #5310 · ros-navigation/navigation2

roncapat · 2025-06-27T10:16:28Z

Closes #5242
Inspired by: #5202
AI generated software: NO

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 )

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
Should this be backported to current distributions? If so, tag with backport-*.

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 <ronca.pat@gmail.com>

codecov · 2025-06-27T11:08:14Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Files with missing lines	Coverage Δ
...clude/nav2_behavior_tree/bt_action_server_impl.hpp	`91.36% <100.00%> (+1.59%)`	⬆️
...tmap_2d/include/nav2_costmap_2d/costmap_2d_ros.hpp	`100.00% <ø> (ø)`
nav2_costmap_2d/src/costmap_2d_ros.cpp	`89.35% <100.00%> (+1.49%)`	⬆️
..._ros_common/include/nav2_ros_common/node_utils.hpp	`91.20% <100.00%> (+0.73%)`	⬆️

... and 8 files with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

roncapat · 2025-06-27T11:15:20Z

replaceOrAddParameter has been copied, at least for now, in this file. @SteveMacenski could we move it somewhere else to deduplicate code?

SteveMacenski · 2025-06-27T17:49:47Z

Yeah, put that in the node_utils file in nav2_ros_common with doxygen for usage and can merge!

mergify · 2025-06-28T09:22:01Z

@roncapat, your PR has failed to build. Please check CI outputs and resolve issues.
You may need to rebase or pull in main due to API changes (or your contribution genuinely fails).

SteveMacenski · 2025-07-01T18:46:45Z

@roncapat please sign off with DCO and I can merge

Copilot

Pull Request Overview

This PR centralizes argument replacement logic and forwards namespace and node options from parent to child nodes.

Extracts and reuses replaceOrAddArgument utility in nav2_ros_common
Updates Costmap2DROS to use the new utility for namespacing and use_sim_time
Refactors BtActionServer to forward parent node options instead of reconstructing arguments inline

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File	Description
nav2_ros_common/include/nav2_ros_common/node_utils.hpp	Added `replaceOrAddArgument` helper to manage ROS arguments
nav2_costmap_2d/src/costmap_2d_ros.cpp	Removed local copy and wired up `nav2::replaceOrAddArgument` in `getChildNodeOptions`
nav2_costmap_2d/include/nav2_costmap_2d/costmap_2d_ros.hpp	Deleted redundant declaration of `replaceOrAddArgument`
nav2_behavior_tree/include/nav2_behavior_tree/bt_action_server_impl.hpp	Refactored action server to pull and modify parent `NodeOptions` rather than hardcoding args

Comments suppressed due to low confidence (1)

nav2_ros_common/include/nav2_ros_common/node_utils.hpp:386

Add unit tests for replaceOrAddArgument covering both the replace and add branches to ensure correct behavior when the argument exists or not.

inline void replaceOrAddArgument(

nav2_ros_common/include/nav2_ros_common/node_utils.hpp

Copilot · 2025-07-01T18:47:09Z

nav2_behavior_tree/include/nav2_behavior_tree/bt_action_server_impl.hpp

+  auto new_arguments = node->get_node_options().arguments();
+  nav2::replaceOrAddArgument(new_arguments, "-r", "__node", std::string("__node:=") +
+    std::string(node->get_name()) + "_" + client_node_name + "_rclcpp_node");
+  auto options = node->get_node_options();
+  options = options.arguments(new_arguments);


[nitpick] You could combine fetching and updating NodeOptions into a single expression (e.g., auto options = node->get_node_options().arguments(new_arguments);) to reduce variable shadowing and improve clarity.

Suggested change

auto new_arguments = node->get_node_options().arguments();

nav2::replaceOrAddArgument(new_arguments, "-r", "__node", std::string("__node:=") +

std::string(node->get_name()) + "_" + client_node_name + "_rclcpp_node");

auto options = node->get_node_options();

options = options.arguments(new_arguments);

auto options = node->get_node_options().arguments(

nav2::replaceOrAddArgument(

node->get_node_options().arguments(), "-r", "__node", std::string("__node:=") +

std::string(node->get_name()) + "_" + client_node_name + "_rclcpp_node"));

Moved to `nav2_ros_common/node_utils.hpp`, namespace `nav2` Signed-off-by: Patrick Roncagliolo <ronca.pat@gmail.com>

roncapat · 2025-07-02T15:56:01Z

@SteveMacenski done :)

SteveMacenski · 2025-07-02T16:48:18Z

Thank you kindly - and merged!

…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 <ronca.pat@gmail.com> * Deduplicate `replaceOrAddArgument` implementation Moved to `nav2_ros_common/node_utils.hpp`, namespace `nav2` Signed-off-by: Patrick Roncagliolo <ronca.pat@gmail.com> --------- Signed-off-by: Patrick Roncagliolo <ronca.pat@gmail.com>

SteveMacenski approved these changes Jun 27, 2025

View reviewed changes

SteveMacenski approved these changes Jul 1, 2025

View reviewed changes

SteveMacenski requested a review from Copilot July 1, 2025 18:46

Copilot AI reviewed Jul 1, 2025

View reviewed changes

Deduplicate replaceOrAddArgument implementation

b25f3f0

Moved to `nav2_ros_common/node_utils.hpp`, namespace `nav2` Signed-off-by: Patrick Roncagliolo <ronca.pat@gmail.com>

roncapat force-pushed the patch-1 branch from e69f29d to b25f3f0 Compare July 2, 2025 15:55

SteveMacenski merged commit 157c0c4 into ros-navigation:main Jul 2, 2025
10 of 12 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

`bt_navigator_***_rclcpp_node` - Namespacing + NodeOptions forwarding#5310
SteveMacenski merged 2 commits intoros-navigation:mainfrom
roncapat:patch-1

roncapat commented Jun 27, 2025

Uh oh!

codecov bot commented Jun 27, 2025 •

edited

Loading

Uh oh!

roncapat commented Jun 27, 2025

Uh oh!

SteveMacenski commented Jun 27, 2025

Uh oh!

mergify bot commented Jun 28, 2025

Uh oh!

SteveMacenski commented Jul 1, 2025

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Copilot AI Jul 1, 2025

Uh oh!

roncapat commented Jul 2, 2025

Uh oh!

Uh oh!

SteveMacenski commented Jul 2, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

roncapat commented Jun 27, 2025

For Maintainers:

Uh oh!

codecov bot commented Jun 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

roncapat commented Jun 27, 2025

Uh oh!

SteveMacenski commented Jun 27, 2025

Uh oh!

mergify bot commented Jun 28, 2025

Uh oh!

SteveMacenski commented Jul 1, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

Copilot AI Jul 1, 2025

Choose a reason for hiding this comment

Uh oh!

roncapat commented Jul 2, 2025

Uh oh!

Uh oh!

SteveMacenski commented Jul 2, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

codecov bot commented Jun 27, 2025 •

edited

Loading