Skip to content

Pick plan tutorial - #182

Merged
mlautman merged 9 commits into
moveit:kinetic-develfrom
Ridhwanluthra:pick_plan_tutorial
Jul 13, 2018
Merged

Pick plan tutorial#182
mlautman merged 9 commits into
moveit:kinetic-develfrom
Ridhwanluthra:pick_plan_tutorial

Conversation

@Ridhwanluthra

Copy link
Copy Markdown
Contributor

No description provided.

@mlautman

Copy link
Copy Markdown
Contributor

Include descriptions with your PR's

@davetcoleman davetcoleman left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is such an awesome tutorial, thanks!

Comment thread doc/pick_place/pick_place_tutorial.rst Outdated
Pick and Place Tutorial
============================

In MoveIt!, grasping is done using the move group interface. In order to grasp an object we need to create ``moveit_msgs::Grasp`` msg which will allow defining the various poses and postures involved in a grasping operation.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

move group -> MoveGroup


<div style="position: relative; padding-bottom: 5%; height: 0; overflow: hidden; max-width: 100%; height: auto;">
<iframe width="700px" height="400px" src="https://www.youtube.com/embed/cHxkQwSyrgY?rel=0" frameborder="0" allow="autoplay; encrypted-media" allowfullscreen></iframe>
</div>

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!

Comment thread doc/pick_place/pick_place_tutorial.rst Outdated

Running The Demo
----------------
Open two shells. In the first shell start RViz and wait for everything to finish loading: ::

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shells -> terminals?

* POSSIBILITY OF SUCH DAMAGE.
*********************************************************************/

/* Author: Ioan Sucan */

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You must keep the BSD license! Add your name next to Ioan

@mlautman mlautman left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is really good! I'm excited about this


.. raw:: html

<div style="position: relative; padding-bottom: 5%; height: 0; overflow: hidden; max-width: 100%; height: auto;">

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes!!!!


In the second shell run the pick and place tutorial: ::

rosrun moveit_tutorials pick_place_tutotrial

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You need a roslaunch for this

@Ridhwanluthra Ridhwanluthra Jun 27, 2018

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i had initially created a launch but the problem is that we need to wait for everything to load before running this node.
So this seemed better than a hacky launch

@@ -1,244 +1,215 @@
/*********************************************************************

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why did you delete this?

co.primitive_poses[0].position.y = -0.7;
co.primitive_poses[0].position.z = 0.5;
pub_co.publish(co);
// BEGIN_SUB_TUTORIAL table1

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be in it's own method not main


collision_objects[0].operation = collision_objects[0].ADD;

// BEGIN_SUB_TUTORIAL table2

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Separate method

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ping

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you suggesting that the collision objects addition to be done in separate functions?

Comment thread index.rst
doc/visualizing_collisions/visualizing_collisions_tutorial
doc/time_parameterization/time_parameterization_tutorial
doc/planning_with_approximated_constraint_manifolds/planning_with_approximated_constraint_manifolds_tutorial
doc/pick_place/pick_place_tutorial

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This makes me really happy :)

// END_SUB_TUTORIAL
}

void place(moveit::planning_interface::MoveGroupInterface& group)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When I run locally place fails with "All supplied place locations failed. Retrying last location in verbose mode."

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It happens here also rarely. I am guessing its because there is only one grasp given and it only tries to plan once.

@mlautman mlautman Jul 5, 2018

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add a comment that the "All supplied place locations failed. Retrying last location in verbose mode." error is a known issue with a TODO to fix.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would generating multiple grasps fix this?

Comment thread doc/pick_place/pick_place_tutorial.rst Outdated

Understanding ``moveit_msgs::Grasp``
------------------------------------
For complete documentation refer to `this. <http://docs.ros.org/kinetic/api/moveit_msgs/html/msg/Grasp.html>`_

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Replace this with moveit_msgs/Grasp.msg

// Setting the grasp pose
grasps[0].grasp_pose.header.frame_id = "panda_link0";
grasps[0].grasp_pose.pose.orientation =
tf::createQuaternionMsgFromRollPitchYaw(-1.5707963267948966, -0.7853981633974483, -1.5707963267948966);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use M_PI from math.h

grasps[0].grasp_pose.pose.orientation =
tf::createQuaternionMsgFromRollPitchYaw(-1.5707963267948966, -0.7853981633974483, -1.5707963267948966);
// This is the pose of panda_link8.
// From panda_link8 to the palm of the eef the distance is 0.058, the cube starts 0.01 before 5.0 (half of the length

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great comments!


// Setting the place location pose.
loc[0].place_pose.header.frame_id = "panda_link0";
loc[0].place_pose.pose.orientation = tf::createQuaternionMsgFromRollPitchYaw(0, 0, 1.5707963267948966);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

M_PI from math here too


// Setting the pre_place_approach with direction as -ve z-axis.
loc[0].pre_place_approach.direction.vector.z = -1.0;
loc[0].pre_place_approach.direction.header.frame_id = "panda_link0";

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should frame_id be moved to one line above to keep it consistent with others?

@mlautman

Copy link
Copy Markdown
Contributor

@Ridhwanluthra Ping us when this is ready for another review

ridhwanluthra added 2 commits June 27, 2018 20:08
* making sections for better explaination
* using M_PI
* separate function for adding collision objects
* using /**/ comments to not break flow
* other minor fixes
@Ridhwanluthra

