moveit
moveit copied to clipboard
Configurable time parametrization method default cartesian path planner in moveit
Description
This PR modifies the default cartesian path planning functionality of the moveit, allowing the user to specify the time parametrization method which is hard coded otherwise. The need for this functionality is reported in the #issue. In the issue referred, there was two possible approaches suggested to solve it. This PR deals with the second approach. And this method requires changes in the moveit_msgs package as well.
Checklist
- [x] Required by CI: Code is auto formatted using clang-format
- [ ] Extend the tutorials / documentation reference
- [ ] Document API changes relevant to the user in the MIGRATION.md notes
- [ ] Create tests, which fail without this PR reference
- [ ] Include a screenshot if changing a GUI
- [ ] While waiting for someone to review your request, please help review another open pull request to support the maintainers
Thanks for helping in improving MoveIt
The code is failing because of it's dependency with moveit_msgs where I have added one more parameter in the service msg.
Could you review this pull request? @IanTheEngineer @rhaschke @jonbinney
I added some comments in a review, but I have a higher level question. We already have a concept of a "PlanningPipeline" for doing multiple steps where we have a planner, and then have a smoother, etc. It seems like we should move in that direction for cartesian path planning as well, instead of adding ad-hoc variables for things like time parameterization. @davetcoleman what do you think?
Yes, I certainly agree we want to move towards unifying Cartesian planners as full fledged motion planners in moveit.
Is there a discussion anywhere on a consensus about that or how to go about it? I don't think we should cut the interpolation-based method out completely, since it is deterministic and lightweight.
Is there a discussion anywhere on a consensus about that or how to go about it? I don't think we should cut the interpolation-based method out completely, since it is deterministic and lightweight.
I don't believe the plan is to completely remove computeCartesianPath(..)
, but to refactor it into a regular planner plugin.
It would then be possible to use the regular planning request adapter infrastructure to add things like time-parameterisation, instead of having to change computeCartesianPath(..)
every time with ad-hoc and local hacks.
With the ability to load multiple plugins, you could then use computeCartesianPath(..)
in combination with other planners.
@gokul-gokz Are you still working with MoveIt and will get a chance to give this a do-over?