-
Notifications
You must be signed in to change notification settings - Fork 48
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 #180
Conversation
Connected to ros2/rmw#299 Signed-off-by: Jacob Perron <jacob@openrobotics.org>
ret = rmw_qos_profile_check_compatible( | ||
rmw_qos_profile_system_default, rmw_qos_profile_unknown, &compatible, nullptr, 0u); | ||
EXPECT_EQ(ret, RMW_RET_INVALID_ARGUMENT); | ||
} |
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.
I assume the purpose of the tests here are to ensure that the function is vectored properly to its implementation and that the full test suite for QoS compatibility is over in 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.
Yes. These tests also are run for every RMW. This is the reason that the tests are not as comprehensive as the tests in rmw_dds_common
. I don't think we can make many assumptions about QoS compatibility if we consider a non-DDS RMW.
Unknown values no longer cause errors, but possibly warnings instead. We can't test this since it is dependent on the RMW. We can test for compatibility with no default or unknown values as well as invalid input. Signed-off-by: Jacob Perron <jacob@openrobotics.org>
After a discussion in ros2/rmw_dds_common#45 (comment), I had to refactor the tests (e4cf875 |
@ros-pull-request-builder retest this please |
2 similar comments
@ros-pull-request-builder retest this please |
@ros-pull-request-builder retest this please |
Connected to ros2/rmw#299