Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Moveit core python tutorials #624

Open
wants to merge 18 commits into
base: master
Choose a base branch
from

Conversation

PeterMitrano
Copy link

Description

Add tutorials for the new python interface to moveit_core. This is adapted from the C++ planning scene tutorial

Checklist

  • Required by CI: Code is auto formatted using clang-format
  • While waiting for someone to review your request, please consider reviewing another open pull request to support the maintainers

@PeterMitrano
Copy link
Author

Things to be aware of:

  • these links are not tested, I made educated guesses on how things will appear on docs.ros.org, but I'm not sure how to test it
  • there may be lots of other places in this repo and others where the URLs for the C++ documentation need to be updated, this only handles the ones I noticed (in conf.py)

@v4hn
Copy link
Contributor

v4hn commented Apr 28, 2021

@PeterMitrano we (hopefully) fixed the links in this independent PR, so your patches do not apply anymore. Could you please clean up this PR?

@PeterMitrano
Copy link
Author

Seems the python docs are still not being generated.

Copy link
Member

@davetcoleman davetcoleman left a comment

Choose a reason for hiding this comment

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

Can you add a photo to the top of the tutorial, to provider readers more visual appeal?

# but this method of instantiation is only intended for illustration.

urdf_path = "/opt/ros/noetic/share/moveit_resources_panda_description/urdf/panda.urdf"
srdf_path = "/opt/ros/noetic/share/moveit_resources_panda_moveit_config/config/panda.srdf"
Copy link
Member

Choose a reason for hiding this comment

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

This is a brittle way to load resoruces in ROS. Use the resource_load (I think that's the name) to resolve package names, without hard coding paths to someone's specific computer

Copy link
Author

@PeterMitrano PeterMitrano May 6, 2021

Choose a reason for hiding this comment

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

do you mean this? http://wiki.ros.org/resource_retriever
or just the rospkg python API? I think rospkg would work fine, assuming the person has the moveit_resources_panda_* packages available

@tylerjw tylerjw self-assigned this May 6, 2021
@PeterMitrano
Copy link
Author

note to self this PR is currently pending on resolving the issues mentioned in moveit/moveit#2606

# on the end-effector of the panda_arm group of the Panda robot. Note the
# use of convenience functions for filling up the constraints

end_effector_name = joint_model_group.getLinkModelNames()[-1]

Choose a reason for hiding this comment

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

Is there a way to explicitly get the end effector link name from the robot model, instead of implicitly finding it as the last entry in the list of all links?

Copy link
Author

Choose a reason for hiding this comment

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

that would be nice, alas there aren't bindings for that yet.

Copy link
Contributor

@JStech JStech left a comment

Choose a reason for hiding this comment

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

I was able to build and run this, with some extra imports. It looks good and is informative.

Some of the tutorial comment blocks have very uneven line lengths--I'd recommend adjusting them to be more regular (there's probably a tool that will do this for you).

doc/planning_scene_python/planning_scene_tutorial.rst Outdated Show resolved Hide resolved
doc/planning_scene_python/planning_scene_tutorial.rst Outdated Show resolved Hide resolved
doc/planning_scene_python/src/planning_scene_tutorial.py Outdated Show resolved Hide resolved
doc/planning_scene_python/src/planning_scene_tutorial.py Outdated Show resolved Hide resolved
doc/planning_scene_python/src/planning_scene_tutorial.py Outdated Show resolved Hide resolved
@PeterMitrano
Copy link
Author

Did a rebase, sorry if that messed up the review. This PR is still blocked by issues mentioned in moveit/moveit#2606

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.

6 participants