diff --git a/rcl/src/rcl/client.c b/rcl/src/rcl/client.c index 09ec8cd78..b66fd368a 100644 --- a/rcl/src/rcl/client.c +++ b/rcl/src/rcl/client.c @@ -103,7 +103,7 @@ rcl_client_fini(rcl_client_t * client, rcl_node_t * node) RCL_CHECK_ARGUMENT_FOR_NULL(node, RCL_RET_INVALID_ARGUMENT); if (client->impl) { rmw_ret_t ret = - rmw_destroy_client(client->impl->rmw_handle); + rmw_destroy_client(rcl_node_get_rmw_handle(node), client->impl->rmw_handle); if (ret != RMW_RET_OK) { RCL_SET_ERROR_MSG(rmw_get_error_string_safe()); result = RCL_RET_ERROR; diff --git a/rcl/src/rcl/service.c b/rcl/src/rcl/service.c index 739906928..932003eb7 100644 --- a/rcl/src/rcl/service.c +++ b/rcl/src/rcl/service.c @@ -99,7 +99,7 @@ rcl_service_fini(rcl_service_t * service, rcl_node_t * node) RCL_CHECK_ARGUMENT_FOR_NULL(node, RCL_RET_INVALID_ARGUMENT); if (service->impl) { rmw_ret_t ret = - rmw_destroy_service(service->impl->rmw_handle); + rmw_destroy_service(rcl_node_get_rmw_handle(node), service->impl->rmw_handle); if (ret != RMW_RET_OK) { RCL_SET_ERROR_MSG(rmw_get_error_string_safe()); result = RCL_RET_ERROR; diff --git a/rcl/test/CMakeLists.txt b/rcl/test/CMakeLists.txt index ea04bd723..7c774dbd2 100644 --- a/rcl/test/CMakeLists.txt +++ b/rcl/test/CMakeLists.txt @@ -60,8 +60,13 @@ function(test_target_function) AMENT_DEPENDENCIES ${rmw_implementation} ) - # TODO(wjwwood): remove this when the graph API works properly for more than OpenSplice. - if(rmw_implementation STREQUAL "rmw_opensplice_cpp") + # TODO(wjwwood): remove this when the graph API works properly for FastRTPS + if( + rmw_implementation STREQUAL "rmw_connext_dynamic_cpp" OR + rmw_implementation STREQUAL "rmw_fastrtps_cpp" + ) + message(STATUS "Skipping test_graph${target_suffix} test.") + else() rcl_add_custom_gtest(test_graph${target_suffix} SRCS rcl/test_graph.cpp ENV ${extra_test_env} @@ -69,8 +74,6 @@ function(test_target_function) LIBRARIES ${PROJECT_NAME}${target_suffix} ${extra_test_libraries} AMENT_DEPENDENCIES ${rmw_implementation} "example_interfaces" "std_msgs" ) - else() - message(STATUS "Skipping test_graph${target_suffix} test.") endif() rcl_add_custom_gtest(test_rcl${target_suffix} diff --git a/rcl/test/rcl/test_graph.cpp b/rcl/test/rcl/test_graph.cpp index 9d87c32ed..e185da060 100644 --- a/rcl/test/rcl/test_graph.cpp +++ b/rcl/test/rcl/test_graph.cpp @@ -44,6 +44,9 @@ # define CLASSNAME(NAME, SUFFIX) NAME #endif +bool is_connext = + std::string(rmw_get_implementation_identifier()).find("rmw_connext") == 0; + class CLASSNAME (TestGraphFixture, RMW_IMPLEMENTATION) : public ::testing::Test { public: @@ -254,6 +257,7 @@ check_graph_state( ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string_safe(); rcl_reset_error(); + tnat = rcl_get_zero_initialized_topic_names_and_types(); ret = rcl_get_topic_names_and_types(node_ptr, &tnat); ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string_safe(); rcl_reset_error(); @@ -519,9 +523,23 @@ TEST_F(CLASSNAME(TestGraphFixture, RMW_IMPLEMENTATION), test_rcl_service_server_ std::to_string(time_to_sleep.count()).c_str()); ret = rcl_wait(this->wait_set_ptr, time_to_sleep.count()); if (ret == RCL_RET_TIMEOUT) { - continue; + if (!is_connext) { + // TODO(wjwwood): + // Connext has a race condition which can cause the graph guard + // condition to wake up due to the necessary topics going away, + // but afterwards rcl_service_server_is_available() still does + // not reflect that the service is "no longer available". + // The result is that some tests are flaky unless you not only + // check right after a graph change but again in the future where + // rcl_service_server_is_available() eventually reports the + // service is no longer there. This condition can be removed and + // we can always continue when we get RCL_RET_TIMEOUT once that + // is fixed. + continue; + } + } else { + ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string_safe(); } - ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string_safe(); ret = rcl_service_server_is_available(this->node_ptr, &client, &is_available); ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string_safe(); if (is_available == expected_state) {