new GraspPlanning service to replace manipulation_msgs version #32
Conversation
Until now the move group pick action uses the grasp planning service from manipulation_msgs. However, this doesn't use moveit_msgs::Grasp which has evolved since then. The new service in moveit_msgs uses its own Grasp.msg and overall aims to replace the old functionality using moveit types.
|
According to the discussion in ros-interactive-manipulation/manipulation_msgs#9 While this should replace the old service in MoveIt kinetic, moveit/moveit#264 proposes to make an additional interface for the new service definition available in the |
| @@ -0,0 +1,27 @@ | |||
| # Requests that grasp planning be performed for the target | |||
| @@ -0,0 +1,27 @@ | |||
| # Requests that grasp planning be performed for the target | |||
| # returns a list of grasps to be tested and executed | |||
| string group_name | ||
|
|
||
| # the object to be grasped | ||
| CollisionObject target |
There was a problem hiding this comment.
how about target_object or collision_object?
There was a problem hiding this comment.
The name is taken from the original manipulation_msgs/GraspPlanning.srv.
I prefer the short name here.
There was a problem hiding this comment.
+1 for the short name target.
| string collision_support_surface_name | ||
|
|
||
| # an optional list of grasps to be evaluated by the planner | ||
| Grasp[] grasps_to_evaluate |
There was a problem hiding this comment.
This is also taken from the original message definition. candidate_grasps sounds better to me too.
|
|
||
| # the name that the support surface (e.g. table) has in the collision map | ||
| # can be left empty if no name is available | ||
| string collision_support_surface_name |
There was a problem hiding this comment.
while creating this message, should we make this an array? (Or maybe even a CollisionObject array?) This always seemed a bit underspecified to me, and I know I've at least once ran into the issue that an object is sitting half on one support surface and half on another.
There was a problem hiding this comment.
At the moment, the MoveGroupInterface only supports setting one support surface and the Pickup and Place actions do too.
If we change this here, we should extend it at all other places in the kinetic branch. (Let's do that in a separate request)
I'm not sure this is useful in practice, but as at least three of you agree, let's change it.
There was a problem hiding this comment.
Or for now just add a warning if someone tries to pass in more than one supporting surface. ROS messages are just much harder/annoying to change after the fact, so they should be general enough in the first run
|
+1 to message consolidation, I'd suggest we give this at least 48 hours for review, since there might be additional good feedback on how make this message great again. |
|
Did I hear "Make MoveIt great again?" |
|
|
||
| # an optional list of obstacles that we have semantic information about | ||
| # and that can be moved in the course of grasping | ||
| CollisionObject[] movable_obstacles |
There was a problem hiding this comment.
How is movable_obstacles different from the allowed_touch_objects field in the Grasp.msg message?
There was a problem hiding this comment.
I think the idea is that moveable_obstacles can actually be shifted by colliding with the gripper -- I'm pretty sure that none of our current planners can actually do anything with this, but there was some work going on at willow garage to do "push-based" manipulation and grasping, where the planner could figure out how to nudge things out of the way in order to get to the grasp and get a desired grasp.
There was a problem hiding this comment.
Right. I think the potential usage you described is already encompassed in the allowed_touch_objects entry in the Grasp.msg messages in this service definition; in fact they even have the same description. I think we should avoid having entries with similar purpose as that could make the interface confusing.
There was a problem hiding this comment.
How does one specify the list of allowed touch objects as an input to the service though, if they have no initial estimate of grasps? The "grasps_to_evaluate"/"candidate_grasps" field is optional, and I presume most users probably won't be specifying it, but if we remove the moveable objects, then we have to pass at least one candidate grasp that contains only the allowed_touch_objects, but is not actually a candidate? That seems pretty unclean, and potentially leading to the sort of headaches we currently have with "is_diff" in the planning scene.
|
I changed the service according to your feedback except for the "target object". Also we've change the name for the collision surfaces. This fits better with the new array. |
|
ok, I'll merge this next Tuesday if nobody requests additional changes until then. |
|
ok, merging. I'll also pick this change to jade (no kinetic branch yet) |
* new GraspPlanning service to replace manipulation_msgs version Until now the move group pick action uses the grasp planning service from manipulation_msgs. However, this doesn't use moveit_msgs::Grasp which has evolved since then. The new service in moveit_msgs uses its own Grasp.msg and overall aims to replace the old functionality using moveit types. * Adapt variable names and comments
I might have heard "MoveIt! is great because it's good". |
Until now the move group pick action uses the grasp planning service from manipulation_msgs.
However, this doesn't use moveit_msgs::Grasp which has evolved since then.
The new service in moveit_msgs uses its own Grasp.msg and overall aims to replace the old functionality
using moveit types.