Skip to content
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

Added collision detection for docking #4752

Merged
merged 7 commits into from
Nov 21, 2024

Conversation

ajtudela
Copy link
Contributor


Basic Info

Info Please fill out this column
Ticket(s) this addresses #4404
Primary OS tested on Ubuntu 24.04
Robotic platform tested on Morphia
Does this PR contain AI generated software? No

Description of contribution in a few bullet points

  • Added optional collision checking on docking / undocking.

Description of documentation updates required from your changes


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

Copy link
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.

Only addition I would like to see (which I think is the intent of the collision_tolerance_ param) is a parameter for a tolerance to the dock or from the dock to ignore collisions. If we're checking within N distance of the dock (or proxy of distance along the trajectory control law), ignore collisions since we're literally going to collide with the dock.

So while docking, its at the end (and at the start of the trajectory for undocking)

Otherwise, this looks very sensible!

nav2_docking/opennav_docking/src/controller.cpp Outdated Show resolved Hide resolved
nav2_docking/opennav_docking/src/controller.cpp Outdated Show resolved Hide resolved
nav2_docking/opennav_docking/src/controller.cpp Outdated Show resolved Hide resolved
Signed-off-by: Alberto Tudela <[email protected]>
Copy link

codecov bot commented Nov 15, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Files with missing lines Coverage Δ
...docking/include/opennav_docking/docking_server.hpp 100.00% <ø> (ø)
nav2_docking/opennav_docking/src/controller.cpp 100.00% <100.00%> (+1.66%) ⬆️
...av2_docking/opennav_docking/src/docking_server.cpp 89.63% <100.00%> (ø)

... and 1 file with indirect coverage changes


🚨 Try these New Features:

nav2_docking/opennav_docking/src/controller.cpp Outdated Show resolved Hide resolved
nav2_docking/opennav_docking/src/controller.cpp Outdated Show resolved Hide resolved
nav2_docking/opennav_docking/src/controller.cpp Outdated Show resolved Hide resolved
@SteveMacenski
Copy link
Member

SteveMacenski commented Nov 20, 2024

@ajtudela do you want me to review again? I see updates but wasn't sure if we came to agreement on the trajectory checking for dock/undock (or if what I'm asking for isn't sensible)

@ajtudela
Copy link
Contributor Author

@ajtudela do you want me to review again? I see updates but wasn't sure if we came to agreement on the trajectory checking for dock/undock (or if what I'm asking for isn't sensible)

Yes, please. I came to the same conclusion as you when doing the test: skip the last part of the trajectory when docking and the first part when undocking.

It's implemented in the latest commit, and I've added new tests to check for collisions in the 4 scenarios: dock/undock forward/backward.

If you can review it, I'll fix the python test and your possible changes tomorrow.

Thanks!

Copy link
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.

Otherwise, pretty much ready to rumble!

nav2_docking/opennav_docking/src/controller.cpp Outdated Show resolved Hide resolved
Signed-off-by: Alberto Tudela <[email protected]>
Signed-off-by: Alberto Tudela <[email protected]>
Signed-off-by: Alberto Tudela <[email protected]>
Signed-off-by: Alberto Tudela <[email protected]>
@ajtudela
Copy link
Contributor Author

ajtudela commented Nov 21, 2024

Update docs here: ros-navigation/docs.nav2.org#610

@ajtudela
Copy link
Contributor Author

Can be backported to humble?

Copy link
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.

That's a nice solution to the docking/undocking problem - very elegant

Can be backported to humble?

I'm going to say 'yes'. It technically breaks the controller's constructor & computeVelocityCommand, but that's not a public facing object and this is an important update that Humble users will want over the next ~3 years of LTS support. This is still a pretty new package, so I'm willing to make the exception here

@ajtudela
Copy link
Contributor Author

That's a nice solution to the docking/undocking problem - very elegant

Can be backported to humble?

I'm going to say 'yes'. It technically breaks the controller's constructor & computeVelocityCommand, but that's not a public facing object and this is an important update that Humble users will want over the next ~3 years of LTS support. This is still a pretty new package, so I'm willing to make the exception here

Great. I'll make another PR in the other repo.

@SteveMacenski SteveMacenski merged commit 90a6c8d into ros-navigation:main Nov 21, 2024
11 checks passed
@SteveMacenski
Copy link
Member

