support local graph changes in Connext (#67)

* pass node to client and service destroy funcs

* address a flaky test with connext, will ticket

* invert test_graph exclusion logic

* use zero initialization of tnat in test_graph

* change is_connext logic

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;

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;

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.
+  # TODO(wjwwood): remove this when the graph API works properly for FastRTPS
-  if(rmw_implementation STREQUAL "rmw_opensplice_cpp")
+  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}

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) {