(moveit_py) Python Bindings for MoveGroupInterface

[m-elwin]

trafficstars

Description

Implements preliminary python bindings for the C++ MoveGroupInterface

Only the minimum needed to create a python version of the Your First C++ MoveIt Project Tutorial, is currently implemented.

A draft of a python version of the tutorial can be found here.

For example, ros2 launch moveit2_tutorials demo.launch.py. Then, with this PR, running the following python script will move end-effector to a target pose:

import moveit
from moveit import MoveGroupInterface
from geometry_msgs.msg import Pose, Quaternion, Point

# Creates a node called "hello_moveit_interface" 
# (in C++ with a separate executor) and connects to the "manipulator" move_group
move_group_interface = MoveGroupInterface("hello_moveit_interface", "manipulator")

# set the desired end-effector target pose
target_pose = Pose(quaternion=Quaternion(w=1),
               position=Point(x=0.28, y=-0.2, z=0.5))
move_group_interface.setPoseTarget(target_pose)

# Plan the motion
(result, plan) = move_group_interface.plan()

# Execute the motion if planning was successful
if result.val == result.SUCCESS:
    move_group_interface.execute(plan)
else:
    print("Planning Failed")

Checklist

• [x] Required by CI: Code is auto formatted using clang-format
• [x] Extend the tutorials / documentation reference
• [ ] Document API changes relevant to the user in the MIGRATION.md notes
• [ ] Create tests, which fail without this PR reference
• [ ] While waiting for someone to review your request, please help review another open pull request to support the maintainers

[codecov[bot]]

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 50.73%. Comparing base (87d5945) to head (b0eaa9a). Report is 67 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #2574   +/-   ##
=======================================
  Coverage   50.73%   50.73%           
=======================================
  Files         386      386           
  Lines       32087    32087           
=======================================
  Hits        16276    16276           
  Misses      15811    15811           

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

[m-elwin]

Re-based off the latest main branch.

[m-elwin]

Thanks for the feedback @sjahr.

There are two primary motivations behind why I decided to work on this:

1. Keep the beginner python moveit2 experience as close as possible to that of the beginner C++ experience.
2. Enable every C++ moveit tutorial to have a direct python equivalent and help ensure that what can be done in C++ can also be done in python with similar code.

Here's the main motivating example: The Your First C++ MoveIt Project

I wrote an equivalent python tutorial Your First Python MoveIt Project (which relies on this PR).

The python tutorial could not be written with existing moveit_py, because it would require setting up moveit parameters in a launchfile. Nor could it be written with pymoveit2 because the python user would need to deal with executors, callback groups, and threads (as far as I can tell). Either way, the alternative approaches would have resulted in a python tutorial that was arguably more complicated than the C++ equivalent (which seems wrong).

It would definitely be possible to do all this with a third-party python wrapper for the ROS API (such as pymoveit2) but there are a few disadvantages of this approach that I can see:

1. These API's being third-party makes them seem less official and less integrated and supported.
2. There is potentially a lot of duplicated code in maintaining a separate pure-python API wrapper, especially because moveit_py already provides the infrastructure for making python bindings.
3. It is easier to ensure feature-parity and similar usage patterns between C++ and python by using bindings rather than a separate ROS API wrapper.

I agree that there are advantages to the approach pymoveit2 takes, including avoiding "wrapping the wrapper", but I think of both approaches as complimentary rather than mutually exclusive and targeted at slightly different users and use-cases.

[MatthijsBurgh]

I think I agree with @sjahr.

The move group interface is some cpp code to talk to ROS API of moveit. I don't think we should wrap that with pybind. I do think it is a good idea to create a move group interface for python, but written in python.

If that library is close to the cpp interfaces, it could live in this repo, maybe even be part of moveit_py.

[m-elwin]

Thanks for the feedback @MatthijsBurgh.

It seems like the main reluctance here is that MoveGroupInterface is already a wrapper around the ROS API so this is "wrapping a wrapper".

My concern is that MoveGroupInterface does not seem to be just a simple ROS API wrapper: it uses threads, futures, and matrix math,all of which a pure python approach needs to re-implement and maintain. And the existing python API wrappers (such as pymoveit2) are non-trivial.

Anyway, thanks for taking the time to consider this. I'd like to help contribute to making the python and C++ moveit experiences as similar as possible (especially in terms of ROS 2 knowledge required for a given task) regardless of the outcome of this PR, so I'd appreciate a bit more feedback on how to proceed.

Is the pybind11 approach in this PR ruled out completely (seems like it might be)? If so I'll close the PR (and can make a new one removing the currently unused python wrapping code in the C++ MoveGroupInterface).

If there is a possibility of acceptance down the road, I'm happy to attempt writing a full wrapper implementation with tests and tutorials to prove the concept further.

[sjahr]

@m-elwin We really appreciate your effort here!

Is the pybind11 approach in this PR ruled out completely (seems like it might be)? If so I'll close the PR (and can make a new one removing the currently unused python wrapping code in the C++ MoveGroupInterface).

Maybe this would be a good step forward. Furthermore, you could try writing a minimal python move_group interface (e.g. as a tutorial for moveit and we'll see where to go from there). If you're interested you could join the public maintainer meeting later this month to discuss any further ideas

[mergify[bot]]

This pull request is in conflict. Could you fix it @m-elwin?

[github-actions[bot]]

This PR is stale because it has been open for 45 days with no activity. Please tag a maintainer for help on completing this PR, or close it if you think it has become obsolete.

[mergify[bot]]

This pull request is in conflict. Could you fix it @m-elwin?

[github-actions[bot]]

This PR is stale because it has been open for 45 days with no activity. Please tag a maintainer for help on completing this PR, or close it if you think it has become obsolete.

[codecov-commenter]

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 50.73%. Comparing base (87d5945) to head (b0eaa9a). Report is 281 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #2574   +/-   ##
=======================================
  Coverage   50.73%   50.73%           
=======================================
  Files         386      386           
  Lines       32087    32087           
=======================================
  Hits        16276    16276           
  Misses      15811    15811           

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

:rocket: New features to boost your workflow:

• :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[github-actions[bot]]

This PR is stale because it has been open for 45 days with no activity. Please tag a maintainer for help on completing this PR, or close it if you think it has become obsolete.

[mergify[bot]]

This pull request is in conflict. Could you fix it @m-elwin?

[MatthijsBurgh]

We should close this PR

[github-actions[bot]]

This PR is stale because it has been open for 45 days with no activity. Please tag a maintainer for help on completing this PR, or close it if you think it has become obsolete.

[github-actions[bot]]

This PR is stale because it has been open for 45 days with no activity. Please tag a maintainer for help on completing this PR, or close it if you think it has become obsolete.

[MatthijsBurgh]

We should close this PR

[github-actions[bot]]

This PR is stale because it has been open for 45 days with no activity. Please tag a maintainer for help on completing this PR, or close it if you think it has become obsolete.

[MatthijsBurgh]

@sjahr please close this one

[rhaschke]

Closing. See https://github.com/moveit/moveit_task_constructor/issues/647#issuecomment-2619945410 for a proposed roadmap.