new param to disable collision checking in DriveOnHeading and BackUp actions #4785

david-wb · 2024-12-05T21:17:30Z

Basic Info

Info	Please fill out this column
Ticket(s) this addresses	#4783
Primary OS tested on	Ubuntu
Robotic platform tested on	Gazebo sim of custom robot
Does this PR contain AI generated software?	No

Description of contribution in a few bullet points

Adds a new bool param called no_collision_checks to the DriveOnHeading and BackUp actions. When the param is set to true, collision checking is disabled on these actions. This is useful, for example, when you want to push through known obstacles.

Description of documentation updates required from your changes

Updated the README for the commander API to include the flag in the listed example.

The new param needs to be added to the docs page.

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

…lision checking Signed-off-by: David Brown <david.brown@inria.fr>

Signed-off-by: David Brown <david.brown@inria.fr>

Signed-off-by: David Brown <davewbrwn@gmail.com>

SteveMacenski

Otherwise generally looks good to me! The BT node config guide pages need updating with the new port, migration guide to highlight this change

nav2_behaviors/include/nav2_behaviors/plugins/drive_on_heading.hpp

nav2_msgs/action/BackUp.action

Signed-off-by: David Brown <davewbrwn@gmail.com>

codecov · 2024-12-05T22:25:45Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Files with missing lines	Coverage Δ
...v2_behavior_tree/plugins/action/back_up_action.hpp	`100.00% <100.00%> (ø)`
...or_tree/plugins/action/drive_on_heading_action.hpp	`100.00% <100.00%> (ø)`
.../nav2_behavior_tree/plugins/action/spin_action.hpp	`100.00% <100.00%> (ø)`
...v2_behavior_tree/plugins/action/back_up_action.cpp	`90.00% <100.00%> (+0.71%)`	⬆️
...or_tree/plugins/action/drive_on_heading_action.cpp	`90.00% <100.00%> (+0.71%)`	⬆️
...nclude/nav2_behaviors/plugins/drive_on_heading.hpp	`93.24% <100.00%> (+0.28%)`	⬆️
..._behaviors/include/nav2_behaviors/plugins/spin.hpp	`100.00% <ø> (ø)`
nav2_behaviors/plugins/back_up.cpp	`86.66% <100.00%> (+0.95%)`	⬆️
nav2_behaviors/plugins/spin.cpp	`90.24% <100.00%> (+0.24%)`	⬆️

... and 2 files with indirect coverage changes

david-wb · 2024-12-06T08:25:11Z

Otherwise generally looks good to me! The BT node config guide pages need updating with the new port, migration guide to highlight this change

I'm not yet up to speed on the release process. Would this change be published as an update to the nav2 Jazzy package (ros-jazzy-navigation2 or will it not be available until the next ros2 release? I think I need to update the jazzy.rst migration guide, is that right?

Signed-off-by: David Brown <davewbrwn@gmail.com>

david-wb · 2024-12-06T12:27:35Z

Here is the corresponding docs PR: ros-navigation/docs.nav2.org#617

SteveMacenski

Small details - but otherwise LGTM!

nav2_behavior_tree/plugins/action/drive_on_heading_action.cpp

nav2_behavior_tree/plugins/action/back_up_action.cpp

nav2_simple_commander/README.md

Signed-off-by: David Brown <davewbrwn@gmail.com>

SteveMacenski

Waiting on CI + docs PR to be updated to a non-draft state

david-wb · 2024-12-06T20:13:16Z

Looks like a couple opennav_docking tests are failing.

SteveMacenski · 2024-12-06T21:49:32Z

I retriggered. I think its just a flaky test. I dont see why this would cause that. Please update the docs PR - I suspect it’ll pass this time 🙂

SteveMacenski · 2024-12-10T19:51:50Z

@david-wb thanks, this is a great contribution!

