perception_pcl icon indicating copy to clipboard operation
perception_pcl copied to clipboard

Humble Porting

Open AntoBrandi opened this issue 6 months ago • 4 comments

Replaces https://github.com/ros-perception/perception_pcl/pull/505 and https://github.com/ros-perception/perception_pcl/pull/501 by properly cherry-picking ros2 commits into humble.

Addresses: https://github.com/ros-perception/perception_pcl/issues/423

Additionally, to fix the build on Humble, small fixes were required:

  • QoS profiles to use .get_rmw_qos_profile() in subscribers
  • Disable lazy subscribers as pub_options.event_callbacks.matched_callback = [this](rclcpp::MatchedInfo & /*info*/) ... is not available in Humble

AntoBrandi avatar Aug 26 '25 09:08 AntoBrandi

A note on my last commit: I can try to make it work on different ROS distros so that it can be cherry-picked in the master. I can use the same approach used in the laser_filters package. Alternatively, the simpler ros2_control approach can also be used.

This way, future syncs should be easier

AntoBrandi avatar Aug 26 '25 11:08 AntoBrandi

Thank you for putting the effort to cherry-pick all relevant commits to humble

I have questions about two specific commits:

  1. Modern CMake: This changes the exported targets. Will this cause downstream compilation failures?
  2. Fix warning: 'subscribe<>' is deprecated: This commit is only needed for jazzy and higher. You corrected this in a lter commit but it might be better to just remove this commit from this PR so you don't need to correct it
  3. In your last commit you did a linter fix. Does it make more sense to disable uncrustify so we can freely cherry-pick from the ros2 branch?

In general I'm a bit worries the changes will lead to downstream compilation failures. Can @SteveMacenski maybe give his opinion about this PR?

Btw, I'm going on vacation for 3 weeks so it might take a while before I come back to this.

Rayman avatar Aug 29 '25 08:08 Rayman

My original idea was to minimize the differences between the ros2 branch and the humble branch, so that future commits could be easily backported to humble. However, I understand your point, especially for the possible downstream compilation failures. In that case, we can follow a different approach: Making the ros2 branch buildable on humble, like the humble_main branch in Nav2

AntoBrandi avatar Sep 02 '25 07:09 AntoBrandi

I wouldn't change the exported targets / variables in an already released distribution. Rclcpp I see still uses the old ament macro for this in Humble https://github.com/ros2/rclcpp/blob/humble/rclcpp/CMakeLists.txt#L193-L207 so that makes me further think we shouldn't backport that to humble as a ABI breaking change.

P.S. I don't particularly recommend the humble_main strategy, its a pain in the ass to keep synced up and I've actually already since decided to stop after introducing nav2_ros_common due to extensive use of ROS 2 advanced features not available in Humble at all. It won't be updated post-July 2025 (though up to date in a broad sense compared to humble itself). The changes for targets + APIs makes keeping Humble x Rolling unrealistically difficult to maintain compatibility for in a project unless someone wants to take explicit ownership for it as a prime objective with regular care and upkeep.

SteveMacenski avatar Sep 03 '25 16:09 SteveMacenski