ros-navigation / navigation2

ROS 2 Navigation Framework and System
https://nav2.org/
Other
2.52k stars 1.28k forks source link

Rotate to goal heading when reaching goal #4289

Closed gennartan closed 4 months ago

gennartan commented 5 months ago

Feature request

Rotate to goal heading when reaching goal

Feature description

When reaching the goal, the rotation shim controller could take back control of the robot to rotate to the goal heading.

Implementation considerations

The current implementation does not take it into account. It is the responsibility of the primary controller to reach the correct heading.

Here is a first idea of the implementation: https://github.com/open-navigation/navigation2/commit/94d3a9d8610d367088ad48fa066d378f31844601 Note: I could not test it yet, probably it is not fully working as is :smile:

SteveMacenski commented 5 months ago

I believe RPP/DWB already do this internally. I had planned to implement something like this but paused since it was already baked in and I didn't want them fighting over control to do it.

But, hey, not a bad idea and to be fair others have their own controller plugins. That seems like a logical contribution. I'd just make it a parameter and when we describe it in documentation, explain that some controllers do this automatically so it doesn't need to be enabled.

Also another good PR contribution, thanks @gennartan !

gennartan commented 5 months ago

Hello,

I updated my code (and tested it). You can find it here: https://github.com/gennartan/navigation2/commit/2587f63fd65908a7e9fa64f7b34847d3b11cb61c . I have a few questions before I can make a pull request. Maybe you can help ^^

Thank you in advance for your responses !

SteveMacenski commented 5 months ago

A path is a vector of ´PoseStamped´. Why is it not a vector of ´Pose´ ?

Future proofing so that paths can be time series for kinodynamic planning. That's not how they're used currently, so you should use the timestamp in the Path message, no each individual pose's timestamp.

Or does it vary depending on the integration ?

In general assume nothing. Make it generalized so that you get that data from either parameters or from other content that has that parameter so it can be changed or adjusted for someone's situation. In this case, you have the costmap, so you have the reference frames that you're interested in as API calls (for example https://github.com/gennartan/navigation2/blob/main/nav2_rotation_shim_controller/src/nav2_rotation_shim_controller.cpp#L205)

Would it make sense to add this in nav2_utils

Sure! Do in this PR.

gennartan commented 5 months ago

The PR is now open.

Finally I did not add the function to check if the goal is within tolerance to ´nav2_util´ because of a circular dependency issue. The goal checker is defined in the header file of ´nav2_core´, which depends on ´nav2_utils´. So I updated the code to have a similar structure than the one for the MPPI controller.