-
Notifications
You must be signed in to change notification settings - Fork 59
Replace rosbag2_cpp typesupport helpers with rclcpp typesupport helpers #105
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
base: main
Are you sure you want to change the base?
Conversation
|
does it work in Humble? |
|
as expected (see CI), this change is not backcompatible with Humble. If you fix it, I will be happy to merge it |
|
What would be your preferred way to fix this? I like the fact that this repository has a single main branch compatible with all active ROS versions. However, since the change doesn't seem to be backported to Humble, these versions have already diverged with respect to the changed header (humble contains the header, while e.g. for jazzy the file has been removed. In my view there are a few options: |
|
Humble is detected by cmake and a defintion is added to compile code conditionally. https://github.com/PlotJuggler/plotjuggler-ros-plugins/blob/main/src/CMakeLists.txt#L50-L54 Therefore, a clear approach would be hide those functions behing our own, and internally use Anyway, as long as CI is happy and there are no conflict, I will be happy to merge |
Signed-off-by: Dominik Authaler <dominik.authaler@uni-ulm.de>
…pp::get_typesupport_handle Signed-off-by: Dominik Authaler <dominik.authaler@uni-ulm.de>
Signed-off-by: Dominik Authaler <dominik.authaler@uni-ulm.de>
3dc6149 to
fe30da2
Compare
Signed-off-by: Dominik Authaler <dominik.authaler@uni-ulm.de>
fe30da2 to
0c1710d
Compare
|
pre-commit :) |
Signed-off-by: Dominik Authaler <dominik.authaler@uni-ulm.de>
Hey @facontidavide ,
first of all thanks for making such an amazing tool availabe as open source!
While trying to update a docker image for Jazzy, I stumbled upon an issue that the plotjuggler ros plugins currently do not build against the current jazzy branch.
More specifically, this backported commit removed the typesupport helpers header from rosbag2_cpp. Consequently, I replaced them the same way they did it in the rosbag repository (with their rclcpp equivalent).
I solely tested this against Jazzy as that is the only version I am currently using , but given it is a backported commit I assume rolling to face the same problem. Not sure about older versions.