[mppi] Add pluginized motion models#6076
Conversation
Signed-off-by: Alessio Parmeggiani <[email protected]>
Signed-off-by: Alessio Parmeggiani <[email protected]>
Signed-off-by: Alessio Parmeggiani <[email protected]>
411031d to
110c98d
Compare
Codecov Report✅ All modified and coverable lines are covered by tests.
... and 19 files with indirect coverage changes 🚀 New features to boost your workflow:
|
SteveMacenski
left a comment
There was a problem hiding this comment.
At some point I had to stop reviewing this, this looks too AI generated and not reviewed by a human. Revert all unnecessary changes and TMI commentary. Please try again after you give this a deeper look and change only what is necessay
|
Sorry for that, will fix asap |
Signed-off-by: Alessio Parmeggiani <[email protected]>
9df2a58 to
1878353
Compare
Signed-off-by: Alessio Parmeggiani <[email protected]>
Signed-off-by: Alessio Parmeggiani <[email protected]>
There was a problem hiding this comment.
Pedantic; I consider this to be done with these changes and our conversation on the pluginlib stuff -- thanks!
The docs definitely need updates on docs.nav2.org for MPPI new parameter / plugins & the migration guide to tell people what to change to make this work for them
Signed-off-by: Alessio Parmeggiani <[email protected]>
Signed-off-by: Alessio Parmeggiani <[email protected]>
| motion_model_->initialize(parameters_handler_, plugin_ns); | ||
| motion_model_->setConstraints(settings_.constraints, settings_.model_dt); |
There was a problem hiding this comment.
Put these in the try/catch as well as a protection in case the initialization can cause an exception to be thrown
|
Please fix the small conflict + add a test for the Otherwise LGTM. The docs updat is the last thing before merge.
|
Signed-off-by: Steve Macenski <[email protected]>
|
This pull request is in conflict. Could you fix it @Alessio-Parmeggiani? |
Signed-off-by: Alessio Parmeggiani <[email protected]>
How should I manage conflicts? Is it ok to merge the main branch into my fork and fix the conflicts like this?
Sure, working on it! EDIT: made a PR on docs repository |
|
Fix however you like - we squash merge anyway so feel free to add commits or get messy. |
Signed-off-by: Alessio Parmeggiani <[email protected]>
|
CI failed + a small adjustment to your docs PR, then we can merge the pair once this CI passes! |
|
Should this be removed from draft if you're done? |
Signed-off-by: Alessio Parmeggiani <[email protected]>
* first draft, compiles correctly Signed-off-by: Alessio Parmeggiani <[email protected]> * minor fixes, unit tests ok Signed-off-by: Alessio Parmeggiani <[email protected]> * minor doc update Signed-off-by: Alessio Parmeggiani <[email protected]> * addressing review comments, removing unnecessary changes Signed-off-by: Alessio Parmeggiani <[email protected]> * minor changes to pass tests, removed some descriptions Signed-off-by: Alessio Parmeggiani <[email protected]> * using casting to determine motion model type and using existing utils Signed-off-by: Alessio Parmeggiani <[email protected]> * minor changes Signed-off-by: Alessio Parmeggiani <[email protected]> * removed default diffDrive plugin Signed-off-by: Alessio Parmeggiani <[email protected]> * Update nav2_mppi_controller/src/optimizer.cpp Signed-off-by: Steve Macenski <[email protected]> * small change and add test for coverage Signed-off-by: Alessio Parmeggiani <[email protected]> * minor refactor Signed-off-by: Alessio Parmeggiani <[email protected]> --------- Signed-off-by: Alessio Parmeggiani <[email protected]> Signed-off-by: Steve Macenski <[email protected]> Co-authored-by: Steve Macenski <[email protected]> Signed-off-by: Tony Najjar <[email protected]>
Basic Info
Description of contribution in a few bullet points
As discussed in #6032 I added motion models for the mppi controller as plugins instead of hardcoded modes.
In this way it is possible for an user to add its own motion model and constraints.
Custom plugins can be added in another package and used with:
Description of documentation updates required from your changes
plugin needs to be specified with parameters. For example, instead of:
Now is:
Implementation notes
I tried to keep the modifications at minimum, some notable changes are:
Description of how this change was tested
First I ensured that the package nav2_mppi_controller compiles correctly and I run the tests of the nav2_mppi_controller package (colcon test ...) to see if they passed.
I created some node to test the correct behavior with a dummy empty global costmap, a local costmap containing some obstacles and a node that emualtes actuation by converting velocity commands into TF publishing.
I verified that the existing motion models works correctly:
I also created a new motion model StraightOnlyMotionModel that can only go straight and it works.
Future work that may be required in bullet points
For Maintainers:
backport-*.