We have a little bit of a problem with perception_pcl for ROS2. We have pcl_conversions ported to ROS2 but so far no such port for pcl_ros containing a number of nodelets (should now be components).
This is a call for help from the community to help port this to ROS2. I can help review and merge things but need help from users to get to that point.
There’s been 2 PRs that have started this in crystal / dashing but largely needs completion and fixing now that ROS2 is at mostly feature parity.
Hi, I won’t have the needed time or experience with ROS 2, but as this is the largest opportunity for API/ABI breaks this library will have for a long time, I wanted to bring in some ideas we implemented in our ImprovedPCLFilters. They’re not published anywhere as the implementation required a number of ugly hacks I wouldn’t dare to put in a publicly available code. But if the ROS 2 port is done correctly, they could either be a direct part of it, or they could at least be much easier and cleaner done.
First, a few general things:
Please, make all methods of Filter virtual and protected. Currently, computePublish() is non-virtual, and input_indices_callback() is private and non-virtual, which makes extensions difficult. There’s also a hard blocker in ROS 1 for people to be generally able to make their own extensions based on the official pcl_ros nodelets - class_loader_hide_library_symbols. I’m not sure whether ROS 2 can do without it, but it’d be great.
Review both the ROS and C++ API and unify it. As seen in the previous point, the Filter class defines the camel-case computePublish and snake-case input_indices_callback. For the ROS part, PassThroughFilter uses parameter filter_limit_negative, whereas CropBox uses negative. Also the part with the dynamic reconfigure servers would deserve re-thinking to be more general (maybe it’d be possible to completely leave it out as ROS 2 support dynamic parameters out of the box).
If PCL 1.9 is used, make sure that you either only allow the keep_organized flag for filters that actually support it, or write a workaround to apply the flag manually. E.g. for CropBox in PCL 1.9, you can set the flag, but it has no effect. (I have the code for this workaround).
It also seems important to me that the nodelets correctly transform not only the x, y, z channels of the pointcloud, but all position- or orientation-related channels like normal_* or vp_*. It’s probably best to leave the names of the channels as a user configuration option (see further).
In the next post, I’ll just re-post an excerpt from the documentation of our ImprovedFilter which can serve as a source for improvement ideas. I can privately share our code implementing these functionalities with anyone who’s interested.
publish_periodically : If true, the filter will publish pointclouds transformed at the current time on rate publish/rate/desired . When a cloud comes in, it is converted to input_frame and stored. When the publication timer triggers, the stored cloud is converted to input_frame at current time using the given fixed_frame . Then the filter is applied, the pointcloud is converted to the output_frame and published.
fixed_frame : Only needed when publish_periodically is true. See the description above.
Improved filter - general:
tf_wait_timeout : Timeout for TF canTransform calls.
produce_diagnostics : Whether to compute and publish diagnostics of the publisher and subscriber.
disable_orig_tf_listener : If true and a shared TF2 buffer has been set to the nodelet before the onInit() call, the original TF listener from pcl_ros::PCLNodelet is disabled (by redirecting its tf topics to nonexistent topics). To launch the nodelet with a shared TF2 buffer, load it into cras_cpp_common/nodelet_manager_sharing_tf_buffer and not a normal nodelet manager. The shared buffer is accessible using the getBuffer() method.
Improved filter - transform arbitrary channels:
transform_channels_point : List of prefixes of channels to be transformed as points (6DOF). If channel consists of vp_x , vp_y and vp_z , pass only vp_ as the parameter here.
transform_channels_direction : List of prefixes of channels to be transformed as vectors or directions (only rotation). If channel consists of normal_x , normal_y and normal_z , pass only normal_ as the parameter here.
Improved filter - diagnostics:
There are two identical sets of settings, one for the subscriber (input pointclouds, param namespace receive/ ), and one for the publisher (output pointclouds, param namespace publish/ ). We describe them here generically with prefix ‘*’.
diagnostic_period : How often the diagnostics should be published. Only whole seconds are supported.
*/rate/desired : The expected publishing/receiving rate. publish/rate/desired is also used to configure the periodic publishing.
*/rate/min / */rate/max : The minimum/maximum expected publishing/receiving rate. If *_rate is set, it is used as a default value for these two.
*/rate/tolerance : Real min rate is ( */rate/min * (1-tolerance)), real max rate is ( */rate/max * (1+tolerance)).
*/rate/window_size : For how many updates (called once in diagnostic_period ) should the topic statistics be stored. If you set the window too small, the computed rates will have high variance. Also, consider setting this to a higher value when the expected rate of the topic is lower than 1.
*/delay/min / */delay/max : Min/max allowed difference between current time and time in the processed message’s header.
Sorry, I’m not actively working with PCL or even ROS2 at the moment, so I can’t contribute much. I’m happy to opine on architecture and provide review, but I’m leaning on my co-maintainer @smac to drive, which he’s doing very effectively here
@smac do you have any specific priorities or coordination with others in mind? Otherwise I’ll start by digging in to the PRs you linked and move them along.
I think the PRs would be a good starting point. PCL ROS is made up of 5 major areas
filters
segmentation
io
features
surface
so it would probably make sense to do one at a time. It would also be good for us to have a discussion looking over PCL and seeing if there’s any new stuff we want to also expose for ROS2 (not part of porting but we can open new tickets for it)
I think we should start with Foxy since that’s what will get immediate use. We can always discuss and backport to Dashing later if required. Though Dashing is only around for another 11 months. By the time we’re done with this, I suspect it will be 9-10 months.
Just FYI that I’m still looking at this, though it’s been a little slow going the last couple weeks. I plan to use the linked PRs as a starting point, but I will keep the work small and piecemeal for the sake of easier PRs and to make it easier for others to contribute too if they want.
Hoping to get the transforms library (including unit tests) stood up some time next week.
Just curious if there’s any plans on converting the old nodelets to components? The sloretz fork seemed to have some support for it but it seems the PR was closed and the current branch has no support at all.
