Skip to content

Add 6dof support for velocity smoother#5243

Merged
SteveMacenski merged 7 commits intoros-navigation:mainfrom
mini-1235:feat/6dof-velocity-smoother
Jul 10, 2025
Merged

Add 6dof support for velocity smoother#5243
SteveMacenski merged 7 commits intoros-navigation:mainfrom
mini-1235:feat/6dof-velocity-smoother

Conversation

@mini-1235
Copy link
Copy Markdown
Collaborator

@mini-1235 mini-1235 commented Jun 6, 2025

Basic Info

Info Please fill out this column
Ticket(s) this addresses #5238
Primary OS tested on (Ubuntu, MacOS, Windows)
Robotic platform tested on (Steve's Robot, gazebo simulation of Tally, hardware turtlebot)
Does this PR contain AI generated software? (No; Yes and it is marked inline in the code)

Description of contribution in a few bullet points

Description of documentation updates required from your changes

ros-navigation/docs.nav2.org#706

Description of how this change was tested

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

@mini-1235 mini-1235 marked this pull request as draft June 6, 2025 15:25
@mini-1235
Copy link
Copy Markdown
Collaborator Author

mini-1235 commented Jun 6, 2025

Still adding unit tests

@codecov
Copy link
Copy Markdown

codecov bot commented Jun 6, 2025 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Files with missing lines Coverage Δ
nav2_velocity_smoother/src/velocity_smoother.cpp 94.03% <100.00%> (+0.50%) ⬆️

... and 3 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
SteveMacenski requested changes Jun 6, 2025
View reviewed changes
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.

There's alot of test gaps, but beyond that LGTM!

The tests for this node are remarkably simple due to its pointedness: https://github.com/ros-navigation/navigation2/blob/main/nav2_velocity_smoother/test/test_velocity_smoother.cpp

I think you just need to add a couple of test cases doing 6DOF and you should be good to go.

@mini-1235
Copy link
Copy Markdown
Collaborator Author

mini-1235 commented Jun 13, 2025

Thanks Steve for the review, I will aim to complete this over the weekend

@SteveMacenski
Copy link
Copy Markdown
Member

SteveMacenski commented Jun 26, 2025 •
edited
Loading

@mini-1235 this seems like an easy win to complete - do you know when you'll have time to finish this? 😄

@mini-1235
Copy link
Copy Markdown
Collaborator Author

mini-1235 commented Jun 30, 2025

@mini-1235 this seems like an easy win to complete - do you know when you'll have time to finish this? 😄

Sorry I was on a vacation, will do in a few days!

mini-1235 added 2 commits July 2, 2025 12:54
@mini-1235
Add 6dof support for velocity smoother
bcc7f3a 
Signed-off-by: mini-1235 <mauricepurnawan@gmail.com>
@mini-1235
Fix formatting and logic error
5f2d817 
Signed-off-by: mini-1235 <mauricepurnawan@gmail.com>
@mini-1235 mini-1235 force-pushed the feat/6dof-velocity-smoother branch from 4c8c658 to 5f2d817 Compare July 3, 2025 11:05
mini-1235 added 4 commits July 3, 2025 11:11
@mini-1235
Check size first
06e8b70 
Signed-off-by: mini-1235 <mauricepurnawan@gmail.com>
@mini-1235
Add tests for param size
f28e59e 
Signed-off-by: mini-1235 <mauricepurnawan@gmail.com>
@mini-1235
Revert param changes
5f74c17 
Signed-off-by: mini-1235 <mauricepurnawan@gmail.com>
@mini-1235
Add more tests
c7d9177 
Signed-off-by: mini-1235 <mauricepurnawan@gmail.com>
@mini-1235 mini-1235 marked this pull request as ready for review July 3, 2025 16:37
SteveMacenski
SteveMacenski requested changes Jul 7, 2025
View reviewed changes
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.

Small change and then can merge!

@mini-1235
Fix as suggested
3f8375b 
Signed-off-by: Maurice <mauricepurnawan@gmail.com>
@SteveMacenski SteveMacenski merged commit b9817db into ros-navigation:main Jul 10, 2025
16 checks passed
@SteveMacenski
Copy link
Copy Markdown
Member

SteveMacenski commented Jul 10, 2025

Merged - thanks for the iteration!

@mini-1235 mini-1235 deleted the feat/6dof-velocity-smoother branch August 5, 2025 02:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@SteveMacenski SteveMacenski SteveMacenski approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@mini-1235 @SteveMacenski