-
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
Better management of RPP path pruning when cusp points are detected #4763
base: humble
Are you sure you want to change the base?
Conversation
This pull request is in conflict. Could you fix it @andreacelani? |
@andreacelani, all pull requests must be targeted towards the |
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.
Heed the CI bots - we need this in main
before we can have it in humble
so that all new distributions and current releases can have this update (i.e. jazzy
which I assume you'll also want eventually 😉 ). Also on linting and DCO sign offs for quality control.
You can leave this PR open after fixing the DCO and linting issues, but also need a main
version of it. Generally I'd recommend closing this PR and opening a main
for us to discuss over and fix issues on - then once we merge open a humble
version. That way any fixes for main
get waterfalled into humble
without trying to maintain 2 PRs simultaneously and perhaps miss some details by accident.
I did a good sweep at it and generally look good to me other than missing test coverage / linting issues. I'll do a detailed sweep after the main
version of this PR is opened, but generally seems good. I assume you tested this fix?
{ | ||
auto && theta = fmod(angles + M_PI, 2.0 * M_PI); | ||
if (theta < 0) | ||
theta += 360; |
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.
Isn't this in radians?
* @return Shortest distance between angles | ||
*/ | ||
template<typename F, typename T> | ||
auto shortest_angular_distance( |
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.
I believe both of these functions can be used from angles
rather than reimplemented https://github.com/ros/angles
@@ -166,38 +160,6 @@ TEST(RegulatedPurePursuitTest, createCarrotMsg) | |||
EXPECT_EQ(rtn->point.z, 0.01); | |||
} | |||
|
|||
TEST(RegulatedPurePursuitTest, findVelocitySignChange) |
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.
make sure to add tests for your new methods (isWithinInversionTolerances
prunePlan
removePosesAfterFirstInversion
findFirstPathInversion
)
Basic Info
Description of contribution in a few bullet points
the contribution make RPP controller follows better the path given by planner. There is no skip of path points even with tight maneuvers.
This apply to all models (diff drive, ackermann and omni)
fixed.RPP.webm
Description of documentation updates required from your changes
Added the following parameters. They are the same found in MPPI controller
inversion_xy_tolerance
inversion_yaw_tolerance
the parameters are useful to terminate an inversion and start the following the next portion of the path
Future work that may be required in bullet points
original RPP coding style has been follow (one single file cpp file). MPPI has more structured organization with a dedicated path_handler.cpp file
-->
For Maintainers: