Add support for user-specified content filters#68
Merged
Conversation
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR is a follow up to #12 which takes a different approach in introducing support for content-filtered topics by trying to simplify the implementation and overcome some issues that arose during development of that PR, particularly with respect to concurrency.
The main problem introduced by #12 is the need to delete and re-create the DDS DataReader if the content-filter expression of the associated ROS2 Subscription is modified. Specifically, if the reader was associated with a content-filtered topic and the expression is empty, the reader must be recreated on the base, unfiltered, topic, and viceversa (if the reader didn't have a filter and must now use one, it will be deleted and recreated on a new content-filtered topic).
Beside introducing a whole lot of concurrency problems, a big issue with this strategy is that it makes "enabling/disabling filtering" a very expensive operation both locally (because the existing reader must be finalized and a new one created, causing potentially a whole lot of memory deallocation/allocation), and remotely/on the network (since the deletion of reader will be announced to other participants in order to unmatch it from any remote writer that was communicating with it, and then the new reader will need to be announced and re-matched again).
This is an unfortunate byproduct of the existing DDS API which differentiates between "regular topics" and "content-fitlered topics" instead of making the "filter" a property of the DataReader: if it were possible to create a content-filtered topic with an empty expression (which would cause it to behave like the base unfiltered topic), it would be possible for
rmw_connextddsto always create a content-filtered topic for every new subscription, even when no filter expression is specified by the user.In order to enable this use case, we take advantage of RTI Connext DDS' ability to register custom content-filter implementations and create a new custom content-filter which extends the built-in SQL-like filter included in Connext by adding the ability to handle empty filter expressions.
All DataReaders created by
rmw_connextddsnow use a content-filtered topic, and the ROS2 layer can easily manipulate the filtering expression.The custom content filter is encapsulated in a new package,
rti_connext_dds_custom_sql_filter.The filter supports writer-side filtering but it might introduce some minor overhead in the case of an empty expression. This overhead should be characterized through performance testing to make sure any degradation is within an acceptable range.
The branch for this PR was created off of the current
master(944da9d) with the addition of the implementation for new RMW APIs and some changes to existing internal functions to create content-filtered topics that were introduced by @iuhilnehc-ynos in #12.