diff --git a/rcl/include/rcl/context.h b/rcl/include/rcl/context.h index 805b5b39c..4a40f4466 100644 --- a/rcl/include/rcl/context.h +++ b/rcl/include/rcl/context.h @@ -314,22 +314,15 @@ RCL_WARN_UNUSED rmw_context_t * rcl_context_get_rmw_context(rcl_context_t * context); -/// Returns the thread attribute context. +/// Returns pointer to the thread attribute list. /** - * \param[in] context from which the thread attribute should be retrieved. - * \param[out] thread_attrs output variable where the thread attribute will be returned. - * \return #RCL_RET_INVALID_ARGUMENT if `context` is invalid (see rcl_context_is_valid()), or - * \return #RCL_RET_INVALID_ARGUMENT if `context->impl` is `NULL`, or - * \return #RCL_RET_INVALID_ARGUMENT if `*thread_attrs` is not `NULL`, or - * \return #RCL_RET_INVALID_ARGUMENT if `context->impl->thread_context.thread_attrs` is `NULL`, or - * \return #RCL_RET_OK if the thread attribute was correctly retrieved. + * \param[in] context object from which the thread attribute list should be retrieved. + * \return pointer to thread attribute list if valid, otherwise `NULL` */ RCL_PUBLIC RCL_WARN_UNUSED -rcl_ret_t -rcl_context_get_thread_attrs( - const rcl_context_t * context, - rcutils_thread_attrs_t ** thread_attrs); +rcutils_thread_attrs_t * +rcl_context_get_thread_attrs(const rcl_context_t * context); #ifdef __cplusplus } diff --git a/rcl/src/rcl/arguments.c b/rcl/src/rcl/arguments.c index 64627388f..2ed84b76b 100644 --- a/rcl/src/rcl/arguments.c +++ b/rcl/src/rcl/arguments.c @@ -581,18 +581,18 @@ rcl_parse_arguments( rcl_parse_yaml_thread_attrs_value(argv[i + 1], &args_impl->thread_attrs)) { RCUTILS_LOG_DEBUG_NAMED( - ROS_PACKAGE_NAME, "Got thread attribute rule : %s\n", argv[i + 1]); + ROS_PACKAGE_NAME, "Got thread attributes value : %s\n", argv[i + 1]); ++i; // Skip flag here, for loop will skip rule. continue; } rcl_error_string_t prev_error_string = rcl_get_error_string(); rcl_reset_error(); RCL_SET_ERROR_MSG_WITH_FORMAT_STRING( - "Couldn't parse thread attribute rule: '%s %s'. Error: %s", argv[i], argv[i + 1], + "Couldn't parse thread attributes value: '%s %s'. Error: %s", argv[i], argv[i + 1], prev_error_string.str); } else { RCL_SET_ERROR_MSG_WITH_FORMAT_STRING( - "Couldn't parse trailing %s flag. No thread attribute rule found.", argv[i]); + "Couldn't parse trailing %s flag. No thread attributes value found.", argv[i]); } ret = RCL_RET_INVALID_ROS_ARGS; goto fail; @@ -615,13 +615,15 @@ rcl_parse_arguments( RCL_RET_OK == rcl_parse_yaml_thread_attrs_file( argv[i + 1], &args_impl->thread_attrs)) { + RCUTILS_LOG_DEBUG_NAMED( + ROS_PACKAGE_NAME, "Got thread attributes file : %s\n", argv[i + 1]); ++i; // Skip flag here, for loop will skip rule. continue; } rcl_error_string_t prev_error_string = rcl_get_error_string(); rcl_reset_error(); RCL_SET_ERROR_MSG_WITH_FORMAT_STRING( - "Couldn't parse thread attr file: '%s %s'. Error: %s", argv[i], argv[i + 1], + "Couldn't parse thread attributes file: '%s %s'. Error: %s", argv[i], argv[i + 1], prev_error_string.str); } else { RCL_SET_ERROR_MSG_WITH_FORMAT_STRING( @@ -1000,6 +1002,18 @@ rcl_arguments_copy( return RCL_RET_BAD_ALLOC; } args_out->impl->enclave = enclave_copy; + + // Copy thread attributes + if (0 < args->impl->thread_attrs.num_attributes) { + rcutils_ret_t thread_attrs_ret = + rcutils_thread_attrs_copy(&args->impl->thread_attrs, &args_out->impl->thread_attrs); + if (RCUTILS_RET_OK != thread_attrs_ret) { + if (RCL_RET_OK != rcl_arguments_fini(args_out)) { + RCL_SET_ERROR_MSG("Error while finalizing arguments due to another error"); + } + return RCL_RET_BAD_ALLOC; + } + } return RCL_RET_OK; } diff --git a/rcl/src/rcl/context.c b/rcl/src/rcl/context.c index c30497857..cc8477b9e 100644 --- a/rcl/src/rcl/context.c +++ b/rcl/src/rcl/context.c @@ -106,21 +106,20 @@ rcl_context_get_rmw_context(rcl_context_t * context) return &(context->impl->rmw_context); } -rcl_ret_t -rcl_context_get_thread_attrs( - const rcl_context_t * context, - rcutils_thread_attrs_t ** thread_attrs) +rcutils_thread_attrs_t * +rcl_context_get_thread_attrs(const rcl_context_t * context) { - RCL_CHECK_ARGUMENT_FOR_NULL(context, RCL_RET_INVALID_ARGUMENT); - RCL_CHECK_ARGUMENT_FOR_NULL(context->impl, RCL_RET_INVALID_ARGUMENT); - RCL_CHECK_ARGUMENT_FOR_NULL(thread_attrs, RCL_RET_INVALID_ARGUMENT); + RCL_CHECK_ARGUMENT_FOR_NULL(context, NULL); + RCL_CHECK_FOR_NULL_WITH_MSG(context->impl, "context is zero-initialized", return NULL); - if (0 < context->impl->thread_attrs.num_attributes) { - *thread_attrs = &context->impl->thread_attrs; - return RCL_RET_OK; - } else { - return RCL_RET_ERROR; + rcutils_thread_attrs_t * attrs = NULL; + rcl_ret_t ret = rcl_arguments_get_thread_attrs(&context->global_arguments, &attrs); + if (RCL_RET_OK != ret) { + if (0 < context->impl->thread_attrs.num_attributes) { + attrs = &context->impl->thread_attrs; + } } + return attrs; } rcl_ret_t diff --git a/rcl/test/CMakeLists.txt b/rcl/test/CMakeLists.txt index 1b73ab5e6..ae128aace 100644 --- a/rcl/test/CMakeLists.txt +++ b/rcl/test/CMakeLists.txt @@ -81,6 +81,7 @@ function(test_target_function) SRCS rcl/test_context.cpp ENV ${rmw_implementation_env_var} ${memory_tools_ld_preload_env_var} APPEND_LIBRARY_DIRS ${extra_lib_dirs} + INCLUDE_DIRS ${CMAKE_CURRENT_SOURCE_DIR}/../src/rcl/ LIBRARIES ${PROJECT_NAME} mimick osrf_testing_tools_cpp::memory_tools AMENT_DEPENDENCIES ${rmw_implementation} ) diff --git a/rcl/test/rcl/test_arguments.cpp b/rcl/test/rcl/test_arguments.cpp index 02f8371a7..c3a34897d 100644 --- a/rcl/test/rcl/test_arguments.cpp +++ b/rcl/test/rcl/test_arguments.cpp @@ -201,6 +201,16 @@ TEST_F(CLASSNAME(TestArgumentsFixture, RMW_IMPLEMENTATION), check_known_vs_unkno EXPECT_FALSE(are_known_ros_args({"--ros-args", "stdout-logs"})); EXPECT_FALSE(are_known_ros_args({"--ros-args", "external-lib-logs"})); EXPECT_FALSE(are_known_ros_args({"--ros-args", "external-lib-logs"})); + + // Thread attributes + EXPECT_TRUE( + are_known_ros_args( + {"--ros-args", "--thread-attrs-file", + (test_path / "test_thread_attrs.yaml").string().c_str()})); + EXPECT_TRUE( + are_known_ros_args( + {"--ros-args", "--thread-attrs-value", + "[{priority: 10, scheduling_policy: FIFO, name: thread-1, core_affinity: 1}]"})); } bool @@ -226,7 +236,9 @@ TEST_F(CLASSNAME(TestArgumentsFixture, RMW_IMPLEMENTATION), check_valid_vs_inval { "--ros-args", "-p", "foo:=bar", "-r", "__node:=node_name", "--params-file", parameters_filepath.c_str(), "--log-level", "INFO", - "--log-config-file", "file.config" + "--log-config-file", "file.config", + "--thread-attrs-value", + "[{priority: 10, scheduling_policy: IDLE, name: thread-1, core_affinity: 1}]" })); // ROS args unknown to rcl are not (necessarily) invalid diff --git a/rcl/test/rcl/test_context.cpp b/rcl/test/rcl/test_context.cpp index 31e4100b3..925cac57b 100644 --- a/rcl/test/rcl/test_context.cpp +++ b/rcl/test/rcl/test_context.cpp @@ -24,6 +24,11 @@ #include "../mocking_utils/patch.hpp" +#include "rcutils/thread_attr.h" + +#include "./arguments_impl.h" +#include "./context_impl.h" + #ifdef RMW_IMPLEMENTATION # define CLASSNAME_(NAME, SUFFIX) NAME ## __ ## SUFFIX # define CLASSNAME(NAME, SUFFIX) CLASSNAME_(NAME, SUFFIX) @@ -147,6 +152,45 @@ TEST_F(CLASSNAME(TestContextFixture, RMW_IMPLEMENTATION), nominal) { EXPECT_NE(rmw_context_ptr, nullptr) << rcl_get_error_string().str; rcl_reset_error(); + // test rcl_context_get_thread_attrs + rcutils_thread_attrs_t * thread_attrs; + EXPECT_NO_MEMORY_OPERATIONS( + { + thread_attrs = rcl_context_get_thread_attrs(nullptr); + }); + EXPECT_EQ(nullptr, thread_attrs); + EXPECT_TRUE(rcl_error_is_set()); + rcl_reset_error(); + + EXPECT_NO_MEMORY_OPERATIONS( + { + EXPECT_EQ(nullptr, rcl_context_get_thread_attrs(&context)); + }); + + { + rcutils_thread_attrs_t * arg_attrs = &context.global_arguments.impl->thread_attrs; + rcutils_thread_attrs_t * ctx_attrs = &context.impl->thread_attrs; + rcutils_ret_t ret; + + ret = rcutils_thread_attrs_init(ctx_attrs, rcl_get_default_allocator()); + EXPECT_EQ(RCUTILS_RET_OK, ret); + ret = rcutils_thread_attrs_add_attr( + ctx_attrs, RCUTILS_THREAD_SCHEDULING_POLICY_FIFO, 1, 10, "arg"); + EXPECT_EQ(RCUTILS_RET_OK, ret); + + EXPECT_EQ(ctx_attrs, rcl_context_get_thread_attrs(&context)); + + ret = rcutils_thread_attrs_copy(ctx_attrs, arg_attrs); + EXPECT_EQ(RCUTILS_RET_OK, ret); + + EXPECT_EQ(arg_attrs, rcl_context_get_thread_attrs(&context)); + + ret = rcutils_thread_attrs_fini(ctx_attrs); + EXPECT_EQ(RCUTILS_RET_OK, ret); + ret = rcutils_thread_attrs_fini(arg_attrs); + EXPECT_EQ(RCUTILS_RET_OK, ret); + } + ret = rcl_init_options_fini(&init_options); EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; } diff --git a/rcl/test/resources/test_arguments/test_thread_attrs.yaml b/rcl/test/resources/test_arguments/test_thread_attrs.yaml new file mode 100644 index 000000000..e784b8cac --- /dev/null +++ b/rcl/test/resources/test_arguments/test_thread_attrs.yaml @@ -0,0 +1,8 @@ +- name: thread-1 + priority: 10 + scheduling_policy: FIFO + core_affinity: 1 +- name: thread-2 + priority: 20 + scheduling_policy: RR + core_affinity: 2