Add --content-filter support to ros2 topic echo|hz|bw#1213
Add --content-filter support to ros2 topic echo|hz|bw#1213PavelGuzenfeld wants to merge 2 commits intoros2:rollingfrom
Conversation
9a6125b to
59c1186
Compare
|
Thank you for the PR! Please see my note here: ros2/rclcpp#3109 (comment) |
|
Thanks for the review! AI disclosure: This PR was authored with the assistance of Claude (Anthropic) as a generative AI coding tool. I have reviewed and validated the changes. Branch target: Will retarget to |
|
Correction on the AI disclosure: the model used was Claude Opus (Anthropic). |
59c1186 to
8d77247
Compare
|
Rebased onto The original feature request (#1126) applies to |
Add DDS content filter expression support to `ros2 topic echo`, `ros2 topic hz`, and `ros2 topic bw` commands via new `--content-filter` and `--content-filter-params` CLI arguments. Content filters are applied at the middleware level, so only matching messages are delivered to the subscriber — reducing bandwidth and CPU usage compared to client-side Python filtering. Closes ros2#1126 Signed-off-by: Pavel Guzenfeld <me@pavelguzenfeld.com> Signed-off-by: Pavel Guzenfeld <pavelguzenfeld@gmail.com>
8d77247 to
237c9d9
Compare
|
The CI failure is a flaky test, not caused by this PR: The test uses This PR only adds Could a maintainer please re-trigger the CI? |
| 'placeholders with --content-filter-params.') | ||
| parser.add_argument( | ||
| '--content-filter-params', dest='content_filter_params', nargs='*', default=[], | ||
| help='Parameters for the content filter expression') |
There was a problem hiding this comment.
It seems like the content filtering feature might be able to replace the Python expression-based filtering above (--filter), which has security implications.
@fujitatomoya since you're familiar with content filtering, what do you think?
|
Can you add tests to the commands you've added these new options to? At least just to exercise the happy path and check that it does filter with a simpler filter, nothing complicated. |
Integration tests: - test_echo_content_filter: matching and non-matching DDS content filter expressions - test_echo_content_filter_once: --content-filter combined with --once - test_echo_content_filter_combined_with_filter: DDS content filter combined with Python --filter (dual filtering) - test_hz_content_filter / test_bw_content_filter: happy path - test_hz_content_filter_no_match / test_bw_content_filter_no_match: non-matching filter produces no rate/bandwidth output Contract tests (test_content_filter_contract.py): - Argument parsing for --content-filter and --content-filter-params across echo, hz, and bw verbs - ContentFilterOptions construction from parsed arguments - None propagation when --content-filter is not provided
1f9b34d to
85997ec
Compare
|
Added tests in the latest push: test_echo_pub.py — test_bw_delay_hz.py — test_content_filter_contract.py — arg parsing and Note: Fast DDS doesn't support bare string |
Summary
Closes #1126
Adds DDS content filter expression support to
ros2 topic echo,ros2 topic hz, andros2 topic bwcommands via new--content-filterand--content-filter-paramsCLI arguments.Content filters are applied at the middleware level (via
rclpy.subscription_content_filter_options.ContentFilterOptions), so only matching messages are delivered to the subscriber — reducing bandwidth and CPU usage compared to client-side Python--filterexpressions.Usage examples:
Changes:
echo.py: Add--content-filterand--content-filter-paramsarguments, passContentFilterOptionstocreate_subscriptionhz.py: Same additions, passed through_rostopic_hzbw.py: Same additions, passed through_rostopic_bwTest plan
ros2 topic echo --helpshows new--content-filterand--content-filter-paramsoptionsros2 topic hz --helpshows new optionsros2 topic bw --helpshows new optionsdata LIKE '%Hello%') receives messagesdata LIKE '%NOMATCH%') receives no messages--content-filterworks unchanged