moveit / moveit_ros

THIS REPO HAS MOVED TO https://github.com/ros-planning/moveit
69 stars 118 forks source link

add velocity & acceleration values to cartesian trajectory #733

Closed v4hn closed 7 years ago

v4hn commented 7 years ago

This also adequately fixes the TODO to add proper time_from_start values.

davetcoleman commented 7 years ago

wow that was easy. i think you should retain the TODO "compute time stemps based on eef distance and param m/s speed for eef" - you've only added joint-based smoothing

v4hn commented 7 years ago

Can we do better than that in theory? Could you please elaborate on the TODO then?

davetcoleman commented 7 years ago

this PR adds time parameterization in joints space, but the cartesian velocity of the gripper will still vary across the duration of the trajectory. you might have even added duplicate functionality - are you sure the interative function isn't called later in the pipeline?

v4hn commented 7 years ago

On Fri, Jul 29, 2016 at 11:54:39AM -0700, Dave Coleman wrote:

this PR adds time parameterization in joints space, but the cartesian velocity of the gripper will still vary across the duration of the trajectory.

Yes. Would you prefer to slow down the speed of the eef over the whole trajectory to the slowest interval of the whole motion? To me, this feels like it should either be a new option in the service request or should be done in a post-processing step. I added a todo comment again.

you might have even added duplicate functionality - are you sure the interative function isn't called later in the pipeline?

There is no pipeline here. The service call returns directly after I add the parameterization.

davetcoleman commented 7 years ago

Would you prefer to slow down the speed of the eef over the whole trajectory to the slowest interval of the whole motion?

I have had a use case (and have implemented it in private projects) that used that feature, yes. I don't think we need to add it here, but I just was pointing out that this fix didn't fully address the original comment TODO

it should either be a new option in the service request or should be done in a post-processing step.

Agreed

Thanks for adding back the comment

v4hn commented 7 years ago

@davetcoleman Could you please directly cherry-pick requests you merge via your browser? Yes, you have to open a command line for that. :P Leaving it to you this time.

davetcoleman commented 7 years ago

created PRs