…actions (ros-navigation#4785) * add parameter to the DriveOnHeading and BackUp actions to disable collision checking Signed-off-by: David Brown <david.brown@inria.fr> * update commander api Signed-off-by: David Brown <david.brown@inria.fr> * use a less verbose param name Signed-off-by: David Brown <davewbrwn@gmail.com> * fix flake8 error Signed-off-by: David Brown <davewbrwn@gmail.com> * rename param Signed-off-by: David Brown <davewbrwn@gmail.com> * set defaults in action messages and popular instance field for param Signed-off-by: David Brown <davewbrwn@gmail.com> * populate field in backup action as well Signed-off-by: David Brown <davewbrwn@gmail.com> * also add the new param to the spin action Signed-off-by: David Brown <davewbrwn@gmail.com> * fix incorrect type Signed-off-by: David Brown <davewbrwn@gmail.com> --------- Signed-off-by: David Brown <david.brown@inria.fr> Signed-off-by: David Brown <davewbrwn@gmail.com> Co-authored-by: David Brown <david.brown@inria.fr>

…actions (ros-navigation#4785) * add parameter to the DriveOnHeading and BackUp actions to disable collision checking Signed-off-by: David Brown <david.brown@inria.fr> * update commander api Signed-off-by: David Brown <david.brown@inria.fr> * use a less verbose param name Signed-off-by: David Brown <davewbrwn@gmail.com> * fix flake8 error Signed-off-by: David Brown <davewbrwn@gmail.com> * rename param Signed-off-by: David Brown <davewbrwn@gmail.com> * set defaults in action messages and popular instance field for param Signed-off-by: David Brown <davewbrwn@gmail.com> * populate field in backup action as well Signed-off-by: David Brown <davewbrwn@gmail.com> * also add the new param to the spin action Signed-off-by: David Brown <davewbrwn@gmail.com> * fix incorrect type Signed-off-by: David Brown <davewbrwn@gmail.com> --------- Signed-off-by: David Brown <david.brown@inria.fr> Signed-off-by: David Brown <davewbrwn@gmail.com> Co-authored-by: David Brown <david.brown@inria.fr> Signed-off-by: stevedanomodolor <stevedan.o.omodolor@gmail.com>

David Brown and others added 3 commits December 4, 2024 10:50

add parameter to the DriveOnHeading and BackUp actions to disable col…

4f2d1ae

…lision checking Signed-off-by: David Brown <david.brown@inria.fr>

update commander api

4ef1ac8

Signed-off-by: David Brown <david.brown@inria.fr>

use a less verbose param name

69e0491

Signed-off-by: David Brown <davewbrwn@gmail.com>

david-wb changed the title ~~Disable collision checking param~~ new param to disable collision checking in DriveOnHeading and BackUp actions Dec 5, 2024

SteveMacenski requested changes Dec 5, 2024

View reviewed changes

nav2_behaviors/include/nav2_behaviors/plugins/drive_on_heading.hpp Outdated Show resolved Hide resolved

nav2_msgs/action/BackUp.action Outdated Show resolved Hide resolved

david-wb added 3 commits December 5, 2024 21:31

fix flake8 error

4de9265

Signed-off-by: David Brown <davewbrwn@gmail.com>

rename param

3ea2011

Signed-off-by: David Brown <davewbrwn@gmail.com>

set defaults in action messages and popular instance field for param

accc5f4

Signed-off-by: David Brown <davewbrwn@gmail.com>

david-wb marked this pull request as ready for review December 6, 2024 08:10

populate field in backup action as well

fe7b040

Signed-off-by: David Brown <davewbrwn@gmail.com>

david-wb requested a review from SteveMacenski December 6, 2024 09:09

also add the new param to the spin action

e482156

Signed-off-by: David Brown <davewbrwn@gmail.com>

david-wb mentioned this pull request Dec 6, 2024

document the new disable_collision_checks param ros-navigation/docs.nav2.org#617

Merged

SteveMacenski requested changes Dec 6, 2024

View reviewed changes

nav2_behavior_tree/plugins/action/drive_on_heading_action.cpp Outdated Show resolved Hide resolved

nav2_behavior_tree/plugins/action/back_up_action.cpp Outdated Show resolved Hide resolved

nav2_simple_commander/README.md Show resolved Hide resolved

fix incorrect type

423dea8

Signed-off-by: David Brown <davewbrwn@gmail.com>

SteveMacenski approved these changes Dec 6, 2024

View reviewed changes

david-wb requested a review from SteveMacenski December 10, 2024 08:35

SteveMacenski merged commit 684a4d0 into ros-navigation:main Dec 10, 2024
11 checks passed

new param to disable collision checking in DriveOnHeading and BackUp actions #4785

new param to disable collision checking in DriveOnHeading and BackUp actions #4785

Uh oh!

Conversation

david-wb commented Dec 5, 2024

Basic Info

Description of contribution in a few bullet points

Description of documentation updates required from your changes

The new param needs to be added to the docs page.

Future work that may be required in bullet points

For Maintainers:

Uh oh!

SteveMacenski left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

codecov bot commented Dec 5, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

david-wb commented Dec 6, 2024

Uh oh!

david-wb commented Dec 6, 2024

Uh oh!

SteveMacenski left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

SteveMacenski left a comment

Choose a reason for hiding this comment

Uh oh!

david-wb commented Dec 6, 2024

Uh oh!

SteveMacenski commented Dec 6, 2024

Uh oh!

Uh oh!

SteveMacenski commented Dec 10, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

codecov bot commented Dec 5, 2024 •

edited

Loading