Split rclpy module for easier porting to pybind11#675
Conversation
rclpy/src/rclpy/exceptions.hpp
Outdated
| std::string append_rcl_error(std::string prepend) | ||
| { | ||
| prepend += ": "; | ||
| prepend += rcl_get_error_string().str; | ||
| rcl_reset_error(); | ||
| return prepend; | ||
| } |
There was a problem hiding this comment.
Won't including this header in multiple *.cpp files that get compiled into the same binary cause a symbol collision?
There was a problem hiding this comment.
I suppose so. It looks like inline would solve the issue (Explanation 2. 1.)
There was a problem hiding this comment.
Even if this gets inlined, I would assume the RCLError constructor would still have the issue. Any reason not to create a .cpp file for the implementation?
There was a problem hiding this comment.
No reason other than laziness 🙃
850e84b to
531d36e
Compare
c04dc58 to
674b02c
Compare
30621b6 to
0a68445
Compare
cottsay
left a comment
There was a problem hiding this comment.
A few comments that you should take a look at, but nothing blocking.
The stateful module change looks like it was a lot of work - well done.
Adds _rclpy_pybind11 module for incrementally converting to pybind11 Makes _rclpy use multiphase initialization so it can import _rclpy_pybind11 Signed-off-by: Shane Loretz <sloretz@openrobotics.org> Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>
Signed-off-by: Shane Loretz <sloretz@openrobotics.org> Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>
Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>
Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>
Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>
Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>
Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>
Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>
Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>
Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>
Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>
0a68445 to
8662933
Compare
|
CI (build:
|
Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>
|
CI again after fixing warning on OSX
|
Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>
|
CI (build:
|
@cottsay specifically, here's a way to split the giant
_rclpymodule in pieces that can be ported a little at a time. This PR has split out exceptions (first commit) andrclpy_context_get_domain_id(second commit).There are two modules:
_rclpyand_rclpy_pybind11. Insiderclpy.impl.implementation_singletonthe functions and exceptions on_rclpy_pybind11gets set on_rclpy._rclpyitself needs the exception objects to raise them in C code, so it also imports_rclpy_pybind11. To do that, I had to convert_rclpyto use Multi phase Initialization.@azeey @IanTheEngineer @cottsay FYI
Part of #665