moveit icon indicating copy to clipboard operation
moveit copied to clipboard

Configurable time parametrization method default cartesian path planner in moveit

Open gokul-gokz opened this issue 5 years ago • 8 comments

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

gokul-gokz avatar Nov 05 '19 03:11 gokul-gokz

Thanks for helping in improving MoveIt

welcome[bot] avatar Nov 05 '19 04:11 welcome[bot]

The code is failing because of it's dependency with moveit_msgs where I have added one more parameter in the service msg.

gokul-gokz avatar Nov 05 '19 19:11 gokul-gokz

Could you review this pull request? @IanTheEngineer @rhaschke @jonbinney

gokul-gokz avatar Nov 11 '19 18:11 gokul-gokz

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?

jonbinney avatar Nov 11 '19 20:11 jonbinney

Yes, I certainly agree we want to move towards unifying Cartesian planners as full fledged motion planners in moveit.

davetcoleman avatar Nov 20 '19 19:11 davetcoleman

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.

felixvd avatar Nov 22 '19 13:11 felixvd

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.

gavanderhoorn avatar Nov 22 '19 13:11 gavanderhoorn

@gokul-gokz Are you still working with MoveIt and will get a chance to give this a do-over?

felixvd avatar Oct 21 '20 14:10 felixvd