[RotationShimController] Rotate to goal heading

gennartan commented 4 months ago

When arriving in the goal xy tolerance, the rotation shim controller takes back the control to command the robot to rotate in the goal heading orientation.

The initial goal of the rotationShimController was to rotate the robot at the beginning of a navigation towards the paths orientation because some controllers are not good at performing in place rotations. For the same reason, the rotationShimController should be able to rotate the robot towards the goal heading.

Basic Info

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

Description of contribution in a few bullet points

Add new feature to rotationShimController: It can rotate towards the goal heading when arriving in goal XY tolerance

Description of documentation updates required from your changes

Added new parameter ´rotate_to_goal_heading´, so need to add that to default configs and documentation 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

SteveMacenski commented 4 months ago

@gennartan please see https://github.com/ros-navigation/navigation2/pull/4290#issuecomment-2108757778 if you hadn't already

SteveMacenski commented 4 months ago

Please also make sure to sign off your commits. See the DCO failed job for details

SteveMacenski commented 4 months ago

Essentially done though, this looks good to me. Did you test this out?

gennartan commented 4 months ago

Essentially done though, this looks good to me. Did you test this out?

Yes I tested it in simulation. It works fine.

@gennartan please see #4290 (comment) if you hadn't already

I missed this comment the other day. Now it is fixed (and tested) :smile:

SteveMacenski commented 4 months ago

@gennartan see my new review comments, some are outstanding still. Then I think the last thing left if just adding the new parameter to the configuration guide for this plugin + add to the migration guide that this is a new available feature!

SteveMacenski commented 4 months ago

I think just docs left now!

SteveMacenski commented 4 months ago

gennartan commented 4 months ago

here is the PR: https://github.com/ros-navigation/docs.nav2.org/pull/561

SteveMacenski commented 4 months ago

Thanks @gennartan !

SteveMacenski commented 4 months ago

@gennartan your PR has a failing test now! https://app.circleci.com/pipelines/github/ros-navigation/navigation2/11752/workflows/7593ca53-120e-481f-9fba-326f26f6e32e/jobs/36042/tests

Can you take a look and fix the issue?

Benjamin-Tan commented 3 months ago

Just a curious thought, will this be backported to humble branch?

SteveMacenski commented 3 months ago

That's the plan, we haven't done a new sync since this was merged, it missed the sync date by a few days. The next one is at the end of this month. If you want it in the branch sooner though, you can open a cherry-picked backport PR to Humble and I can merge that so its not a blocker

ros-navigation / navigation2