diff --git a/rcl/include/rcl/node.h b/rcl/include/rcl/node.h index e0213b7de..58a2163ec 100644 --- a/rcl/include/rcl/node.h +++ b/rcl/include/rcl/node.h @@ -562,7 +562,8 @@ rcl_get_disable_loaned_message(bool * disable_loaned_message); * must register rcl_node_type_description_service_handle_request or a custom callback * to handle incoming requests, via that client's executor/waitset capabilities. * - * This will initialize the node's type cache, if it has not been initialized already. + * Note that the returned service must be cleaned up by the caller by calling + * rcl_service_fini. * *
* Attribute | Adherence @@ -572,6 +573,7 @@ rcl_get_disable_loaned_message(bool * disable_loaned_message); * Uses Atomics | No * Lock-Free | Yes * + * \param[in] service the handle to the type description service to be initialized * \param[in] node handle to the node for which to initialize the service * \return #RCL_RET_OK if the service was successfully initialized, or * \return #RCL_RET_INVALID_ARGUMENT if any arguments are invalid, or @@ -581,59 +583,9 @@ rcl_get_disable_loaned_message(bool * disable_loaned_message); */ RCL_PUBLIC RCL_WARN_UNUSED -rcl_ret_t rcl_node_type_description_service_init(rcl_node_t * node); - -/// Finalizes the node's ~/get_type_description service. -/** - * This function finalizes the node's private ~/get_type_description service. - * - *
- * Attribute | Adherence - * ------------------ | ------------- - * Allocates Memory | No - * Thread-Safe | No - * Uses Atomics | No - * Lock-Free | Yes - * - * \param[in] node the handle to the node whose type cache should be initialized - * \return #RCL_RET_OK if service was deinitialized successfully, or - * \return #RCL_RET_INVALID_ARGUMENT if any arguments are invalid, or - * \return #RCL_RET_SERVICE_INVALID if the service is invalid, or - * \return #RCL_RET_NODE_INVALID if the node is invalid, or - * \return #RCL_RET_ERROR if an unspecified error occurs. - */ -RCL_PUBLIC -RCL_WARN_UNUSED -rcl_ret_t rcl_node_type_description_service_fini(rcl_node_t * node); - - -/// Returns a pointer to the node's ~/get_type_description service. -/** - * On success, sets service_out to the initialized service. - * rcl_node_type_description_service_init must be called before this. - * - *
- * Attribute | Adherence - * ------------------ | ------------- - * Allocates Memory | No - * Thread-Safe | No - * Uses Atomics | No - * 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. - */ -RCL_PUBLIC -RCL_WARN_UNUSED -rcl_ret_t rcl_node_get_type_description_service( - const rcl_node_t * node, - rcl_service_t ** service_out); - +rcl_ret_t rcl_node_type_description_service_init( + rcl_service_t * service, + const rcl_node_t * node); /// Process a single pending request to the GetTypeDescription service. /** diff --git a/rcl/src/rcl/node.c b/rcl/src/rcl/node.c index 35f995e42..1d4305bf2 100644 --- a/rcl/src/rcl/node.c +++ b/rcl/src/rcl/node.c @@ -194,7 +194,6 @@ rcl_node_init( node->impl, "allocating memory failed", ret = RCL_RET_BAD_ALLOC; goto fail); node->impl->options = rcl_node_get_default_options(); node->impl->registered_types_by_type_hash = rcutils_get_zero_initialized_hash_map(); - node->impl->get_type_description_service = rcl_get_zero_initialized_service(); node->context = context; // Initialize node impl. ret = rcl_node_options_copy(options, &(node->impl->options)); @@ -580,14 +579,15 @@ 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_ret_t rcl_node_type_description_service_init( + rcl_service_t * service, + const rcl_node_t * node) { + RCL_CHECK_ARGUMENT_FOR_NULL(service, RCL_RET_INVALID_ARGUMENT); 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; - - if (rcl_service_is_valid(&node->impl->get_type_description_service)) { + if (rcl_service_is_valid(service)) { return RCL_RET_ALREADY_INIT; } rcl_reset_error(); // Reset the error message set by rcl_service_is_valid() @@ -601,7 +601,7 @@ rcl_ret_t rcl_node_type_description_service_init(rcl_node_t * node) rcl_allocator_t allocator = node->context->impl->allocator; // Construct service name - ret = rcl_node_resolve_name( + rcl_ret_t ret = rcl_node_resolve_name( node, "~/get_type_description", allocator, true, true, &service_name); if (RCL_RET_OK != ret) { @@ -612,43 +612,9 @@ rcl_ret_t rcl_node_type_description_service_init(rcl_node_t * node) // Initialize service ret = rcl_service_init( - &node->impl->get_type_description_service, node, + 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) -{ - RCL_CHECK_ARGUMENT_FOR_NULL(node, RCL_RET_INVALID_ARGUMENT); - RCL_CHECK_ARGUMENT_FOR_NULL(node->impl, RCL_RET_NODE_INVALID); - RCL_CHECK_ARGUMENT_FOR_NULL(service_out, RCL_RET_SERVICE_INVALID); - - if (!rcl_service_is_valid(&node->impl->get_type_description_service)) { - return RCL_RET_NOT_INIT; - } - - *service_out = &node->impl->get_type_description_service; - return RCL_RET_OK; -} diff --git a/rcl/src/rcl/node_impl.h b/rcl/src/rcl/node_impl.h index 8d4f5847e..5445c5aab 100644 --- a/rcl/src/rcl/node_impl.h +++ b/rcl/src/rcl/node_impl.h @@ -31,7 +31,6 @@ struct rcl_node_impl_s const char * logger_name; const char * fq_name; rcutils_hash_map_t registered_types_by_type_hash; - rcl_service_t get_type_description_service; }; #endif // RCL__NODE_IMPL_H_ diff --git a/rcl/test/rcl/test_get_type_description_service.cpp b/rcl/test/rcl/test_get_type_description_service.cpp index baac08013..235e7937a 100644 --- a/rcl/test/rcl/test_get_type_description_service.cpp +++ b/rcl/test/rcl/test_get_type_description_service.cpp @@ -163,8 +163,6 @@ class CLASSNAME (TestGetTypeDescSrvFixture, RMW_IMPLEMENTATION) : public ::testi rcl_node_options_t node_options = rcl_node_get_default_options(); ret = rcl_node_init(this->node_ptr, name, "", this->context_ptr, &node_options); ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; - ret = rcl_node_type_description_service_init(node_ptr); - ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; const char * node_fqn = rcl_node_get_fully_qualified_name(this->node_ptr); snprintf( @@ -174,9 +172,7 @@ class CLASSNAME (TestGetTypeDescSrvFixture, RMW_IMPLEMENTATION) : public ::testi void TearDown() { - rcl_ret_t ret = rcl_node_type_description_service_fini(node_ptr); - EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; - ret = rcl_node_fini(this->node_ptr); + rcl_ret_t ret = rcl_node_fini(this->node_ptr); delete this->node_ptr; EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; ret = rcl_shutdown(this->context_ptr); @@ -197,23 +193,30 @@ class CLASSNAME (TestGetTypeDescSrvFixture, RMW_IMPLEMENTATION) : public ::testi TEST_F( CLASSNAME(TestGetTypeDescSrvFixture, RMW_IMPLEMENTATION), test_service_init_and_fini_functions) { + rcl_service_t service = rcl_get_zero_initialized_service(); + + // Service does not initially exist EXPECT_TRUE( - service_exists( + service_not_exists( this->node_ptr, this->get_type_description_service_name, GET_TYPE_DESCRIPTION_SRV_TYPE_NAME, std::chrono::seconds(5))); - EXPECT_EQ(RCL_RET_OK, rcl_node_type_description_service_fini(this->node_ptr)); + + // Once the type descrition service is init, then it appear in the graph + EXPECT_EQ(RCL_RET_OK, rcl_node_type_description_service_init(&service, this->node_ptr)); EXPECT_TRUE( - service_not_exists( + service_exists( this->node_ptr, this->get_type_description_service_name, GET_TYPE_DESCRIPTION_SRV_TYPE_NAME, std::chrono::seconds(5))); - EXPECT_EQ(RCL_RET_NOT_INIT, rcl_node_type_description_service_fini(this->node_ptr)); - EXPECT_EQ(RCL_RET_OK, rcl_node_type_description_service_init(this->node_ptr)); + // Once the type description service is fini, then it no longer appears in the graph + EXPECT_EQ(RCL_RET_OK, rcl_service_fini(&service, this->node_ptr)); EXPECT_TRUE( - service_exists( + service_not_exists( this->node_ptr, this->get_type_description_service_name, GET_TYPE_DESCRIPTION_SRV_TYPE_NAME, std::chrono::seconds(5))); - EXPECT_EQ(RCL_RET_ALREADY_INIT, rcl_node_type_description_service_init(this->node_ptr)); + + // Repeatedly destroying the service should not cause faults. + EXPECT_EQ(RCL_RET_OK, rcl_service_fini(&service, this->node_ptr)); } /* Basic nominal test of the ~/get_type_description service. */ @@ -222,6 +225,10 @@ TEST_F(CLASSNAME(TestGetTypeDescSrvFixture, RMW_IMPLEMENTATION), test_service_no const rosidl_service_type_support_t * ts = ROSIDL_GET_SRV_TYPE_SUPPORT( type_description_interfaces, srv, GetTypeDescription); + // Create type description service. + auto service = rcl_get_zero_initialized_service(); + EXPECT_EQ(RCL_RET_OK, rcl_node_type_description_service_init(&service, this->node_ptr)); + // Create client. rcl_client_t client = rcl_get_zero_initialized_client(); rcl_client_options_t client_options = rcl_client_get_default_options(); @@ -233,6 +240,9 @@ TEST_F(CLASSNAME(TestGetTypeDescSrvFixture, RMW_IMPLEMENTATION), test_service_no { rcl_ret_t ret = rcl_client_fini(&client, this->node_ptr); EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + + ret = rcl_service_fini(&service, this->node_ptr); + EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; }); ASSERT_TRUE(wait_for_server_to_be_available(this->node_ptr, &client, 10, 1000)); @@ -262,8 +272,8 @@ TEST_F(CLASSNAME(TestGetTypeDescSrvFixture, RMW_IMPLEMENTATION), test_service_no // This scope simulates handling request in a different context { - auto service = &node_ptr->impl->get_type_description_service; - ASSERT_TRUE(wait_for_service_to_be_ready(service, context_ptr, 10, 100)); + auto * service_ptr = &service; + ASSERT_TRUE(wait_for_service_to_be_ready(service_ptr, context_ptr, 10, 100)); type_description_interfaces__srv__GetTypeDescription_Response service_response; OSRF_TESTING_TOOLS_CPP_SCOPE_EXIT( @@ -278,7 +288,7 @@ TEST_F(CLASSNAME(TestGetTypeDescSrvFixture, RMW_IMPLEMENTATION), test_service_no type_description_interfaces__srv__GetTypeDescription_Request__fini(&service_request); }); rmw_service_info_t header; - ret = rcl_take_request_with_info(service, &header, &service_request); + ret = rcl_take_request_with_info(service_ptr, &header, &service_request); ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; rcl_node_type_description_service_handle_request( @@ -287,7 +297,7 @@ TEST_F(CLASSNAME(TestGetTypeDescSrvFixture, RMW_IMPLEMENTATION), test_service_no &service_request, &service_response); - ret = rcl_send_response(service, &header.request_id, &service_response); + ret = rcl_send_response(service_ptr, &header.request_id, &service_response); ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; } @@ -315,6 +325,10 @@ TEST_F( const rosidl_service_type_support_t * ts = ROSIDL_GET_SRV_TYPE_SUPPORT( type_description_interfaces, srv, GetTypeDescription); + // Create type description service. + auto service = rcl_get_zero_initialized_service(); + EXPECT_EQ(RCL_RET_OK, rcl_node_type_description_service_init(&service, this->node_ptr)); + // Create client. rcl_client_t client = rcl_get_zero_initialized_client(); rcl_client_options_t client_options = rcl_client_get_default_options(); @@ -326,6 +340,9 @@ TEST_F( { rcl_ret_t ret = rcl_client_fini(&client, this->node_ptr); EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + + ret = rcl_service_fini(&service, this->node_ptr); + EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; }); ASSERT_TRUE(wait_for_server_to_be_available(this->node_ptr, &client, 10, 1000)); @@ -346,8 +363,8 @@ TEST_F( // This scope simulates handling request in a different context { - auto service = &node_ptr->impl->get_type_description_service; - ASSERT_TRUE(wait_for_service_to_be_ready(service, context_ptr, 10, 100)); + auto * service_ptr = &service; + ASSERT_TRUE(wait_for_service_to_be_ready(service_ptr, context_ptr, 10, 100)); type_description_interfaces__srv__GetTypeDescription_Response service_response; OSRF_TESTING_TOOLS_CPP_SCOPE_EXIT( @@ -362,7 +379,7 @@ TEST_F( type_description_interfaces__srv__GetTypeDescription_Request__fini(&service_request); }); rmw_service_info_t header; - ret = rcl_take_request_with_info(service, &header, &service_request); + ret = rcl_take_request_with_info(service_ptr, &header, &service_request); ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; rcl_node_type_description_service_handle_request( @@ -371,7 +388,7 @@ TEST_F( &service_request, &service_response); - ret = rcl_send_response(service, &header.request_id, &service_response); + ret = rcl_send_response(service_ptr, &header.request_id, &service_response); ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; }