ros-perception / perception_pcl

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

pcl_conversions incompatible with latest version of PCL #435

Closed kgreenek closed 9 months ago

kgreenek commented 9 months ago

When using the latest version of pcl with pcl_conversions, I see the following compiler errors:

pcl_conversions/include/pcl_conversions/pcl_conversions.h: In function 'void pcl_conversions::fromPCL(const pcl::PointIndices&, pcl_msgs::msg::PointIndices&)':
external/perception_pcl/pcl_conversions/include/pcl_conversions/pcl_conversions.h:304:25: error: no match for 'operator=' (operand types are 'pcl_msgs::msg::PointIndices_<std::allocator<void> >::_indices_type' {aka 'std::vector<int>'} and 'const Indices' {aka 'const std::vector<unsigned int, std::allocator<unsigned int> >'})
  304 |     pi.indices = pcl_pi.indices;

It appears as if the pcl::PointIndices type is an unsigned int, but the pcl_conversions code is expecting it to be a signed int.

SteveMacenski commented 9 months ago

The main branch of this targets ROS 2 rolling, so if the rolling version of PCL doesn't have that change then we should keep it the way it is for compatability with the REP 2000 officially supported versions for a distribution.

However, we should definitely have this fixed for newer versions. I'd appreciate a PR to implement something like the following for the new version of PCL that this applies to:

#if PCL_VERSION_COMPARE(>=, Z, X, Y)
    // new setting
#else
    // old setting
#endif

That would then allow it to work with your newer version as well

mvieth commented 9 months ago

@kgreenek Do you build pcl yourself, with PCL_INDEX_SIGNED=false ( https://github.com/PointCloudLibrary/pcl/blob/master/cmake/pcl_options.cmake#L99 ) ? If PCL_INDEX_SIGNED=true (the default), then indices in pcl::PointIndices is a vector of (signed) int. If PCL_INDEX_SIGNED=false, then indices in pcl::PointIndices is a vector of unsigned int. Note: this is not a very recent change, it has been around for several PCL versions. pcl_conversions currently indeed only works with the default case, PCL_INDEX_SIGNED=true.

kgreenek commented 9 months ago

Oh! This is almost certainly my problem. I am using the bazel implementation and I just noticed that it has that flag set to false for some reason. I'll fix it in the bazel rules for building pcl. Thank you!

SteveMacenski commented 9 months ago

Happy to help! (Though @mvieth did all the thing-knowing)