-
Notifications
You must be signed in to change notification settings - Fork 23
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
Add function for checking QoS profile compatibility #45
Changes from 10 commits
17df0fb
f549bae
cc81b3f
132b3e1
1a156da
e7d4b0a
9c43a6e
70dc190
ff1ece3
55e56f3
26f2e63
530c42f
6c789b4
6a67320
a8862ee
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,58 @@ | ||
// Copyright 2021 Open Source Robotics Foundation, Inc. | ||
// | ||
// Licensed under the Apache License, Version 2.0 (the "License"); | ||
// you may not use this file except in compliance with the License. | ||
// You may obtain a copy of the License at | ||
// | ||
// http://www.apache.org/licenses/LICENSE-2.0 | ||
// | ||
// Unless required by applicable law or agreed to in writing, software | ||
// distributed under the License is distributed on an "AS IS" BASIS, | ||
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
// See the License for the specific language governing permissions and | ||
// limitations under the License. | ||
|
||
#ifndef RMW_DDS_COMMON__QOS_HPP_ | ||
#define RMW_DDS_COMMON__QOS_HPP_ | ||
|
||
#include "rmw/qos_profiles.h" | ||
#include "rmw/types.h" | ||
|
||
#include "rmw_dds_common/visibility_control.h" | ||
|
||
namespace rmw_dds_common | ||
{ | ||
|
||
/// Check if two QoS profiles are compatible | ||
/** | ||
* Two QoS profiles are compatible if a publisher and subcription | ||
* using the QoS policies can communicate with each other. | ||
* | ||
* This implements the rmw API \ref rmw_qos_profile_check_compatible(). | ||
* See \ref rmw_qos_profile_check_compatible() for more information. | ||
* | ||
* \param[in] publisher_profile: The QoS profile used for a publisher. | ||
* \param[in] subscription_profile: The QoS profile used for a subscription. | ||
* \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. | ||
jacobperron marked this conversation as resolved.
Show resolved
Hide resolved
|
||
* \param[out] reason: A detailed reason for a QoS incompatibility. | ||
jacobperron marked this conversation as resolved.
Show resolved
Hide resolved
|
||
* Must be pre-allocated by the caller. This parameter is optional and may be set to `NULL`. | ||
jacobperron marked this conversation as resolved.
Show resolved
Hide resolved
|
||
* \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 `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_DDS_COMMON_PUBLIC | ||
rmw_ret_t | ||
qos_profile_check_compatible( | ||
const rmw_qos_profile_t publisher_qos, | ||
const rmw_qos_profile_t subscription_qos, | ||
rmw_qos_compatibility_type_t * compatibility, | ||
char * reason, | ||
size_t reason_size); | ||
|
||
} // namespace rmw_dds_common | ||
|
||
#endif // RMW_DDS_COMMON__QOS_HPP_ |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,308 @@ | ||
// Copyright 2021 Open Source Robotics Foundation, Inc. | ||
// | ||
// Licensed under the Apache License, Version 2.0 (the "License"); | ||
// you may not use this file except in compliance with the License. | ||
// You may obtain a copy of the License at | ||
// | ||
// http://www.apache.org/licenses/LICENSE-2.0 | ||
// | ||
// Unless required by applicable law or agreed to in writing, software | ||
// distributed under the License is distributed on an "AS IS" BASIS, | ||
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
// See the License for the specific language governing permissions and | ||
// limitations under the License. | ||
|
||
#include "rmw_dds_common/qos.hpp" | ||
|
||
#include <cstring> | ||
|
||
#include "rcutils/snprintf.h" | ||
#include "rmw/error_handling.h" | ||
#include "rmw/qos_profiles.h" | ||
|
||
namespace rmw_dds_common | ||
{ | ||
|
||
static bool | ||
jacobperron marked this conversation as resolved.
Show resolved
Hide resolved
|
||
operator==(rmw_time_t t1, rmw_time_t t2) | ||
{ | ||
return t1.sec == t2.sec && t1.nsec == t2.nsec; | ||
} | ||
|
||
static bool | ||
operator!=(rmw_time_t t1, rmw_time_t t2) | ||
{ | ||
return !(t1 == t2); | ||
} | ||
|
||
static bool | ||
operator<(rmw_time_t t1, rmw_time_t t2) | ||
{ | ||
if (t1.sec < t2.sec) { | ||
return true; | ||
} else if (t1.sec == t2.sec && t1.nsec < t2.nsec) { | ||
return true; | ||
} | ||
return false; | ||
} | ||
|
||
// Returns RMW_RET_OK if successful or no buffer was provided | ||
// Returns RMW_RET_ERROR if there as an error copying the message to the buffer | ||
static rmw_ret_t | ||
_append_to_buffer(const char * message, char * buffer, size_t buffer_size) | ||
{ | ||
// Only write if a buffer is provided | ||
if (!buffer || buffer_size == 0u) { | ||
return RMW_RET_OK; | ||
} | ||
// Determine available space left in buffer | ||
size_t offset = strnlen(buffer, buffer_size); | ||
size_t write_size = buffer_size - offset; | ||
int snprintf_ret = rcutils_snprintf(buffer + offset, write_size, "%s", message); | ||
if (snprintf_ret < 0) { | ||
RMW_SET_ERROR_MSG("failed to append to character buffer"); | ||
return RMW_RET_ERROR; | ||
} | ||
return RMW_RET_OK; | ||
} | ||
|
||
rmw_ret_t | ||
qos_profile_check_compatible( | ||
const rmw_qos_profile_t publisher_qos, | ||
const rmw_qos_profile_t subscription_qos, | ||
rmw_qos_compatibility_type_t * compatibility, | ||
char * reason, | ||
size_t reason_size) | ||
jacobperron marked this conversation as resolved.
Show resolved
Hide resolved
|
||
{ | ||
if (!compatibility) { | ||
RMW_SET_ERROR_MSG("compatibility parameter is null"); | ||
return RMW_RET_INVALID_ARGUMENT; | ||
} | ||
|
||
if (!reason && reason_size != 0u) { | ||
RMW_SET_ERROR_MSG("reason parameter is null, but reason_size parameter is not zero"); | ||
return RMW_RET_INVALID_ARGUMENT; | ||
} | ||
|
||
// If there are any "unknown" values, then there is an error | ||
if (RMW_QOS_POLICY_RELIABILITY_UNKNOWN == publisher_qos.reliability) { | ||
RMW_SET_ERROR_MSG("Publisher reliability is unknown"); | ||
return RMW_RET_INVALID_ARGUMENT; | ||
} | ||
if (RMW_QOS_POLICY_RELIABILITY_UNKNOWN == subscription_qos.reliability) { | ||
RMW_SET_ERROR_MSG("Subscription reliability is unknown"); | ||
return RMW_RET_INVALID_ARGUMENT; | ||
} | ||
if (RMW_QOS_POLICY_DURABILITY_UNKNOWN == publisher_qos.durability) { | ||
RMW_SET_ERROR_MSG("Publisher durability is unknown"); | ||
return RMW_RET_INVALID_ARGUMENT; | ||
} | ||
if (RMW_QOS_POLICY_DURABILITY_UNKNOWN == subscription_qos.durability) { | ||
RMW_SET_ERROR_MSG("Subscription durability is unknown"); | ||
return RMW_RET_INVALID_ARGUMENT; | ||
} | ||
if (RMW_QOS_POLICY_LIVELINESS_UNKNOWN == publisher_qos.liveliness) { | ||
RMW_SET_ERROR_MSG("Publisher liveliness is unknown"); | ||
return RMW_RET_INVALID_ARGUMENT; | ||
} | ||
if (RMW_QOS_POLICY_LIVELINESS_UNKNOWN == subscription_qos.liveliness) { | ||
RMW_SET_ERROR_MSG("Subscription liveliness is unknown"); | ||
return RMW_RET_INVALID_ARGUMENT; | ||
} | ||
|
||
// Presume profiles are compatible until proven otherwise | ||
*compatibility = RMW_QOS_COMPATIBILITY_OK; | ||
|
||
// Best effort publisher and reliable subscription | ||
if (publisher_qos.reliability == RMW_QOS_POLICY_RELIABILITY_BEST_EFFORT && | ||
subscription_qos.reliability == RMW_QOS_POLICY_RELIABILITY_RELIABLE) | ||
{ | ||
*compatibility = RMW_QOS_COMPATIBILITY_ERROR; | ||
rmw_ret_t append_ret = _append_to_buffer( | ||
"ERROR: Best effort publisher and reliable subscription;", | ||
reason, | ||
reason_size); | ||
if (RMW_RET_OK != append_ret) { | ||
return append_ret; | ||
} | ||
} | ||
|
||
// Volatile publisher and transient local subscription | ||
if (publisher_qos.durability == RMW_QOS_POLICY_DURABILITY_VOLATILE && | ||
subscription_qos.durability == RMW_QOS_POLICY_DURABILITY_TRANSIENT_LOCAL) | ||
{ | ||
*compatibility = RMW_QOS_COMPATIBILITY_ERROR; | ||
rmw_ret_t append_ret = _append_to_buffer( | ||
"ERROR: Volatile publisher and transient local subscription;", | ||
reason, | ||
reason_size); | ||
if (RMW_RET_OK != append_ret) { | ||
return append_ret; | ||
} | ||
} | ||
|
||
const rmw_time_t & pub_deadline = publisher_qos.deadline; | ||
const rmw_time_t & sub_deadline = subscription_qos.deadline; | ||
const rmw_time_t deadline_default = RMW_QOS_DEADLINE_DEFAULT; | ||
|
||
// No deadline for publisher and deadline for subscription | ||
if (pub_deadline == deadline_default && sub_deadline != deadline_default) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The policies in ROS have this annoying If that interpretation is correct (which it may not be, given that the If you then define the comparison functions such that ∞ is handled correctly, this all becomes much simpler, IMHO. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Yes, that should be the case.
But the "system default" could be different from infinite, e.g. if you specify in a qos profile file a default profile with a different deadline value (which would be weird, but possible). There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Actually, this new function is failing when a policy has value "UNKNOWN", which is used when an entity has a qos value valid for the underlying middleware that hasn't an equivalent in ROS 2. I'm not sure why this function should return a compatibility warning when "SYSTEM_DEFAULT" and an error when "UNKNOWN". @jacobperron what do you think? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I wasn't sure if
Unfortunately, all of the widely used profiles defined in rmw_qos_profile_check_compatible( rmw_qos_profile_sensor_data, rmw_qos_profile_sensor_data, ...); Although, producing a warning isn't much better... Maybe we should change the default values for those profiles 😅
I don't know if this applies here. This is a common implementation for DDS that presumes the RMW supports all the ROS 2 QoS settings (we could document it here to be explicit). If an RMW doesn't support all of the QoS policies, then they should probably provide their own implementation of this function, especially if the RMW is not a DDS implementation. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What is this function for? I imagine it being used in two ways:
In case 1, "SYSTEM_DEFAULT" is impossible, and "UNKNOWN" is a possible value.
Yes, and in that case you can still get "UNKNOWN".
Same applies to any application mixing native DDS with ROS 2. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @ivanpauno Yeah, those are the two uses of this function that I had in mind. And the first one is the primary motivation for this API. AFAIK, tools that want to introspect the ROS graph (e.g. ros2doctor and rqt_graph) cannot get "QoS incompatible" events for all nodes in the graph. This API let's them warn about (possible) compatibility issues. I guess it is technically possible to get a "SYSTEM_DEFAULT" back from There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Ah yes, that's true.
Using a warning for both system default and unknown sounds good. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Warning on "unknown" values: 6c789b4 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I think that There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. To circle back to the original comment, I think I'll leave the current implementation as-is since it seems to agree with the RMW API docs and https://index.ros.org/doc/ros2/Concepts/About-Quality-of-Service-Settings/#qos-compatibilities. While endpoints may not return "SYSTEM_DEFAULT" values, it's possible that other use-cases not involving endpoint queries will call this function. So I think it's nice to warn about "SYSTEM_DEFAULT" values, instead of returning an error with no info about other possible incompatibilities. |
||
*compatibility = RMW_QOS_COMPATIBILITY_ERROR; | ||
rmw_ret_t ret = _append_to_buffer( | ||
"ERROR: Subscription has a deadline, but publisher does not;", | ||
reason, | ||
reason_size); | ||
if (RMW_RET_OK != ret) { | ||
return ret; | ||
} | ||
} | ||
|
||
// Subscription deadline is less than publisher deadline | ||
if (pub_deadline != deadline_default && sub_deadline != deadline_default) { | ||
if (sub_deadline < pub_deadline) { | ||
*compatibility = RMW_QOS_COMPATIBILITY_ERROR; | ||
rmw_ret_t append_ret = _append_to_buffer( | ||
"ERROR: Subscription deadline is less than publisher deadline;", | ||
reason, | ||
reason_size); | ||
if (RMW_RET_OK != append_ret) { | ||
return append_ret; | ||
} | ||
} | ||
} | ||
|
||
// Automatic liveliness for publisher and manual by topic for subscription | ||
if (publisher_qos.liveliness == RMW_QOS_POLICY_LIVELINESS_AUTOMATIC && | ||
subscription_qos.liveliness == RMW_QOS_POLICY_LIVELINESS_MANUAL_BY_TOPIC) | ||
{ | ||
*compatibility = RMW_QOS_COMPATIBILITY_ERROR; | ||
rmw_ret_t append_ret = _append_to_buffer( | ||
"ERROR: Publisher's liveliness is automatic and subscription's is manual by topic;", | ||
reason, | ||
reason_size); | ||
if (RMW_RET_OK != append_ret) { | ||
return append_ret; | ||
} | ||
} | ||
|
||
const rmw_time_t & pub_lease = publisher_qos.liveliness_lease_duration; | ||
const rmw_time_t & sub_lease = subscription_qos.liveliness_lease_duration; | ||
const rmw_time_t lease_default = RMW_QOS_LIVELINESS_LEASE_DURATION_DEFAULT; | ||
|
||
// No lease duration for publisher and lease duration for subscription | ||
if (pub_lease == lease_default && sub_lease != lease_default) { | ||
ivanpauno marked this conversation as resolved.
Show resolved
Hide resolved
|
||
*compatibility = RMW_QOS_COMPATIBILITY_ERROR; | ||
rmw_ret_t append_ret = _append_to_buffer( | ||
"ERROR: Subscription has a liveliness lease duration, but publisher does not;", | ||
reason, | ||
reason_size); | ||
if (RMW_RET_OK != append_ret) { | ||
return append_ret; | ||
} | ||
} | ||
|
||
// Subscription lease duration is less than publisher lease duration | ||
if (pub_lease != lease_default && sub_lease != lease_default) { | ||
if (sub_lease < pub_lease) { | ||
*compatibility = RMW_QOS_COMPATIBILITY_ERROR; | ||
rmw_ret_t append_ret = _append_to_buffer( | ||
"ERROR: Subscription liveliness lease duration is less than publisher;", | ||
reason, | ||
reason_size); | ||
if (RMW_RET_OK != append_ret) { | ||
return append_ret; | ||
} | ||
} | ||
} | ||
|
||
// Only check for warnings if there are no errors | ||
if (RMW_QOS_COMPATIBILITY_OK == *compatibility) { | ||
// Reliability for publisher is "system default" and subscription is reliable | ||
if (publisher_qos.reliability == RMW_QOS_POLICY_RELIABILITY_SYSTEM_DEFAULT && | ||
subscription_qos.reliability == RMW_QOS_POLICY_RELIABILITY_RELIABLE) | ||
{ | ||
*compatibility = RMW_QOS_COMPATIBILITY_WARNING; | ||
rmw_ret_t append_ret = _append_to_buffer( | ||
"WARNING: Reliable subscription, but publisher is system default;", | ||
reason, | ||
reason_size); | ||
if (RMW_RET_OK != append_ret) { | ||
return append_ret; | ||
} | ||
} else { | ||
// Reliability for publisher is best effort and subscription is "system default" | ||
if (publisher_qos.reliability == RMW_QOS_POLICY_RELIABILITY_BEST_EFFORT && | ||
subscription_qos.reliability == RMW_QOS_POLICY_RELIABILITY_SYSTEM_DEFAULT) | ||
{ | ||
*compatibility = RMW_QOS_COMPATIBILITY_WARNING; | ||
rmw_ret_t append_ret = _append_to_buffer( | ||
"WARNING: Best effort publisher, but subscription is system default;", | ||
reason, | ||
reason_size); | ||
if (RMW_RET_OK != append_ret) { | ||
return append_ret; | ||
} | ||
} | ||
} | ||
|
||
// Durability for publisher is "system default" and subscription is transient local | ||
if (publisher_qos.durability == RMW_QOS_POLICY_DURABILITY_SYSTEM_DEFAULT && | ||
subscription_qos.durability == RMW_QOS_POLICY_DURABILITY_TRANSIENT_LOCAL) | ||
{ | ||
*compatibility = RMW_QOS_COMPATIBILITY_WARNING; | ||
rmw_ret_t ret = _append_to_buffer( | ||
"WARNING: Transient local subscription, but publisher is system default;", | ||
reason, | ||
reason_size); | ||
if (RMW_RET_OK != ret) { | ||
return ret; | ||
} | ||
} else { | ||
// Durability for publisher is volatile and subscription is "system default" | ||
if (publisher_qos.durability == RMW_QOS_POLICY_DURABILITY_VOLATILE && | ||
subscription_qos.durability == RMW_QOS_POLICY_DURABILITY_SYSTEM_DEFAULT) | ||
{ | ||
*compatibility = RMW_QOS_COMPATIBILITY_WARNING; | ||
rmw_ret_t ret = _append_to_buffer( | ||
"WARNING: Volatile publisher, but subscription is system default;", | ||
reason, | ||
reason_size); | ||
if (RMW_RET_OK != ret) { | ||
return ret; | ||
} | ||
} | ||
} | ||
|
||
// Automatic liveliness for publisher and "system default" for subscription | ||
if (publisher_qos.liveliness == RMW_QOS_POLICY_LIVELINESS_AUTOMATIC && | ||
subscription_qos.liveliness == RMW_QOS_POLICY_LIVELINESS_SYSTEM_DEFAULT) | ||
{ | ||
*compatibility = RMW_QOS_COMPATIBILITY_WARNING; | ||
rmw_ret_t ret = _append_to_buffer( | ||
"WARNING: Publisher's liveliness is automatic, but subscription's is system default;", | ||
reason, | ||
reason_size); | ||
if (RMW_RET_OK != ret) { | ||
return ret; | ||
} | ||
} else { | ||
if (publisher_qos.liveliness == RMW_QOS_POLICY_LIVELINESS_SYSTEM_DEFAULT && | ||
subscription_qos.liveliness == RMW_QOS_POLICY_LIVELINESS_MANUAL_BY_TOPIC) | ||
jacobperron marked this conversation as resolved.
Show resolved
Hide resolved
|
||
{ | ||
*compatibility = RMW_QOS_COMPATIBILITY_WARNING; | ||
rmw_ret_t ret = _append_to_buffer( | ||
"WARNING: Subscription's liveliness is manual by topic, but publisher's is system " | ||
"default;", | ||
reason, | ||
reason_size); | ||
if (RMW_RET_OK != ret) { | ||
return ret; | ||
} | ||
} | ||
} | ||
} | ||
|
||
return RMW_RET_OK; | ||
} | ||
|
||
} // namespace rmw_dds_common |
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.
Just for my edification: I've always wondered about the inconsistent terminology, IMO: "publisher" and subscription" versus, "publisher" and "subscriber" or "publication" and "subscription." Any quick background info on that?
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.
TBH, I can't recall why that terminology. I'm just following suit with the terminology used throughout the rest of the ROS 2 stack, though I think we're also inconsistent about what terminology we're using 🙃 The tutorials seem to use "subscriber" instead of "subscription".
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.
@wjwwood Probably has some insight.
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.
A publisher publishes, a subscription is a receipt of a subscription, it isn't a subscriber. So subscriber is the wrong term. A node is kind of a subscriber as it is the thing that subscribes. A publication could be a thing, but because our object publishes, it is a publisher.
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.
@wjwwood Interesting. I guess I just think of it more like this (probably attracted to the symmetry/uniformity):
Anyways, thanks for the info. It helps to fill in some of the blanks.