From 49a97b97e1486cfa9ecccaad5b244b30ab72877f Mon Sep 17 00:00:00 2001 From: Ivan Santiago Paunovic Date: Thu, 20 Feb 2020 16:51:55 -0300 Subject: [PATCH 1/3] Remove initialization/finalization of logger from rcl_init()/rcl_shutdown() Signed-off-by: Ivan Santiago Paunovic --- rcl/include/rcl/init_options.h | 5 +++++ rcl/src/rcl/init.c | 17 ----------------- rcl/src/rcl/init_options.c | 8 ++++++++ 3 files changed, 13 insertions(+), 17 deletions(-) diff --git a/rcl/include/rcl/init_options.h b/rcl/include/rcl/init_options.h index 7b1533cba..5956012c7 100644 --- a/rcl/include/rcl/init_options.h +++ b/rcl/include/rcl/init_options.h @@ -152,6 +152,11 @@ RCL_WARN_UNUSED rmw_init_options_t * rcl_init_options_get_rmw_init_options(rcl_init_options_t * init_options); +RCL_PUBLIC +RCL_WARN_UNUSED +const rcl_allocator_t * +rcl_init_options_get_allocator(const rcl_init_options_t * init_options); + #ifdef __cplusplus } #endif diff --git a/rcl/src/rcl/init.c b/rcl/src/rcl/init.c index 539f02835..a18f675bb 100644 --- a/rcl/src/rcl/init.c +++ b/rcl/src/rcl/init.c @@ -131,16 +131,6 @@ rcl_init( goto fail; } - ret = rcl_logging_configure(&context->global_arguments, &allocator); - if (RCL_RET_OK != ret) { - fail_ret = ret; - RCUTILS_LOG_ERROR_NAMED( - ROS_PACKAGE_NAME, - "Failed to configure logging: %s", - rcutils_get_error_string().str); - goto fail; - } - // Set the instance id. uint64_t next_instance_id = rcutils_atomic_fetch_add_uint64_t(&__rcl_next_unique_id, 1); if (0 == next_instance_id) { @@ -260,13 +250,6 @@ rcl_shutdown(rcl_context_t * context) return rcl_convert_rmw_ret_to_rcl_ret(rmw_ret); } - rcl_ret_t rcl_ret = rcl_logging_fini(); - RCUTILS_LOG_ERROR_EXPRESSION_NAMED( - RCL_RET_OK != rcl_ret, ROS_PACKAGE_NAME, - "Failed to fini logging, rcl_ret_t: %i, rcl_error_str: %s", rcl_ret, - rcl_get_error_string().str); - rcl_reset_error(); - return RCL_RET_OK; } diff --git a/rcl/src/rcl/init_options.c b/rcl/src/rcl/init_options.c index 12b17fd68..713337d60 100644 --- a/rcl/src/rcl/init_options.c +++ b/rcl/src/rcl/init_options.c @@ -139,6 +139,14 @@ rcl_init_options_get_rmw_init_options(rcl_init_options_t * init_options) return &(init_options->impl->rmw_init_options); } +const rcl_allocator_t * +rcl_init_options_get_allocator(const rcl_init_options_t * init_options) +{ + RCL_CHECK_ARGUMENT_FOR_NULL(init_options, NULL); + RCL_CHECK_ARGUMENT_FOR_NULL(init_options->impl, NULL); + return &(init_options->impl->allocator); +} + #ifdef __cplusplus } #endif From 4172497f6c66d9917257ad8235214151b9d474ed Mon Sep 17 00:00:00 2001 From: Ivan Santiago Paunovic Date: Thu, 20 Feb 2020 17:31:40 -0300 Subject: [PATCH 2/3] Update tests Signed-off-by: Ivan Santiago Paunovic --- rcl/test/rcl/test_graph.cpp | 7 +++++++ rcl/test/rcl/test_logging_rosout.cpp | 10 +++++++++- 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/rcl/test/rcl/test_graph.cpp b/rcl/test/rcl/test_graph.cpp index d4f923fb8..d72a08997 100644 --- a/rcl/test/rcl/test_graph.cpp +++ b/rcl/test/rcl/test_graph.cpp @@ -30,6 +30,7 @@ #include "rcl/error_handling.h" #include "rcl/graph.h" +#include "rcl/logging.h" #include "rcl/rcl.h" #include "rcutils/logging_macros.h" @@ -66,6 +67,7 @@ class CLASSNAME (TestGraphFixture, RMW_IMPLEMENTATION) : public ::testing::Test void SetUp() { rcl_ret_t ret; + rcl_allocator_t allocator = rcl_get_default_allocator(); rcl_init_options_t init_options = rcl_get_zero_initialized_init_options(); ret = rcl_init_options_init(&init_options, rcl_get_default_allocator()); ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; @@ -77,6 +79,10 @@ class CLASSNAME (TestGraphFixture, RMW_IMPLEMENTATION) : public ::testing::Test *this->old_context_ptr = rcl_get_zero_initialized_context(); ret = rcl_init(0, nullptr, &init_options, this->old_context_ptr); ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + EXPECT_EQ( + RCL_RET_OK, + rcl_logging_configure(&this->old_context_ptr->global_arguments, &allocator) + ) << rcl_get_error_string().str; this->old_node_ptr = new rcl_node_t; *this->old_node_ptr = rcl_get_zero_initialized_node(); const char * old_name = "old_node_name"; @@ -127,6 +133,7 @@ class CLASSNAME (TestGraphFixture, RMW_IMPLEMENTATION) : public ::testing::Test ret = rcl_context_fini(this->old_context_ptr); EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; delete this->old_context_ptr; + EXPECT_EQ(RCL_RET_OK, rcl_logging_fini()) << rcl_get_error_string().str; } }; diff --git a/rcl/test/rcl/test_logging_rosout.cpp b/rcl/test/rcl/test_logging_rosout.cpp index 0c87058b6..8816a55a9 100644 --- a/rcl/test/rcl/test_logging_rosout.cpp +++ b/rcl/test/rcl/test_logging_rosout.cpp @@ -19,6 +19,7 @@ #include "osrf_testing_tools_cpp/scope_exit.hpp" #include "rcl/error_handling.h" +#include "rcl/logging.h" #include "rcl/rcl.h" #include "rcl/subscription.h" #include "rcl_interfaces/msg/log.h" @@ -70,8 +71,9 @@ class TEST_FIXTURE_P_RMW (TestLoggingRosoutFixture) { auto param = GetParam(); rcl_ret_t ret; + rcl_allocator_t allocator = rcl_get_default_allocator(); rcl_init_options_t init_options = rcl_get_zero_initialized_init_options(); - ret = rcl_init_options_init(&init_options, rcl_get_default_allocator()); + ret = rcl_init_options_init(&init_options, allocator); ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; OSRF_TESTING_TOOLS_CPP_SCOPE_EXIT( { @@ -83,6 +85,11 @@ class TEST_FIXTURE_P_RMW (TestLoggingRosoutFixture) ret = rcl_init(param.argc, param.argv, &init_options, this->context_ptr); ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + EXPECT_EQ( + RCL_RET_OK, + rcl_logging_configure(&this->context_ptr->global_arguments, &allocator) + ) << rcl_get_error_string().str; + // create node rcl_node_options_t node_options = rcl_node_get_default_options(); if (!param.enable_node_option_rosout) { @@ -120,6 +127,7 @@ class TEST_FIXTURE_P_RMW (TestLoggingRosoutFixture) ret = rcl_context_fini(this->context_ptr); delete this->context_ptr; EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + EXPECT_EQ(RCL_RET_OK, rcl_logging_fini()) << rcl_get_error_string().str; } protected: From 91367126a25c1bf342d24d674f4f83d7dfcf41b1 Mon Sep 17 00:00:00 2001 From: Ivan Santiago Paunovic Date: Wed, 22 Apr 2020 18:19:40 -0300 Subject: [PATCH 3/3] Add docblock Signed-off-by: Ivan Santiago Paunovic --- rcl/include/rcl/init_options.h | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/rcl/include/rcl/init_options.h b/rcl/include/rcl/init_options.h index 5956012c7..1d33a0739 100644 --- a/rcl/include/rcl/init_options.h +++ b/rcl/include/rcl/init_options.h @@ -144,7 +144,7 @@ rcl_init_options_fini(rcl_init_options_t * init_options); * Lock-Free | Yes * * \param[in] init_options object from which the rmw init options should be retrieved - * \return pointer to the the rmw init options, or + * \return pointer to the the rcl init options, or * \return `NULL` if there was an error */ RCL_PUBLIC @@ -152,6 +152,26 @@ RCL_WARN_UNUSED rmw_init_options_t * rcl_init_options_get_rmw_init_options(rcl_init_options_t * init_options); +/// Return the allocator stored in the init_options. +/** + * This function can fail and return `NULL` if: + * - init_options is NULL + * - init_options is invalid, e.g. init_options->impl is NULL + * + * If NULL is returned an error message will have been set. + * + *
+ * Attribute | Adherence + * ------------------ | ------------- + * Allocates Memory | No + * Thread-Safe | Yes + * Uses Atomics | No + * Lock-Free | Yes + * + * \param[in] init_options object from which the allocator should be retrieved + * \return pointer to the rcl allocator, or + * \return `NULL` if there was an error + */ RCL_PUBLIC RCL_WARN_UNUSED const rcl_allocator_t *