add a demo of content filter listener

[iuhilnehc-ynos]

Signed-off-by: Chen Lihui lihui.chen@sony.com

[iuhilnehc-ynos]

@asorbini @MiguelCompany

As we know, the content filter has operations as below,
`RelOp ::= ‘=’ | ‘>’ | ‘>=’ | ‘<’ | ‘<=’ | ‘<>’ | like``

I expected that the keyword like can work as https://www.w3schools.com/sql/sql_ref_like.asp.
Unfortunately, it seems it doesn't work.

Is there a limitation in the DDS?

[MiguelCompany]

@iuhilnehc-ynos On Fast DDS it should work as expected. See the test cases here

RTI Connext, according to this comment, does not support like

[iuhilnehc-ynos]

@MiguelCompany

RTI Connext, according to this comment, does not support like

Thanks for your information. Sorry, I didn't notice it before.

On Fast DDS it should work as expected.

After confirming, it works fine with rmw_fastrtps_cpp.

[ivanpauno]

LGTM!

Could you add a test here?

[ivanpauno]

@wjwwood @clalancette any objection to merge this now?
We're past the API break, but nothing depends on the demos and it seems worthwile to have a demo of a new feature.

[fujitatomoya]

Once this demo is merged, i will link this demo to Humble Release Note. (see ros2/ros2_documentation#2396)

[wjwwood]

I don't mind it getting merged this late, following your logic, but @clalancette should give his thoughts. I didn't review the demo code yet.

[fujitatomoya]

CI:

• Linux
• Linux-aarch64
• Windows

[ivanpauno]

@iuhilnehc-ynos On Fast DDS it should work as expected. See the test cases here

RTI Connext, according to ros2/design#282 (comment), does not support like

Could we modify the example to work with both?

[iuhilnehc-ynos]

@ivanpauno

Could you add a test here?

Sorry, I don't know how to add a test for this listener_content_filter, it seems the current test_executables_tutorial.py.in can't work as I expect.

I expect that running with colcon test --packages-select demo_nodes_cpp --ctest-args -R test_tutorial_talker_listener_content_filter__rmw_cyclonedds_cpp should fail with the patch, but it doesn't.

What I mean is if the test in the above patch can be passed by rmw_cyclonedds_cpp, it seems there is no reason to add it.

[ivanpauno]

Maybe the number (currently 10) could be configurable by the user.
Maybe that makes the demo more interesting (?).
Feel free to ignore the comment though.

[fujitatomoya]

I actually agree with this comment. probably string chatter example is interesting enough to show this feature. how about filtering specific range of data? that would be one of the most common case?

[ivanpauno]

I expect that running with colcon test --packages-select demo_nodes_cpp --ctest-args -R test_tutorial_talker_listener_content_filter__rmw_cyclonedds_cpp should fail with the patch, but it doesn't.

Yes, that's not going to work because the test is waiting for the output in the file to be printed

demos/demo_nodes_cpp/test/test_executables_tutorial.py.in

Line 55 in 7a15d0b

proc_output.assertWaitFor(

, and that happens as well if no content filter is applied.
You'll need to adapt the test, or adapt the file so it doesn't match the output if "Content filter is not enabled since it's not supported" was logged before (I'm not sure if that's possible).

---

Please don't share a patch, they are hard to understand without being applied to the code to have more context.
It's better to push the commit and ask the question, as github gives more context (or if you don't want to push in this branch, you can do that in a new branch/fork and share the link to the github diff).

[fujitatomoya]

@iuhilnehc-ynos all comments are addressed, right? can you confirm? so that we can ask final review to @ivanpauno @wjwwood

[wjwwood]

Just curious why not put this kind of thing in ros2/examples instead?

[ivanpauno]

LGTM with @wjwwood approval

[fujitatomoya]

@wjwwood friendly ping.

[wjwwood]

This is really fragile, since if someone updated the expression and forgot to update this is would be wrong. Violates DRY.

Can we print out the filter expression and parameters from the content_filter_options directly instead?

[wjwwood]

Is this comment block needed?

[wjwwood]

w.r.t. putting it in examples. I think having something here in demos is fine so long as we have a corresponding page about it or the executable is used in some documentation or tutorial. For example, the intra-process demos have this page which shows how to run it, what you should expect to see when you run it, and how it demonstrates that the intra-process feature is working:

https://github.com/ros2/ros2_documentation/blob/rolling/source/Tutorials/Intra-Process-Communication.rst

If we had something similar then this executable makes sense to me. Otherwise, I'd still say examples makes more sense. And I don't think you need a talker equivalent if it is put in examples, the purpose there is to show how to use the API, and it's not part of a tutorial or demo document that tells you to run it and observe some effect.

I hope that distinction makes sense.

[fujitatomoya]

@wjwwood @ivanpauno
CC: @iuhilnehc-ynos

i just created ros2/examples#341.

how about the following, sounds like a plan?

• we could keep this demo. but this demo could be adjusted more demonstration perspective. IMO, i would use float or integer to simulate the sensing data, and filtering the specific range. this would be common use case and easy to get a picture how it works. any comments are welcome.
• add ROS 2 ContentFilteredTopic for Humble Release Note. ros2_documentation#2396 can link this demonstration.
• adding tutorial how to use, and that doc links to add ContentFilteredTopic example. examples#341

[wjwwood]

ros2/ros2_documentation#2396 can link this demonstration.
adding tutorial how to use, and that doc links to ros2/examples#341

I'd flip this around so that:

• add ROS 2 ContentFilteredTopic for Humble Release Note. ros2_documentation#2396 links to the example (add ContentFilteredTopic example. examples#341)
• a new how to use "tutorial" and have that doc link to this demo application (and use talker from demo_nodes_cpp as part of the "how to use")
• After it is in place, add ROS 2 ContentFilteredTopic for Humble Release Note. ros2_documentation#2396 could also link to the tutorial/demo doc

We could set up a new example that uses floats or something like you said, but that would be for the demo/tutorial combo and not the minimal example.

[fujitatomoya]

@iuhilnehc-ynos

We could set up a new example that uses floats or something like you said, but that would be for the demo/tutorial combo and not the minimal example.

can we address this? current code looks good to us, just adjusting data type for using float to simulate the situation to filtering the sensing data. for doing that, we need to add publisher as well.

how about following?

• emergency temperature demonstration
  • say publisher is publishing temperature data with std_msgs/msg/Float64. make a loop to publish data -100 to +150 F in order with certain offset.
  • subscription use content filtering (if it is supported), and only receives data with “data < %0 OR data > %1” with {"-30.0", "100.0"}, then print emergency temperature.

i believe demonstrating with expected common use case would be much easier to get the picture of this feature for user.

[iuhilnehc-ynos]

@ros-pull-request-builder retest this please

[fujitatomoya]

implementation looks good to me.

[fujitatomoya]

Suggested change

	src/topics/temperature_publisher.cpp
	src/topics/temperature_subscriber.cpp
	src/topics/content_filtering_publisher.cpp
	src/topics/content_filtering_subscriber.cpp

sorry i was not clear on this, i think files, classes and components can be general names for content filtering, so that user can tell these are related to content filtering.

[ivanpauno]

Example looks really nice!
Thanks for the work!

[wjwwood]

Do you guys have demo documentation to go with this? Something along the lines of this document: https://docs.ros.org/en/rolling/Tutorials/Intra-Process-Communication.html

[fujitatomoya]

@wjwwood i will be back with PR for that, let me have a couple of days.

[wjwwood]

Sure, no problem, I would just hold this until that is ready.

[fujitatomoya]

@wjwwood @ivanpauno @iuhilnehc-ynos
CC: @clalancette

i just opened ros2/ros2_documentation#2441, could you guys take a look at to give me some feedback?

[fujitatomoya]

either @wjwwood or @clalancette

can you merge this with ros2/ros2_documentation#2441 at the same time?

[ivanpauno]

can you merge this with ros2/ros2_documentation#2441 at the same time?

It seems that CI wasn't run after the last changes, running it:

• Linux
• Linux-aarch64
• Windows

[fujitatomoya]

@ivanpauno thanks! all green, can you merge this?

[fujitatomoya]

@clalancette can you merge this? i already did ros2/ros2_documentation#2441

[fujitatomoya]

this is only required to backport humble package.

[wjwwood]

@Mergifyio backport humble

[mergify]

backport humble

✅ Backports have been created

Details

• #559 add a demo of content filter listener (backport #557) has been created for branch humble