Add trajectory_parameterization parameter to CartesianPath & JointInterpolationPlanner

[JafarAbdi]

Fix: #330

Depend on moveit/moveit#3021

I think this's simpler than the plugin based solution

[rhaschke]

I think this's simpler than the plugin-based solution.

Sure, it's easier, but less flexible! A plugin-based solution would allow using the other time parameterization methods for MoveIt planning pipelines as well.

[JafarAbdi]

A plugin-based solution would allow using the other time parameterization methods for MoveIt planning pipelines as well.

I thought this's already the case TimeOptimalTrajectoryGeneration, IterativeParabolicTimeParameterization, IterativeSplineParameterization am I missing something ?

[rhaschke]

I thought this's already the case TimeOptimalTrajectoryGeneration, IterativeParabolicTimeParameterization, IterativeSplineParameterization.

You are right. I was misled by the too generic name AddTimeParameterization used for the IPTP plugin...

Approving. Depends on moveit/moveit#3021 and moveit/moveit#3023.

[AndyZe]

I've tested this. It works well. 👍

Edit: I didn't test properly. Please disregrad.

[codecov]

Codecov Report

Merging #333 (a536bc2) into master (184fab8) will increase coverage by 0.01%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #333      +/-   ##
==========================================
+ Coverage   53.38%   53.38%   +0.01%     
==========================================
  Files         102      102              
  Lines        7620     7621       +1     
==========================================
+ Hits         4067     4068       +1     
  Misses       3553     3553              

Impacted Files	Coverage Δ
...e/moveit/task_constructor/solvers/cartesian_path.h	100.00% <ø> (ø)
...eit/task_constructor/solvers/joint_interpolation.h	100.00% <ø> (ø)
core/src/solvers/cartesian_path.cpp	83.79% <100.00%> (ø)
core/src/solvers/joint_interpolation.cpp	88.00% <100.00%> (+0.25%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 184fab8...a536bc2. Read the comment docs.

[rhaschke]

@JafarAbdi, may I ask you to change the API such that the time parameterization is specified as a property of those classes?
Time parameterization is also used by merge(...):
https://github.com/ros-planning/moveit_task_constructor/blob/4fa8c10f4456e893e09212b51a06b3d2ec4e9266/core/src/merge.cpp#L168-L170
which is used by Connect and Merger stages. Do you mind adding this feature there as well?

[henningkayser]

This looks very straight-forward to me. I agree it makes much more sense than using plugins here. Do you think you could address Robert's comments so that this can get merged?

[rhaschke]

@JafarAbdi, I implemented my suggestion (#333 (review)) in #339. Please review. I'm closing here.