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, uninitialized 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,6 +551,73 @@ rcl_get_disable_loaned_message(bool * disable_loaned_message)
   return RCL_RET_OK;
 }
 
+void rcl_node_type_description_service_handle_request(
+  rcl_node_t * node,
+  rmw_request_id_t request_header,
+  const type_description_interfaces__srv__GetTypeDescription_Request * request,
+  type_description_interfaces__srv__GetTypeDescription_Response * response)
+{
+  (void)request_header;
+  rcl_type_info_t type_info;
+  RCL_CHECK_FOR_NULL_WITH_MSG(node, "invalid node handle", return;);
+  RCL_CHECK_FOR_NULL_WITH_MSG(node->impl, "invalid node", return;);
+
+  if (!type_description_interfaces__srv__GetTypeDescription_Response__init(response)) {
+    RCUTILS_LOG_ERROR_NAMED(
+      ROS_PACKAGE_NAME,
+      "Failed to initialize service response.");
+    return;
+  }
+
+  rosidl_type_hash_t type_hash;
+  if (RCUTILS_RET_OK !=
+    rosidl_parse_type_hash_string(request->type_hash.data, &type_hash))
+  {
+    RCUTILS_LOG_ERROR_NAMED(
+      ROS_PACKAGE_NAME, "Failed to parse type hash '%s'",
+      request->type_hash.data);
+    response->successful = false;
+    rosidl_runtime_c__String__assign(
+      &response->failure_reason,
+      "Failed to parse type hash");
+    return;
+  }
+
+  if (RCUTILS_RET_OK !=
+    rcl_node_type_cache_get_type_info(node, &type_hash, &type_info))
+  {
+    response->successful = false;
+    rosidl_runtime_c__String__assign(
+      &response->failure_reason,
+      "Type not currently in use by this node");
+    return;
+  }
+
+  if (!type_description_interfaces__msg__TypeDescription__copy(
+      type_info.type_description, &response->type_description))
+  {
+    response->successful = false;
+    rosidl_runtime_c__String__assign(
+      &response->failure_reason,
+      "Failed to populate TypeDescription to response.");
+    return;
+  }
+
+  if (request->include_type_sources) {
+    if (!type_description_interfaces__msg__TypeSource__Sequence__copy(
+        type_info.type_sources, &response->type_sources))
+    {
+      response->successful = false;
+      rosidl_runtime_c__String__assign(
+        &response->failure_reason,
+        "Failed to populate TypeSource_Sequence to response.");
+      return;
+    }
+  }
+
+  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);
@@ -631,73 +698,6 @@ rcl_ret_t rcl_node_get_type_description_service(
   return RCL_RET_OK;
 }
 
-void rcl_node_type_description_service_handle_request(
-  rcl_node_t * node,
-  rmw_request_id_t request_header,
-  const type_description_interfaces__srv__GetTypeDescription_Request * request,
-  type_description_interfaces__srv__GetTypeDescription_Response * response)
-{
-  (void)request_header;
-  rcl_type_info_t type_info;
-  RCL_CHECK_FOR_NULL_WITH_MSG(node, "invalid node handle", return;);
-  RCL_CHECK_FOR_NULL_WITH_MSG(node->impl, "invalid node", return;);
-
-  if (!type_description_interfaces__srv__GetTypeDescription_Response__init(response)) {
-    RCUTILS_LOG_ERROR_NAMED(
-      ROS_PACKAGE_NAME,
-      "Failed to initialize service response.");
-    return;
-  }
-
-  rosidl_type_hash_t type_hash;
-  if (RCUTILS_RET_OK !=
-    rosidl_parse_type_hash_string(request->type_hash.data, &type_hash))
-  {
-    RCUTILS_LOG_ERROR_NAMED(
-      ROS_PACKAGE_NAME, "Failed to parse type hash '%s'",
-      request->type_hash.data);
-    response->successful = false;
-    rosidl_runtime_c__String__assign(
-      &response->failure_reason,
-      "Failed to parse type hash");
-    return;
-  }
-
-  if (RCUTILS_RET_OK !=
-    rcl_node_type_cache_get_type_info(node, &type_hash, &type_info))
-  {
-    response->successful = false;
-    rosidl_runtime_c__String__assign(
-      &response->failure_reason,
-      "Type not currently in use by this node");
-    return;
-  }
-
-  if (!type_description_interfaces__msg__TypeDescription__copy(
-    type_info.type_description, &response->type_description))
-  {
-    response->successful = false;
-    rosidl_runtime_c__String__assign(
-      &response->failure_reason,
-      "Failed to populate TypeDescription to response.");
-    return;
-  }
-
-  if (request->include_type_sources) {
-    if (!type_description_interfaces__msg__TypeSource__Sequence__copy(
-      type_info.type_sources, &response->type_sources))
-    {
-      response->successful = false;
-      rosidl_runtime_c__String__assign(
-        &response->failure_reason,
-        "Failed to populate TypeSource_Sequence to response.");
-      return;
-    }
-  }
-
-  response->successful = true;
-}
-
 #ifdef __cplusplus
 }
 #endif

rcl/src/rcl/node_options.c

@@ -36,7 +36,6 @@ rcl_node_get_default_options()
     .arguments = rcl_get_zero_initialized_arguments(),
     .enable_rosout = true,
     .rosout_qos = rcl_qos_profile_rosout_default,
-    .enable_type_description_service = false,
   };
   return default_options;
 }
@@ -62,7 +61,6 @@ rcl_node_options_copy(
   options_out->use_global_arguments = options->use_global_arguments;
   options_out->enable_rosout = options->enable_rosout;
   options_out->rosout_qos = options->rosout_qos;
-  options_out->enable_type_description_service = options->enable_type_description_service;
   if (NULL != options->arguments.impl) {
     return rcl_arguments_copy(&(options->arguments), &(options_out->arguments));
   }

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;
 }

rcl/test/rcl/test_get_type_description_service.cpp

@@ -105,7 +105,6 @@ class CLASSNAME (TestGetTypeDescSrvEnabledFixture, RMW_IMPLEMENTATION) : public
   rcl_context_t * context_ptr;
   rcl_node_t * node_ptr;
   char get_type_description_service_name[256];
-  bool enable_get_type_description_service;
 
   virtual bool get_type_description_service_enabled() const
   {
@@ -132,7 +131,6 @@ class CLASSNAME (TestGetTypeDescSrvEnabledFixture, RMW_IMPLEMENTATION) : public
     *this->node_ptr = rcl_get_zero_initialized_node();
     const char * name = "test_service_node";
     rcl_node_options_t node_options = rcl_node_get_default_options();
-    node_options.enable_type_description_service = get_type_description_service_enabled();
     ret = rcl_node_init(this->node_ptr, name, "", this->context_ptr, &node_options);
     ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str;
 

rcl/test/rcl/test_node_type_cache.cpp

@@ -59,7 +59,6 @@ class CLASSNAME (TestNodeTypeCacheFixture, RMW_IMPLEMENTATION)
     *this->node_ptr = rcl_get_zero_initialized_node();
     constexpr char name[] = "test_type_cache_node";
     rcl_node_options_t node_options = rcl_node_get_default_options();
-    node_options.enable_type_description_service = true;
     ret = rcl_node_init(
       this->node_ptr, name, "", this->context_ptr,
       &node_options);