moveit_py design flaws

[peterdavidfagan]

While revising the moveit_py library it is clear that the current implementation is less than ideal. The crux of the issue appears to be the dependency of moveit_cpp and moveit_core on the rclcpp client library for logging and node parameters handling.

When interacting with ROS via Python the rclpy client library is used. Ideally, when instantiating the Python equivalent of moveit_cpp one would follow standard ROS 2 design patterns (e.g. initialise client library, create node, spin node etc.). The current design of the moveit_py library doesn't allow for this, with the current design rclcpp is initialised and a moveit_cpp instance is created in a custom constructor, a thread that spins the underlying rclcpp Node is detached and a custom deleter is defined to manage this thread (seen here). As a result, the user never explicitly instantiates a Node in Python using rclpy.

I wished to open this conversation in order to consider a potential refactor of the existing library. Most of the MoveIt C++ codebase depends on rclcpp (at the very least for logging). I am wondering if there might be a simple way to overcome this dependency issue in the Python bindings. The interoperability between rclcpp and rclpy is low, under the hood they both interact with rcl but currently it is not possible to take a pointer to underlying rcl instance to reconstruct nodes from either library. As a result, I don't believe it is possible to leverage a Node instantiated with rclpy in the construction of moveit_cpp (Note: I briefly investigated this approach during GSoC.)

Any thoughts on approaches to address the rclcpp dependency issue in the Python bindings is appreciated.

Related Issue: ros2/rclpy#291
Related Approach: https://github.com/RobotLocomotion/drake-ros/pull/285/files (doesn't address the underlying issue)

Tagging GSoC mentors: @henningkayser @v4hn
Tagging Eric as he has worked on a similar issue: @EricCousineau-TRI

[henningkayser]

I think part of the design flaw is that moveit_cpp itself depends on rclcpp. This comes down to the discussion if it is possible to design a core version of MoveIt that is ROS-agnostic, but then supports interacting with ROS interfaces that are provided at runtime.
There have been several discussions in the past around injecting ROS dependencies or relying on wrapper interfaces more, but in the end we would have to touch the C++ parts first.