Convert nav2_loopback_sim to C++

[tonynajjar]

---

Basic Info

Info	Please fill out this column
Ticket(s) this addresses	N/A
Primary OS tested on	Ubuntu 24.04
Robotic platform tested on	Loopback simulation
Does this PR contain AI generated software?	Yup
Was this PR description generated by AI software?	yes but revised

---

Description of contribution in a few bullet points

• Rewrote nav2_loopback_sim from Python (ament_python) to C++ (ament_cmake)
• Converted from a plain rclpy.Node to a nav2::LifecycleNode with full lifecycle support, bond connection, and composable node registration
• Extracted clock publishing into a separate ClockPublisher node to avoid the circular dependency of a use_sim_time node publishing its own /clock -> All the timers in the loopback simulation node should use sim time, just not the timer publishing the clock.
• Added speed_factor parameter to ClockPublisher — allows running sim time faster or slower than real time, dynamically reconfigurable at runtime
• Added separate odom_publish_dur parameter — odometry can now be published at a different rate than the TF update rate
• Added scan_noise_std parameter — configurable Gaussian noise on laser scan measurements
• Simplified getLaserScan raycasting — replaced ~150-line inline LineIterator (6 directional branches with separate slope/intercept tracking) with a simple polar ray-march loop (~15 lines)
• Uses nav2_util::TwistSubscriber to handle both stamped and unstamped cmd_vel (replaces the manual enable_stamped_cmd_vel parameter)
• Follows nav2 C++ conventions: nav2::LifecycleNode, nav2::Publisher<T>, declare_or_get_parameter, nav2::qos::SensorDataQoS, nav2_common CMake macros

Description of documentation updates required from your changes

ros-navigation/docs.nav2.org#897

Description of how this change was tested

---

Future work that may be required in bullet points

• Add unit tests for the C++ implementation
• Consider adding a speed_factor parameter to the launch file for easier configuration

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-*.

[Copilot]

Pull request overview

This PR migrates nav2_loopback_sim from a Python-based loopback simulator to a C++ (ament_cmake) implementation, adding C++ nodes for loopback simulation and simulated /clock publishing.

Changes:

• Replaces the Python loopback simulator implementation with a C++ LoopbackSimulator lifecycle node (TF + odom + optional LaserScan).
• Adds a C++ ClockPublisher node and standalone mains for both nodes.
• Converts the package to ament_cmake and updates dependencies / removes Python packaging + pytest-based lint tests.

Reviewed changes

Copilot reviewed 16 out of 17 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
nav2_loopback_sim/test/test_pep257.py	Removes Python PEP257 linter test (package no longer Python-based).
nav2_loopback_sim/test/test_flake8.py	Removes Python flake8 linter test.
nav2_loopback_sim/test/test_copyright.py	Removes Python copyright linter test.
nav2_loopback_sim/src/main.cpp	Adds C++ main for spinning the loopback simulator lifecycle node.
nav2_loopback_sim/src/loopback_simulator.cpp	Adds C++ loopback simulator implementation (TF, odom, optional scan from map).
nav2_loopback_sim/src/clock_publisher.cpp	Adds simulated /clock publisher node with runtime-tunable parameters.
nav2_loopback_sim/src/clock_publisher_main.cpp	Adds standalone executable main for the clock publisher node.
nav2_loopback_sim/setup.py	Removes Python packaging entrypoint.
nav2_loopback_sim/setup.cfg	Removes Python install script configuration.
nav2_loopback_sim/pytest.ini	Removes pytest configuration used for Python tests.
nav2_loopback_sim/package.xml	Switches build type to ament_cmake and updates dependencies accordingly.
nav2_loopback_sim/include/nav2_loopback_sim/loopback_simulator.hpp	Adds C++ public header for the loopback simulator.
nav2_loopback_sim/include/nav2_loopback_sim/clock_publisher.hpp	Adds C++ public header for the clock publisher.
nav2_loopback_sim/CMakeLists.txt	Adds ament_cmake build, library/executables, component registration, and lint config.

[codecov]

Codecov Report

❌ Patch coverage is 0% with 283 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
nav2_loopback_sim/src/loopback_simulator.cpp	0.00%	238 Missing ⚠️
nav2_loopback_sim/src/clock_publisher.cpp	0.00%	35 Missing ⚠️
nav2_loopback_sim/src/main.cpp	0.00%	5 Missing ⚠️
nav2_loopback_sim/src/clock_publisher_main.cpp	0.00%	4 Missing ⚠️
...m/include/nav2_loopback_sim/loopback_simulator.hpp	0.00%	1 Missing ⚠️

