add support for lazy subscribers #815
Conversation
mikeferguson
force-pushed
the
lazy_subscriber_support
branch
3 times, most recently
from
2484f5f to
f5c7818
Compare
|
@JWhitleyWork this is finally reviewable/mergeable - since the image_common stuff is now released in rolling. It will not pass CI on Iron since there are missing API changes in image_common (not sure why Iron CI is running against rolling PRs) |
|
@JWhitleyWork pinging again - I'd really like to get this into rolling, and cut a new release, so that we can get a number of downstream projects ready for eventual J-turtle release. I notice we also have quite a few PRs piling up here - would you be interested in an additional maintainer joining on to help review and test things? This is a pretty critical component within ROS 2. |
|
@ahcorde @adityapande-1995 I see you reviewed/merged #845 (which has this PR now in conflict, so I'll have to go update around your change) - are either of you now a maintainer of this package? Can we get a review/feedback on this? |
mikeferguson
force-pushed
the
lazy_subscriber_support
branch
from
f5c7818 to
8a85eaf
Compare
|
rebased on latest changes to handle conflict |
|
We should also create an iron branch, per #837, before we actually merge this |
There was a problem hiding this comment.
Umm, I noticed this is failing in iron
/tmp/ws/src/image_pipeline/image_proc/src/resize.cpp:72:56: error: too many arguments to function ‘image_transport::CameraPublisher image_transport::create_camera_publisher(rclcpp::Node*, const string&, rmw_qos_profile_t)’
14:26:27 72 | pub_image_ = image_transport::create_camera_publisher(
14:26:27 | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^
14:26:27 73 | this, "resize/image_raw", qos_profile, pub_options);
14:26:27 | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
14:26:27 In file included from /tmp/ws/src/image_pipeline/image_proc/include/image_proc/resize.hpp:38,
14:26:27 from /tmp/ws/src/image_pipeline/image_proc/src/resize.cpp:39:
14:26:27 /opt/ros/iron/include/image_transport/image_transport/image_transport.hpp:75:17: note: declared here
14:26:27 75 | CameraPublisher create_camera_publisher(
14:26:27 | ^~~~~~~~~~~~~~~~~~~~~~~
14:26:27 gmake[2]: *** [CMakeFiles/resize.dir/build.make:76: CMakeFiles/resize.dir/src/resize.cpp.o] Error 1
14:26:27 gmake[1]: *** [CMakeFiles/Makefile2:230: CMakeFiles/resize.dir/all] Error 2
14:26:27 gmake: *** [Makefile:146: all] Error 2
14:26:27 ---
Maybe something was not backported to iron in image_common ?
|
Yeah - this won't work Iron - only rolling and forward (it requires an ABI-breaking change that couldn't go into iron, we ran out of time back in April) - so we should fork off an iron branch first |
|
I just created an iron branch - https://github.com/ros-perception/image_pipeline/tree/iron - will update CI next |
I saw your other PR here ros-perception/image_common@4965283 |
|
@mikeferguson I see you added yourself as a maintainer, I created this PR ros2-gbp/ros2-gbp-github-org#425 to be able to create releases, Let me know I should add you there too. |
|
Once ros/rosdistro#39567 is merged, it will update what CI jobs run and this can be merged |
|
PR to rosdistro is merged ... but buildfarm hasn't reconfigured yet I guess (since Iron is still triggering) |
|
Ok - closing and reopening the PR this time - iron didn't retrigger so the build farm has reconfigured (just wanted to make sure that we didn't have issues for new PRs that get opened after this one is merged). |
missed a few components in #815
This implements ros-perception#780 for ROS 2 distributions after Iron, where we have: * Connect/disconnect callbacks, per ros2/rmw#330 (this made it into Iron) * Updated APIs in ros-perception/image_common#272 (this is only in Rolling currently)
missed a few components in ros-perception#815
This implements #780 for ROS 2 distributions after Iron, where we have: