Fix rosbag2_py transport test for py capsule#707
Conversation
Change introduced by ros2/rclpy#741 conflicted with parallel work in #702, the combination causing nightly test failure for `rosbag2_py`'s `_transport.cpp` tests. Fix to use the new `py::object` for `rmw_qos_profile_t` to fix the tests. Signed-off-by: Emerson Knapp <eknapp@amazon.com>
|
Gist: https://gist.githubusercontent.com/emersonknapp/e386fd1aa55069d16d879d0dd81d0ee8/raw/9247bbca23a4ab96ba857b45011d824c225e377b/ros2.repos
|
|
Note: the PR and Action builds are using the Rolling release - which is not yet updated with these changes for |
|
Windows CMake warning preexisting - reported here eclipse-cyclonedds/cyclonedds#741 |
99470d8 to
f2b2401
Compare
Signed-off-by: Emerson Knapp <eknapp@amazon.com>
| return rclcpp::QoS{qos_init, *rmw_qos_profile}; | ||
| PyObject * raw_obj = PyObject_CallMethod(source.ptr(), "get_c_qos_profile", ""); | ||
| const auto py_obj = py::cast<py::object>(raw_obj); | ||
| const auto rmw_qos_profile = py_obj.cast<rmw_qos_profile_t>(); |
There was a problem hiding this comment.
This looks like it will work, though I wouldn't rely on rclpy's C/C++ interface being API/ABI stable. It's possible (but very unlikely) that this will break in the same ROS distro in the future.
get_c_qos_profile() itself should really be documented as internal-use-only.
There was a problem hiding this comment.
Do you have a suggestion on how rosbag2's pybind could take rclpy.QoS as an argument in a more stable way?
There was a problem hiding this comment.
Alternatively - we could pass the YAML file directly and have the C++ layer do the parsing, which it does already have utilities for. Maybe that's a better way, though I had preferred that the CLI layer do input validation
* Fix rosbag2_py transport test for py capsule Change introduced by ros2/rclpy#741 conflicted with parallel work in ros2#702, the combination causing nightly test failure for `rosbag2_py`'s `_transport.cpp` tests. Fix to use the new `py::object` for `rmw_qos_profile_t` to fix the tests. Signed-off-by: Emerson Knapp <eknapp@amazon.com>
Change introduced by ros2/rclpy#741 conflicted with parallel work in #702, the combination causing nightly test failure for
rosbag2_py's_transport.cpptests. Fix to use the newpy::objectforrmw_qos_profile_tto fix the tests.One instance https://ci.ros2.org/view/nightly/job/nightly_linux-aarch64_debug/1559/testReport/junit/(root)/projectroot/test_transport_py/