-
Notifications
You must be signed in to change notification settings - Fork 499
Support to configure feedback subscription content filter for action client #3034
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: rolling
Are you sure you want to change the base?
Support to configure feedback subscription content filter for action client #3034
Conversation
|
@Barry-Xu-2018 Can you provide more information of the intention behind this ? |
Although the content filter is set on the subscriber side, it actually takes effect on the publisher side. In other words, only data that meets the filtering criteria will be transmitted from the publisher to the subscriber. Of course, this also depends on the implementation of DDS. (Refer to https://fast-dds.docs.eprosima.com/en/latest/fastdds/dds_layer/topic/contentFilteredTopic/writerFiltering.html#dds-layer-topic-contentfilteredtopic-writer-side) Another benefit is that currently, feedback messages are received via the internal feedback topic. For the same action name, each action client receives all feedback messages sent by the action server to every action client. The current implementation of the action client is to receive the message, determine whether it is intended for itself by checking the goal ID, and process it further if it is; otherwise, the message is discarded. In fact, this approach of filtering after receiving messages is inefficient. With content filtering enabled, the action client will only receive feedback messages that are relevant to itself. Why am I considering adding a new interface here in rcl? Please refer to my comment ros2/rcl#1258 (comment) |
@jmachowinski as @Barry-Xu-2018 mentioned above, this is not our understanding (at lease from our verification and experience when we were developing content filtering 4-5 years ago...). if this is true, content filtering is not really useful feature in DDS.
our understanding (@Barry-Xu-2018 please correct me, if i got anything wrong... cz i might be... 😅 ) is,
i just want to make sure that if we are missing something here! thanks for posting the comment anyway 👍 CC: @MiguelCompany @fgallegosalido @lobolanja feel free to chime in. |
|
Thank you for your correction. The Content Filter on the DataWriter side involves many conditions. Currently, the Content Filter is used on the DataReader side. Therefore, it does not help with network load. The advantage is the second point I mentioned in #3034 (comment). |
…client Signed-off-by: Barry Xu <barry.xu@sony.com>
66a898e to
74f08ea
Compare
Signed-off-by: Barry Xu <barry.xu@sony.com>
|
Background information: Each goal ID requires 16 parameters. Therefore, an action client can handle up to 6 goals simultaneously. Once this limit is exceeded, the current approach is to stop using the content filter and revert to the original processing method. |
Signed-off-by: Barry Xu <barry.xu@sony.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR adds support in rclcpp_action clients (typed and generic) for configuring feedback subscription content filters to enable feedback message optimization, and verifies that enabling this optimization does not change normal feedback behavior.
Changes:
- Extend
ClientBase,Client<ActionT>, andGenericClientto accept anenable_feedback_msg_optimizationflag, manage an internal atomic state, and call newrcl_action_client_configure_feedback_subscription_filter_*APIs to add/remove goal IDs from the feedback subscription content filter during send, result, and cancel flows. - Add wrapper methods on
ClientBasefor configuring the feedback subscription filter and wire them into the typed and generic client send/result/cancel paths. - Introduce unit and integration tests to validate the new configuration helpers and to ensure that enabling feedback optimization does not affect normal feedback reception.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
rclcpp_action/include/rclcpp_action/client_base.hpp |
Adds internal APIs for configuring the feedback subscription filter, an enable_feedback_msg_optimization_ flag, and associated mutex; minor documentation issues noted. |
rclcpp_action/src/client_base.cpp |
Implements the new feedback subscription filter configuration helpers and wires the new optimization flag into the constructor. |
rclcpp_action/include/rclcpp_action/client.hpp |
Extends the typed Client constructor and integrates feedback optimization into async_send_goal, result handling, and cancel handling; new behavior needs additional test coverage for error paths. |
rclcpp_action/src/generic_client.cpp |
Mirrors the typed client changes for GenericClient, adding/removing goal IDs to the filter on send, result, and cancel. |
rclcpp_action/include/rclcpp_action/generic_client.hpp |
Extends GenericClient constructor and documents the feedback optimization behavior and limit. |
rclcpp_action/include/rclcpp_action/create_client.hpp |
Propagates the enable_feedback_msg_optimization option through the create_client helpers and documents it. |
rclcpp_action/include/rclcpp_action/create_generic_client.hpp |
Propagates the enable_feedback_msg_optimization option through the create_generic_client helpers and documents it. |
rclcpp_action/src/create_generic_client.cpp |
Updates the factory to pass the new optimization flag through to GenericClient. |
rclcpp_action/test/test_client.cpp |
Adds a helper client subclass to access the new configuration helpers, tests their return-value behavior under different rcl return codes, updates an existing construction test call for the new parameter, and adds an integration test verifying feedback behavior when optimization is enabled. |
rclcpp_action/test/test_generic_client.cpp |
Adds an integration test ensuring feedback behavior is unchanged when feedback message optimization is enabled for the generic client. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Signed-off-by: Barry Xu <barry.xu@sony.com>
|
@fujitatomoya filtering in Connext will be performed on the DataWriter side automatically if a bunch of conditions are met (you can check them in our documentation). Otherwise, filtering is done on the DataReader side. |
|
@fgallegosalido thanks for the confirmation 👍 |
|
Similar behavior in Fast DDS. Documentation here |
Description
Use Content Filtering for action feedback topic on action clients ros2/rcl#1258
This PR depends on ros2/rcl#1287
Is this user-facing behavior change?
No
Did you use Generative AI?
No
Additional Information