ctu-vras/sensor_filters

More Documentation needed

Closed this issue · 5 comments

uahic commented

I kind of like the idea of a unified filter chain package but the documentation is really really sparse. For example I have never worked with nodlets the pcl_ros perception filters etc. before and can't infer if I could for example use the passthrough filter of pcl_ros directly with your chain or not (probably not?).

peci1 commented

Hi, thanks for letting me know you're struggling.

Based on your comments, I have a few ideas regarding improvements of the docs. Maybe the most important would be putting a link to https://wiki.ros.org/filters near the top of the readme. This page explains the concept of ROS filters.

Regarding pcl_ros "filters" - no, you can't use them with this package. They are filters from the PCL point of view, but from the ROS point of view, they are nodelets, and thus cannot be plugged inside a ROS filter chain. I'll also try to explain that in the readme.

Last idea - I'll take the example files typed verbatim at the end of the readme and turn them into real files inside a launch folder, just where many ROS users are used to look for examples.

Do you have any further ideas on how the docs could be improved?

Generally speaking, the concept of ROS filters is largely underused in the ROS ecosystem, although they offer the highest performance of all alternatives. I don't exactly know why. But that's the reason why you won't find many ready-made filters out there. So the expected use-case for this package is that you write a few of your own filters, and sensor_filters will save you writing the boilerplate of a node(let) that's setting up the chain of filters and is running it. One of the few non-trivial filters available out there is for example https://github.com/peci1/robot_body_filter .

uahic commented

@peci1 Hi! thanks for the quick response, I've managed to implement my own LaserScan and PointCloud2 filters (partially using PCL filters inside). To know that filters::FilterBase is the core helps alot, yep I've had a look into robot_body_filter which gave me all the clues :)

Another thing you should point out is in terms of performance. Currently I'm a bit under project time pressure so didnt crawled your source code (or that of filter base) yet, but I wonder how big the complexity is to pass data from one filter to another; Best would be if its possible to pass a message through the filter chain with zero copy (assuming that each filter does nothing else than identity mapping, of course its up to the user if they operate in-place on the message or copy it into other formats)

Currently, I'm not sure about this and assume that between each filter there is a copy indeed but for the moment I'm willing to sacrafice a little performance for the sake of having my filters chainable and reusable instead of writing a standalone node that chains them

peci1 commented

Glad to hear the additional info helped you. I'll incorporate it here shortly.

Support for zero-copy filters has been proposed here: ros/filters#38 . I'd be glad if you could chime in and vouch for the PR. It only has to be updated now because recent changes to the filters packages made it incompatible.

The standard filters are not zero-copy, but they should still be the most performant way of processing a chain of data modifiers. Passing data from filter to filter is just about copying the message structure. In nodelets, you have to use shared pointers to get the most performant way of sharing data, so there you pay the extra price of dynamic memory allocations (unless you do tricks with pool-based allocation). Not speaking about the other ways of sharing data, which include (de)serialization or even travel via the TCP stack.

uahic commented

Awesome. I will have a look on the PR in late September during my vacations, currently I have to meet 3 deadlines very soon

Seems like the copying situation is much better than expected, worst case would have been that each filter is instantiated as a separate ROS node that passes around messages over the network stack. Most filter implementations I have seen so far seem to convert the sensor_msgs:: to PCL anyways and dont work in-place and prefer simplicity over performance. Yeah ok fiddling around with data field structures and computing offsets for iterators isnt the nicest thing to do, so I get it :D

peci1 commented

I've finished the extension of the docs. I hope they will be more useful from now on! Thanks for triggering this.