Added plan_only flags to pick and place in Kinetic #862
Conversation
| @@ -68,7 +68,7 @@ void demoPick(moveit::planning_interface::MoveGroupInterface& group) | |||
|
|
|||
| grasps.push_back(g); | |||
| } | |||
| group.pick("bubu", grasps); | |||
| group.pick("bubu", grasps, false); | |||
There was a problem hiding this comment.
for readability can you make this a const?
const bool plan_only = false;
(..., plan_only)
There was a problem hiding this comment.
I had actually meant to just change this line (and other similar lines) to not have a hidden false value but instead make it a const right before the call:
const bool plan_only = false;
group.pick("bubu", grasps, plan_only);
There was a problem hiding this comment.
But the approach you used of having default values in the function headers i actually like better, except please remove the const
| @@ -795,7 +797,7 @@ class MoveGroupInterface::MoveGroupInterfaceImpl | |||
| return MoveItErrorCode(moveit_msgs::MoveItErrorCodes::FAILURE); | |||
| } | |||
|
|
|||
| return pick(object.id, response.grasps); | |||
| return pick(object.id, response.grasps, false); | |||
| @@ -343,15 +343,15 @@ void MotionPlanningFrame::pickObject() | |||
| { | |||
| move_group_->setSupportSurfaceName(support_surface_name_); | |||
| } | |||
| if (move_group_->pick(pick_object_name_[group_name])) | |||
| if (move_group_->pick(pick_object_name_[group_name], false)) | |||
|
I added the change to the arguments so that they are constant. Let me know if that doesn't line up with what you meant. |
| @@ -754,15 +754,15 @@ class MoveGroupInterface | |||
| /** \brief Pick up an object | |||
|
|
|||
| This applies a number of hard-coded default grasps */ | |||
| MoveItErrorCode pick(const std::string& object); | |||
| MoveItErrorCode pick(const std::string& object, const bool plan_only=false); | |||
There was a problem hiding this comment.
please remove the const here and throughout
|
Thanks for the reminder. Updated it again. |
|
@davetcoleman looks like @DavidWatkins addressed all your feedback. Anything else stopping this PR from moving forward? |
|
@mcevoyandy this PR needs a second review, do you +1 it? |
moveit_ros/planning_interface/move_group_interface/src/move_group_interface.cpp
Show resolved
Hide resolved
moveit_ros/visualization/motion_planning_rviz_plugin/src/motion_planning_frame_manipulation.cpp
Outdated
Show resolved
Hide resolved
|
I've addressed the new concerns, any additional thoughts? |
Added optional plan_only flags to pick and place Added plan_only flag to planGraspsAndPick
|
Thank you for the swift cleanup! Merged and cherry-picked to melodic. |
Added optional plan_only flags to pick and place Added plan_only flag to planGraspsAndPick
Description
The pick/place functions in moveit_commander did not have a way to only plan the action. I have added optional parameters onto the pick/place calls in moveit_commander which is defaulted to false to ensure no changes in existing code will be required.
Checklist