Skip to content

Add new parameter to nav2_velocity_smoother to also smooth the timestamp of TwistStamped messages#5858

Merged
SteveMacenski merged 8 commits intoros-navigation:jazzyfrom
AndreiCostinescu:jazzy
Jan 14, 2026
Merged

Add new parameter to nav2_velocity_smoother to also smooth the timestamp of TwistStamped messages#5858
SteveMacenski merged 8 commits intoros-navigation:jazzyfrom
AndreiCostinescu:jazzy

Conversation

@AndreiCostinescu
Copy link
Copy Markdown
Contributor

@AndreiCostinescu AndreiCostinescu commented Jan 11, 2026

Add parameter to velocity_smoother controlling whether to overwite the timestamp of the smoothed message or to keep the timestamp of the last received command.


Basic Info

Info Please fill out this column
Ticket(s) this addresses #5857
Primary OS tested on Ubuntu
Robotic platform tested on Clearpath ridgeback r100
Does this PR contain AI generated software? No
Was this PR description generated by AI software? No

Description of contribution in a few bullet points

  • I have added the boolean stamp_smoothed_velocity_with_smoothing_time parameter to velocity_smoother node.
  • When enabled, the builtin-interfaces/msg/Time stamp message header of the TwistStamped output is also smoothed based on the smoothing time.

Description of documentation updates required from your changes

  • Added new parameter, so added it to the default configs and documentation page

Description of how this change was tested

Previous output:
cmd_vel_data_sim.txt

Current output with stamp_smoothed_velocity_with_smoothing_time parameter set to true:
cmd_vel_data_sim_withParameterEnabled.txt


Future work that may be required in bullet points

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

@mergify
Copy link
Copy Markdown
Contributor

mergify Bot commented Jan 11, 2026

@AndreiCostinescu, all pull requests must be targeted towards the main development branch.
Once merged into main, it is possible to backport to @jazzy, but it must be in main
to have these changes reflected into new distributions.

@codecov
Copy link
Copy Markdown

codecov Bot commented Jan 11, 2026

Codecov Report

❌ Patch coverage is 90.00000% with 1 line in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
nav2_velocity_smoother/src/velocity_smoother.cpp 90.00% 1 Missing ⚠️
Files with missing lines Coverage Δ
nav2_velocity_smoother/src/velocity_smoother.cpp 94.19% <90.00%> (-0.21%) ⬇️

... and 6 files with indirect coverage changes

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

@AndreiCostinescu
Copy link
Copy Markdown
Contributor Author

AndreiCostinescu commented Jan 11, 2026

If the PR should target the main branch, should I make the same changes in the main branch and then open a new pull request for that branch?
What should happen to this PR then? ☺️

Copy link
Copy Markdown
Member

@SteveMacenski SteveMacenski left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think think this necessarily needs to be parameterized. I think this is a totally sane way for this to be done. Since we still have the timeout in place to stop if it gets too old, marching the interpolation seems fine.

We do need this forward ported to main so that this is available in Rolling and newer distributions so that when you update, you get this also included.

Here's what I think: For Jazzy keep the parameterized nature so that the existing behavior doesn't change for existing users. For the main PR, remove parameterization and simply do the time interpolation as a part of the main code. Then I can merge the pair.

Open a PR for docs.nav2.org and add to the configuration guide the parameter with a note that it is only in Jazzy and otherwise is the default behavior in all other newer distributions.

@AndreiCostinescu
Copy link
Copy Markdown
Contributor Author

AndreiCostinescu commented Jan 13, 2026

Thanks for you input. I have opened a new pull request #5861 for the main branch to smooth the timestamps of the sent cmd_vel commands by default (not needing a parameter for this).
I also created the pull request for the documentation update in the docs.nav2.org repository.

@SteveMacenski
Copy link
Copy Markdown
Member

Great! I commented on the other PR with a couple of changes that should be reflected here as well. Otherwise, this should be good to merge by tomorrow :-)

@SteveMacenski
Copy link
Copy Markdown
Member

With that frame_id change, can you also fix the DCO job?

…p of the smoothed velocity header message

Signed-off-by: Andrei Costinescu <andreicostinescu96@gmail.com>
…g_time in nav2_velocity_smoother/README.md

Signed-off-by: Andrei Costinescu <andreicostinescu96@gmail.com>
…meter examples in nav2_bringup

Signed-off-by: Andrei Costinescu <andreicostinescu96@gmail.com>
Signed-off-by: Andrei Costinescu <andreicostinescu96@gmail.com>
Signed-off-by: Andrei Costinescu <andreicostinescu96@gmail.com>
Signed-off-by: Andrei Costinescu <andreicostinescu96@gmail.com>
Signed-off-by: Andrei Costinescu <andreicostinescu96@gmail.com>
…e and 2) make the difference explicit between using and not using the new parameter

Signed-off-by: Andrei Costinescu <andreicostinescu96@gmail.com>
@SteveMacenski SteveMacenski merged commit 77d6579 into ros-navigation:jazzy Jan 14, 2026
10 checks passed
redvinaa pushed a commit to EnjoyRobotics/navigation2 that referenced this pull request Mar 2, 2026
…amp of TwistStamped messages (ros-navigation#5858)

* Add new parameter to nav2_velocity_smoother that updates the timestamp of the smoothed velocity header message

Signed-off-by: Andrei Costinescu <andreicostinescu96@gmail.com>

* Update parameter description of stamp_smoothed_velocity_with_smoothing_time in nav2_velocity_smoother/README.md

Signed-off-by: Andrei Costinescu <andreicostinescu96@gmail.com>

* Add new parameter stamp_smoothed_velocity_with_smoothing_time in parameter examples in nav2_bringup

Signed-off-by: Andrei Costinescu <andreicostinescu96@gmail.com>

* Fix lines too long cpplint issues in velocity_smoother.cpp

Signed-off-by: Andrei Costinescu <andreicostinescu96@gmail.com>

* Rephrase parameter description for improved clarity

Signed-off-by: Andrei Costinescu <andreicostinescu96@gmail.com>

* Rename delta time variable for clarity.

Signed-off-by: Andrei Costinescu <andreicostinescu96@gmail.com>

* Make the time-smoothing computation more concise.

Signed-off-by: Andrei Costinescu <andreicostinescu96@gmail.com>

* Change code to 1) not write the timestamp in the cmd_vel->header twice and 2) make the difference explicit between using and not using the new parameter

Signed-off-by: Andrei Costinescu <andreicostinescu96@gmail.com>

---------

Signed-off-by: Andrei Costinescu <andreicostinescu96@gmail.com>
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.

Jerky behavior caused by same timestamps in TwistStamped messages published by velocity_smoother

2 participants