From 6d3b31d77aa38a72cf42865ae2ef61467736bcc1 Mon Sep 17 00:00:00 2001 From: Michel Hidalgo Date: Tue, 22 Sep 2020 16:02:41 -0300 Subject: [PATCH 1/3] Update service server/client creation/destruction API documentation. Signed-off-by: Michel Hidalgo --- rmw/include/rmw/rmw.h | 150 ++++++++++++++++++++++++++++++++++++------ 1 file changed, 129 insertions(+), 21 deletions(-) diff --git a/rmw/include/rmw/rmw.h b/rmw/include/rmw/rmw.h index 4f842704..01ee3445 100644 --- a/rmw/include/rmw/rmw.h +++ b/rmw/include/rmw/rmw.h @@ -1749,13 +1749,42 @@ rmw_return_loaned_message_from_subscription( const rmw_subscription_t * subscription, void * loaned_message); -/// Create an rmw client to communicate with the specified service +/// Create a service client that can send requests to a service server. /** - * \param[in] node Handle to node with which to register this client - * \param[in] type_support The type_support of this rosidl service - * \param[in] service_name The name of the ROS 2 service to connect with - * \param[in] qos_policies The QoS profile policies to utilize for this connection - * \return The initialized client if successful, `nullptr` if not + * This function can fail, and therefore return `NULL`, if: + * - `node` is `NULL`, or + * - `node` does not belong to this implementation i.e. it does not have + * a matching implementation identifier, or + * - `type_support` is `NULL`, or + * - `service_name` is `NULL`, or + * - `service_name` is an empty string, or + * - (if ROS namespace conventions apply) `service_name` is invalid by + * rmw_validate_full_topic_name() definition, or + * - `qos_profile` is `NULL`, or + * - `qos_profile` has invalid or unknown policies, or + * - memory allocation fails during service server creation, or + * - an unspecified error occurs. + * + *
+ * Attribute | Adherence + * ------------------ | ------------- + * Allocates Memory | Yes + * Thread-Safe | No + * Uses Atomics | Maybe [1] + * Lock-Free | Maybe [1] + * [1] rmw implementation defined, check the implementation documentation + * + * \pre Given `node` must be a valid node, as returned by rmw_create_node(). + * \pre Given `type_support` must be a valid `rosidl` service type support, as + * returned by ROSIDL_GET_SRV_TYPE_SUPPORT(). + * + * \param[in] node Node with which to register this service client. + * \param[in] type_support Type support of the service to be used. + * \param[in] service_name Name of the service to be used, often a fully qualified + * service name unless `qos_profile` is configured to avoid ROS namespace conventions + * i.e. to create a native service client. + * \param[in] qos_profile QoS policies for this service client's connections. + * \return rmw service client handle, or `NULL` if there was an error. */ RMW_PUBLIC RMW_WARN_UNUSED @@ -1766,11 +1795,36 @@ rmw_create_client( const char * service_name, const rmw_qos_profile_t * qos_policies); -/// Destroy and unregister a service client +/// Destroy and unregister a service client from its node. /** - * \param[in] node The associated node whose client will be destroyed - * \param[in] client The service client to destroy - * \return RMW_RET_OK if successful, otherwise an appropriate error code + * This function will reclaim all associated resources, including the service client itself. + * Use of a destroyed service client is undefined behavior. + * This function will return early if a logical error, such as `RMW_RET_INVALID_ARGUMENT` + * or `RMW_RET_INCORRECT_RMW_IMPLEMENTATION`, ensues, leaving the given service client unchanged. + * Otherwise, it will proceed despite errors. + * + *
+ * Attribute | Adherence + * ------------------ | ------------- + * Allocates Memory | No + * Thread-Safe | No + * Uses Atomics | Maybe [1] + * Lock-Free | Maybe [1] + * [1] rmw implementation defined, check the implementation documentation + * + * \pre Given `node` must be the one the service client was registered with. + * \pre Given `client` must be a valid service client, as returned by rmw_create_service(). + * + * \param[in] node Node with which the given service client is registered. + * \param[in] client Service client to be destroyed. + * \return `RMW_RET_OK` if successful, or + * \return `RMW_RET_INVALID_ARGUMENT` if `node` is `NULL`, or + * \return `RMW_RET_INVALID_ARGUMENT` if `client` is `NULL`, or + * \return `RMW_RET_INCORRECT_RMW_IMPLEMENTATION` if the `node` + * implementation identifier does not match this implementation, or + * \return `RMW_RET_INCORRECT_RMW_IMPLEMENTATION` if the `client` + * implementation identifier does not match this implementation, or + * \return `RMW_RET_ERROR` if an unspecified error occurs. */ RMW_PUBLIC RMW_WARN_UNUSED @@ -1809,13 +1863,42 @@ rmw_take_response( void * ros_response, bool * taken); -/// Create an rmw service server that responds to requests +/// Create a service server that can reply to client requests. /** - * \param[in] node The node that responds the service requests - * \param[in] type_support The type support description of this service - * \param[in] service_name The name of this service advertised across the ROS graph - * \param[in] qos_policies The QoS profile policies to utilize for connections - * \return The created service object if successful, otherwise a nullptr + * This function can fail, and therefore return `NULL`, if: + * - `node` is `NULL` + * - `node` does not belong to this implementation i.e. it does not have + * a matching implementation identifier + * - `type_support` is `NULL` + * - `service_name` is `NULL` + * - `service_name` is an empty string + * - (if ROS namespace conventions apply) `service_name` is invalid by + * rmw_validate_full_topic_name() definition + * - `qos_profile` is `NULL` + * - `qos_profile` has invalid or unknown policies + * - memory allocation fails during service server creation + * - an unspecified error occurs + * + *
+ * Attribute | Adherence + * ------------------ | ------------- + * Allocates Memory | Yes + * Thread-Safe | No + * Uses Atomics | Maybe [1] + * Lock-Free | Maybe [1] + * [1] rmw implementation defined, check the implementation documentation + * + * \pre Given `node` must be a valid node, as returned by rmw_create_node(). + * \pre Given `type_support` must be a valid `rosidl` service type support, as + * returned by ROSIDL_GET_SRV_TYPE_SUPPORT(). + * + * \param[in] node Node with which to register this service server. + * \param[in] type_support Type support of the service to be served. + * \param[in] service_name Name of the service to be served, often a fully qualified + * service name unless `qos_profile` is configured to avoid ROS namespace conventions + * i.e. to create a native service server. + * \param[in] qos_profile QoS policies for this service server's connections. + * \return rmw service handle, or `NULL` if there was an error. */ RMW_PUBLIC RMW_WARN_UNUSED @@ -1824,13 +1907,38 @@ rmw_create_service( const rmw_node_t * node, const rosidl_service_type_support_t * type_support, const char * service_name, - const rmw_qos_profile_t * qos_policies); + const rmw_qos_profile_t * qos_profile); -/// Destroy and unregister the service from this node +/// Destroy and unregister a service server from its node. /** - * \param[in] node The node that owns the service that is being destroyed - * \param[in] service Pointer to the service type created in `rmw_create_service` - * \return RMW_RET_OK if successful, otherwise an appropriate error code + * This function will reclaim all associated resources, including the service server itself. + * Use of a destroyed service server is undefined behavior. + * This function will return early if a logical error, such as `RMW_RET_INVALID_ARGUMENT` + * or `RMW_RET_INCORRECT_RMW_IMPLEMENTATION`, ensues, leaving the given service server unchanged. + * Otherwise, it will proceed despite errors. + * + *
+ * Attribute | Adherence + * ------------------ | ------------- + * Allocates Memory | No + * Thread-Safe | No + * Uses Atomics | Maybe [1] + * Lock-Free | Maybe [1] + * [1] rmw implementation defined, check the implementation documentation + * + * \pre Given `node` must be the one the service server was registered with. + * \pre Given `service` must be a valid service server, as returned by rmw_create_service(). + * + * \param[in] node Node with which the given service server is registered. + * \param[in] service Service server to be destroyed. + * \return `RMW_RET_OK` if successful, or + * \return `RMW_RET_INVALID_ARGUMENT` if `node` is `NULL`, or + * \return `RMW_RET_INVALID_ARGUMENT` if `service` is `NULL`, or + * \return `RMW_RET_INCORRECT_RMW_IMPLEMENTATION` if the `node` + * implementation identifier does not match this implementation, or + * \return `RMW_RET_INCORRECT_RMW_IMPLEMENTATION` if the `service` + * implementation identifier does not match this implementation, or + * \return `RMW_RET_ERROR` if an unspecified error occurs. */ RMW_PUBLIC RMW_WARN_UNUSED From e2f44e84a2b81058d5659213940ca80515ee2a34 Mon Sep 17 00:00:00 2001 From: Michel Hidalgo Date: Tue, 22 Sep 2020 16:07:12 -0300 Subject: [PATCH 2/3] Minor cleanup Signed-off-by: Michel Hidalgo --- rmw/include/rmw/rmw.h | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/rmw/include/rmw/rmw.h b/rmw/include/rmw/rmw.h index 01ee3445..9acc2e70 100644 --- a/rmw/include/rmw/rmw.h +++ b/rmw/include/rmw/rmw.h @@ -1753,8 +1753,8 @@ rmw_return_loaned_message_from_subscription( /** * This function can fail, and therefore return `NULL`, if: * - `node` is `NULL`, or - * - `node` does not belong to this implementation i.e. it does not have - * a matching implementation identifier, or + * - `node` does not belong to this implementation + * i.e. it does not have a matching implementation identifier, or * - `type_support` is `NULL`, or * - `service_name` is `NULL`, or * - `service_name` is an empty string, or @@ -1762,7 +1762,7 @@ rmw_return_loaned_message_from_subscription( * rmw_validate_full_topic_name() definition, or * - `qos_profile` is `NULL`, or * - `qos_profile` has invalid or unknown policies, or - * - memory allocation fails during service server creation, or + * - memory allocation fails during service client creation, or * - an unspecified error occurs. * *
@@ -1866,17 +1866,17 @@ rmw_take_response( /// Create a service server that can reply to client requests. /** * This function can fail, and therefore return `NULL`, if: - * - `node` is `NULL` - * - `node` does not belong to this implementation i.e. it does not have - * a matching implementation identifier - * - `type_support` is `NULL` - * - `service_name` is `NULL` - * - `service_name` is an empty string + * - `node` is `NULL`, or + * - `node` does not belong to this implementation + * i.e. it does not have a matching implementation identifier, or + * - `type_support` is `NULL`, or + * - `service_name` is `NULL`, or + * - `service_name` is an empty string, or * - (if ROS namespace conventions apply) `service_name` is invalid by - * rmw_validate_full_topic_name() definition - * - `qos_profile` is `NULL` - * - `qos_profile` has invalid or unknown policies - * - memory allocation fails during service server creation + * rmw_validate_full_topic_name() definition, or + * - `qos_profile` is `NULL`, or + * - `qos_profile` has invalid or unknown policies, or + * - memory allocation fails during service server creation, or * - an unspecified error occurs * *
From 7a37488e7f19efc941724bc2ceed6d60e20543cf Mon Sep 17 00:00:00 2001 From: Michel Hidalgo Date: Tue, 22 Sep 2020 18:38:22 -0300 Subject: [PATCH 3/3] Rephrase rmw_create_service() and rmw_create_client() briefs Signed-off-by: Michel Hidalgo --- rmw/include/rmw/rmw.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/rmw/include/rmw/rmw.h b/rmw/include/rmw/rmw.h index 9acc2e70..adc179f8 100644 --- a/rmw/include/rmw/rmw.h +++ b/rmw/include/rmw/rmw.h @@ -1749,7 +1749,7 @@ rmw_return_loaned_message_from_subscription( const rmw_subscription_t * subscription, void * loaned_message); -/// Create a service client that can send requests to a service server. +/// Create a service client that can send requests to and receive replies from a service server. /** * This function can fail, and therefore return `NULL`, if: * - `node` is `NULL`, or @@ -1863,7 +1863,7 @@ rmw_take_response( void * ros_response, bool * taken); -/// Create a service server that can reply to client requests. +/// Create a service server that can receive requests from and send replies to a service client. /** * This function can fail, and therefore return `NULL`, if: * - `node` is `NULL`, or