search

ros-perception / perception_pcl

PCL (Point Cloud Library) ROS interface stack
http://wiki.ros.org/perception_pcl
421 stars 332 forks source link

Fixed message filter api #457

Open ahcorde opened 3 months ago

ahcorde commented 3 months ago

Related with this PR https://github.com/ros2/message_filters/pull/132

ahcorde commented 3 months ago

There is an issue with rosdistro and branches. The problem is described here https://github.com/ros-perception/perception_pcl/issues/458

If we merge this PR we will break humble, iron and jazzy deb builds

clalancette commented 3 months ago

To the maintainers here: it is usually our policy to not hard-break API like this in the ROS 2 core. We inadvertently did that when we merged in https://github.com/ros2/message_filters/pull/129 ; we have a revert PR open in https://github.com/ros2/message_filters/pull/132. However, we do eventually want to move in the direction of deprecating the current API and moving to a new one.

What's your feeling here? Would it be a lot better for you if we maintained the current API, and then somehow figured out how to move to a new one?

SteveMacenski commented 3 months ago

ros2 targets Rolling, humble/iron are branched separately. I don't think this is a problem, no?

clalancette commented 3 months ago

ros2 targets Rolling, humble/iron are branched separately. I don't think this is a problem, no?

It doesn't look like that is the case currently; all of Humble, Iron, Jazzy, and Rolling are using the ros2 branch:

SteveMacenski commented 3 months ago

See https://github.com/ros-perception/perception_pcl/issues/458

clalancette commented 3 months ago

See https://github.com/ros2/message_filters/pull/132#issuecomment-2192225078 , where we reverted the problematic change for now. We'll probably move back in that direction, so this PR will eventually be needed; up to you whether you want to leave this open for now or close it, and we can reopen later.