Copy link
Copy Markdown
Contributor Author

@mlautman could you please review this again.

/* This is the pose of panda_link8. */
/* From panda_link8 to the palm of the eef the distance is 0.058, the cube starts 0.01 before 5.0 (half of the length */
/* of the cube). */
/* Therfore, the posotion for panda_link8 = 5 - (length of cube/2 - distance b/w panda_link8 and palm of eef - some */

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

position

You should use a spell checker to catch typos like this.

// END_SUB_TUTORIAL
}

void place(moveit::planning_interface::MoveGroupInterface& group)

@mlautman mlautman Jul 5, 2018

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add a comment that the "All supplied place locations failed. Retrying last location in verbose mode." error is a known issue with a TODO to fix.

@mlautman mlautman left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 after addressing my two additional comments

Great job!

@mlautman
mlautman self-requested a review July 11, 2018 22:15
// ^^^^^^^^^^^^^^
// CALL_SUB_TUTORIAL place
// END_TUTORIAL No newline at end of file
// END_TUTORIAL

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great job!

/* of the cube). */
/* Therefore, the position for panda_link8 = 5 - (length of cube/2 - distance b/w panda_link8 and palm of eef - some
*/
/* extra padding) */

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

these comments have weird line breaks. please cleanup and consider just using //

void place(moveit::planning_interface::MoveGroupInterface& group)
{
// BEGIN_SUB_TUTORIAL place
// Note(TODO) - Calling place function may lead to "All supplied place locations failed. Retrying last location in

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

todo format is:

// TODO(githubname): blah...

per GStyle C++

// Note(TODO) - Calling place function may lead to "All supplied place locations failed. Retrying last location in
// verbose mode." This is a known issue and we are working on fixing it.
// Create a vector of placings to be attempted, currently only creating single place location.
std::vector<moveit_msgs::PlaceLocation> loc;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

better var name: loc -> "place_location"


// Setting posture of eef after placing object
// +++++++++++++++++++++++++++++++++++++++++++
/* similar to the pick case */

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similar


collision_objects[0].operation = collision_objects[0].ADD;

// BEGIN_SUB_TUTORIAL table2

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ping

// wait a bit for ros things to initialize
addCollisionObjects(planning_scene_interface);

// wait a bit for ROS things to initialize

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wait

The entire code can be seen :codedir:`here <pick_place>` in the moveit_tutorials GitHub project.

.. tutorial-formatter:: ./src/pick_place_tutorial.cpp No newline at end of file
.. |br| raw:: html

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this just for a line break? Seems like overkill

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I couldn't find a better way to do this and it looks real shabby without it.

@mlautman
mlautman merged commit d094fdb into moveit:kinetic-devel Jul 13, 2018
@mlautman

Copy link
Copy Markdown
Contributor

Congrats @Ridhwanluthra!

@Ridhwanluthra
Ridhwanluthra deleted the pick_plan_tutorial branch July 16, 2018 16:06
Ridhwanluthra added a commit to Ridhwanluthra/moveit_tutorials that referenced this pull request Aug 6, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants