ParameterEventHandler can be extended with ContentFiltering

[fujitatomoya]

Description

Currently ParameterEventHandler creates the subscription for /parameter_events and registered callbacks will be called when the parameter events are cominig in. This means that all the parameter events need to be received by this subscription no matter what, but it is likely expected that user application focus on the specific parameter of the particular node.
This can be much more efficient with ContentFiltering feature.

reference: https://github.com/ros2/design/blob/gh-pages/articles/content_filtering.md

Motivation

We can save the network traffic and much better efficiency, especially resource constrained edge devices.

Design / Implementation Considerations

IMO,

• ParameterEventHandler constructor can support SubscriptionOption that includes the content filtering expression when it is created.
• Introduce methods to set and get the filtering expression and parameters to the internal subscription object event_subscription_, so that user can handle the content filtering expression and parameters as they like.

Additional Information

No response

[Barry-Xu-2018]

@fujitatomoya

About implementation, I want to discuss with you.
Writing content filtering for parameter events requires developers to understand SQL syntax and the message format of ParameterEvent, which is not convenient for ordinary developers.

Therefore, I consider another way to implement it.
ParameterEventHandler provides two functions to add callbacks.

1. Add a callback for all parameter events

   rclcpp/rclcpp/include/rclcpp/parameter_event_handler.hpp

   Lines 206 to 220 in b14af74

   	/// Set a callback for all parameter events.
   	/**
   	* This function may be called multiple times to set multiple parameter event callbacks.
   	*
   	* Note: if the returned callback handle smart pointer is not captured, the callback is
   	* immediatedly unregistered. A compiler warning should be generated to warn of this.
   	*
   	* \param[in] callback Function callback to be invoked on parameter updates.
   	* \returns A handle used to refer to the callback.
   	*/
   	RCLCPP_PUBLIC
   	RCUTILS_WARN_UNUSED
   	ParameterEventCallbackHandle::SharedPtr
   	add_parameter_event_callback(
   	ParameterEventCallbackType callback);

2. Add a callback for a specified parameter.

   rclcpp/rclcpp/include/rclcpp/parameter_event_handler.hpp

   Lines 233 to 252 in b14af74

   	/// Add a callback for a specified parameter.
   	/**
   	* If a node_name is not provided, defaults to the current node.
   	*
   	* Note: if the returned callback handle smart pointer is not captured, the callback
   	* is immediately unregistered. A compiler warning should be generated to warn
   	* of this.
   	*
   	* \param[in] parameter_name Name of parameter to monitor.
   	* \param[in] callback Function callback to be invoked upon parameter update.
   	* \param[in] node_name Name of node which hosts the parameter.
   	* \returns A handle used to refer to the callback.
   	*/
   	RCLCPP_PUBLIC
   	RCUTILS_WARN_UNUSED
   	ParameterCallbackHandle::SharedPtr
   	add_parameter_callback(
   	const std::string & parameter_name,
   	ParameterCallbackType callback,
   	const std::string & node_name = "");

If the user sets the first type of callback function, then no content filter will be set, or any previously set content filter will be reset. This is because the user is concerned about all parameter changes.

The content filter is only set when the second type of callback function is configured. This content filter is internally created based on the parameter name and node name passed in by the user when calling this function. If a content filter has already been configured, the new content filter expression needs to be merged with the existing one.

What do you think?

[Barry-Xu-2018]

I found a problem: currently, filtering by parameter name is not supported.

• The format of ParameterEvent

# This message contains a parameter event.
# Because the parameter event was an atomic update, a specific parameter name
# can only be in one of the three sets.

# The time stamp when this parameter event occurred.
builtin_interfaces/Time stamp

# Fully qualified ROS path to node.
string node

# New parameters that have been set for this node.
Parameter[] new_parameters

# Parameters that have been changed during this event.
Parameter[] changed_parameters

# Parameters that have been deleted during this event.
Parameter[] deleted_parameters

• The format of Parameter

# This is the message to communicate a parameter. It is an open struct with an enum in
# the descriptor to select which value is active.

# The full name of the parameter.
string name

# The parameter's value which can be one of several types, see
# `ParameterValue.msg` and `ParameterType.msg`.
ParameterValue value

When we want to match the parameter name, we need to perform element matching operations within an array, but the SQL statements for content filters in FastDDS do not support array operations.
Refer to https://fast-dds.docs.eprosima.com/en/stable/fastdds/dds_layer/topic/contentFilteredTopic/defaultFilter.html
In DDS spec，the section "Annex B - Syntax for Queries and Filters" doesn't mention operation for array.

So the only item that can be filtered is the node.

[fujitatomoya]

Writing content filtering for parameter events requires developers to understand SQL syntax and the message format of ParameterEvent, which is not convenient for ordinary developers.

correct, requiring the user to understand the SQL syntax is not ideal. i was thinking that we can do the filtering based on the node name and parameter names.

When we want to match the parameter name, we need to perform element matching operations within an array, but the SQL statements for content filters in FastDDS do not support array operations.

related to above comment. after all, i think what we can do right now is to filter based on the node name only. i believe that this is still better to reduce the significant traffic when it subscribes the /parameter_events giant global topic.

Add a callback for a specified parameter.

besides, this indicates that filtering based on only node name is still good. i think this API is originally introduced because it is likely that user application is only interested in the specific node name and parameters.

If the user sets the first type of callback function, then no content filter will be set, or any previously set content filter will be reset. This is because the user is concerned about all parameter changes.
The content filter is only set when the second type of callback function is configured. This content filter is internally created based on the parameter name and node name passed in by the user when calling this function. If a content filter has already been configured, the new content filter expression needs to be merged with the existing one.

personally i do not really like this coupling... i think it should be decoupled.
cft setting API should be dedicated to set the content filtering expression and parameters, that takes the node names list. and callback registration functions do not change the content filtering setting at all. i think that this is much clear behavior for the user application.
in this way, user application configure more efficient filtering based on node names. for example, user can add a callback for all parameter events, but waiting for the parameter events only for specific node list. (not all parameter events for all nodes.)

[Barry-Xu-2018]

@fujitatomoya

in this way, user application configure more efficient filtering based on node names. for example, user can add a callback for all parameter events, but waiting for the parameter events only for specific node list. (not all parameter events for all nodes.)

My understanding is that you want to use a new API that allows users to specify which parameter events of the nodes will be handled.

My original idea was obtaining the nodes that users care about from the callback settings, without adding new interfaces. This way, existing programs can automatically take advantage of the content filter feature.

[Barry-Xu-2018]

@fujitatomoya friendly ping. I just want to make sure I understand correctly. We need to add a new API.

[fujitatomoya]

My original idea was obtaining the nodes that users care about from the callback settings, without adding new interfaces. This way, existing programs can automatically take advantage of the content filter feature.

yes, i am suggesting to add the new method to manage the filtering expression and parameters for user application, decoupled with other existing APIs.
your approach still makes sense, but i was thinking that we do not want to affect any user application at all (including possible bug we could generate). so already existing methods can stay totally unaware of content filtering concept, in this way all the user application will be able to stay without having any affection from this new methods. in other words, user application needs to understand how to use and how it works in this new methods to control content filtering feature.

besides above, content filtering is not supported by all rmw implementation. that means, we probably do not interfere already existed methods with content filtering (works for some application but some with warning messages). and in this way, it is much clearer for user that they do understand they what they are doing with this new API / the content filtering feature.

what do you think?

[Barry-Xu-2018]

I agree with your opinion. I will implement it with a new API.

[Barry-Xu-2018]

The rclcpp code is ready. Please help to review #2971.
The rclpy code is under development.

[Barry-Xu-2018]

The rclpy code is ready. Please help to review ros2/rclpy#1531.