add fields to set cartesian end effector speed #80
Conversation
There was a problem hiding this comment.
This is an interesting extension for the MotionPlanRequest, even if the main use for this is with Cartesian motions (and these are currently not planned through this interface).
Shouldn't this be the maximum Cartesian speed of the trajectory though?
Does anything else make sense, given that most trajectories have to accelerate first and decelerate in the end?
msg/MotionPlanRequest.msg
Outdated
| # For the speed only values larger than 0 are allowed. | ||
| # For values outside the range, the trajectory is not modified. | ||
| string cartesian_speed_end_effector_link | ||
| float64 cartesian_speed |
There was a problem hiding this comment.
| float64 cartesian_speed | |
| float64 cartesian_speed # m/s |
?
|
The parts of the trajectory where the original speed was lower than the Cartesian speed provided here remain unchanged. |
msg/MotionPlanRequest.msg
Outdated
| @@ -47,3 +47,8 @@ float64 allowed_planning_time | |||
| float64 max_velocity_scaling_factor | |||
| float64 max_acceleration_scaling_factor | |||
|
|
|||
| # Maximum cartesian speed for the given end effector. | |||
| # For the speed only values larger than 0 are allowed. | |||
There was a problem hiding this comment.
Speeds are always non-negative. Otherwise they would be velocities.
msg/MotionPlanRequest.msg
Outdated
| @@ -47,3 +47,8 @@ float64 allowed_planning_time | |||
| float64 max_velocity_scaling_factor | |||
| float64 max_acceleration_scaling_factor | |||
|
|
|||
| # Maximum cartesian speed for the given end effector. | |||
| # For the speed only values larger than 0 are allowed. | |||
| # For values outside the range, the trajectory is not modified. | |||
There was a problem hiding this comment.
I changed the line to clarify that the trajectory is not modified when max_cartesian_speed <= 0.
There was a problem hiding this comment.
I see your need for this addition, but I don't like the approach very much.
Adding it in the general MotionPlanRequest actually suggests that this max Cartesian speed will be obeyed in all situations, which is not the case.
IMO, we need a mechanism to pass specific parameters to individual adapters, for example with a generic key-value map. This would clearly state that corresponding parameters are only considered for specific adapters.
As a minimum requirement, this PR should include a comment mentioning the adapter that is actually utilizing the parameter.
|
I see your need for this addition, but I don't like the approach very much.
[...]
IMO, we need a mechanism to pass specific parameters to individual adapters, for example with a generic key-value map.
I see what you mean but we don't *have* a reasonable way to pass such information in ROS.
Generic maps are simply not an option, `MoveGroup` is overly generic already.
As a minimum requirement, this PR should include a comment mentioning the adapter that is actually utilizing the parameter.
👍 That makes a lot of sense to add. I would also support adding the PRA to the default list in the assistant.
|
the fields are used by a method in the move_group_interface to pass these values to a planning_request_adapter that recomputes the trajectories time parameterization to reach the given cartesian speed.
Done.
That is already part of moveit/moveit#1790. |
See moveit/moveit#1790 for more details.
These additional fields are needed to pass these values from the
move_group_interfaceto a planning adapter, that recomputes the time parameterization of the trajectory to match the given Cartesian speed.