Files with missing lines	Coverage Δ
...m/include/nav2_loopback_sim/loopback_simulator.hpp	0.00% <0.00%> (ø)
nav2_loopback_sim/src/clock_publisher_main.cpp	0.00% <0.00%> (ø)
nav2_loopback_sim/src/main.cpp	0.00% <0.00%> (ø)
nav2_loopback_sim/src/clock_publisher.cpp	0.00% <0.00%> (ø)
nav2_loopback_sim/src/loopback_simulator.cpp	0.00% <0.00%> (ø)

... and 16 files with indirect coverage changes

🚀 New features to boost your workflow:

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

[SteveMacenski]

With the ClockPublisher Node question, I only did a medium fidelity review. The next one would be looking at the python vs c++ version implementations feature by feature, but generally looks good on a first look

[SteveMacenski]

Extracted clock publishing into a separate ClockPublisher node to avoid the circular dependency of a use_sim_time node publishing its own /clock -> All the timers in the loopback simulation node should use sim time, just not the timer publishing the clock.

I'm not entirely sure I'm following this statement. I don't think a Node with use_sim_time = true is publishing its own clock. I think its always taking a subscription from the /clock topic to use for the time [1]. The simulator is the one publishing htis clock topic for all other nodes ot use, this is not an internal mechanism, this is how Nav2 gets the time too. I don't think you can run the simulator without the time publisher. Please clarify.

I think this feature was something that could be toggled on/off as a param whether to publish the clock or not. If I'm reading "...of a use_sim_time node publishing its own /clock" to mean that you have a separate node publishing clocks in your implementation, then you could just toggle that param off and keep the publishing of the clock in the main simulator code.

