Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Add support for rmw_connextdds #895

Merged
merged 4 commits into from
Mar 11, 2021
Merged

Add support for rmw_connextdds #895

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
24afd46
Reset arguments to rcl_get_node_names() on each test iteration.
asorbini Jan 28, 2021
d7ecefb
Replace rmw_connext_cpp with rmw_connextdds
asorbini Mar 5, 2021
33595be
Remove references to OpenSplice
asorbini Mar 8, 2021
eca1b0c
Restore exceptions for ros2/rmw_connext to ease transition to rticomm…
asorbini Mar 9, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 11 additions & 9 deletions rcl/test/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,10 +73,9 @@ function(test_target_function)
AMENT_DEPENDENCIES ${rmw_implementation}
)

# TODO(hidmic): re-enable timer tests against RTI Connext once
# https://github.com/ros2/rcl/issues/687 is resolved
# TODO(asorbini): Remove these exceptions once ros2/rmw_connext is deprecated.
set(AMENT_GTEST_ARGS "")
if(rmw_implementation STREQUAL "rmw_connext_cpp")
if(rmw_implementation MATCHES "rmw_connext(.*)_cpp")
message(STATUS "Skipping test_timer${target_suffix} test.")
set(AMENT_GTEST_ARGS "SKIP_TEST")
endif()
Expand Down Expand Up @@ -122,16 +121,16 @@ function(test_target_function)
AMENT_DEPENDENCIES ${rmw_implementation} "osrf_testing_tools_cpp"
)

# TODO(asorbini): Remove these exceptions once ros2/rmw_connext is deprecated.
set(AMENT_GTEST_ARGS "")
# TODO(wjwwood): remove this when the graph API works properly for connext dynamic
if(rmw_implementation STREQUAL "rmw_connext_dynamic_cpp")
message(STATUS "Skipping test_graph${target_suffix} test.")
set(AMENT_GTEST_ARGS "SKIP_TEST")
# TODO(mm318): why rmw_connext tests run much slower than rmw_fastrtps and rmw_opensplice tests
elseif(rmw_implementation STREQUAL "rmw_connext_cpp")
message(STATUS "Increasing test_graph${target_suffix} test timeout.")
set(AMENT_GTEST_ARGS TIMEOUT 180)
endif()

rcl_add_custom_gtest(test_graph${target_suffix}
SRCS rcl/test_graph.cpp
ENV ${rmw_implementation_env_var}
Expand All @@ -142,19 +141,19 @@ function(test_target_function)
${AMENT_GTEST_ARGS}
)

# TODO(asorbini): Remove these exceptions once ros2/rmw_connext is deprecated.
set(AMENT_GTEST_ARGS "")
# TODO(mm318): why rmw_connext tests run much slower than rmw_fastrtps and rmw_opensplice tests
if(rmw_implementation STREQUAL "rmw_connext_cpp")
if(rmw_implementation MATCHES "rmw_connext(.*)_cpp")
message(STATUS "Increasing test_info_by_topic${target_suffix} test timeout.")
set(AMENT_GTEST_ARGS TIMEOUT 120)
endif()

rcl_add_custom_gtest(test_info_by_topic${target_suffix}
SRCS rcl/test_info_by_topic.cpp rcl/wait_for_entity_helpers.cpp
ENV ${rmw_implementation_env_var}
APPEND_LIBRARY_DIRS ${extra_lib_dirs}
LIBRARIES ${PROJECT_NAME}
AMENT_DEPENDENCIES ${rmw_implementation} "osrf_testing_tools_cpp" "test_msgs"
${AMENT_GTEST_ARGS}
)

rcl_add_custom_gtest(test_count_matched${target_suffix}
Expand Down Expand Up @@ -254,8 +253,11 @@ function(test_target_function)
LIBRARIES ${PROJECT_NAME} mimick
AMENT_DEPENDENCIES ${rmw_implementation} "osrf_testing_tools_cpp" "test_msgs"
)
# TODO(asorbini) Enable message timestamp tests for rmw_connextdds on Windows
# once clock incompatibilities are resolved.
if(rmw_implementation STREQUAL "rmw_fastrtps_cpp" OR
rmw_implementation STREQUAL "rmw_fastrtps_dynamic_cpp")
rmw_implementation STREQUAL "rmw_fastrtps_dynamic_cpp" OR
(rmw_implementation STREQUAL "rmw_connextdds" AND NOT WIN32))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My major question here is why this is different with rmw_connextdds_cpp, when it seemingly worked before with rmw_connext_cpp. Do we have a good idea?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the rmw_connext_cpp were being completely skipped, now they aren't skipped except than on Windows.
i.e.: it looks better now 😄

message(STATUS "Enabling message timestamp test for ${rmw_implementation}")
target_compile_definitions(test_subscription${target_suffix}
PUBLIC "RMW_TIMESTAMPS_SUPPORTED=1" "RMW_RECEIVED_TIMESTAMP_SUPPORTED=1")
Expand Down
1 change: 0 additions & 1 deletion rcl/test/rcl/test_events.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -606,7 +606,6 @@ TEST_F(TestEventFixture, test_pubsub_liveliness_kill_pub)

/*
* Basic test of publisher and subscriber incompatible qos callback events.
* Only implemented in opensplice at the moment.
*/
TEST_P(TestEventFixture, test_pubsub_incompatible_qos)
{
Expand Down
4 changes: 2 additions & 2 deletions rcl/test/rcl/test_get_actual_qos.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -430,9 +430,9 @@ get_parameters(bool for_publisher)
});
}
} else {
// TODO(asorbini): Remove this block once ros2/rmw_connext is deprecated.
if (rmw_implementation_str == "rmw_connext_cpp" ||
rmw_implementation_str == "rmw_connext_dynamic_cpp" ||
rmw_implementation_str == "rmw_opensplice_cpp")
rmw_implementation_str == "rmw_connext_dynamic_cpp")
{
/*
* Test with non-default settings.
Expand Down
3 changes: 0 additions & 3 deletions rcl/test/rcl/test_graph.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,9 +51,6 @@
bool is_connext =
std::string(rmw_get_implementation_identifier()).find("rmw_connext") == 0;

bool is_opensplice =
std::string(rmw_get_implementation_identifier()).find("rmw_opensplice") == 0;

class CLASSNAME (TestGraphFixture, RMW_IMPLEMENTATION) : public ::testing::Test
{
public:
Expand Down
8 changes: 8 additions & 0 deletions rcl_action/test/rcl_action/test_graph.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -358,6 +358,14 @@ class TestActionGraphMultiNodeFixture : public CLASSNAME(TestActionGraphFixture,
ret = rcl_get_node_names(&this->remote_node, allocator, &node_names, &node_namespaces);
++attempts;
ASSERT_LE(attempts, max_attempts) << "Unable to attain all required nodes";
if (node_names.size < 3u) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice catch 👍

ret = rcutils_string_array_fini(&node_names);
ASSERT_EQ(RCUTILS_RET_OK, ret);
ret = rcutils_string_array_fini(&node_namespaces);
ASSERT_EQ(RCUTILS_RET_OK, ret);
node_names = rcutils_get_zero_initialized_string_array();
node_namespaces = rcutils_get_zero_initialized_string_array();
}
}
}

Expand Down