-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
[DWB] Option to limit velocity commands in trajectory generator #4663
Conversation
Signed-off-by: huiyulhy <[email protected]>
Signed-off-by: huiyulhy <[email protected]>
Signed-off-by: huiyulhy <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great! We also need:
- Nav2 migration guide updated to include this change and the new parameter
- Nav2 configuration guide for DWB including this new parameter
- You have a couple of linting errors: https://app.circleci.com/pipelines/github/ros-navigation/navigation2/12645/workflows/2c298f6b-d296-44b0-a985-b33143906d5d/jobs/38206/tests
nav2_dwb_controller/dwb_plugins/src/standard_traj_generator.cpp
Outdated
Show resolved
Hide resolved
nav2_dwb_controller/dwb_plugins/src/standard_traj_generator.cpp
Outdated
Show resolved
Hide resolved
nav2_dwb_controller/dwb_plugins/src/standard_traj_generator.cpp
Outdated
Show resolved
Hide resolved
Thanks for the review and pointing the required changes out! Am updating for the documentation for Nav2 and I wanted to confirm that the addition of this change in the migration guide would be from Jazzy to K-Turtle? |
That is correct! |
Signed-off-by: huiyulhy <[email protected]>
@SteveMacenski thanks for the feedback! Fixed the linting errors and updated the documentation in this PR on nav2 docs! |
Signed-off-by: huiyulhy <[email protected]>
Signed-off-by: huiyulhy <[email protected]>
Codecov ReportAttention: Patch coverage is
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A couple of docs updates needed, then we can merge the pair!
Basic Info
Description of contribution in a few bullet points
-based on current trajectory through a new parameter (
limit_vel_cmd_in_traj_
)limit_vel_cmd_in_traj_
is set to false by default, but when set to true, would use the first velocity computed in the trajectory rollout as the trajectory velocityDefault behavior:
When
limit_vel_cmd_in_traj_
is set totrue
:We observed slightly improved velocity tracking, and similar time to reach the same goal setting this parameter to true.
Description of documentation updates required from your changes
limit_vel_cmd_in_traj_
to DWB local planner (in standard trajectory generator)Future work that may be required in bullet points
For Maintainers: