Cleanup code

Signed-off-by: Emerson Knapp <emerson.b.knapp@gmail.com>

rcl/include/rcl/node.h

@@ -618,7 +618,9 @@ rcl_ret_t rcl_node_type_description_service_fini(rcl_node_t * node);
  * Lock-Free          | Yes
  *
  * \param[in] node the handle to the node
+ * \param[out] service_out Handle to pointer that will be set
  * \return #RCL_RET_OK if valid service was returned successfully, or
+ * \return #RCL_RET_NODE_INVALID if node is invalid, or
  * \return #RCL_RET_INVALID_ARGUMENT if any arguments are invalid, or
  * \return #RCL_RET_NOT_INIT if the service hasn't yet been initialized, or
  * \return #RCL_RET_ERROR if an unspecified error occurs.
@@ -632,8 +634,7 @@ rcl_ret_t rcl_node_get_type_description_service(
 
 /// Process a single pending request to the GetTypeDescription service.
 /**
- * Should be registered as the callback for the type description service
- * by any client instantiating that service.
+ * This function may be called to handle incoming requests by any client starting the service.
  * It is not intended to be called directly by users.
  *
  * <hr>
@@ -645,6 +646,10 @@ rcl_ret_t rcl_node_get_type_description_service(
  * Lock-Free          | Yes
  *
  * \param[in] node the handle to the node
+ * \param[in] request_header ID of the incoming request
+ * \param[in] request Request that came in to the service
+ * \param[out] response Allocated, but not necessarily initialized, response to the request
+ * \return void
  */
 RCL_PUBLIC
 void rcl_node_type_description_service_handle_request(

rcl/include/rcl/node_options.h

@@ -54,10 +54,6 @@ typedef struct rcl_node_options_s
 
   /// Middleware quality of service settings for /rosout.
   rmw_qos_profile_t rosout_qos;
-
-  /// Register the ~/get_type_description service. Defaults to false.
-  // Deprecated: use parameter "enable_type_description_service" in language clients instead.
-  bool enable_type_description_service;
 } rcl_node_options_t;
 
 /// Return the default node options in a rcl_node_options_t.

rcl/include/rcl/node_type_cache.h

@@ -37,6 +37,8 @@ typedef struct rcl_type_info_t
 /**
  * This function initializes hash map of the node's type cache such that types
  * can be registered and retrieved.
+ * Note that to correctly capture all types used by a node, this needs to be called
+ * before any "builtin" publishers or services are created.
  *
  * <hr>
  * Attribute          | Adherence

rcl/src/rcl/node.c

@@ -551,70 +551,6 @@ rcl_get_disable_loaned_message(bool * disable_loaned_message)
   return RCL_RET_OK;
 }
 
-rcl_ret_t rcl_node_type_description_service_init(rcl_node_t * node)
-{
-  RCL_CHECK_ARGUMENT_FOR_NULL(node, RCL_RET_INVALID_ARGUMENT);
-  RCL_CHECK_ARGUMENT_FOR_NULL(node->impl, RCL_RET_NODE_INVALID);
-
-  rcl_ret_t ret;
-
-  ret = rcl_node_type_cache_init(node);
-  if (ret == RCL_RET_ALREADY_INIT) {
-    rcl_reset_error();
-  } else if (ret != RCL_RET_OK) {
-    return ret;
-  }
-
-  if (rcl_service_is_valid(&node->impl->get_type_description_service)) {
-    return RCL_RET_ALREADY_INIT;
-  }
-  rcl_reset_error();  // Reset the error message set by rcl_service_is_valid()
-
-  char * service_name = NULL;
-  const rosidl_service_type_support_t * type_support =
-    ROSIDL_GET_SRV_TYPE_SUPPORT(
-    type_description_interfaces, srv,
-    GetTypeDescription);
-  rcl_service_options_t service_ops = rcl_service_get_default_options();
-  rcl_allocator_t allocator = node->context->impl->allocator;
-
-  // Construct service name
-  ret = rcl_node_resolve_name(
-    node, "~/get_type_description",
-    allocator, true, true, &service_name);
-  if (RCL_RET_OK != ret) {
-    RCL_SET_ERROR_MSG(
-      "Failed to construct ~/get_type_description service name");
-    return ret;
-  }
-
-  // Initialize service
-  ret = rcl_service_init(
-    &node->impl->get_type_description_service, node,
-    type_support, service_name, &service_ops);
-  allocator.deallocate(service_name, allocator.state);
-
-  return ret;
-}
-
-rcl_ret_t rcl_node_type_description_service_fini(rcl_node_t * node)
-{
-  RCL_CHECK_ARGUMENT_FOR_NULL(node, RCL_RET_INVALID_ARGUMENT);
-  RCL_CHECK_ARGUMENT_FOR_NULL(node->impl, RCL_RET_NODE_INVALID);
-  if (!rcl_service_is_valid(&node->impl->get_type_description_service)) {
-    rcl_reset_error();
-    return RCL_RET_NOT_INIT;
-  }
-
-  const rcl_ret_t ret =
-    rcl_service_fini(&node->impl->get_type_description_service, node);
-  if (RCL_RET_OK == ret) {
-    node->impl->get_type_description_service = rcl_get_zero_initialized_service();
-  }
-
-  return ret;
-}
-
 rcl_ret_t rcl_node_get_type_description_service(
   const rcl_node_t * node,
   rcl_service_t ** service_out)
@@ -698,6 +634,70 @@ void rcl_node_type_description_service_handle_request(
   response->successful = true;
 }
 
+rcl_ret_t rcl_node_type_description_service_init(rcl_node_t * node)
+{
+  RCL_CHECK_ARGUMENT_FOR_NULL(node, RCL_RET_INVALID_ARGUMENT);
+  RCL_CHECK_ARGUMENT_FOR_NULL(node->impl, RCL_RET_NODE_INVALID);
+
+  rcl_ret_t ret;
+
+  ret = rcl_node_type_cache_init(node);
+  if (ret == RCL_RET_ALREADY_INIT) {
+    rcl_reset_error();
+  } else if (ret != RCL_RET_OK) {
+    return ret;
+  }
+
+  if (rcl_service_is_valid(&node->impl->get_type_description_service)) {
+    return RCL_RET_ALREADY_INIT;
+  }
+  rcl_reset_error();  // Reset the error message set by rcl_service_is_valid()
+
+  char * service_name = NULL;
+  const rosidl_service_type_support_t * type_support =
+    ROSIDL_GET_SRV_TYPE_SUPPORT(
+    type_description_interfaces, srv,
+    GetTypeDescription);
+  rcl_service_options_t service_ops = rcl_service_get_default_options();
+  rcl_allocator_t allocator = node->context->impl->allocator;
+
+  // Construct service name
+  ret = rcl_node_resolve_name(
+    node, "~/get_type_description",
+    allocator, true, true, &service_name);
+  if (RCL_RET_OK != ret) {
+    RCL_SET_ERROR_MSG(
+      "Failed to construct ~/get_type_description service name");
+    return ret;
+  }
+
+  // Initialize service
+  ret = rcl_service_init(
+    &node->impl->get_type_description_service, node,
+    type_support, service_name, &service_ops);
+  allocator.deallocate(service_name, allocator.state);
+
+  return ret;
+}
+
+rcl_ret_t rcl_node_type_description_service_fini(rcl_node_t * node)
+{
+  RCL_CHECK_ARGUMENT_FOR_NULL(node, RCL_RET_INVALID_ARGUMENT);
+  RCL_CHECK_ARGUMENT_FOR_NULL(node->impl, RCL_RET_NODE_INVALID);
+  if (!rcl_service_is_valid(&node->impl->get_type_description_service)) {
+    rcl_reset_error();
+    return RCL_RET_NOT_INIT;
+  }
+
+  const rcl_ret_t ret =
+    rcl_service_fini(&node->impl->get_type_description_service, node);
+  if (RCL_RET_OK == ret) {
+    node->impl->get_type_description_service = rcl_get_zero_initialized_service();
+  }
+
+  return ret;
+}
+
 #ifdef __cplusplus
 }
 #endif

rcl/src/rcl/node_type_cache.c

@@ -205,17 +205,6 @@ rcl_ret_t rcl_node_type_cache_register_type(
     return RCL_RET_ERROR;
   }
 
-  // char * hash_str = NULL;
-  // rcutils_allocator_t allocator = rcutils_get_default_allocator();
-  // rcutils_ret_t hash_ret = rosidl_stringify_type_hash(type_hash, allocator, &hash_str);
-  // if (hash_ret == RCUTILS_RET_OK) {
-    // RCUTILS_LOG_WARN("Registered %s: %s.\n",
-    //   type_description->type_description.type_name.data, hash_str);
-  // } else {
-  //   assert(false);
-  // }
-  // allocator.deallocate(hash_str, allocator.state);
-
   return RCL_RET_OK;
 }
 
@@ -228,23 +217,13 @@ rcl_ret_t rcl_node_type_cache_unregister_type(
   RCL_CHECK_ARGUMENT_FOR_NULL(node->impl, RCL_RET_NODE_INVALID);
   RCL_CHECK_ARGUMENT_FOR_NULL(type_hash, RCL_RET_INVALID_ARGUMENT);
 
-  // char * hash_str = NULL;
-  // rcutils_allocator_t allocator = rcutils_get_default_allocator();
-  // rcutils_ret_t rcutils_ret = rosidl_stringify_type_hash(type_hash, allocator, &hash_str);
-
   if (RCUTILS_RET_OK !=
     rcutils_hash_map_get(
       &node->impl->registered_types_by_type_hash,
       type_hash, &type_info))
   {
     RCL_SET_ERROR_MSG("Failed to unregister hash");
     return RCL_RET_ERROR;
-    // if (rcutils_ret == RCUTILS_RET_OK) {
-    //   RCUTILS_LOG_ERROR("Type not registered with type cache %s.", hash_str);
-    // } else {
-    //   RCL_SET_ERROR_MSG("Type not registered with type cache - type hash failed to print.");
-    // }
-    // return RCL_RET_INVALID_ARGUMENT;
   }
 
   if (--type_info.num_registrations > 0) {
@@ -272,8 +251,5 @@ rcl_ret_t rcl_node_type_cache_unregister_type(
       type_info.type_info.type_sources);
   }
 
-  // RCUTILS_LOG_WARN("UNREgistered %s.", hash_str);
-  // allocator.deallocate(hash_str, allocator.state);
-
   return RCL_RET_OK;
 }