-
Notifications
You must be signed in to change notification settings - Fork 227
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
Interoperability between C / C++ and other Python binding libraries? #291
Comments
There was a proof-of-concept developed, but it's probably quite stale at this point: https://github.com/ros2/rclpy/tree/proof_of_concept_pybind11 @sloretz (the original author of this PoC) would probably be the best person to weigh in here. |
When you say "interoperability between rclpy and rclcpp" can you please elaborate what exactly you mean / expect? |
Gotcha, thanks! It looks like that is namely tailored towards providing a subset of functionality (
I'd like to define an API in C++, and bind it in As a toy example, "combining" both // pybind bindings of `MinimalPublisher`
PYBIND11_MODULE(cpp_pub, m) {
m.class_<MinimalPublisher, rclcpp::Node>(m, "MinimalPublisher")
.def(py::init());
} and then in Python: import cpp_pub
rclpy.init(args=args)
node = cpp_pub.MinimalPublisher()
rclpy.spin(node)
node.destroy_node()
rclpy.shutdown() (assuming the GIL issues are avoided by only dispatching using python Disclaimer: I haven't yet used ROS2 messages; if there's a nice generic type-erasure mechanism for C++ & Python generated classes via static or dynamic polymorphism, then I'm sure it'd be easy to resolve that via |
I think maybe what he's asking is "to what end?" Are you trying to do intra-process communication between them, or mix C++ class instances with "pure" Python Node's and spin them in the same threads (if so why?), etc... What's the use case?
Not sure you can rely on that.
Honestly I cannot answer this, without knowing what you intend to use the type erasure for, because it can be type erased, but the underlying storage for Python is our C message class rather than our C++ message class, so type erasing it will not allow you to use the messages in both systems without conversion, i.e. Python stores strings as |
This one! My goal is to avoid this (in overview) when providing C++ and Python interfaces for Drake:
Short-term, sure, we can duplicate stuff if it's the best short-term path; long-term, it would be nice to minimize API + implementation duplication, especially in light of rigorous testing.
Sure, there needs to be an understood contract in the implementation (e.g. do not threading dispatch in the implementation, but rather let the consumer do the handling, as is done by the
Sorry, didn't clarify; I don't expect to access the direct C++ data from Python. I'm completely fine with there being a duplication of data; e.g. if passing a Python message to C++, serialization happens at that boundary. Simple enough if the interface handles it at some layer; if not, then whatevs, we can wrap it. Example (but blech that this was needed in the first place...): |
Hi everyone, I see it has been a while since this thread was active but nonetheless I thought it may be worthwhile contributing to the discussion. I am working on creating a Python library for the MoveIt project as part of GSoC this year and I am also using pybind11. As part of this project I was hoping that users would be able to leverage the rclpy library to create nodes which could be passed to MoveIt through the Python API. I am currently encountering issues in doing so, I have a class whose constructor accepts a shared pointer reference to a node instance. I wish to create this node using rclpy and pass it to MoveIt as follows. I believe this relates to this subject of interoperability; the moveit library I am binding is dependent on the rclcpp library. Any comments or suggestions would be appreciated, at the moment I am considering adding utilities to the library I am creating in order to allow users to create nodes using rclcpp library via python bindings. |
It looks like your API expects an
Note that |
Thanks @sloretz this is an awesome response I appreciate the detail.
Agreed on the above point.
Thanks @sloretz I wasn't already aware of |
The ROS1 wrapper relied on python <-> c++ type casting via message serialization. A corresponding mechanism doesn't yet exist in ROS2: - https://answers.ros.org/question/356542/ros2-message-serialization-adapting-types/ - ros2/rclpy#291 (comment)
The ROS1 wrapper relied on python <-> c++ type casting via message serialization. A corresponding mechanism doesn't yet exist in ROS2: - https://answers.ros.org/question/356542/ros2-message-serialization-adapting-types/ - ros2/rclpy#291 (comment)
The ROS1 wrapper relied on python <-> c++ type casting via message serialization. A corresponding mechanism doesn't yet exist in ROS2: - https://answers.ros.org/question/356542/ros2-message-serialization-adapting-types/ - ros2/rclpy#291 (comment)
I am currently revising my implementation for A new approach would be to decouple the MoveIt C++ implementation from the Another new approach is to enable interoperability between |
I think, creating interoperability of nodes between rclpy/rclpy/src/rclpy/node.hpp Lines 216 to 217 in 070132a
while a While both implementations essentially build on the |
Thanks for the detailed overview and links @rhaschke.
I was reluctant to create my own wrapper for the This unfortunately could lead to confusion if end-users are not aware of how nodes are being handled by the library. |
My comment was not targeting you, @peterdavidfagan, but the developers of rclpy. |
Ah ok my bad and understood. |
We didn't go that way for the original design for a couple of reasons:
You can argue whether the design is successful, and whether those design goals are worth it. But that's why the current implementation is like it is.
Yes, this is exactly it. ROS 2 tends to view interoperability at the message level, not the API level, which is why the design is like this.
Given enough motivation, it would of course be possible by making changes to both the rclcpp and rclpy layers to make them interoperable. But someone would need to have that motivation and the time to put in the large amount of work to get there. |
Thanks for this explanation, Chris. I also had in mind, that a major goal of ROS2 was to concentrate functionality in the rcl layer. However, the current design of rclpy and rclcpp - adding more (and different) functionality on top of |
It's unclear to me. Personally, I like the current design, and I think the problem is that we have implemented too much at the But I'll admit I am not the expert in this repository, so I'd be interested to hear what @sloretz had to say about it. |
Haven't yet hit a need for this, but wanted to ask anywho:
We use
pybind11
in Drake, and it'd be nice if we had the ability to get interop betweenrclpy
andrclcpp
within our own binding layer.For some external examples, people have done this with packages like OpenCV, VTK, etc.:
https://github.com/virtuald/pybind11_opencv_numpy
https://github.com/EricCousineau-TRI/repro/tree/b9e02d6d5a71f6315b80759ba1628b4bb383c0b8/python/vtk_pybind (my stuff, but points to the motivating examples / questions)
Granted, VTK has a rich object model which makes it relatively easy to do the interop. OpenCV is implemented in pure C++, hence the ease of use with pybind C++.
My technical guesses:
rcl
C andrclpy
should be more or less straightforward, would just require experimental / unstable APIs to be exposed (or copied + pasted) from the bindings here.rclpp
andrclpy
would be much more challenging, as the C++ API (rightly so) encapsulates the underlying C structs.rclpy
API. It would require more overhead in terms of maintenance, compilation, size, etc; however, the unittests here look very extensive, so it should be relatively easy to confirm useful parity :)As an alternative, we can do what we do for LCM, where we introduce our own wrapper interface... but this has given me personally many sad faces for API interop + decoupling (heavy frameworks...), utilities, generalization, etc.:
https://drake.mit.edu/pydrake/pydrake.lcm.html
https://drake.mit.edu/doxygen_cxx/classdrake_1_1lcm_1_1_drake_lcm.html
(doc produced at or around
drake@361223416
)The text was updated successfully, but these errors were encountered: