Skip to content

Commit

Permalink
Return service from node_type_description_service_init (#1112)
Browse files Browse the repository at this point in the history
This changes the mechanism of rcl_node_type_description_service_init to populate a user-provided service.

Signed-off-by: Michael Carroll <[email protected]>
  • Loading branch information
mjcarroll authored Oct 30, 2023
1 parent 45fc952 commit 1b79535
Show file tree
Hide file tree
Showing 4 changed files with 50 additions and 116 deletions.
60 changes: 6 additions & 54 deletions rcl/include/rcl/node.h
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*
* <hr>
* Attribute | Adherence
Expand All @@ -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
Expand All @@ -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.
*
* <hr>
* 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.
*
* <hr>
* 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.
/**
Expand Down
48 changes: 7 additions & 41 deletions rcl/src/rcl/node.c
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand Down Expand Up @@ -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()
Expand All @@ -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) {
Expand All @@ -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;
}
1 change: 0 additions & 1 deletion rcl/src/rcl/node_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -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_
57 changes: 37 additions & 20 deletions rcl/test/rcl/test_get_type_description_service.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -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);
Expand All @@ -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. */
Expand All @@ -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();
Expand All @@ -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));

Expand Down Expand Up @@ -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(
Expand All @@ -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(
Expand All @@ -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;
}

Expand Down Expand Up @@ -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();
Expand All @@ -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));

Expand All @@ -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(
Expand All @@ -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(
Expand All @@ -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;
}

Expand Down

0 comments on commit 1b79535

Please sign in to comment.