Thanks, this is great work!

mergify bot pushed a commit that referenced this pull request Nov 21, 2024
* Added collision detection for docking

Signed-off-by: Alberto Tudela <[email protected]>

* Minor fixes

Signed-off-by: Alberto Tudela <[email protected]>

* Improve collision  while undocking and test

Signed-off-by: Alberto Tudela <[email protected]>

* Fix smoke testing

Signed-off-by: Alberto Tudela <[email protected]>

* Rename dock_collision_threshold

Signed-off-by: Alberto Tudela <[email protected]>

* Added docs and params

Signed-off-by: Alberto Tudela <[email protected]>

* Minor changes in README

Signed-off-by: Alberto Tudela <[email protected]>

---------

Signed-off-by: Alberto Tudela <[email protected]>
(cherry picked from commit 90a6c8d)
SteveMacenski pushed a commit that referenced this pull request Nov 21, 2024
* Added collision detection for docking

Signed-off-by: Alberto Tudela <[email protected]>

* Minor fixes

Signed-off-by: Alberto Tudela <[email protected]>

* Improve collision  while undocking and test

Signed-off-by: Alberto Tudela <[email protected]>

* Fix smoke testing

Signed-off-by: Alberto Tudela <[email protected]>

* Rename dock_collision_threshold

Signed-off-by: Alberto Tudela <[email protected]>

* Added docs and params

Signed-off-by: Alberto Tudela <[email protected]>

* Minor changes in README

Signed-off-by: Alberto Tudela <[email protected]>

---------

Signed-off-by: Alberto Tudela <[email protected]>
(cherry picked from commit 90a6c8d)

Co-authored-by: Alberto Tudela <[email protected]>
@ajtudela ajtudela deleted the feature-collision branch November 22, 2024 07:37
Jakubach added a commit to Jakubach/navigation2 that referenced this pull request Nov 22, 2024
* Added collision detection for docking

Signed-off-by: Alberto Tudela <[email protected]>

* Minor fixes

Signed-off-by: Alberto Tudela <[email protected]>

* Improve collision  while undocking and test

Signed-off-by: Alberto Tudela <[email protected]>

* Fix smoke testing

Signed-off-by: Alberto Tudela <[email protected]>

* Rename dock_collision_threshold

Signed-off-by: Alberto Tudela <[email protected]>

* Added docs and params

Signed-off-by: Alberto Tudela <[email protected]>

* Minor changes in README

Signed-off-by: Alberto Tudela <[email protected]>

---------

Signed-off-by: Alberto Tudela <[email protected]>
Signed-off-by: Jakubach <[email protected]>
Jakubach added a commit to Jakubach/navigation2 that referenced this pull request Nov 22, 2024
* Added collision detection for docking

Signed-off-by: Alberto Tudela <[email protected]>

* Minor fixes

Signed-off-by: Alberto Tudela <[email protected]>

* Improve collision  while undocking and test

Signed-off-by: Alberto Tudela <[email protected]>

* Fix smoke testing

Signed-off-by: Alberto Tudela <[email protected]>

* Rename dock_collision_threshold

Signed-off-by: Alberto Tudela <[email protected]>

* Added docs and params

Signed-off-by: Alberto Tudela <[email protected]>

* Minor changes in README

Signed-off-by: Alberto Tudela <[email protected]>

---------

Signed-off-by: Alberto Tudela <[email protected]>
Signed-off-by: Jakubach <[email protected]>
Jakubach added a commit to Jakubach/navigation2 that referenced this pull request Nov 22, 2024
* Added collision detection for docking

Signed-off-by: Alberto Tudela <[email protected]>

* Minor fixes

Signed-off-by: Alberto Tudela <[email protected]>

* Improve collision  while undocking and test

Signed-off-by: Alberto Tudela <[email protected]>

* Fix smoke testing

Signed-off-by: Alberto Tudela <[email protected]>

* Rename dock_collision_threshold

Signed-off-by: Alberto Tudela <[email protected]>

* Added docs and params

Signed-off-by: Alberto Tudela <[email protected]>

* Minor changes in README

Signed-off-by: Alberto Tudela <[email protected]>

---------

Signed-off-by: Alberto Tudela <[email protected]>
Signed-off-by: Jakubach <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants