From eb099bb4eb9a4247891fdf4195cb56a3381adf70 Mon Sep 17 00:00:00 2001 From: Jacob Perron Date: Wed, 10 Feb 2021 17:20:19 -0800 Subject: [PATCH 01/10] Add declaration for function to check QoS profile compatibility Currently, users who are creating a publisher or subscription can receive 'QoS incompatibility' events from the RMW if an incompatible endpoint is discovered. While this is useful, we currently don't have a nice way for application to generally check if two QoS profiles are compatible. For example, it would be nice if tooling could query the communication graph and report any detected QoS incompatibilities. In order to reduce code duplication, I think an API for checking QoS compatibilty should live in a common place. I've opted for `rmw` (over a place like `rcl`) since it's possible QoS compatiblity rules may vary per RMW vendor. Since rules for all DDS implementations should be the same, we could put that common logic in `rmw_dds_common`. Signed-off-by: Jacob Perron --- rmw/include/rmw/qos_profiles.h | 24 ++++++++++++++++++++++++ rmw/include/rmw/rmw.h | 3 +++ 2 files changed, 27 insertions(+) diff --git a/rmw/include/rmw/qos_profiles.h b/rmw/include/rmw/qos_profiles.h index 0e39c6f2..1f9858dd 100644 --- a/rmw/include/rmw/qos_profiles.h +++ b/rmw/include/rmw/qos_profiles.h @@ -113,6 +113,30 @@ static const rmw_qos_profile_t rmw_qos_profile_unknown = false }; +/// Check if two QoS profiles are compatible. +/** + * Two QoS profiles are compatible if the a publisher and subcription + * using their QoS policies can communicate with each other. + * + *
+ * Attribute | Adherence + * ------------------ | ------------- + * Allocates Memory | No + * Thread-Safe | No + * Uses Atomics | No + * Lock-Free | No + * + * \param[in] publisher_profile: The QoS profile used for a publisher. + * \param[in] subscription_profile: The QoS profile used for a subscription. + * \return `true` if the publisher and subscription profiles are compatible, `false` otherwise. + */ +RMW_PUBLIC +RMW_WARN_UNUSED +bool +rmw_qos_profile_are_compatible( + const rmw_qos_profile_t publisher_profile, + const rmw_qos_profile_t subscription_profile); + #ifdef __cplusplus } #endif diff --git a/rmw/include/rmw/rmw.h b/rmw/include/rmw/rmw.h index 08da4eec..c2d55cc3 100644 --- a/rmw/include/rmw/rmw.h +++ b/rmw/include/rmw/rmw.h @@ -40,6 +40,9 @@ * - A function to validate a node's name * - rmw_validate_node_name() * - rmw/validate_node_name.h + * - A function to validate the compatibility of two QoS profiles + * - rmw_qos_profile_are_compatible() + * - rmw/qos_profiles.h * * It also has some machinery that is necessary to wait on and act on these concepts: * From 219217abd1a444cfd808c4c10f478946dc3e05bf Mon Sep 17 00:00:00 2001 From: Jacob Perron Date: Tue, 16 Feb 2021 19:03:05 -0800 Subject: [PATCH 02/10] Refactor API Use enum for output; if one or more policies is set to 'system default', then it's better to warn the caller that we aren't sure if QoS profiles are compatible. Add optional 'reason', and 'reason_size', parameters for outputing a description of what is (or might be) the incompatiblity. Update API docs. Signed-off-by: Jacob Perron --- rmw/include/rmw/qos_profiles.h | 47 ++++++++++++++++++++++++++++------ 1 file changed, 39 insertions(+), 8 deletions(-) diff --git a/rmw/include/rmw/qos_profiles.h b/rmw/include/rmw/qos_profiles.h index 1f9858dd..78d5df68 100644 --- a/rmw/include/rmw/qos_profiles.h +++ b/rmw/include/rmw/qos_profiles.h @@ -113,29 +113,60 @@ static const rmw_qos_profile_t rmw_qos_profile_unknown = false }; +typedef enum RMW_PUBLIC_TYPE rmw_qos_compatibility_type_t +{ + /// QoS policies are compatible + RMW_QOS_COMPATIBILITY_OK = 0, + + /// QoS policies may not be compatible + RMW_QOS_COMPATIBILITY_WARNING, + + /// QoS policies are not compatible + RMW_QOS_COMPATIBILITY_ERROR +} rmw_qos_compatibility_type_t; + + /// Check if two QoS profiles are compatible. /** - * Two QoS profiles are compatible if the a publisher and subcription - * using their QoS policies can communicate with each other. + * Two QoS profiles are compatible if a publisher and subcription + * using the QoS policies can communicate with each other. + * + * If any of the profile policies has the value "system default", then it may not be + * possible to determine the compatibilty. + * In this case, the output parameter `compatibility` is set to `RMW_QOS_COMPATIBILITY_WARNING` + * and `reason` is populated + * + * Profile policies must not have the value "unknown". This is considered an error, and the + * output parameter `compatiblity` is not set. * *
* Attribute | Adherence * ------------------ | ------------- * Allocates Memory | No - * Thread-Safe | No + * Thread-Safe | Yes * Uses Atomics | No - * Lock-Free | No + * Lock-Free | Yes * * \param[in] publisher_profile: The QoS profile used for a publisher. * \param[in] subscription_profile: The QoS profile used for a subscription. - * \return `true` if the publisher and subscription profiles are compatible, `false` otherwise. + * \param[out] compatibility: `RMW_QOS_COMPATIBILITY_OK` if the QoS profiles are compatible, or + * `RMW_QOS_COMPATIBILITY_WARNING` if the QoS profiles might be compatible, or + * `RMW_QOS_COMPATIBILITY_ERROR` if the QoS profiles are not compatible. + * \param[out] reason: A detailed reason for a QoS incompatibility. + * Must be pre-allocated by the caller. This parameter is optional and may be set to `NULL`. + * \param[in] reason_size: Size of the string buffer `reason`, if one is provided. + * \return `RMW_RET_OK` if the check was successful, or + * \return `RMW_RET_INVALID_ARGUMENT` if any of the policies have value "unknown" */ RMW_PUBLIC RMW_WARN_UNUSED -bool -rmw_qos_profile_are_compatible( +rmw_ret_t +rmw_qos_profile_check_compatible( const rmw_qos_profile_t publisher_profile, - const rmw_qos_profile_t subscription_profile); + const rmw_qos_profile_t subscription_profile, + rmw_qos_compatibility_type_t * compatibility, + char * reason, + size_t reason_size); #ifdef __cplusplus } From 6805ea3efe5aa4534ecc4a8aa422afad7b854814 Mon Sep 17 00:00:00 2001 From: Jacob Perron Date: Tue, 16 Feb 2021 19:12:03 -0800 Subject: [PATCH 03/10] Fix docs Signed-off-by: Jacob Perron --- rmw/include/rmw/rmw.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rmw/include/rmw/rmw.h b/rmw/include/rmw/rmw.h index c2d55cc3..d9714eda 100644 --- a/rmw/include/rmw/rmw.h +++ b/rmw/include/rmw/rmw.h @@ -41,7 +41,7 @@ * - rmw_validate_node_name() * - rmw/validate_node_name.h * - A function to validate the compatibility of two QoS profiles - * - rmw_qos_profile_are_compatible() + * - rmw_qos_profile_check_compatible() * - rmw/qos_profiles.h * * It also has some machinery that is necessary to wait on and act on these concepts: From 63cee7dc4875405a25415025a2a329b94d4796b9 Mon Sep 17 00:00:00 2001 From: Jacob Perron Date: Tue, 16 Feb 2021 19:16:34 -0800 Subject: [PATCH 04/10] Refactor docs Signed-off-by: Jacob Perron --- rmw/include/rmw/qos_profiles.h | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/rmw/include/rmw/qos_profiles.h b/rmw/include/rmw/qos_profiles.h index 78d5df68..52f3011c 100644 --- a/rmw/include/rmw/qos_profiles.h +++ b/rmw/include/rmw/qos_profiles.h @@ -136,8 +136,8 @@ typedef enum RMW_PUBLIC_TYPE rmw_qos_compatibility_type_t * In this case, the output parameter `compatibility` is set to `RMW_QOS_COMPATIBILITY_WARNING` * and `reason` is populated * - * Profile policies must not have the value "unknown". This is considered an error, and the - * output parameter `compatiblity` is not set. + * Profile policies must not have the value "unknown". An "unknown" value is considered an error + * and `RMW_RET_INVALID_ARGUMENT` is returned. * *
* Attribute | Adherence @@ -156,7 +156,8 @@ typedef enum RMW_PUBLIC_TYPE rmw_qos_compatibility_type_t * Must be pre-allocated by the caller. This parameter is optional and may be set to `NULL`. * \param[in] reason_size: Size of the string buffer `reason`, if one is provided. * \return `RMW_RET_OK` if the check was successful, or - * \return `RMW_RET_INVALID_ARGUMENT` if any of the policies have value "unknown" + * \return `RMW_RET_INVALID_ARGUMENT` if `compatiblity` is NULL, or + * \return `RMW_RET_INVALID_ARGUMENT` if any of the policies have value "unknown". */ RMW_PUBLIC RMW_WARN_UNUSED From 28a9f5f399b1faeccd0ece5bdffc791daf25a704 Mon Sep 17 00:00:00 2001 From: Jacob Perron Date: Wed, 17 Feb 2021 21:24:31 -0800 Subject: [PATCH 05/10] Improve docs Signed-off-by: Jacob Perron --- rmw/include/rmw/qos_profiles.h | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/rmw/include/rmw/qos_profiles.h b/rmw/include/rmw/qos_profiles.h index 52f3011c..d4effbad 100644 --- a/rmw/include/rmw/qos_profiles.h +++ b/rmw/include/rmw/qos_profiles.h @@ -134,10 +134,21 @@ typedef enum RMW_PUBLIC_TYPE rmw_qos_compatibility_type_t * If any of the profile policies has the value "system default", then it may not be * possible to determine the compatibilty. * In this case, the output parameter `compatibility` is set to `RMW_QOS_COMPATIBILITY_WARNING` - * and `reason` is populated + * and `reason` is populated. * * Profile policies must not have the value "unknown". An "unknown" value is considered an error * and `RMW_RET_INVALID_ARGUMENT` is returned. + * `reason` will be set, identifying the offending policy. + * + * If there is a compatibility warning or error, and a buffer is provided for `reason`, then an + * explanation of all warnings and errors will be populated into the buffer, separated by + * semi-colons (`;`). + * Errors will appear before warnings in the string buffer. + * If the provided buffer is not large enough, this function will still write to the buffer, up to + * the `reason_size` number of characters. + * Therefore, it is possible that not errors and warnings are communicated if the buffer size limit + * is reached. + * A buffer size of 2048 should be more than enough to capture all possible errors and warnings. * *
* Attribute | Adherence From e003ba87dccef4b2aca392079065e9180252ae53 Mon Sep 17 00:00:00 2001 From: Jacob Perron Date: Wed, 17 Feb 2021 21:29:35 -0800 Subject: [PATCH 06/10] Fix docs Signed-off-by: Jacob Perron --- rmw/include/rmw/qos_profiles.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rmw/include/rmw/qos_profiles.h b/rmw/include/rmw/qos_profiles.h index d4effbad..b5029df4 100644 --- a/rmw/include/rmw/qos_profiles.h +++ b/rmw/include/rmw/qos_profiles.h @@ -146,7 +146,7 @@ typedef enum RMW_PUBLIC_TYPE rmw_qos_compatibility_type_t * Errors will appear before warnings in the string buffer. * If the provided buffer is not large enough, this function will still write to the buffer, up to * the `reason_size` number of characters. - * Therefore, it is possible that not errors and warnings are communicated if the buffer size limit + * Therefore, it is possible that not all errors and warnings are communicated if the buffer size limit * is reached. * A buffer size of 2048 should be more than enough to capture all possible errors and warnings. * From 041f02c1d7d3d7e9cbf4e779a3fd89e3b5c14b42 Mon Sep 17 00:00:00 2001 From: Jacob Perron Date: Thu, 18 Feb 2021 14:51:14 -0800 Subject: [PATCH 07/10] Do not set output parameters if there is an error This behaves more like other RMW functions. Signed-off-by: Jacob Perron --- rmw/include/rmw/qos_profiles.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/rmw/include/rmw/qos_profiles.h b/rmw/include/rmw/qos_profiles.h index b5029df4..c5afcb0a 100644 --- a/rmw/include/rmw/qos_profiles.h +++ b/rmw/include/rmw/qos_profiles.h @@ -136,9 +136,9 @@ typedef enum RMW_PUBLIC_TYPE rmw_qos_compatibility_type_t * In this case, the output parameter `compatibility` is set to `RMW_QOS_COMPATIBILITY_WARNING` * and `reason` is populated. * - * Profile policies must not have the value "unknown". An "unknown" value is considered an error - * and `RMW_RET_INVALID_ARGUMENT` is returned. - * `reason` will be set, identifying the offending policy. + * Profile policies must not have the value "unknown". + * An "unknown" value is considered an error and `RMW_RET_INVALID_ARGUMENT` is returned. + * `compatibility` and `reason` will not be set. * * If there is a compatibility warning or error, and a buffer is provided for `reason`, then an * explanation of all warnings and errors will be populated into the buffer, separated by From 25ab9732cf346dd4c10cea2da550a616c7188703 Mon Sep 17 00:00:00 2001 From: Jacob Perron Date: Thu, 18 Feb 2021 15:13:40 -0800 Subject: [PATCH 08/10] Account for possible RMW_RET_ERROR return value Signed-off-by: Jacob Perron --- rmw/include/rmw/qos_profiles.h | 1 + 1 file changed, 1 insertion(+) diff --git a/rmw/include/rmw/qos_profiles.h b/rmw/include/rmw/qos_profiles.h index c5afcb0a..41bebcfe 100644 --- a/rmw/include/rmw/qos_profiles.h +++ b/rmw/include/rmw/qos_profiles.h @@ -169,6 +169,7 @@ typedef enum RMW_PUBLIC_TYPE rmw_qos_compatibility_type_t * \return `RMW_RET_OK` if the check was successful, or * \return `RMW_RET_INVALID_ARGUMENT` if `compatiblity` is NULL, or * \return `RMW_RET_INVALID_ARGUMENT` if any of the policies have value "unknown". + * \return `RMW_RET_ERROR` if there is an unexpected error. */ RMW_PUBLIC RMW_WARN_UNUSED From c21f5cc0b7705f6f414f9e2f83b64410c462a54e Mon Sep 17 00:00:00 2001 From: Jacob Perron Date: Fri, 19 Feb 2021 16:15:48 -0800 Subject: [PATCH 09/10] Improve docs Signed-off-by: Jacob Perron --- rmw/include/rmw/qos_profiles.h | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/rmw/include/rmw/qos_profiles.h b/rmw/include/rmw/qos_profiles.h index 41bebcfe..1bbf754f 100644 --- a/rmw/include/rmw/qos_profiles.h +++ b/rmw/include/rmw/qos_profiles.h @@ -163,11 +163,15 @@ typedef enum RMW_PUBLIC_TYPE rmw_qos_compatibility_type_t * \param[out] compatibility: `RMW_QOS_COMPATIBILITY_OK` if the QoS profiles are compatible, or * `RMW_QOS_COMPATIBILITY_WARNING` if the QoS profiles might be compatible, or * `RMW_QOS_COMPATIBILITY_ERROR` if the QoS profiles are not compatible. - * \param[out] reason: A detailed reason for a QoS incompatibility. - * Must be pre-allocated by the caller. This parameter is optional and may be set to `NULL`. + * \param[out] reason: A detailed reason for a QoS incompatibility or potential incompatibility. + * Must be pre-allocated by the caller. + * This parameter is optional and may be set to `NULL` if the reason information is not + * desired. * \param[in] reason_size: Size of the string buffer `reason`, if one is provided. + * If `reason` is `nullptr`, then this parameter must be zero. * \return `RMW_RET_OK` if the check was successful, or - * \return `RMW_RET_INVALID_ARGUMENT` if `compatiblity` is NULL, or + * \return `RMW_RET_INVALID_ARGUMENT` if `compatiblity` is `nullptr`, or + * \return `RMW_RET_INVALID_ARGUMENT` if `reason` is `NULL` and `reason_size` is not zero, or * \return `RMW_RET_INVALID_ARGUMENT` if any of the policies have value "unknown". * \return `RMW_RET_ERROR` if there is an unexpected error. */ From 93e9aa2399d91cd50ad9cc71ef9f8a36e4bb5fe9 Mon Sep 17 00:00:00 2001 From: Jacob Perron Date: Mon, 22 Feb 2021 13:58:54 -0800 Subject: [PATCH 10/10] Warn on 'unknown' value Since it's possible for rmw's to return an unknown value if the policy is set to an unsupported value by ROS 2. Signed-off-by: Jacob Perron --- rmw/include/rmw/qos_profiles.h | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/rmw/include/rmw/qos_profiles.h b/rmw/include/rmw/qos_profiles.h index 1bbf754f..be5c26c8 100644 --- a/rmw/include/rmw/qos_profiles.h +++ b/rmw/include/rmw/qos_profiles.h @@ -131,15 +131,11 @@ typedef enum RMW_PUBLIC_TYPE rmw_qos_compatibility_type_t * Two QoS profiles are compatible if a publisher and subcription * using the QoS policies can communicate with each other. * - * If any of the profile policies has the value "system default", then it may not be + * If any of the profile policies has the value "system default" or "unknown", then it may not be * possible to determine the compatibilty. * In this case, the output parameter `compatibility` is set to `RMW_QOS_COMPATIBILITY_WARNING` * and `reason` is populated. * - * Profile policies must not have the value "unknown". - * An "unknown" value is considered an error and `RMW_RET_INVALID_ARGUMENT` is returned. - * `compatibility` and `reason` will not be set. - * * If there is a compatibility warning or error, and a buffer is provided for `reason`, then an * explanation of all warnings and errors will be populated into the buffer, separated by * semi-colons (`;`). @@ -172,7 +168,6 @@ typedef enum RMW_PUBLIC_TYPE rmw_qos_compatibility_type_t * \return `RMW_RET_OK` if the check was successful, or * \return `RMW_RET_INVALID_ARGUMENT` if `compatiblity` is `nullptr`, or * \return `RMW_RET_INVALID_ARGUMENT` if `reason` is `NULL` and `reason_size` is not zero, or - * \return `RMW_RET_INVALID_ARGUMENT` if any of the policies have value "unknown". * \return `RMW_RET_ERROR` if there is an unexpected error. */ RMW_PUBLIC