design
design copied to clipboard
ContentFilteredTopic Design Proposal.
ROS 2 ContentFilteredTopic Feature Support.
- adding content-filtered-topic interfaces for Humble API freeze
- [x] ros2/rmw
- [x] ros2/rmw_implementation
- [x] ros2/rmw_connextdds (CFT supported)
- [x] ros2/rmw_fastrtps (CFT supported)
- [x] ros2/rmw_cyclonedds (not supported)
- [x] ros2/rcl
- [x] ros2/rclcpp
- follow-ups for Humble Release
- [x] Tutorial
- [x] Update Humble Release Note
- [x] demo_nodes_cpp
- [x] examples
- Fallback Filtering after Humble Release
-
/parameter_events
& actionfeedback
topics.
- [ ] ros2/rcl
- [ ] ros2/rclcpp
- rclpy support
- [ ] ros2/rclpy
Signed-off-by: Tomoya.Fujita [email protected]
@wjwwood @ivanpauno CC: @eboasson
would you give us some feedback what/how you feel about this feature extension? pointing you, because i usually have communication with you these layers, such as rmw/rcl/rclxxx.
also anyone, any idea and comments will do good.
This pull request has been mentioned on ROS Discourse. There might be relevant details there:
https://discourse.ros.org/t/content-filtering-topic/13889/1
This pull request has been mentioned on ROS Discourse. There might be relevant details there:
https://discourse.ros.org/t/ros-2-real-time-working-group-online-meeting-16-apr-29-2020-meeting-minutes/13999/4
@wjwwood @dirk-thomas
friendly ping, but not urgent.
Someone will take a look at this but likely only after the Foxy release is out.
@fujitatomoya I see you have been working on draft PRs, but bare in mind that this has high chances of being rejected (specially, because how DDS specific is this feature). I would really wait for the design approval before having a full implementation.
Sorry for the delay in reviewing.
@ivanpauno
really appreciate for checking on this and advice that is helpful.
I would really wait for the design approval before having a full implementation.
i was thinking that too, i would discuss more how we could make approach on this before implementation.
Adding an optional feature sounds fine to me, but you're proposing making use of cft in some built-in use cases (parameter events and actions).
It does not have to be only for built-in use cases, but user classes also. that is what we want to discuss.
P.S.: parameter events and multi-client actions are a great example of where using cft is a performance win.
I believe so. Something we'd like to know from the others is is there any approach that we can solve the problems defined in this draft?
. I mean, if there is and it is better, we will do that to support the requirement instead of cft.
How will parameter events work in the case the rmw implementation doesn't support content filtering? i.e. How will filtering happen in that case?
i guess that we have options,
- filtering will be done in subscription side. (not rmw implementation, but rclcpp/rclpy instead)
- return NOT_SUPPORT if rmw implementation does not support cft.
thinking about good user experience for it just works out-of-the-box
, option-1 can be an actual choice. we can support this as well.
If we add this, I'm not sure if we should support the content filtering expressions supported in the DDS standard, or a custom designed one for ROS2 (with a way to convert between the two).
as far as i see, DDS standard expressions are really flexible so it would be nice to keep the standard.
return NOT_SUPPORT if rmw implementation does not support cft.
This is tempting, but it would have the side-effect of preventing a non-DDS-based RMW being used for a node that decides it wants content filtering. I feel that whether or not to content-filter a topic should either be decidable by the system integrator outside of the node's implementation, or by the node implementer and done in the node's implementation (not in the RMW layer) if we want to remain completely portable.
I think that if we want content filtering, we would have to provide a generic interface for doing it that can internally switch between using RMW-provided content filtering when available or doing it at a higher layer when not available in the RMW layer. Without that generic support also being provided I think adding access to it in the RMW layer is risky.
@gbiggs
thanks for sharing you thoughts! i do agree on that.
- filtering will be done in subscription side. (not rmw implementation, but rclcpp/rclpy instead)
- return NOT_SUPPORT if rmw implementation does not support cft.
Yeah, in the case content filtered topics are added, I think that having a default implementation of subscription side filtering for when the rmw
implementation don't support the feature is a good idea.
We will need full support of dds cft expressions somewhere.
but rclcpp/rclpy instead
It could be implemented in rcl
instead.
It could be implemented in rcl instead.
i was thinking about that, it will be better.
This pull request has been mentioned on ROS Discourse. There might be relevant details there:
https://discourse.ros.org/t/rmw-proposal-content-filtered-topic-suport/16113/1
@wjwwood friendly ping
we are doing some test about efficiency of ContentFitleredTopic based on RTI Connext with scenarios.
- Confirm filtering happens either on writer or reader side. (Network transmission if that happens or not.)
note,
- DDS specification does not specify ContentFilteredTopic implementation, so it is up to implementation.
- To support different DDS implementation, reader must be responsible to filter the message at the end.
- filtering depends on how many actual messages need to be delivered to reader. that said, if # of receiver is more than 80%, it would be better to do the filtering process in the reader side. and vice-versa.
This pull request has been mentioned on ROS Discourse. There might be relevant details there:
https://discourse.ros.org/t/cyclonedds-roadmap-multi-network-async-content-filtering-ql1-et-al/17561/5
To add content-filtered-topic interfaces and an empty stub implementation for rmw_connext,rmw_fastrtps,rmw_cyclonedds
- [ ] ros2/rmw
- [ ] ros2/rmw_implementation
- [ ] ros2/rmw_connext
- [ ] ros2/rmw_fastrtps
- [ ] ros2/rmw_cyclonedds
- [ ] ros2/rcl
- [ ] ros2/rclcpp
@wjwwood @ivanpauno @clalancette CC: @iuhilnehc-ynos
friendly ping, ContentFiteredTopic 1st step is out here: https://github.com/ros2/design/pull/282#issuecomment-785522547
i know you are busy and there are other stuff to review in the queue. but we really appreciate if you have some time to take a look and give us feedback 😄 thanks in advance.
Note for myself,
- enabling ContentFilteredTopic, we need https://github.com/rticommunity/rmw_connextdds to be merged before this.
- ideally, we need to follow-up with examples for https://github.com/ros2/demos/tree/master/demo_nodes_cpp.
This pull request has been mentioned on ROS Discourse. There might be relevant details there:
https://discourse.ros.org/t/rmw-proposal-content-filtered-topic-suport/16113/7
I'd like to suggest renaming the two new RMW functions used to access and set the content-filter expression and parameters to be slightly more appropriate.
Quoting myself from a comment in ros2/rmw_connextdds#68:
I feel like
[get|set]_cft_expression_parameters()
is a bit of a misnomer, since "parameters" are kind of an "advanced use case" and in many cases users might be interested in getting and setting only the expression.I think something like
[get|set]_content_filter()
(without "expression" or "parameter") would be more appropriate. So, we could rename the functions to:
rmw_subscription_get_content_filter
rmw_subscription_set_content_filter
Does that sound reasonable?
@asorbini it does for me.
@iuhilnehc-ynos do you happen to have a repo file for CFT development?
@iuhilnehc-ynos do you happen to have a repo file for CFT development?
@iuhilnehc-ynos friendly ping!
@fujitatomoya
Sorry, I missed the https://github.com/ros2/design/pull/282#issuecomment-989205246. I need to re-check it. Some repositories might be rebased. (After that, I'll provide it in gist.github.com)
@fujitatomoya
Please refer to https://raw.githubusercontent.com/iuhilnehc-ynos/ros2/topic-debug-connextdds-cft/ros2.repos And I think we need to update the https://github.com/ros2/design/pull/282#issue-610568380
Remove ros2/rmw_connext
Update rticommunity/rmw_connextdds
from https://github.com/rticommunity/rmw_connextdds/pull/12 to https://github.com/ros2/rmw_connextdds/pull/68
@iuhilnehc-ynos thanks 👍
@iuhilnehc-ynos
which one is official fix for CFT? https://github.com/ros2/rmw_connextdds/tree/asorbini/cft or https://github.com/ros2/rmw_connextdds/pull/12? could you confirm and check?
@fujitatomoya
Please use https://github.com/ros2/rmw_connextdds/tree/asorbini/cft created in the https://github.com/ros2/rmw_connextdds/pull/68 by @asorbini , I'll close https://github.com/ros2/rmw_connextdds/pull/12.
@ros2/team could you have some time to review this and shar comments?