Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

pass context to wait set, and fini rmw context #373

Merged
merged 2 commits into from
Jan 25, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion rcl/include/rcl/wait.h
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ rcl_get_zero_initialized_wait_set(void);
*
* rcl_wait_set_t wait_set = rcl_get_zero_initialized_wait_set();
* rcl_ret_t ret =
* rcl_wait_set_init(&wait_set, 42, 42, 42, 42, 42, rcl_get_default_allocator());
* rcl_wait_set_init(&wait_set, 42, 42, 42, 42, 42, &context, rcl_get_default_allocator());
* // ... error handling, then use it, then call the matching fini:
* ret = rcl_wait_set_fini(&wait_set);
* // ... error handling
Expand All @@ -105,9 +105,11 @@ rcl_get_zero_initialized_wait_set(void);
* \param[in] number_of_timers non-zero size of the timers set
* \param[in] number_of_clients non-zero size of the clients set
* \param[in] number_of_services non-zero size of the services set
* \param[in] context the context that the wait set should be associated with
* \param[in] allocator the allocator to use when allocating space in the sets
* \return `RCL_RET_OK` if the wait set is initialized successfully, or
* \return `RCL_RET_ALREADY_INIT` if the wait set is not zero initialized, or
* \return `RCL_RET_NOT_INIT` if the given context is invalid, or
* \return `RCL_RET_INVALID_ARGUMENT` if any arguments are invalid, or
* \return `RCL_RET_BAD_ALLOC` if allocating memory failed, or
* \return `RCL_RET_ERROR` if an unspecified error occurs.
Expand All @@ -122,6 +124,7 @@ rcl_wait_set_init(
size_t number_of_timers,
size_t number_of_clients,
size_t number_of_services,
rcl_context_t * context,
rcl_allocator_t allocator);

/// Finalize a rcl wait set.
Expand Down
13 changes: 13 additions & 0 deletions rcl/src/rcl/context.c
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,19 @@ __cleanup_context(rcl_context_t * context)
}
}

// clean up rmw_context
if (NULL != context->impl->rmw_context.implementation_identifier) {
rmw_ret_t rmw_ret = rmw_context_fini(&(context->impl->rmw_context));
if (RMW_RET_OK != rmw_ret) {
RCUTILS_SAFE_FWRITE_TO_STDERR(
"[rcl|init.c:" RCUTILS_STRINGIFY(__LINE__)
"] failed to finalize rmw context while cleaning up context, memory may be leaked: ");
RCUTILS_SAFE_FWRITE_TO_STDERR(rcutils_get_error_string().str);
RCUTILS_SAFE_FWRITE_TO_STDERR("\n");
rcutils_reset_error();
}
}

// clean up copy of argv if valid
if (NULL != context->impl->argv) {
int64_t i;
Expand Down
4 changes: 3 additions & 1 deletion rcl/src/rcl/init.c
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,9 @@ rcl_init(
RCL_CHECK_FOR_NULL_WITH_MSG(
context->impl, "failed to allocate memory for context impl", return RCL_RET_BAD_ALLOC);

// Zero initialize rmw context first so its validity can by checked in cleanup.
context->impl->rmw_context = rmw_get_zero_initialized_context();

// Copy the options into the context for future reference.
rcl_ret_t ret = rcl_init_options_copy(options, &(context->impl->init_options));
if (RCL_RET_OK != ret) {
Expand Down Expand Up @@ -132,7 +135,6 @@ rcl_init(
context->impl->init_options.impl->rmw_init_options.instance_id = next_instance_id;

// Initialize rmw_init.
context->impl->rmw_context = rmw_get_zero_initialized_context();
rmw_ret_t rmw_ret = rmw_init(
&(context->impl->init_options.impl->rmw_init_options),
&(context->impl->rmw_context));
Expand Down
26 changes: 23 additions & 3 deletions rcl/src/rcl/wait.c
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@ extern "C"
#include "rmw/error_handling.h"
#include "rmw/rmw.h"

#include "./context_impl.h"

typedef struct rcl_wait_set_impl_t
{
// number of subscriptions that have been added to the wait set
Expand All @@ -47,6 +49,9 @@ typedef struct rcl_wait_set_impl_t
rmw_wait_set_t * rmw_wait_set;
// number of timers that have been added to the wait set
size_t timer_index;
// context with which the wait set is associated
rcl_context_t * context;
// allocator used in the wait set
rcl_allocator_t allocator;
} rcl_wait_set_impl_t;

Expand Down Expand Up @@ -97,6 +102,7 @@ rcl_wait_set_init(
size_t number_of_timers,
size_t number_of_clients,
size_t number_of_services,
rcl_context_t * context,
rcl_allocator_t allocator)
{
RCUTILS_LOG_DEBUG_NAMED(
Expand All @@ -112,6 +118,14 @@ rcl_wait_set_init(
RCL_SET_ERROR_MSG("wait_set already initialized, or memory was uninitialized.");
return RCL_RET_ALREADY_INIT;
}
// Make sure rcl has been initialized.
RCL_CHECK_ARGUMENT_FOR_NULL(context, RCL_RET_INVALID_ARGUMENT);
if (!rcl_context_is_valid(context)) {
RCL_SET_ERROR_MSG(
"the given context is not valid, "
"either rcl_init() was not called or rcl_shutdown() was called.");
return RCL_RET_NOT_INIT;
}
// Allocate space for the implementation struct.
wait_set->impl = (rcl_wait_set_impl_t *)allocator.allocate(
sizeof(rcl_wait_set_impl_t), allocator.state);
Expand All @@ -127,13 +141,19 @@ rcl_wait_set_init(
wait_set->impl->rmw_services.services = NULL;
wait_set->impl->rmw_services.service_count = 0;

wait_set->impl->rmw_wait_set = rmw_create_wait_set(
2 * number_of_subscriptions + number_of_guard_conditions + number_of_clients +
number_of_services);
size_t num_conditions =
(2 * number_of_subscriptions) +
number_of_guard_conditions +
number_of_clients +
number_of_services;

wait_set->impl->rmw_wait_set = rmw_create_wait_set(&(context->impl->rmw_context), num_conditions);
if (!wait_set->impl->rmw_wait_set) {
goto fail;
}

// Set context.
wait_set->impl->context = context;
// Set allocator.
wait_set->impl->allocator = allocator;
// Initialize subscription space.
Expand Down
5 changes: 3 additions & 2 deletions rcl/test/rcl/client_fixture.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -57,11 +57,12 @@ wait_for_server_to_be_available(
bool
wait_for_client_to_be_ready(
rcl_client_t * client,
rcl_context_t * context,
size_t max_tries,
int64_t period_ms)
{
rcl_wait_set_t wait_set = rcl_get_zero_initialized_wait_set();
rcl_ret_t ret = rcl_wait_set_init(&wait_set, 0, 0, 0, 1, 0, rcl_get_default_allocator());
rcl_ret_t ret = rcl_wait_set_init(&wait_set, 0, 0, 0, 1, 0, context, rcl_get_default_allocator());
if (ret != RCL_RET_OK) {
RCUTILS_LOG_ERROR_NAMED(
ROS_PACKAGE_NAME, "Error in wait set init: %s", rcl_get_error_string().str);
Expand Down Expand Up @@ -207,7 +208,7 @@ int main(int argc, char ** argv)
memset(&client_response, 0, sizeof(test_msgs__srv__Primitives_Response));
test_msgs__srv__Primitives_Response__init(&client_response);

if (!wait_for_client_to_be_ready(&client, 1000, 100)) {
if (!wait_for_client_to_be_ready(&client, &context, 1000, 100)) {
RCUTILS_LOG_ERROR_NAMED(ROS_PACKAGE_NAME, "Client never became ready");
return -1;
}
Expand Down
5 changes: 3 additions & 2 deletions rcl/test/rcl/service_fixture.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -30,11 +30,12 @@
bool
wait_for_service_to_be_ready(
rcl_service_t * service,
rcl_context_t * context,
size_t max_tries,
int64_t period_ms)
{
rcl_wait_set_t wait_set = rcl_get_zero_initialized_wait_set();
rcl_ret_t ret = rcl_wait_set_init(&wait_set, 0, 0, 0, 0, 1, rcl_get_default_allocator());
rcl_ret_t ret = rcl_wait_set_init(&wait_set, 0, 0, 0, 0, 1, context, rcl_get_default_allocator());
if (ret != RCL_RET_OK) {
RCUTILS_LOG_ERROR_NAMED(
ROS_PACKAGE_NAME, "Error in wait set init: %s", rcl_get_error_string().str);
Expand Down Expand Up @@ -156,7 +157,7 @@ int main(int argc, char ** argv)

// Block until a client request comes in.

if (!wait_for_service_to_be_ready(&service, 1000, 100)) {
if (!wait_for_service_to_be_ready(&service, &context, 1000, 100)) {
RCUTILS_LOG_ERROR_NAMED(ROS_PACKAGE_NAME, "Service never became ready");
return -1;
}
Expand Down
3 changes: 2 additions & 1 deletion rcl/test/rcl/test_count_matched.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,8 @@ class CLASSNAME (TestCountFixture, RMW_IMPLEMENTATION) : public ::testing::Test

this->wait_set_ptr = new rcl_wait_set_t;
*this->wait_set_ptr = rcl_get_zero_initialized_wait_set();
ret = rcl_wait_set_init(this->wait_set_ptr, 0, 1, 0, 0, 0, rcl_get_default_allocator());
ret = rcl_wait_set_init(
this->wait_set_ptr, 0, 1, 0, 0, 0, this->context_ptr, rcl_get_default_allocator());
}

void TearDown()
Expand Down
3 changes: 2 additions & 1 deletion rcl/test/rcl/test_graph.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,8 @@ class CLASSNAME (TestGraphFixture, RMW_IMPLEMENTATION) : public ::testing::Test

this->wait_set_ptr = new rcl_wait_set_t;
*this->wait_set_ptr = rcl_get_zero_initialized_wait_set();
ret = rcl_wait_set_init(this->wait_set_ptr, 0, 1, 0, 0, 0, rcl_get_default_allocator());
ret = rcl_wait_set_init(
this->wait_set_ptr, 0, 1, 0, 0, 0, this->context_ptr, rcl_get_default_allocator());
ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str;
}

Expand Down
7 changes: 4 additions & 3 deletions rcl/test/rcl/test_service.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -78,12 +78,13 @@ class CLASSNAME (TestServiceFixture, RMW_IMPLEMENTATION) : public ::testing::Tes
void
wait_for_service_to_be_ready(
rcl_service_t * service,
rcl_context_t * context,
size_t max_tries,
int64_t period_ms,
bool & success)
{
rcl_wait_set_t wait_set = rcl_get_zero_initialized_wait_set();
rcl_ret_t ret = rcl_wait_set_init(&wait_set, 0, 0, 0, 0, 1, rcl_get_default_allocator());
rcl_ret_t ret = rcl_wait_set_init(&wait_set, 0, 0, 0, 0, 1, context, rcl_get_default_allocator());
ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str;
OSRF_TESTING_TOOLS_CPP_SCOPE_EXIT({
rcl_ret_t ret = rcl_wait_set_fini(&wait_set);
Expand Down Expand Up @@ -174,7 +175,7 @@ TEST_F(CLASSNAME(TestServiceFixture, RMW_IMPLEMENTATION), test_service_nominal)
ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str;

bool success;
wait_for_service_to_be_ready(&service, 10, 100, success);
wait_for_service_to_be_ready(&service, context_ptr, 10, 100, success);
ASSERT_TRUE(success);

// This scope simulates the service responding in a different context so that we can
Expand All @@ -201,7 +202,7 @@ TEST_F(CLASSNAME(TestServiceFixture, RMW_IMPLEMENTATION), test_service_nominal)
ret = rcl_send_response(&service, &header, &service_response);
ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str;
}
wait_for_service_to_be_ready(&service, 10, 100, success);
wait_for_service_to_be_ready(&service, context_ptr, 10, 100, success);

// Initialize the response owned by the client and take the response.
test_msgs__srv__Primitives_Response client_response;
Expand Down
7 changes: 4 additions & 3 deletions rcl/test/rcl/test_subscription.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -78,12 +78,13 @@ class CLASSNAME (TestSubscriptionFixture, RMW_IMPLEMENTATION) : public ::testing
void
wait_for_subscription_to_be_ready(
rcl_subscription_t * subscription,
rcl_context_t * context,
size_t max_tries,
int64_t period_ms,
bool & success)
{
rcl_wait_set_t wait_set = rcl_get_zero_initialized_wait_set();
rcl_ret_t ret = rcl_wait_set_init(&wait_set, 1, 0, 0, 0, 0, rcl_get_default_allocator());
rcl_ret_t ret = rcl_wait_set_init(&wait_set, 1, 0, 0, 0, 0, context, rcl_get_default_allocator());
ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str;
OSRF_TESTING_TOOLS_CPP_SCOPE_EXIT({
rcl_ret_t ret = rcl_wait_set_fini(&wait_set);
Expand Down Expand Up @@ -166,7 +167,7 @@ TEST_F(CLASSNAME(TestSubscriptionFixture, RMW_IMPLEMENTATION), test_subscription
ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str;
}
bool success;
wait_for_subscription_to_be_ready(&subscription, 10, 100, success);
wait_for_subscription_to_be_ready(&subscription, context_ptr, 10, 100, success);
ASSERT_TRUE(success);
{
test_msgs__msg__Primitives msg;
Expand Down Expand Up @@ -217,7 +218,7 @@ TEST_F(CLASSNAME(TestSubscriptionFixture, RMW_IMPLEMENTATION), test_subscription
ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str;
}
bool success;
wait_for_subscription_to_be_ready(&subscription, 10, 100, success);
wait_for_subscription_to_be_ready(&subscription, context_ptr, 10, 100, success);
ASSERT_TRUE(success);
{
test_msgs__msg__Primitives msg;
Expand Down
10 changes: 5 additions & 5 deletions rcl/test/rcl/test_timer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ TEST_F(TestTimerFixture, test_two_timers) {
ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str;

rcl_wait_set_t wait_set = rcl_get_zero_initialized_wait_set();
ret = rcl_wait_set_init(&wait_set, 0, 0, 2, 0, 0, rcl_get_default_allocator());
ret = rcl_wait_set_init(&wait_set, 0, 0, 2, 0, 0, context_ptr, rcl_get_default_allocator());
EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str;

ret = rcl_wait_set_add_timer(&wait_set, &timer, NULL);
Expand Down Expand Up @@ -140,7 +140,7 @@ TEST_F(TestTimerFixture, test_two_timers_ready_before_timeout) {
ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str;

rcl_wait_set_t wait_set = rcl_get_zero_initialized_wait_set();
ret = rcl_wait_set_init(&wait_set, 0, 0, 2, 0, 0, rcl_get_default_allocator());
ret = rcl_wait_set_init(&wait_set, 0, 0, 2, 0, 0, context_ptr, rcl_get_default_allocator());
EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str;

ret = rcl_wait_set_add_timer(&wait_set, &timer, NULL);
Expand Down Expand Up @@ -191,7 +191,7 @@ TEST_F(TestTimerFixture, test_timer_not_ready) {
ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str;

rcl_wait_set_t wait_set = rcl_get_zero_initialized_wait_set();
ret = rcl_wait_set_init(&wait_set, 0, 0, 1, 0, 0, rcl_get_default_allocator());
ret = rcl_wait_set_init(&wait_set, 0, 0, 1, 0, 0, context_ptr, rcl_get_default_allocator());
EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str;

ret = rcl_wait_set_add_timer(&wait_set, &timer, NULL);
Expand Down Expand Up @@ -239,7 +239,7 @@ TEST_F(TestTimerFixture, test_canceled_timer) {
ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str;

rcl_wait_set_t wait_set = rcl_get_zero_initialized_wait_set();
ret = rcl_wait_set_init(&wait_set, 0, 0, 1, 0, 0, rcl_get_default_allocator());
ret = rcl_wait_set_init(&wait_set, 0, 0, 1, 0, 0, context_ptr, rcl_get_default_allocator());
EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str;

ret = rcl_wait_set_add_timer(&wait_set, &timer, NULL);
Expand Down Expand Up @@ -459,7 +459,7 @@ TEST_F(TestTimerFixture, test_ros_time_wakes_wait) {

std::thread wait_thr([&](void) {
rcl_wait_set_t wait_set = rcl_get_zero_initialized_wait_set();
ret = rcl_wait_set_init(&wait_set, 0, 0, 1, 0, 0, rcl_get_default_allocator());
ret = rcl_wait_set_init(&wait_set, 0, 0, 1, 0, 0, context_ptr, rcl_get_default_allocator());
EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str;

ASSERT_EQ(RCL_RET_OK, rcl_wait_set_add_timer(&wait_set, &timer, NULL)) <<
Expand Down
Loading