Personally, I just like this simple simulator to be 1-node since its pretty compact. Though, we could keep that clock publishing as a parameter and keep the Clock Publisher as a separate utility if architecturally you like to keep this separated. In fact, I suppose transform and laser scan handling could also be broken into separate sharper objects if desired (though I'm not asking you / trying to give you more work - only if this is the reason why you did this with the clock. Otherwise, ignore)

[1] https://github.com/ros2/rclcpp/blob/487eee15dcad03c3e5a0ad5cf0ce57ab4f19fdc3/rclcpp/include/rclcpp/time_source.hpp#L39

---

What's the resource utilization comparison for eq. parameters for odom publishing / laser scan publishing of the python vs C++ version? I think that'll be an imporant metric for the migration guide entry.

[tonynajjar]

I'm not entirely sure I'm following this statement.

Let me clarify. In end effect I just mean that the timer for publishing the clock should use wall time whereas all other timers in the loopback simulation should use ROS time (/clock or system time). We could do this in the same node though.
I just thought that it's a clearer separation because the loopback simulator is also a subscriber of this /clock - wouldn't be very nice for a node to publish and subscribe to the same topic i.e. /clock. Also theoretically you could run the clock_publisher without the loopback simulator, although less useful in the context of Nav2 but in general it is a self-contained node.
I do see the appeal of bundling because together they replace gazebo

In fact, I suppose transform and laser scan handling could also be broken into separate sharper objects if desired

I do agree just didn't see a need to separate currently but I think it will come.

[SteveMacenski]

Got it - so I think the alignment is to use ClockPublisher within the LoopbackSimulator parameterized instead of a separate node. Perhaps then it doesn't need to be its own rclcpp::Node then (given a node / interfaces in the constructor to use). We can keep it a separate class though which itself could be used independently. If you wanted to keep the main for the ClockPublisher node, I'm cool with that (just create a node in the main for using the class). Up to you though, I'm fine also not including it too.

I agree on the wall time element for the clock publisher. Perhaps worth a comment in the readme about that so its clear to future readers.

That should be an easy Claude refactor. Then I'll review line by line for feature parity, though I didn't see anything that stuck out. If you add tests before I do that review there's a good chance we can just merge it immediately upon review.

[tonynajjar]

@SteveMacenski CI's build keeps timing out?

[tonynajjar]

I only had the time for a quick successful functional test today so not ready to merge yet but ready for a review

[tonynajjar]

@SteveMacenski CI's build keeps timing out?

[51min 42.5s] [16/18 complete] [nav2_behavior_tree:build 81% - 36min 46.0s]

What's happening 😅

[SteveMacenski]

Finished <<< nav2_loopback_sim [2min 46s]

The time it takes to compile the new C++ version of this is throwing off CI times. Remove loopback sim from the 2x entries in this block https://github.com/ros-navigation/navigation2/blob/main/.circleci/config.yml#L573-L584 and it should then build in system_build_parameters

[tonynajjar]

Finished <<< nav2_loopback_sim [2min 46s]

The time it takes to compile the new C++ version of this is throwing off CI times. Remove loopback sim from the 2x entries in this block https://github.com/ros-navigation/navigation2/blob/main/.circleci/config.yml#L573-L584 and it should then build in system_build_parameters

Still the same. I won't dig into it until you say you run out of ideas because you know this part better than me

[SteveMacenski]

Want to use our Nav2 version of the service client? That wraps up alot of good boilerplate.

[SteveMacenski]

• we have gotten rid of all other rclcpp:: interfaces to use nav2:: now

[tonynajjar]

Good point, I think it's time we make some instructions for the agents to take into account all the specificities in Nav2

[SteveMacenski]

Might be time we add a claude.md file for nav2 with all that perhaps

[tonynajjar]

I've always wondered what would be the best way to enforce some custom rules in a codebase. A quick research gave me https://github.com/semgrep/semgrep. We could make such a rule:

rules:
  - id: use-nav2-service-client
    pattern: rclcpp::Client<$T>
    message: "Use nav2::ServiceClient<$T> instead of rclcpp::Client<$T>"
    severity: WARNING
    languages: [cpp]
    paths:
      include:
        - nav2_loopback_sim/

I think Nav2 has become quite opinionated in many aspects and such rules would help both users without historical context pick the right options and help maintainers focus the review only on functionality (same as code formatters and linters).

That does not replace adding a claude.md, it's on top

[SteveMacenski]

That looks mighty interesting to me. Can you put together a CI job for the ServiceClient and maybe ServiceServer? That can be a recipe I (or some jr developers I recruit) to populate more for subscriptions, publishers, parameter handling, etc. I actually have a ~85% complete list of Nav2 styling requirements that could be used to populate this.

[Copilot]

Pull request overview

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

[SteveMacenski]

The original function does not send the map to odom transformation -- similar to how we don't set it with the gazebo simulator until the user does so

[SteveMacenski]

Should we remove this?

[tonynajjar]

I actually wish we would set it in gazebo as well. What is the reason for not setting it if in gazebo we know where the real robot spawns? In Loopback simulation, since there is no "real robot" or a counter-truth, then really anywhere we set it is valid theoretically

From a practical aspect, the resulting spam is really counter-intuitive for new users - I think their first thought would be that something is broken, not that they need to set the initial pose

[planner_server-7] [INFO 1775848366.459603721] [global_costmap.global_costmap]: Timed out waiting for transform from base_link to map to become available, tf error: Invalid frame ID "map" passed to canTransform argument target_frame - frame does not exist
[controller_server-6] [ERROR 1775848366.509515471] [local_costmap.local_costmap]: StaticLayer: "map" passed to lookupTransform argument target_frame does not exist. 

[SteveMacenski]

For consistency - please remove it :-) We do not know where the robot should be spawned, the user must tell us. The origin could be anywhere and/or nonsensical.

[tonynajjar]

What if we don't set it by default but make the initial pose a launch argument i.e. node param?

[SteveMacenski]

Sure thing - if we add that for the normal bringup as well with Gazebo. That's a different PR though but totally fine with me

[SteveMacenski]

@tonynajjar if you remove this point, I can merge this right now

[SteveMacenski]

@tonynajjar can you stop changing things since I've approved everything except for the one open topic. Can you revert the last comment messing with addYawToQuat?

[tonynajjar]

Can you revert the last comment messing with addYawToQuat?

I mean I found an opportunity to improve it and I was going to re-request review on that part. Or should I open another PR just for that part?

[SteveMacenski]

Another PR please - given I've approved the changes as-is with a pretty time consuming line-by-line python to C++ review. I'd like to not do that again in the context of this large block of code

[SteveMacenski]

Sorry this is annoying, can you move nav2_behavior_tree from the core build to the algorithm_build job? I would do that for you but your fork doesn't allow me to.