From 73718cafb8186d63e65ecdaca12f79e4cefcf97c Mon Sep 17 00:00:00 2001 From: s-morita <32567894+smorita-esol@users.noreply.github.com> Date: Tue, 6 Jun 2023 17:52:00 +0900 Subject: [PATCH 1/7] feat: Thread configuration prototype This is a prototype implementation of RCL for discussion about the thread configuration feature to receive and apply a set of scheduling parameters for the threads controlled by the ROS 2 executor. Our basic idea is as below. 1. Implement a new class rclcpp::thread and modify rclcpp to use it. This class has the same function set as the std::thread but also additional features to control its thread attributions. 2. Modify the rcl layer to receive a set of scheduling parameters. The parameters are described in YAML format and passed via command line parameters, environment variables, or files. 3. the rclcpp reads the parameters from rcl and applies them to each thread in the thread pool. There have been some discussions about this pull request, as below. [ROS Discourse] https://discourse.ros.org/t/adding-thread-attributes-configuration-in-ros-2-framework/30701 [ROS 2 Real-Time Working Group] https://github.com/ros-realtime/ros-realtime.github.io/issues/18 Signed-off-by: Shoji Morita --- rcl/CMakeLists.txt | 1 + rcl/include/rcl/arguments.h | 24 ++ rcl/include/rcl/context.h | 17 + rcl/include/rcl/thread_attr.h | 66 ++++ rcl/include/rcl/types.h | 2 + rcl/src/rcl/arguments.c | 100 ++++++ rcl/src/rcl/arguments_impl.h | 3 + rcl/src/rcl/context.c | 32 ++ rcl/src/rcl/context_impl.h | 3 + rcl/src/rcl/init.c | 32 ++ rcl/src/rcl/thread_attr.c | 90 +++++ rcl_yaml_param_parser/CMakeLists.txt | 2 + .../parser_thread_attr.h | 98 ++++++ .../include/rcl_yaml_param_parser/types.h | 37 +++ rcl_yaml_param_parser/src/impl/parse.h | 24 ++ .../src/impl/parse_thread_attr.h | 61 ++++ rcl_yaml_param_parser/src/impl/types.h | 18 + rcl_yaml_param_parser/src/parse_thread_attr.c | 311 ++++++++++++++++++ .../src/parser_thread_attr.c | 174 ++++++++++ 19 files changed, 1095 insertions(+) create mode 100644 rcl/include/rcl/thread_attr.h create mode 100644 rcl/src/rcl/thread_attr.c create mode 100644 rcl_yaml_param_parser/include/rcl_yaml_param_parser/parser_thread_attr.h create mode 100644 rcl_yaml_param_parser/src/impl/parse_thread_attr.h create mode 100644 rcl_yaml_param_parser/src/parse_thread_attr.c create mode 100644 rcl_yaml_param_parser/src/parser_thread_attr.c diff --git a/rcl/CMakeLists.txt b/rcl/CMakeLists.txt index eef888492..34a860bcf 100644 --- a/rcl/CMakeLists.txt +++ b/rcl/CMakeLists.txt @@ -67,6 +67,7 @@ set(${PROJECT_NAME}_sources src/rcl/service.c src/rcl/service_event_publisher.c src/rcl/subscription.c + src/rcl/thread_attr.c src/rcl/time.c src/rcl/timer.c src/rcl/type_hash.c diff --git a/rcl/include/rcl/arguments.h b/rcl/include/rcl/arguments.h index 44c99f6a9..ee4a0cfcb 100644 --- a/rcl/include/rcl/arguments.h +++ b/rcl/include/rcl/arguments.h @@ -83,6 +83,12 @@ typedef struct rcl_arguments_s /// logging (must be preceded with --enable- or --disable-). #define RCL_LOG_EXT_LIB_FLAG_SUFFIX "external-lib-logs" +/// The ROS flag that precedes the ROS thread attribute file path. +#define RCL_THREAD_ATTRS_FILE_FLAG "--thread-attrs-file" + +/// The ROS flag that precedes the ROS logging thread attribute. +#define RCL_THREAD_ATTRS_VALUE_FLAG "--thread-attrs-value" + /// Return a rcl_arguments_t struct with members initialized to `NULL`. RCL_PUBLIC RCL_WARN_UNUSED @@ -447,6 +453,24 @@ rcl_ret_t rcl_arguments_fini( rcl_arguments_t * args); +/// Return thread attribute parsed from the command line. +/** + * Thread attribute are parsed directly from command line arguments and + * thread attribute files provided in the command line. + * + * \param[in] arguments An arguments structure that has been parsed. + * \param[out] thread_attrs thread attribute as parsed from command line arguments. + * This structure must be finalized by the caller. + * \return #RCL_RET_OK if everything goes correctly, or + * \return #RCL_RET_INVALID_ARGUMENT if any function arguments are invalid, or + */ +RCL_PUBLIC +RCL_WARN_UNUSED +rcl_ret_t +rcl_arguments_get_thread_attrs( + const rcl_arguments_t * arguments, + rcl_thread_attrs_t ** thread_attrs); + #ifdef __cplusplus } #endif diff --git a/rcl/include/rcl/context.h b/rcl/include/rcl/context.h index 68500da81..67a57b05d 100644 --- a/rcl/include/rcl/context.h +++ b/rcl/include/rcl/context.h @@ -314,6 +314,23 @@ RCL_WARN_UNUSED rmw_context_t * rcl_context_get_rmw_context(rcl_context_t * context); +/// Returns the thread attribute context. +/** + * \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. + */ +RCL_PUBLIC +RCL_WARN_UNUSED +rcl_ret_t +rcl_context_get_thread_attrs( + const rcl_context_t * context, + rcl_thread_attrs_t ** thread_attrs); + #ifdef __cplusplus } #endif diff --git a/rcl/include/rcl/thread_attr.h b/rcl/include/rcl/thread_attr.h new file mode 100644 index 000000000..787a69837 --- /dev/null +++ b/rcl/include/rcl/thread_attr.h @@ -0,0 +1,66 @@ +// Copyright 2023 eSOL Co.,Ltd. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +/// @file + +#ifndef RCL__THREAD_ATTR_H_ +#define RCL__THREAD_ATTR_H_ + +#ifdef __cplusplus +extern "C" +{ +#endif + +#include + +#include "rcl/allocator.h" +#include "rcl/macros.h" +#include "rcl/types.h" +#include "rcl/visibility_control.h" +#include "rcl_yaml_param_parser/types.h" + +extern const char * const RCL_THREAD_ATTR_VALUE_ENV_VAR; +extern const char * const RCL_THREAD_ATTR_FILE_ENV_VAR; + +/// Determine the default thread attribute from string, based on the environment. +/// \param[out] thread_attrs Must not be NULL. +/// \param[in] allocator memory allocator to be used +/// \return #RCL_RET_INVALID_ARGUMENT if an argument is invalid, or +/// \return #RCL_RET_ERROR in case of an unexpected error, or +/// \return #RCL_RET_BAD_ALLOC if allocating memory failed, or +/// \return #RCL_RET_OK. +RCL_PUBLIC +rcl_ret_t +rcl_get_default_thread_attrs_from_value( + rcl_thread_attrs_t * thread_attrs, + rcl_allocator_t allocator); + +/// Determine the default thread attribute from file path, based on the environment. +/// \param[out] thread_attrs Must not be NULL. +/// \param[in] allocator memory allocator to be used +/// \return #RCL_RET_INVALID_ARGUMENT if an argument is invalid, or +/// \return #RCL_RET_ERROR in case of an unexpected error, or +/// \return #RCL_RET_BAD_ALLOC if allocating memory failed, or +/// \return #RCL_RET_OK. +RCL_PUBLIC +rcl_ret_t +rcl_get_default_thread_attrs_from_file( + rcl_thread_attrs_t * thread_attrs, + rcl_allocator_t allocator); + +#ifdef __cplusplus +} +#endif + +#endif // RCL__THREAD_ATTR_H_ diff --git a/rcl/include/rcl/types.h b/rcl/include/rcl/types.h index 753fc8eee..e3be92adc 100644 --- a/rcl/include/rcl/types.h +++ b/rcl/include/rcl/types.h @@ -111,6 +111,8 @@ typedef rmw_ret_t rcl_ret_t; #define RCL_RET_INVALID_PARAM_RULE 1010 /// Argument is not a valid log level rule #define RCL_RET_INVALID_LOG_LEVEL_RULE 1020 +/// Argument is not a valid thread attr rule +#define RCL_RET_INVALID_THREAD_ATTRS 1030 // rcl event specific ret codes in 20XX /// Invalid rcl_event_t given return code. diff --git a/rcl/src/rcl/arguments.c b/rcl/src/rcl/arguments.c index 8c3e7c0c7..725c43412 100644 --- a/rcl/src/rcl/arguments.c +++ b/rcl/src/rcl/arguments.c @@ -25,6 +25,7 @@ #include "rcl/lexer_lookahead.h" #include "rcl/validate_topic_name.h" #include "rcl_yaml_param_parser/parser.h" +#include "rcl_yaml_param_parser/parser_thread_attr.h" #include "rcl_yaml_param_parser/types.h" #include "rcutils/allocator.h" #include "rcutils/error_handling.h" @@ -286,6 +287,12 @@ rcl_parse_arguments( goto fail; } + args_impl->thread_attrs = rcl_get_zero_initialized_thread_attrs(); + ret = rcl_thread_attrs_init(&args_impl->thread_attrs, allocator); + if (RCL_RET_OK != ret) { + goto fail; + } + args_impl->parameter_overrides = rcl_yaml_node_struct_init(allocator); if (NULL == args_impl->parameter_overrides) { ret = RCL_RET_BAD_ALLOC; @@ -559,6 +566,73 @@ rcl_parse_arguments( RCL_DISABLE_FLAG_PREFIX, RCL_LOG_EXT_LIB_FLAG_SUFFIX, rcl_get_error_string().str); rcl_reset_error(); + // Attempt to parse argument as thread attribute flag + if (strcmp(RCL_THREAD_ATTRS_VALUE_FLAG, argv[i]) == 0) { + if (i + 1 < argc) { + if (args_impl->thread_attrs.num_attributes != 0) { + RCL_SET_ERROR_MSG_WITH_FORMAT_STRING( + "Thread attributes already set: '%s %s'.", argv[i], argv[i + 1]); + ++i; + continue; + } + // Attempt to parse next argument as thread attribute rule + if (RCL_RET_OK == + 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]); + ++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], + 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]); + } + ret = RCL_RET_INVALID_ROS_ARGS; + goto fail; + } + RCUTILS_LOG_DEBUG_NAMED( + ROS_PACKAGE_NAME, "Arg %d (%s) is not a %s flag.", + i, argv[i], RCL_THREAD_ATTRS_VALUE_FLAG); + + // Attempt to parse argument as thread attribute file rule + if (strcmp(RCL_THREAD_ATTRS_FILE_FLAG, argv[i]) == 0) { + if (i + 1 < argc) { + if (args_impl->thread_attrs.num_attributes != 0) { + RCL_SET_ERROR_MSG_WITH_FORMAT_STRING( + "Thread attributes already setted: '%s %s'.", argv[i], argv[i + 1]); + ++i; + continue; + } + // Attempt to parse next argument as thread attribute file rule + if ( + RCL_RET_OK == rcl_parse_yaml_thread_attrs_file( + argv[i + 1], &args_impl->thread_attrs)) + { + ++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], + prev_error_string.str); + } else { + RCL_SET_ERROR_MSG_WITH_FORMAT_STRING( + "Couldn't parse trailing %s flag. No file path provided.", argv[i]); + } + ret = RCL_RET_INVALID_ROS_ARGS; + goto fail; + } + RCUTILS_LOG_DEBUG_NAMED( + ROS_PACKAGE_NAME, "Arg %d (%s) is not a %s flag.", + i, argv[i], RCL_THREAD_ATTRS_FILE_FLAG); + // Argument is an unknown ROS specific argument args_impl->unparsed_ros_args[args_impl->num_unparsed_ros_args] = i; ++(args_impl->num_unparsed_ros_args); @@ -988,6 +1062,14 @@ rcl_arguments_fini( args->impl->external_log_config_file = NULL; } + rcl_ret_t thread_ret = rcl_thread_attrs_fini(&args->impl->thread_attrs); + if (thread_ret != RCL_RET_OK) { + ret = thread_ret; + RCUTILS_LOG_ERROR_NAMED( + ROS_PACKAGE_NAME, + "Failed to finalize thread attribute while finalizing arguments. Continuing..."); + } + args->impl->allocator.deallocate(args->impl, args->impl->allocator.state); args->impl = NULL; return ret; @@ -2067,11 +2149,29 @@ _rcl_allocate_initialized_arguments_impl(rcl_arguments_t * args, rcl_allocator_t args_impl->log_rosout_disabled = false; args_impl->log_ext_lib_disabled = false; args_impl->enclave = NULL; + args_impl->thread_attrs = rcl_get_zero_initialized_thread_attrs(); args_impl->allocator = *allocator; return RCL_RET_OK; } +rcl_ret_t +rcl_arguments_get_thread_attrs( + const rcl_arguments_t * arguments, + rcl_thread_attrs_t ** thread_attrs) +{ + RCL_CHECK_ARGUMENT_FOR_NULL(arguments, RCL_RET_INVALID_ARGUMENT); + RCL_CHECK_ARGUMENT_FOR_NULL(arguments->impl, RCL_RET_INVALID_ARGUMENT); + RCL_CHECK_ARGUMENT_FOR_NULL(thread_attrs, RCL_RET_INVALID_ARGUMENT); + + if (0 < arguments->impl->thread_attrs.num_attributes) { + *thread_attrs = &arguments->impl->thread_attrs; + return RCL_RET_OK; + } else { + return RCL_RET_ERROR; + } +} + #ifdef __cplusplus } #endif diff --git a/rcl/src/rcl/arguments_impl.h b/rcl/src/rcl/arguments_impl.h index d3aedbb42..79a0a274f 100644 --- a/rcl/src/rcl/arguments_impl.h +++ b/rcl/src/rcl/arguments_impl.h @@ -51,6 +51,9 @@ struct rcl_arguments_impl_s /// Length of remap_rules. int num_remap_rules; + /// thread attribute. + rcl_thread_attrs_t thread_attrs; + /// Log levels parsed from arguments. rcl_log_levels_t log_levels; /// A file used to configure the external logging library diff --git a/rcl/src/rcl/context.c b/rcl/src/rcl/context.c index 5414cc5e9..2de0175b3 100644 --- a/rcl/src/rcl/context.c +++ b/rcl/src/rcl/context.c @@ -24,6 +24,7 @@ extern "C" #include "./common.h" #include "./context_impl.h" #include "rcutils/stdatomic_helper.h" +#include "rcl_yaml_param_parser/parser_thread_attr.h" rcl_context_t rcl_get_zero_initialized_context(void) @@ -105,6 +106,22 @@ 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, + rcl_thread_attrs_t ** thread_attrs) +{ + RCL_CHECK_ARGUMENT_FOR_NULL(context, RCL_RET_INVALID_ARGUMENT); + RCL_CHECK_ARGUMENT_FOR_NULL(context->impl, RCL_RET_INVALID_ARGUMENT); + + if (0 < context->impl->thread_attrs.num_attributes) { + *thread_attrs = &context->impl->thread_attrs; + return RCL_RET_OK; + } else { + return RCL_RET_ERROR; + } +} + rcl_ret_t __cleanup_context(rcl_context_t * context) { @@ -146,6 +163,21 @@ __cleanup_context(rcl_context_t * context) } } + // clean up thread_attrs_context + rcl_ret_t thread_attrs_context_fini_ret = + rcl_thread_attrs_fini(&(context->impl->thread_attrs)); + if (RCL_RET_OK != thread_attrs_context_fini_ret) { + if (RCL_RET_OK == ret) { + ret = thread_attrs_context_fini_ret; + } + RCUTILS_SAFE_FWRITE_TO_STDERR( + "[rcl|context.c:" RCUTILS_STRINGIFY(__LINE__) + "] failed to finalize attr 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 rmw_context if (NULL != context->impl->rmw_context.implementation_identifier) { rmw_ret_t rmw_context_fini_ret = rmw_context_fini(&(context->impl->rmw_context)); diff --git a/rcl/src/rcl/context_impl.h b/rcl/src/rcl/context_impl.h index 10c9e82cc..eff1d0a66 100644 --- a/rcl/src/rcl/context_impl.h +++ b/rcl/src/rcl/context_impl.h @@ -15,6 +15,7 @@ #ifndef RCL__CONTEXT_IMPL_H_ #define RCL__CONTEXT_IMPL_H_ +#include "rcl_yaml_param_parser/parser_thread_attr.h" #include "rcl/context.h" #include "rcl/error_handling.h" @@ -38,6 +39,8 @@ struct rcl_context_impl_s char ** argv; /// rmw context. rmw_context_t rmw_context; + /// thread attributes. + rcl_thread_attrs_t thread_attrs; }; RCL_LOCAL diff --git a/rcl/src/rcl/init.c b/rcl/src/rcl/init.c index 85a36bf73..ecaa018d4 100644 --- a/rcl/src/rcl/init.c +++ b/rcl/src/rcl/init.c @@ -34,7 +34,9 @@ extern "C" #include "rcl/localhost.h" #include "rcl/logging.h" #include "rcl/security.h" +#include "rcl/thread_attr.h" #include "rcl/validate_enclave_name.h" +#include "rcl_yaml_param_parser/parser_thread_attr.h" #include "./arguments_impl.h" #include "./common.h" @@ -93,6 +95,9 @@ rcl_init( // Zero initialize rmw context first so its validity can by checked in cleanup. context->impl->rmw_context = rmw_get_zero_initialized_context(); + // Zero initialize thread attribute context first so its validity can by checked in cleanup. + context->impl->thread_attrs = rcl_get_zero_initialized_thread_attrs(); + // Store the allocator. context->impl->allocator = allocator; @@ -256,6 +261,33 @@ rcl_init( "\t%s", discovery_options->static_peers[ii].peer_address); } + ret = rcl_thread_attrs_init(&(context->impl->thread_attrs), allocator); + if (RCL_RET_OK != ret) { + fail_ret = ret; + goto fail; + } + + if (0 == context->global_arguments.impl->thread_attrs.num_attributes) { + // Get actual thread attribute based on environment variable. + ret = rcl_get_default_thread_attrs_from_value( + &(context->impl->thread_attrs), + allocator); + if (RCL_RET_OK != ret) { + fail_ret = ret; + goto fail; + } + if (0 == context->impl->thread_attrs.num_attributes) { + // Get actual thread attribute file path based on environment variable. + ret = rcl_get_default_thread_attrs_from_file( + &(context->impl->thread_attrs), + allocator); + if (RCL_RET_OK != ret) { + fail_ret = ret; + goto fail; + } + } + } + if (context->global_arguments.impl->enclave) { context->impl->init_options.impl->rmw_init_options.enclave = rcutils_strdup( context->global_arguments.impl->enclave, diff --git a/rcl/src/rcl/thread_attr.c b/rcl/src/rcl/thread_attr.c new file mode 100644 index 000000000..e5c2781e6 --- /dev/null +++ b/rcl/src/rcl/thread_attr.c @@ -0,0 +1,90 @@ +// Copyright 2023 eSOL Co.,Ltd. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#include + +#include "rcl/thread_attr.h" +#include "rcl/allocator.h" +#include "rcl/error_handling.h" +#include "rcl/macros.h" +#include "rcl_yaml_param_parser/parser_thread_attr.h" +#include "rcl_yaml_param_parser/types.h" +#include "rcutils/env.h" +#include "rcutils/strdup.h" + +const char * const RCL_THREAD_ATTRS_FILE_ENV_VAR = "ROS_THREAD_ATTRS_FILE"; +const char * const RCL_THREAD_ATTRS_VALUE_ENV_VAR = "ROS_THREAD_ATTRS_VALUE"; + +rcl_ret_t +rcl_get_default_thread_attrs_from_value( + rcl_thread_attrs_t * thread_attrs, + rcl_allocator_t allocator) +{ + RCUTILS_CAN_SET_MSG_AND_RETURN_WITH_ERROR_OF(RCL_RET_INVALID_ARGUMENT); + RCUTILS_CAN_SET_MSG_AND_RETURN_WITH_ERROR_OF(RCL_RET_ERROR); + RCUTILS_CAN_SET_MSG_AND_RETURN_WITH_ERROR_OF(RCL_RET_BAD_ALLOC); + + RCL_CHECK_ARGUMENT_FOR_NULL(thread_attrs, RCL_RET_INVALID_ARGUMENT); + RCL_CHECK_ALLOCATOR(&allocator, return RCL_RET_INVALID_ARGUMENT); + + const char * ros_thread_attrs_value = NULL; + const char * get_env_error_str = NULL; + + get_env_error_str = rcutils_get_env(RCL_THREAD_ATTRS_VALUE_ENV_VAR, &ros_thread_attrs_value); + if (NULL != get_env_error_str) { + RCL_SET_ERROR_MSG_WITH_FORMAT_STRING( + "Error getting env var '" RCUTILS_STRINGIFY(RCL_THREAD_ATTRS_VALUE_ENV_VAR) "': %s\n", + get_env_error_str); + return RCL_RET_ERROR; + } + if (ros_thread_attrs_value && strcmp(ros_thread_attrs_value, "") != 0) { + rcl_ret_t ret = rcl_parse_yaml_thread_attrs_value(ros_thread_attrs_value, thread_attrs); + if (RCUTILS_RET_OK != ret) { + return ret; + } + } + return RCL_RET_OK; +} + +rcl_ret_t +rcl_get_default_thread_attrs_from_file( + rcl_thread_attrs_t * thread_attrs, + rcl_allocator_t allocator) +{ + RCUTILS_CAN_SET_MSG_AND_RETURN_WITH_ERROR_OF(RCL_RET_INVALID_ARGUMENT); + RCUTILS_CAN_SET_MSG_AND_RETURN_WITH_ERROR_OF(RCL_RET_ERROR); + RCUTILS_CAN_SET_MSG_AND_RETURN_WITH_ERROR_OF(RCL_RET_BAD_ALLOC); + + RCL_CHECK_ARGUMENT_FOR_NULL(thread_attrs, RCL_RET_INVALID_ARGUMENT); + RCL_CHECK_ALLOCATOR(&allocator, return RCL_RET_INVALID_ARGUMENT); + + const char * ros_thread_attrs_file = NULL; + const char * get_env_error_str = NULL; + + get_env_error_str = + rcutils_get_env(RCL_THREAD_ATTRS_FILE_ENV_VAR, &ros_thread_attrs_file); + if (NULL != get_env_error_str) { + RCL_SET_ERROR_MSG_WITH_FORMAT_STRING( + "Error getting env var '" RCUTILS_STRINGIFY(RCL_THREAD_ATTRS_FILE_ENV_VAR) "': %s\n", + get_env_error_str); + return RCL_RET_ERROR; + } + if (ros_thread_attrs_file && strcmp(ros_thread_attrs_file, "") != 0) { + rcutils_ret_t ret = rcl_parse_yaml_thread_attrs_file(ros_thread_attrs_file, thread_attrs); + if (RCUTILS_RET_OK != ret) { + return ret; + } + } + return RCL_RET_OK; +} diff --git a/rcl_yaml_param_parser/CMakeLists.txt b/rcl_yaml_param_parser/CMakeLists.txt index a5082ad1e..b14b5de8c 100644 --- a/rcl_yaml_param_parser/CMakeLists.txt +++ b/rcl_yaml_param_parser/CMakeLists.txt @@ -28,6 +28,8 @@ add_library( src/node_params.c src/parse.c src/parser.c + src/parse_thread_attr.c + src/parser_thread_attr.c src/yaml_variant.c ) target_include_directories(${PROJECT_NAME} PUBLIC diff --git a/rcl_yaml_param_parser/include/rcl_yaml_param_parser/parser_thread_attr.h b/rcl_yaml_param_parser/include/rcl_yaml_param_parser/parser_thread_attr.h new file mode 100644 index 000000000..0ac148d86 --- /dev/null +++ b/rcl_yaml_param_parser/include/rcl_yaml_param_parser/parser_thread_attr.h @@ -0,0 +1,98 @@ +// Copyright 2023 eSOL Co.,Ltd. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#ifndef RCL_YAML_PARAM_PARSER__PARSER_THREAD_ATTR_H_ +#define RCL_YAML_PARAM_PARSER__PARSER_THREAD_ATTR_H_ + +#include + + +#include "rcl_yaml_param_parser/types.h" +#include "rcl_yaml_param_parser/visibility_control.h" + +#include "rcutils/allocator.h" +#include "rcutils/macros.h" +#include "rcutils/types/rcutils_ret.h" + +#ifdef __cplusplus +extern "C" +{ +#endif + +/// \brief Return a rcl_thread_attrs_t struct with members initialized to zero value. +/// \return a rcl_thread_attrs_t struct with members initialized to zero value. +RCL_YAML_PARAM_PARSER_PUBLIC +rcl_thread_attrs_t +rcl_get_zero_initialized_thread_attrs(void); + +/// \brief Initialize list of thread attributes +/// \param[out] thread_attrs the list of thread attributes to be initialized +/// \param[in] allocator memory allocator to be used +/// \return #RCUTILS_RET_OK if the structure was initialized succeessfully, or +/// \return #RCUTILS_RET_INVALID_ARGUMENT if any function arguments are invalid, or +/// \return #RCUTILS_RET_BAD_ALLOC if allocating memory failed, or +/// \return #RCUTILS_RET_ERROR an unspecified error occur. +RCL_YAML_PARAM_PARSER_PUBLIC +rcutils_ret_t +rcl_thread_attrs_init( + rcl_thread_attrs_t * thread_attrs, + rcutils_allocator_t allocator); + +/// \brief Initialize list of thread attributes with a capacity +/// \param[out] thread_attrs the list of thread attributes to be initialized +/// \param[in] allocator memory allocator to be used +/// \return #RCUTILS_RET_OK if the structure was initialized succeessfully, or +/// \return #RCUTILS_RET_INVALID_ARGUMENT if any function arguments are invalid, or +/// \return #RCUTILS_RET_BAD_ALLOC if allocating memory failed, or +/// \return #RCUTILS_RET_ERROR an unspecified error occur. +RCL_YAML_PARAM_PARSER_PUBLIC +rcutils_ret_t +rcl_thread_attrs_init_with_capacity( + rcl_thread_attrs_t * thread_attrs, + rcutils_allocator_t allocator, + size_t capacity); + +/// \brief Free list of thread attributes +/// \param[in] thread_attrs The structure to be deallocated. +/// \return #RCUTILS_RET_OK if the memory was successfully freed, or +/// \return #RCUTILS_RET_INVALID_ARGUMENT if any function arguments are invalid +RCL_YAML_PARAM_PARSER_PUBLIC +rcutils_ret_t +rcl_thread_attrs_fini( + rcl_thread_attrs_t * thread_attrs); + +/// \brief Parse the YAML file and populate \p thread_attrs +/// \pre Given \p thread_attrs must be a valid thread attribute struct +/// \param[in] file_path is the path to the YAML file +/// \param[inout] thread_attrs points to the struct to be populated +/// \return true on success and false on failure +RCL_YAML_PARAM_PARSER_PUBLIC +rcutils_ret_t rcl_parse_yaml_thread_attrs_file( + const char * file_path, + rcl_thread_attrs_t * thread_attrs); + +/// \brief Parse a thread attribute value as a YAML string, updating thread_attrs accordingly +/// \param[in] yaml_value is the thread attribute value as a YAML string to be parsed +/// \param[inout] thread_attrs points to the thread attribute struct +/// \return true on success and false on failure +RCL_YAML_PARAM_PARSER_PUBLIC +rcutils_ret_t rcl_parse_yaml_thread_attrs_value( + const char * yaml_value, + rcl_thread_attrs_t * thread_attrs); + +#ifdef __cplusplus +} +#endif + +#endif // RCL_YAML_PARAM_PARSER__PARSER_THREAD_ATTR_H_ diff --git a/rcl_yaml_param_parser/include/rcl_yaml_param_parser/types.h b/rcl_yaml_param_parser/include/rcl_yaml_param_parser/types.h index e0e4b5f1c..b7bad0b46 100644 --- a/rcl_yaml_param_parser/include/rcl_yaml_param_parser/types.h +++ b/rcl_yaml_param_parser/include/rcl_yaml_param_parser/types.h @@ -108,4 +108,41 @@ typedef struct rcl_params_s rcutils_allocator_t allocator; ///< Allocator used } rcl_params_t; +typedef enum rcl_thread_scheduling_policy_type_e +{ + RCL_THREAD_SCHEDULING_POLICY_UNKNOWN = 0, + RCL_THREAD_SCHEDULING_POLICY_FIFO = 1, + RCL_THREAD_SCHEDULING_POLICY_RR = 2, + RCL_THREAD_SCHEDULING_POLICY_SPORADIC = 3, + RCL_THREAD_SCHEDULING_POLICY_OTHER = 4, + RCL_THREAD_SCHEDULING_POLICY_IDLE = 5, + RCL_THREAD_SCHEDULING_POLICY_BATCH = 6, + RCL_THREAD_SCHEDULING_POLICY_DEADLINE = 7 +} rcl_thread_scheduling_policy_type_t; + +typedef struct rcl_thread_attr_s +{ + /// Thread core affinity + int core_affinity; + /// Thread scheduling policy. + rcl_thread_scheduling_policy_type_t scheduling_policy; + /// Thread priority. + int priority; + /// Thread name + char * name; +} rcl_thread_attr_t; + +/// Hold thread attribute rules. +typedef struct rcl_thread_attrs_s +{ + /// Private implementation array. + rcl_thread_attr_t * attributes; + /// Number of threads attribute + size_t num_attributes; + /// Number of threads attribute capacity + size_t capacity_attributes; + /// Allocator used to allocate objects in this struct + rcutils_allocator_t allocator; +} rcl_thread_attrs_t; + #endif // RCL_YAML_PARAM_PARSER__TYPES_H_ diff --git a/rcl_yaml_param_parser/src/impl/parse.h b/rcl_yaml_param_parser/src/impl/parse.h index 99f5d28e7..87724f9ec 100644 --- a/rcl_yaml_param_parser/src/impl/parse.h +++ b/rcl_yaml_param_parser/src/impl/parse.h @@ -90,6 +90,30 @@ rcutils_ret_t find_parameter( rcl_params_t * param_st, size_t * parameter_idx); +RCL_YAML_PARAM_PARSER_LOCAL +RCUTILS_WARN_UNUSED +rcutils_ret_t parse_thread_attr_key( + const char * value, + thread_attr_key_type_t * key_type); + +RCL_YAML_PARAM_PARSER_LOCAL +RCUTILS_WARN_UNUSED +rcl_thread_scheduling_policy_type_t parse_thread_attr_scheduling_policy( + const char * value); + +RCL_YAML_PARAM_PARSER_LOCAL +RCUTILS_WARN_UNUSED +rcutils_ret_t parse_thread_attr( + yaml_parser_t * parser, + rcl_thread_attr_t * attr, + rcutils_allocator_t allocator); + +RCL_YAML_PARAM_PARSER_PUBLIC +RCUTILS_WARN_UNUSED +rcutils_ret_t parse_thread_attr_events( + yaml_parser_t * parser, + rcl_thread_attrs_t * thread_attrs); + #ifdef __cplusplus } #endif diff --git a/rcl_yaml_param_parser/src/impl/parse_thread_attr.h b/rcl_yaml_param_parser/src/impl/parse_thread_attr.h new file mode 100644 index 000000000..e039853a1 --- /dev/null +++ b/rcl_yaml_param_parser/src/impl/parse_thread_attr.h @@ -0,0 +1,61 @@ +// Copyright 2023 eSOL Co.,Ltd. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#ifndef IMPL__PARSE_THREAD_ATTR_H_ +#define IMPL__PARSE_THREAD_ATTR_H_ + +#include + +#include "rcutils/allocator.h" +#include "rcutils/macros.h" +#include "rcutils/types/rcutils_ret.h" + +#include "./types.h" +#include "rcl_yaml_param_parser/types.h" +#include "rcl_yaml_param_parser/visibility_control.h" + +#ifdef __cplusplus +extern "C" +{ +#endif + +RCL_YAML_PARAM_PARSER_LOCAL +RCUTILS_WARN_UNUSED +rcutils_ret_t parse_thread_attr_key( + const char * value, + thread_attr_key_type_t * key_type); + +RCL_YAML_PARAM_PARSER_LOCAL +RCUTILS_WARN_UNUSED +rcl_thread_scheduling_policy_type_t parse_thread_attr_scheduling_policy( + const char * value); + +RCL_YAML_PARAM_PARSER_LOCAL +RCUTILS_WARN_UNUSED +rcutils_ret_t parse_thread_attr( + yaml_parser_t * parser, + rcl_thread_attr_t * attr, + rcutils_allocator_t allocator); + +RCL_YAML_PARAM_PARSER_PUBLIC +RCUTILS_WARN_UNUSED +rcutils_ret_t parse_thread_attr_events( + yaml_parser_t * parser, + rcl_thread_attrs_t * thread_attrs); + +#ifdef __cplusplus +} +#endif + +#endif // IMPL__PARSE_THREAD_ATTR_H_ diff --git a/rcl_yaml_param_parser/src/impl/types.h b/rcl_yaml_param_parser/src/impl/types.h index 4776dc128..f7f0c27ee 100644 --- a/rcl_yaml_param_parser/src/impl/types.h +++ b/rcl_yaml_param_parser/src/impl/types.h @@ -63,6 +63,24 @@ typedef struct namespace_tracker_s uint32_t num_parameter_ns; } namespace_tracker_t; +typedef enum thread_attr_key_type_e +{ + THREAD_ATTR_KEY_CORE_AFFINITY = 1, + THREAD_ATTR_KEY_SCHEDULING_POLICY = 2, + THREAD_ATTR_KEY_PRIORITY = 4, + THREAD_ATTR_KEY_NAME = 8 +} thread_attr_key_type_t; + +typedef enum thread_attr_key_bits_e +{ + THREAD_ATTR_KEY_BITS_NONE = 0, + THREAD_ATTR_KEY_BITS_ALL = + THREAD_ATTR_KEY_CORE_AFFINITY | + THREAD_ATTR_KEY_SCHEDULING_POLICY | + THREAD_ATTR_KEY_PRIORITY | + THREAD_ATTR_KEY_NAME +} thread_attr_key_bits_t; + #ifdef __cplusplus } #endif diff --git a/rcl_yaml_param_parser/src/parse_thread_attr.c b/rcl_yaml_param_parser/src/parse_thread_attr.c new file mode 100644 index 000000000..ff9f18431 --- /dev/null +++ b/rcl_yaml_param_parser/src/parse_thread_attr.c @@ -0,0 +1,311 @@ +// Copyright 2023 eSOL Co.,Ltd. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#include +#include + +#include + +#include "rcl_yaml_param_parser/parser_thread_attr.h" + +#include "rcutils/allocator.h" +#include "rcutils/error_handling.h" +#include "rcutils/format_string.h" +#include "rcutils/strdup.h" +#include "rcutils/types/rcutils_ret.h" + +#include "./impl/parse.h" // to use get_value +#include "./impl/parse_thread_attr.h" + +#define PARSE_WITH_CHECK_ERROR(out_event) \ + do { \ + int _parse_ret_; \ + _parse_ret_ = yaml_parser_parse(parser, (out_event)); \ + if (0 == _parse_ret_) { \ + RCUTILS_SET_ERROR_MSG("Failed to parse thread attributes"); \ + ret = RCUTILS_RET_ERROR; \ + goto error; \ + } \ + } while (0) + +#define PARSE_WITH_EVENT_CHECK(event_type) \ + do { \ + yaml_event_t _event_; \ + int _parse_ret_; \ + _parse_ret_ = yaml_parser_parse(parser, &_event_); \ + if (0 == _parse_ret_) { \ + RCUTILS_SET_ERROR_MSG("Failed to parse thread attributes"); \ + ret = RCUTILS_RET_ERROR; \ + goto error; \ + } \ + if (_event_.type != (event_type)) { \ + RCUTILS_SET_ERROR_MSG("Unexpected element in a configuration of thread attributes"); \ + ret = RCUTILS_RET_ERROR; \ + goto error; \ + } \ + } while (0) + +/// +/// Parse the key part of a thread attribute +/// +rcutils_ret_t +parse_thread_attr_key( + const char * str, + thread_attr_key_type_t * key_type) +{ + rcutils_ret_t ret; + if (strcmp(str, "core_affinity") == 0) { + *key_type = THREAD_ATTR_KEY_CORE_AFFINITY; + } else if (strcmp(str, "priority") == 0) { + *key_type = THREAD_ATTR_KEY_PRIORITY; + } else if (strcmp(str, "scheduling_policy") == 0) { + *key_type = THREAD_ATTR_KEY_SCHEDULING_POLICY; + } else if (strcmp(str, "name") == 0) { + *key_type = THREAD_ATTR_KEY_NAME; + } else if (*str == '\0') { + RCUTILS_SET_ERROR_MSG("empty name for a thread attribute"); + ret = RCUTILS_RET_ERROR; + goto error; + } else { + RCUTILS_SET_ERROR_MSG_WITH_FORMAT_STRING("unrecognized key for a thread attribute: %s", str); + ret = RCUTILS_RET_ERROR; + goto error; + } + return RCUTILS_RET_OK; + +error: + return ret; +} + +/// +/// Parse the value of the scheduling policy of a thread attribute +/// +rcl_thread_scheduling_policy_type_t parse_thread_attr_scheduling_policy( + const char * value) +{ + rcl_thread_scheduling_policy_type_t ret; + if (strcmp(value, "FIFO") == 0) { + ret = RCL_THREAD_SCHEDULING_POLICY_FIFO; + } else if (strcmp(value, "RR") == 0) { + ret = RCL_THREAD_SCHEDULING_POLICY_RR; + } else if (strcmp(value, "SPORADIC") == 0) { + ret = RCL_THREAD_SCHEDULING_POLICY_SPORADIC; + } else if (strcmp(value, "OTHER") == 0) { + ret = RCL_THREAD_SCHEDULING_POLICY_OTHER; + } else if (strcmp(value, "IDLE") == 0) { + ret = RCL_THREAD_SCHEDULING_POLICY_IDLE; + } else if (strcmp(value, "BATCH") == 0) { + ret = RCL_THREAD_SCHEDULING_POLICY_BATCH; + } else if (strcmp(value, "DEADLINE") == 0) { + ret = RCL_THREAD_SCHEDULING_POLICY_DEADLINE; + } else { + ret = RCL_THREAD_SCHEDULING_POLICY_UNKNOWN; + } + return ret; +} + +/// +/// parse a thread attribute YAML value string and process them +/// +rcutils_ret_t parse_thread_attr( + yaml_parser_t * parser, + rcl_thread_attr_t * attr, + rcutils_allocator_t allocator) +{ + rcutils_ret_t ret; + yaml_event_t event; + thread_attr_key_bits_t key_bits = THREAD_ATTR_KEY_BITS_NONE; + + while (1) { + PARSE_WITH_CHECK_ERROR(&event); + + if (YAML_MAPPING_END_EVENT == event.type) { + break; + } + if (YAML_SCALAR_EVENT != event.type) { + RCUTILS_SET_ERROR_MSG("Unexpected element in a configuration of thread attributes"); + ret = RCUTILS_RET_ERROR; + goto error; + } + + const char * str = (char *)event.data.scalar.value; + thread_attr_key_type_t key_type; + ret = parse_thread_attr_key(str, &key_type); + if (RCUTILS_RET_OK != ret) { + goto error; + } + if (key_bits & (thread_attr_key_bits_t)key_type) { + RCUTILS_SET_ERROR_MSG_WITH_FORMAT_STRING("duplicated key for a thread attribute: %s", str); + ret = RCUTILS_RET_ERROR; + goto error; + } + + PARSE_WITH_CHECK_ERROR(&event); + + const char * value = (char *)event.data.scalar.value; + yaml_scalar_style_t style = event.data.scalar.style; + const yaml_char_t * tag = event.data.scalar.tag; + data_types_t val_type; + void * ret_val = NULL; + + switch (key_type) { + case THREAD_ATTR_KEY_CORE_AFFINITY: + ret_val = get_value(value, style, tag, &val_type, allocator); + if (DATA_TYPE_INT64 != val_type) { + RCUTILS_SET_ERROR_MSG_WITH_FORMAT_STRING( + "Unrecognized value for thread core affinity: %s", value); + ret = RCUTILS_RET_ERROR; + goto error; + } + attr->core_affinity = ((int)*(int64_t *)(ret_val)); + break; + case THREAD_ATTR_KEY_PRIORITY: + ret_val = get_value(value, style, tag, &val_type, allocator); + if (DATA_TYPE_INT64 != val_type) { + RCUTILS_SET_ERROR_MSG_WITH_FORMAT_STRING( + "Unrecognized value for thread priority: %s", value); + ret = RCUTILS_RET_ERROR; + goto error; + } + attr->priority = ((int)*(int64_t *)(ret_val)); + break; + case THREAD_ATTR_KEY_SCHEDULING_POLICY: + attr->scheduling_policy = parse_thread_attr_scheduling_policy(value); + break; + case THREAD_ATTR_KEY_NAME: + if (*value == '\0') { + RCUTILS_SET_ERROR_MSG("Empty thread name"); + ret = RCUTILS_RET_ERROR; + goto error; + } + attr->name = rcutils_strdup(value, allocator); + if (NULL == attr->name) { + ret = RCUTILS_RET_BAD_ALLOC; + goto error; + } + break; + } + + if (NULL != ret_val) { + allocator.deallocate(ret_val, allocator.state); + } + + key_bits |= (thread_attr_key_bits_t)key_type; + } + + if (THREAD_ATTR_KEY_BITS_ALL != key_bits) { + RCUTILS_SET_ERROR_MSG("A thread attribute does not have enough parameters"); + ret = RCUTILS_RET_ERROR; + goto error; + } + + return RCUTILS_RET_OK; + +error: + if (NULL != attr->name) { + allocator.deallocate(attr->name, allocator.state); + } + return ret; +} + +static inline rcutils_ret_t extend_thread_attrs_capacity( + rcl_thread_attrs_t * attrs, + size_t new_cap) +{ + size_t cap = attrs->capacity_attributes; + size_t size = cap * sizeof(rcl_thread_attr_t); + size_t new_size = new_cap * sizeof(rcl_thread_attr_t); + rcl_thread_attr_t * new_attrs = attrs->allocator.reallocate( + attrs->attributes, new_size, attrs->allocator.state); + + if (NULL == new_attrs) { + RCUTILS_SET_ERROR_MSG("Failed to allocate memory for thread attributes"); + return RCUTILS_RET_BAD_ALLOC; + } + + memset(new_attrs + cap, 0, new_size - size); + + attrs->capacity_attributes = new_cap; + attrs->attributes = new_attrs; + + return RCUTILS_RET_OK; +} + +/// +/// Get events from parsing thread attributes YAML value string and process them +/// +rcutils_ret_t parse_thread_attr_events( + yaml_parser_t * parser, + rcl_thread_attrs_t * thread_attrs) +{ + yaml_event_t event; + rcutils_ret_t ret; + + RCUTILS_CHECK_ARGUMENT_FOR_NULL(parser, RCUTILS_RET_INVALID_ARGUMENT); + RCUTILS_CHECK_ARGUMENT_FOR_NULL(thread_attrs, RCUTILS_RET_INVALID_ARGUMENT); + + PARSE_WITH_EVENT_CHECK(YAML_STREAM_START_EVENT); + PARSE_WITH_EVENT_CHECK(YAML_DOCUMENT_START_EVENT); + PARSE_WITH_EVENT_CHECK(YAML_SEQUENCE_START_EVENT); + + while (1) { + PARSE_WITH_CHECK_ERROR(&event); + + if (YAML_SEQUENCE_END_EVENT == event.type) { + break; + } else if (YAML_MAPPING_START_EVENT != event.type) { + RCUTILS_SET_ERROR_MSG("Unexpected element in a configuration of thread attributes"); + ret = RCUTILS_RET_ERROR; + goto error; + } + + if (thread_attrs->num_attributes == thread_attrs->capacity_attributes) { + size_t new_cap = 0; + if (0 == thread_attrs->capacity_attributes) { + new_cap = 1; + } else { + new_cap = thread_attrs->capacity_attributes * 2; + } + // Extend the capacity + ret = extend_thread_attrs_capacity(thread_attrs, new_cap); + if (RCUTILS_RET_OK != ret) { + goto error; + } + } + + rcl_thread_attr_t * attr = thread_attrs->attributes + thread_attrs->num_attributes; + ret = parse_thread_attr(parser, attr, thread_attrs->allocator); + if (RCUTILS_RET_OK != ret) { + goto error; + } + + ++thread_attrs->num_attributes; + } + PARSE_WITH_EVENT_CHECK(YAML_DOCUMENT_END_EVENT); + PARSE_WITH_EVENT_CHECK(YAML_STREAM_END_EVENT); + + if (0 == thread_attrs->num_attributes) { + RCUTILS_SET_ERROR_MSG("No thread attributes."); + ret = RCUTILS_RET_ERROR; + goto error; + } + + return RCUTILS_RET_OK; + +error: + if (0 < thread_attrs->capacity_attributes) { + rcl_thread_attrs_fini(thread_attrs); + } + return ret; +} diff --git a/rcl_yaml_param_parser/src/parser_thread_attr.c b/rcl_yaml_param_parser/src/parser_thread_attr.c new file mode 100644 index 000000000..6281ce183 --- /dev/null +++ b/rcl_yaml_param_parser/src/parser_thread_attr.c @@ -0,0 +1,174 @@ +// Copyright 2023 eSOL Co.,Ltd. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#include +#include +#include +#include + +#include + +#include "rcl_yaml_param_parser/parser_thread_attr.h" +#include "rcl_yaml_param_parser/types.h" + +#include "rcutils/allocator.h" +#include "rcutils/error_handling.h" +#include "rcutils/types/rcutils_ret.h" + +#include "./impl/types.h" +#include "./impl/parse.h" +#include "./impl/parse_thread_attr.h" +#include "./impl/node_params.h" +#include "./impl/yaml_variant.h" + +#define INIT_NUM_THREAD_ATTRIBUTE 0U + +rcl_thread_attrs_t +rcl_get_zero_initialized_thread_attrs(void) +{ + rcl_thread_attrs_t ret = { + NULL, + }; + return ret; +} + +rcutils_ret_t +rcl_thread_attrs_init( + rcl_thread_attrs_t * thread_attrs, + rcutils_allocator_t allocator) +{ + return rcl_thread_attrs_init_with_capacity( + thread_attrs, allocator, INIT_NUM_THREAD_ATTRIBUTE); +} + +rcutils_ret_t +rcl_thread_attrs_init_with_capacity( + rcl_thread_attrs_t * thread_attrs, + rcutils_allocator_t allocator, + size_t capacity) +{ + RCUTILS_CHECK_ARGUMENT_FOR_NULL(thread_attrs, RCUTILS_RET_INVALID_ARGUMENT); + RCUTILS_CHECK_ALLOCATOR_WITH_MSG( + &allocator, "invalid allocator", return RCUTILS_RET_INVALID_ARGUMENT); + + thread_attrs->allocator = allocator; + thread_attrs->num_attributes = 0U; + thread_attrs->capacity_attributes = capacity; + if (capacity > 0) { + thread_attrs->attributes = + allocator.zero_allocate(capacity, sizeof(rcl_thread_attr_t), allocator.state); + if (NULL == thread_attrs->attributes) { + *thread_attrs = rcl_get_zero_initialized_thread_attrs(); + RCUTILS_SET_ERROR_MSG("Failed to allocate memory for thread attributes"); + return RCUTILS_RET_BAD_ALLOC; + } + } + return RCUTILS_RET_OK; +} + +rcutils_ret_t +rcl_thread_attrs_fini(rcl_thread_attrs_t * thread_attrs) +{ + RCUTILS_CHECK_ARGUMENT_FOR_NULL(thread_attrs, RCUTILS_RET_INVALID_ARGUMENT); + rcutils_allocator_t * allocator = &thread_attrs->allocator; + if (NULL == thread_attrs->attributes) { + return RCUTILS_RET_OK; + } + // check the allocator only if attributes is available to avoid checking after zero-initialized + RCUTILS_CHECK_ALLOCATOR(allocator, return RCUTILS_RET_INVALID_ARGUMENT); + for (size_t i = 0; i < thread_attrs->num_attributes; ++i) { + rcl_thread_attr_t * attr = thread_attrs->attributes + i; + if (NULL != attr->name) { + allocator->deallocate(attr->name, allocator->state); + } + } + allocator->deallocate(thread_attrs->attributes, allocator->state); + *thread_attrs = rcl_get_zero_initialized_thread_attrs(); + + return RCUTILS_RET_OK; +} + +/// +/// Parse the YAML file and populate thread_attrs +/// +rcutils_ret_t rcl_parse_yaml_thread_attrs_file( + const char * file_path, + rcl_thread_attrs_t * thread_attrs) +{ + RCUTILS_CHECK_FOR_NULL_WITH_MSG( + file_path, "YAML file path is NULL", return RCUTILS_RET_INVALID_ARGUMENT); + + if (NULL == thread_attrs) { + RCUTILS_SAFE_FWRITE_TO_STDERR("Pass an initialized thread attr structure"); + return RCUTILS_RET_ERROR; + } + + yaml_parser_t parser; + int success = yaml_parser_initialize(&parser); + if (0 == success) { + RCUTILS_SET_ERROR_MSG("Could not initialize the parser"); + return RCUTILS_RET_ERROR; + } + + FILE * yaml_file = fopen(file_path, "r"); + if (NULL == yaml_file) { + yaml_parser_delete(&parser); + RCUTILS_SET_ERROR_MSG("Error opening YAML file"); + return RCUTILS_RET_ERROR; + } + + yaml_parser_set_input_file(&parser, yaml_file); + rcutils_ret_t ret = parse_thread_attr_events(&parser, thread_attrs); + + fclose(yaml_file); + + yaml_parser_delete(&parser); + + return ret; +} + +/// +/// Parse a YAML string and populate thread_attrs +/// +rcutils_ret_t rcl_parse_yaml_thread_attrs_value( + const char * yaml_value, + rcl_thread_attrs_t * thread_attrs) +{ + RCUTILS_CHECK_ARGUMENT_FOR_NULL(yaml_value, false); + + if (0U == strlen(yaml_value)) { + return RCUTILS_RET_ERROR; + } + + if (NULL == thread_attrs) { + RCUTILS_SAFE_FWRITE_TO_STDERR("Pass an initialized thread attr structure"); + return RCUTILS_RET_ERROR; + } + + yaml_parser_t parser; + int success = yaml_parser_initialize(&parser); + if (0 == success) { + RCUTILS_SET_ERROR_MSG("Could not initialize the parser"); + return RCUTILS_RET_ERROR; + } + + yaml_parser_set_input_string( + &parser, (const unsigned char *)yaml_value, strlen(yaml_value)); + + rcutils_ret_t ret = parse_thread_attr_events(&parser, thread_attrs); + + yaml_parser_delete(&parser); + + return ret; +} From e25cf2b939b438c78d1d688923250b2ec007c9c2 Mon Sep 17 00:00:00 2001 From: Shoji Morita Date: Tue, 20 Jun 2023 20:02:58 +0900 Subject: [PATCH 2/7] Decoupled the additional feature from rcl to rcutils, reflecting on the pointing out below. https://github.com/ros2/rclcpp/pull/2205#issuecomment-1593832758 Signed-off-by: Shoji Morita --- rcl/include/rcl/arguments.h | 5 +- rcl/include/rcl/context.h | 2 +- rcl/include/rcl/thread_attr.h | 6 +- rcl/src/rcl/arguments.c | 13 +-- rcl/src/rcl/arguments_impl.h | 2 +- rcl/src/rcl/context.c | 5 +- rcl/src/rcl/context_impl.h | 2 +- rcl/src/rcl/init.c | 4 +- rcl/src/rcl/thread_attr.c | 6 +- .../parser_thread_attr.h | 52 +--------- .../include/rcl_yaml_param_parser/types.h | 37 ------- rcl_yaml_param_parser/src/impl/parse.h | 24 ----- .../src/impl/parse_thread_attr.h | 9 +- rcl_yaml_param_parser/src/parse_thread_attr.c | 97 +++++++------------ .../src/parser_thread_attr.c | 84 ++-------------- 15 files changed, 77 insertions(+), 271 deletions(-) diff --git a/rcl/include/rcl/arguments.h b/rcl/include/rcl/arguments.h index ee4a0cfcb..ee830bfe1 100644 --- a/rcl/include/rcl/arguments.h +++ b/rcl/include/rcl/arguments.h @@ -23,6 +23,7 @@ #include "rcl/types.h" #include "rcl/visibility_control.h" #include "rcl_yaml_param_parser/types.h" +#include "rcutils/thread_attr.h" #ifdef __cplusplus extern "C" @@ -86,7 +87,7 @@ typedef struct rcl_arguments_s /// The ROS flag that precedes the ROS thread attribute file path. #define RCL_THREAD_ATTRS_FILE_FLAG "--thread-attrs-file" -/// The ROS flag that precedes the ROS logging thread attribute. +/// The ROS flag that precedes the ROS thread attribute. #define RCL_THREAD_ATTRS_VALUE_FLAG "--thread-attrs-value" /// Return a rcl_arguments_t struct with members initialized to `NULL`. @@ -469,7 +470,7 @@ RCL_WARN_UNUSED rcl_ret_t rcl_arguments_get_thread_attrs( const rcl_arguments_t * arguments, - rcl_thread_attrs_t ** thread_attrs); + rcutils_thread_attrs_t ** thread_attrs); #ifdef __cplusplus } diff --git a/rcl/include/rcl/context.h b/rcl/include/rcl/context.h index 67a57b05d..805b5b39c 100644 --- a/rcl/include/rcl/context.h +++ b/rcl/include/rcl/context.h @@ -329,7 +329,7 @@ RCL_WARN_UNUSED rcl_ret_t rcl_context_get_thread_attrs( const rcl_context_t * context, - rcl_thread_attrs_t ** thread_attrs); + rcutils_thread_attrs_t ** thread_attrs); #ifdef __cplusplus } diff --git a/rcl/include/rcl/thread_attr.h b/rcl/include/rcl/thread_attr.h index 787a69837..cdf107b1d 100644 --- a/rcl/include/rcl/thread_attr.h +++ b/rcl/include/rcl/thread_attr.h @@ -28,7 +28,7 @@ extern "C" #include "rcl/macros.h" #include "rcl/types.h" #include "rcl/visibility_control.h" -#include "rcl_yaml_param_parser/types.h" +#include "rcutils/thread_attr.h" extern const char * const RCL_THREAD_ATTR_VALUE_ENV_VAR; extern const char * const RCL_THREAD_ATTR_FILE_ENV_VAR; @@ -43,7 +43,7 @@ extern const char * const RCL_THREAD_ATTR_FILE_ENV_VAR; RCL_PUBLIC rcl_ret_t rcl_get_default_thread_attrs_from_value( - rcl_thread_attrs_t * thread_attrs, + rcutils_thread_attrs_t * thread_attrs, rcl_allocator_t allocator); /// Determine the default thread attribute from file path, based on the environment. @@ -56,7 +56,7 @@ rcl_get_default_thread_attrs_from_value( RCL_PUBLIC rcl_ret_t rcl_get_default_thread_attrs_from_file( - rcl_thread_attrs_t * thread_attrs, + rcutils_thread_attrs_t * thread_attrs, rcl_allocator_t allocator); #ifdef __cplusplus diff --git a/rcl/src/rcl/arguments.c b/rcl/src/rcl/arguments.c index 725c43412..64627388f 100644 --- a/rcl/src/rcl/arguments.c +++ b/rcl/src/rcl/arguments.c @@ -33,6 +33,7 @@ #include "rcutils/logging.h" #include "rcutils/logging_macros.h" #include "rcutils/strdup.h" +#include "rcutils/thread_attr.h" #include "rmw/validate_namespace.h" #include "rmw/validate_node_name.h" @@ -287,8 +288,8 @@ rcl_parse_arguments( goto fail; } - args_impl->thread_attrs = rcl_get_zero_initialized_thread_attrs(); - ret = rcl_thread_attrs_init(&args_impl->thread_attrs, allocator); + args_impl->thread_attrs = rcutils_get_zero_initialized_thread_attrs(); + ret = rcutils_thread_attrs_init(&args_impl->thread_attrs, allocator); if (RCL_RET_OK != ret) { goto fail; } @@ -1062,8 +1063,8 @@ rcl_arguments_fini( args->impl->external_log_config_file = NULL; } - rcl_ret_t thread_ret = rcl_thread_attrs_fini(&args->impl->thread_attrs); - if (thread_ret != RCL_RET_OK) { + rcl_ret_t thread_ret = rcutils_thread_attrs_fini(&args->impl->thread_attrs); + if (RCL_RET_OK != thread_ret) { ret = thread_ret; RCUTILS_LOG_ERROR_NAMED( ROS_PACKAGE_NAME, @@ -2149,7 +2150,7 @@ _rcl_allocate_initialized_arguments_impl(rcl_arguments_t * args, rcl_allocator_t args_impl->log_rosout_disabled = false; args_impl->log_ext_lib_disabled = false; args_impl->enclave = NULL; - args_impl->thread_attrs = rcl_get_zero_initialized_thread_attrs(); + args_impl->thread_attrs = rcutils_get_zero_initialized_thread_attrs(); args_impl->allocator = *allocator; return RCL_RET_OK; @@ -2158,7 +2159,7 @@ _rcl_allocate_initialized_arguments_impl(rcl_arguments_t * args, rcl_allocator_t rcl_ret_t rcl_arguments_get_thread_attrs( const rcl_arguments_t * arguments, - rcl_thread_attrs_t ** thread_attrs) + rcutils_thread_attrs_t ** thread_attrs) { RCL_CHECK_ARGUMENT_FOR_NULL(arguments, RCL_RET_INVALID_ARGUMENT); RCL_CHECK_ARGUMENT_FOR_NULL(arguments->impl, RCL_RET_INVALID_ARGUMENT); diff --git a/rcl/src/rcl/arguments_impl.h b/rcl/src/rcl/arguments_impl.h index 79a0a274f..880775993 100644 --- a/rcl/src/rcl/arguments_impl.h +++ b/rcl/src/rcl/arguments_impl.h @@ -52,7 +52,7 @@ struct rcl_arguments_impl_s int num_remap_rules; /// thread attribute. - rcl_thread_attrs_t thread_attrs; + rcutils_thread_attrs_t thread_attrs; /// Log levels parsed from arguments. rcl_log_levels_t log_levels; diff --git a/rcl/src/rcl/context.c b/rcl/src/rcl/context.c index 2de0175b3..c30497857 100644 --- a/rcl/src/rcl/context.c +++ b/rcl/src/rcl/context.c @@ -109,10 +109,11 @@ rcl_context_get_rmw_context(rcl_context_t * context) rcl_ret_t rcl_context_get_thread_attrs( const rcl_context_t * context, - rcl_thread_attrs_t ** thread_attrs) + rcutils_thread_attrs_t ** thread_attrs) { 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); if (0 < context->impl->thread_attrs.num_attributes) { *thread_attrs = &context->impl->thread_attrs; @@ -165,7 +166,7 @@ __cleanup_context(rcl_context_t * context) // clean up thread_attrs_context rcl_ret_t thread_attrs_context_fini_ret = - rcl_thread_attrs_fini(&(context->impl->thread_attrs)); + rcutils_thread_attrs_fini(&(context->impl->thread_attrs)); if (RCL_RET_OK != thread_attrs_context_fini_ret) { if (RCL_RET_OK == ret) { ret = thread_attrs_context_fini_ret; diff --git a/rcl/src/rcl/context_impl.h b/rcl/src/rcl/context_impl.h index eff1d0a66..87c6908bf 100644 --- a/rcl/src/rcl/context_impl.h +++ b/rcl/src/rcl/context_impl.h @@ -40,7 +40,7 @@ struct rcl_context_impl_s /// rmw context. rmw_context_t rmw_context; /// thread attributes. - rcl_thread_attrs_t thread_attrs; + rcutils_thread_attrs_t thread_attrs; }; RCL_LOCAL diff --git a/rcl/src/rcl/init.c b/rcl/src/rcl/init.c index ecaa018d4..6923ce612 100644 --- a/rcl/src/rcl/init.c +++ b/rcl/src/rcl/init.c @@ -96,7 +96,7 @@ rcl_init( context->impl->rmw_context = rmw_get_zero_initialized_context(); // Zero initialize thread attribute context first so its validity can by checked in cleanup. - context->impl->thread_attrs = rcl_get_zero_initialized_thread_attrs(); + context->impl->thread_attrs = rcutils_get_zero_initialized_thread_attrs(); // Store the allocator. context->impl->allocator = allocator; @@ -261,7 +261,7 @@ rcl_init( "\t%s", discovery_options->static_peers[ii].peer_address); } - ret = rcl_thread_attrs_init(&(context->impl->thread_attrs), allocator); + ret = rcutils_thread_attrs_init(&(context->impl->thread_attrs), allocator); if (RCL_RET_OK != ret) { fail_ret = ret; goto fail; diff --git a/rcl/src/rcl/thread_attr.c b/rcl/src/rcl/thread_attr.c index e5c2781e6..4933046a9 100644 --- a/rcl/src/rcl/thread_attr.c +++ b/rcl/src/rcl/thread_attr.c @@ -19,16 +19,16 @@ #include "rcl/error_handling.h" #include "rcl/macros.h" #include "rcl_yaml_param_parser/parser_thread_attr.h" -#include "rcl_yaml_param_parser/types.h" #include "rcutils/env.h" #include "rcutils/strdup.h" +#include "rcutils/thread_attr.h" const char * const RCL_THREAD_ATTRS_FILE_ENV_VAR = "ROS_THREAD_ATTRS_FILE"; const char * const RCL_THREAD_ATTRS_VALUE_ENV_VAR = "ROS_THREAD_ATTRS_VALUE"; rcl_ret_t rcl_get_default_thread_attrs_from_value( - rcl_thread_attrs_t * thread_attrs, + rcutils_thread_attrs_t * thread_attrs, rcl_allocator_t allocator) { RCUTILS_CAN_SET_MSG_AND_RETURN_WITH_ERROR_OF(RCL_RET_INVALID_ARGUMENT); @@ -59,7 +59,7 @@ rcl_get_default_thread_attrs_from_value( rcl_ret_t rcl_get_default_thread_attrs_from_file( - rcl_thread_attrs_t * thread_attrs, + rcutils_thread_attrs_t * thread_attrs, rcl_allocator_t allocator) { RCUTILS_CAN_SET_MSG_AND_RETURN_WITH_ERROR_OF(RCL_RET_INVALID_ARGUMENT); diff --git a/rcl_yaml_param_parser/include/rcl_yaml_param_parser/parser_thread_attr.h b/rcl_yaml_param_parser/include/rcl_yaml_param_parser/parser_thread_attr.h index 0ac148d86..bbaf21ca5 100644 --- a/rcl_yaml_param_parser/include/rcl_yaml_param_parser/parser_thread_attr.h +++ b/rcl_yaml_param_parser/include/rcl_yaml_param_parser/parser_thread_attr.h @@ -18,11 +18,11 @@ #include -#include "rcl_yaml_param_parser/types.h" #include "rcl_yaml_param_parser/visibility_control.h" #include "rcutils/allocator.h" #include "rcutils/macros.h" +#include "rcutils/thread_attr.h" #include "rcutils/types/rcutils_ret.h" #ifdef __cplusplus @@ -30,66 +30,24 @@ extern "C" { #endif -/// \brief Return a rcl_thread_attrs_t struct with members initialized to zero value. -/// \return a rcl_thread_attrs_t struct with members initialized to zero value. -RCL_YAML_PARAM_PARSER_PUBLIC -rcl_thread_attrs_t -rcl_get_zero_initialized_thread_attrs(void); - -/// \brief Initialize list of thread attributes -/// \param[out] thread_attrs the list of thread attributes to be initialized -/// \param[in] allocator memory allocator to be used -/// \return #RCUTILS_RET_OK if the structure was initialized succeessfully, or -/// \return #RCUTILS_RET_INVALID_ARGUMENT if any function arguments are invalid, or -/// \return #RCUTILS_RET_BAD_ALLOC if allocating memory failed, or -/// \return #RCUTILS_RET_ERROR an unspecified error occur. -RCL_YAML_PARAM_PARSER_PUBLIC -rcutils_ret_t -rcl_thread_attrs_init( - rcl_thread_attrs_t * thread_attrs, - rcutils_allocator_t allocator); - -/// \brief Initialize list of thread attributes with a capacity -/// \param[out] thread_attrs the list of thread attributes to be initialized -/// \param[in] allocator memory allocator to be used -/// \return #RCUTILS_RET_OK if the structure was initialized succeessfully, or -/// \return #RCUTILS_RET_INVALID_ARGUMENT if any function arguments are invalid, or -/// \return #RCUTILS_RET_BAD_ALLOC if allocating memory failed, or -/// \return #RCUTILS_RET_ERROR an unspecified error occur. -RCL_YAML_PARAM_PARSER_PUBLIC -rcutils_ret_t -rcl_thread_attrs_init_with_capacity( - rcl_thread_attrs_t * thread_attrs, - rcutils_allocator_t allocator, - size_t capacity); - -/// \brief Free list of thread attributes -/// \param[in] thread_attrs The structure to be deallocated. -/// \return #RCUTILS_RET_OK if the memory was successfully freed, or -/// \return #RCUTILS_RET_INVALID_ARGUMENT if any function arguments are invalid -RCL_YAML_PARAM_PARSER_PUBLIC -rcutils_ret_t -rcl_thread_attrs_fini( - rcl_thread_attrs_t * thread_attrs); - /// \brief Parse the YAML file and populate \p thread_attrs /// \pre Given \p thread_attrs must be a valid thread attribute struct /// \param[in] file_path is the path to the YAML file -/// \param[inout] thread_attrs points to the struct to be populated +/// \param[in,out] thread_attrs points to the struct to be populated /// \return true on success and false on failure RCL_YAML_PARAM_PARSER_PUBLIC rcutils_ret_t rcl_parse_yaml_thread_attrs_file( const char * file_path, - rcl_thread_attrs_t * thread_attrs); + rcutils_thread_attrs_t * thread_attrs); /// \brief Parse a thread attribute value as a YAML string, updating thread_attrs accordingly /// \param[in] yaml_value is the thread attribute value as a YAML string to be parsed -/// \param[inout] thread_attrs points to the thread attribute struct +/// \param[in,out] thread_attrs points to the thread attribute struct /// \return true on success and false on failure RCL_YAML_PARAM_PARSER_PUBLIC rcutils_ret_t rcl_parse_yaml_thread_attrs_value( const char * yaml_value, - rcl_thread_attrs_t * thread_attrs); + rcutils_thread_attrs_t * thread_attrs); #ifdef __cplusplus } diff --git a/rcl_yaml_param_parser/include/rcl_yaml_param_parser/types.h b/rcl_yaml_param_parser/include/rcl_yaml_param_parser/types.h index b7bad0b46..e0e4b5f1c 100644 --- a/rcl_yaml_param_parser/include/rcl_yaml_param_parser/types.h +++ b/rcl_yaml_param_parser/include/rcl_yaml_param_parser/types.h @@ -108,41 +108,4 @@ typedef struct rcl_params_s rcutils_allocator_t allocator; ///< Allocator used } rcl_params_t; -typedef enum rcl_thread_scheduling_policy_type_e -{ - RCL_THREAD_SCHEDULING_POLICY_UNKNOWN = 0, - RCL_THREAD_SCHEDULING_POLICY_FIFO = 1, - RCL_THREAD_SCHEDULING_POLICY_RR = 2, - RCL_THREAD_SCHEDULING_POLICY_SPORADIC = 3, - RCL_THREAD_SCHEDULING_POLICY_OTHER = 4, - RCL_THREAD_SCHEDULING_POLICY_IDLE = 5, - RCL_THREAD_SCHEDULING_POLICY_BATCH = 6, - RCL_THREAD_SCHEDULING_POLICY_DEADLINE = 7 -} rcl_thread_scheduling_policy_type_t; - -typedef struct rcl_thread_attr_s -{ - /// Thread core affinity - int core_affinity; - /// Thread scheduling policy. - rcl_thread_scheduling_policy_type_t scheduling_policy; - /// Thread priority. - int priority; - /// Thread name - char * name; -} rcl_thread_attr_t; - -/// Hold thread attribute rules. -typedef struct rcl_thread_attrs_s -{ - /// Private implementation array. - rcl_thread_attr_t * attributes; - /// Number of threads attribute - size_t num_attributes; - /// Number of threads attribute capacity - size_t capacity_attributes; - /// Allocator used to allocate objects in this struct - rcutils_allocator_t allocator; -} rcl_thread_attrs_t; - #endif // RCL_YAML_PARAM_PARSER__TYPES_H_ diff --git a/rcl_yaml_param_parser/src/impl/parse.h b/rcl_yaml_param_parser/src/impl/parse.h index 87724f9ec..99f5d28e7 100644 --- a/rcl_yaml_param_parser/src/impl/parse.h +++ b/rcl_yaml_param_parser/src/impl/parse.h @@ -90,30 +90,6 @@ rcutils_ret_t find_parameter( rcl_params_t * param_st, size_t * parameter_idx); -RCL_YAML_PARAM_PARSER_LOCAL -RCUTILS_WARN_UNUSED -rcutils_ret_t parse_thread_attr_key( - const char * value, - thread_attr_key_type_t * key_type); - -RCL_YAML_PARAM_PARSER_LOCAL -RCUTILS_WARN_UNUSED -rcl_thread_scheduling_policy_type_t parse_thread_attr_scheduling_policy( - const char * value); - -RCL_YAML_PARAM_PARSER_LOCAL -RCUTILS_WARN_UNUSED -rcutils_ret_t parse_thread_attr( - yaml_parser_t * parser, - rcl_thread_attr_t * attr, - rcutils_allocator_t allocator); - -RCL_YAML_PARAM_PARSER_PUBLIC -RCUTILS_WARN_UNUSED -rcutils_ret_t parse_thread_attr_events( - yaml_parser_t * parser, - rcl_thread_attrs_t * thread_attrs); - #ifdef __cplusplus } #endif diff --git a/rcl_yaml_param_parser/src/impl/parse_thread_attr.h b/rcl_yaml_param_parser/src/impl/parse_thread_attr.h index e039853a1..1be1e6883 100644 --- a/rcl_yaml_param_parser/src/impl/parse_thread_attr.h +++ b/rcl_yaml_param_parser/src/impl/parse_thread_attr.h @@ -19,10 +19,10 @@ #include "rcutils/allocator.h" #include "rcutils/macros.h" +#include "rcutils/thread_attr.h" #include "rcutils/types/rcutils_ret.h" #include "./types.h" -#include "rcl_yaml_param_parser/types.h" #include "rcl_yaml_param_parser/visibility_control.h" #ifdef __cplusplus @@ -38,21 +38,20 @@ rcutils_ret_t parse_thread_attr_key( RCL_YAML_PARAM_PARSER_LOCAL RCUTILS_WARN_UNUSED -rcl_thread_scheduling_policy_type_t parse_thread_attr_scheduling_policy( +rcutils_thread_scheduling_policy_t parse_thread_attr_scheduling_policy( const char * value); RCL_YAML_PARAM_PARSER_LOCAL RCUTILS_WARN_UNUSED rcutils_ret_t parse_thread_attr( yaml_parser_t * parser, - rcl_thread_attr_t * attr, - rcutils_allocator_t allocator); + rcutils_thread_attrs_t * attrs); RCL_YAML_PARAM_PARSER_PUBLIC RCUTILS_WARN_UNUSED rcutils_ret_t parse_thread_attr_events( yaml_parser_t * parser, - rcl_thread_attrs_t * thread_attrs); + rcutils_thread_attrs_t * thread_attrs); #ifdef __cplusplus } diff --git a/rcl_yaml_param_parser/src/parse_thread_attr.c b/rcl_yaml_param_parser/src/parse_thread_attr.c index ff9f18431..2be2c94ee 100644 --- a/rcl_yaml_param_parser/src/parse_thread_attr.c +++ b/rcl_yaml_param_parser/src/parse_thread_attr.c @@ -23,7 +23,7 @@ #include "rcutils/error_handling.h" #include "rcutils/format_string.h" #include "rcutils/strdup.h" -#include "rcutils/types/rcutils_ret.h" +#include "rcutils/thread_attr.h" #include "./impl/parse.h" // to use get_value #include "./impl/parse_thread_attr.h" @@ -91,26 +91,26 @@ parse_thread_attr_key( /// /// Parse the value of the scheduling policy of a thread attribute /// -rcl_thread_scheduling_policy_type_t parse_thread_attr_scheduling_policy( +rcutils_thread_scheduling_policy_t parse_thread_attr_scheduling_policy( const char * value) { - rcl_thread_scheduling_policy_type_t ret; + rcutils_thread_scheduling_policy_t ret; if (strcmp(value, "FIFO") == 0) { - ret = RCL_THREAD_SCHEDULING_POLICY_FIFO; + ret = RCUTILS_THREAD_SCHEDULING_POLICY_FIFO; } else if (strcmp(value, "RR") == 0) { - ret = RCL_THREAD_SCHEDULING_POLICY_RR; + ret = RCUTILS_THREAD_SCHEDULING_POLICY_RR; } else if (strcmp(value, "SPORADIC") == 0) { - ret = RCL_THREAD_SCHEDULING_POLICY_SPORADIC; + ret = RCUTILS_THREAD_SCHEDULING_POLICY_SPORADIC; } else if (strcmp(value, "OTHER") == 0) { - ret = RCL_THREAD_SCHEDULING_POLICY_OTHER; + ret = RCUTILS_THREAD_SCHEDULING_POLICY_OTHER; } else if (strcmp(value, "IDLE") == 0) { - ret = RCL_THREAD_SCHEDULING_POLICY_IDLE; + ret = RCUTILS_THREAD_SCHEDULING_POLICY_IDLE; } else if (strcmp(value, "BATCH") == 0) { - ret = RCL_THREAD_SCHEDULING_POLICY_BATCH; + ret = RCUTILS_THREAD_SCHEDULING_POLICY_BATCH; } else if (strcmp(value, "DEADLINE") == 0) { - ret = RCL_THREAD_SCHEDULING_POLICY_DEADLINE; + ret = RCUTILS_THREAD_SCHEDULING_POLICY_DEADLINE; } else { - ret = RCL_THREAD_SCHEDULING_POLICY_UNKNOWN; + ret = RCUTILS_THREAD_SCHEDULING_POLICY_UNKNOWN; } return ret; } @@ -120,12 +120,16 @@ rcl_thread_scheduling_policy_type_t parse_thread_attr_scheduling_policy( /// rcutils_ret_t parse_thread_attr( yaml_parser_t * parser, - rcl_thread_attr_t * attr, - rcutils_allocator_t allocator) + rcutils_thread_attrs_t * attrs) { rcutils_ret_t ret; yaml_event_t event; thread_attr_key_bits_t key_bits = THREAD_ATTR_KEY_BITS_NONE; + rcutils_thread_scheduling_policy_t sched_policy; + int core_affinity; + int priority; + char const * name = NULL; + rcutils_allocator_t allocator = attrs->allocator; while (1) { PARSE_WITH_CHECK_ERROR(&event); @@ -168,7 +172,7 @@ rcutils_ret_t parse_thread_attr( ret = RCUTILS_RET_ERROR; goto error; } - attr->core_affinity = ((int)*(int64_t *)(ret_val)); + core_affinity = ((int)*(int64_t *)(ret_val)); break; case THREAD_ATTR_KEY_PRIORITY: ret_val = get_value(value, style, tag, &val_type, allocator); @@ -178,10 +182,10 @@ rcutils_ret_t parse_thread_attr( ret = RCUTILS_RET_ERROR; goto error; } - attr->priority = ((int)*(int64_t *)(ret_val)); + priority = ((int)*(int64_t *)(ret_val)); break; case THREAD_ATTR_KEY_SCHEDULING_POLICY: - attr->scheduling_policy = parse_thread_attr_scheduling_policy(value); + sched_policy = parse_thread_attr_scheduling_policy(value); break; case THREAD_ATTR_KEY_NAME: if (*value == '\0') { @@ -189,8 +193,8 @@ rcutils_ret_t parse_thread_attr( ret = RCUTILS_RET_ERROR; goto error; } - attr->name = rcutils_strdup(value, allocator); - if (NULL == attr->name) { + name = rcutils_strdup(value, allocator); + if (NULL == name) { ret = RCUTILS_RET_BAD_ALLOC; goto error; } @@ -210,44 +214,28 @@ rcutils_ret_t parse_thread_attr( goto error; } + ret = rcutils_thread_attrs_add_attr(attrs, sched_policy, core_affinity, priority, name); + if (RCUTILS_RET_OK != ret) { + goto error; + } + + allocator.deallocate((char *)name, allocator.state); + return RCUTILS_RET_OK; error: - if (NULL != attr->name) { - allocator.deallocate(attr->name, allocator.state); + if (NULL != name) { + allocator.deallocate((char *)name, allocator.state); } return ret; } -static inline rcutils_ret_t extend_thread_attrs_capacity( - rcl_thread_attrs_t * attrs, - size_t new_cap) -{ - size_t cap = attrs->capacity_attributes; - size_t size = cap * sizeof(rcl_thread_attr_t); - size_t new_size = new_cap * sizeof(rcl_thread_attr_t); - rcl_thread_attr_t * new_attrs = attrs->allocator.reallocate( - attrs->attributes, new_size, attrs->allocator.state); - - if (NULL == new_attrs) { - RCUTILS_SET_ERROR_MSG("Failed to allocate memory for thread attributes"); - return RCUTILS_RET_BAD_ALLOC; - } - - memset(new_attrs + cap, 0, new_size - size); - - attrs->capacity_attributes = new_cap; - attrs->attributes = new_attrs; - - return RCUTILS_RET_OK; -} - /// /// Get events from parsing thread attributes YAML value string and process them /// rcutils_ret_t parse_thread_attr_events( yaml_parser_t * parser, - rcl_thread_attrs_t * thread_attrs) + rcutils_thread_attrs_t * thread_attrs) { yaml_event_t event; rcutils_ret_t ret; @@ -270,22 +258,7 @@ rcutils_ret_t parse_thread_attr_events( goto error; } - if (thread_attrs->num_attributes == thread_attrs->capacity_attributes) { - size_t new_cap = 0; - if (0 == thread_attrs->capacity_attributes) { - new_cap = 1; - } else { - new_cap = thread_attrs->capacity_attributes * 2; - } - // Extend the capacity - ret = extend_thread_attrs_capacity(thread_attrs, new_cap); - if (RCUTILS_RET_OK != ret) { - goto error; - } - } - - rcl_thread_attr_t * attr = thread_attrs->attributes + thread_attrs->num_attributes; - ret = parse_thread_attr(parser, attr, thread_attrs->allocator); + ret = parse_thread_attr(parser, thread_attrs); if (RCUTILS_RET_OK != ret) { goto error; } @@ -305,7 +278,9 @@ rcutils_ret_t parse_thread_attr_events( error: if (0 < thread_attrs->capacity_attributes) { - rcl_thread_attrs_fini(thread_attrs); + rcutils_ret_t thread_attrs_ret = rcutils_thread_attrs_fini(thread_attrs); + (void)thread_attrs_ret; + // Since an error has already occurred, ignore this result. } return ret; } diff --git a/rcl_yaml_param_parser/src/parser_thread_attr.c b/rcl_yaml_param_parser/src/parser_thread_attr.c index 6281ce183..abddba9b5 100644 --- a/rcl_yaml_param_parser/src/parser_thread_attr.c +++ b/rcl_yaml_param_parser/src/parser_thread_attr.c @@ -24,6 +24,7 @@ #include "rcutils/allocator.h" #include "rcutils/error_handling.h" +#include "rcutils/thread_attr.h" #include "rcutils/types/rcutils_ret.h" #include "./impl/types.h" @@ -32,84 +33,18 @@ #include "./impl/node_params.h" #include "./impl/yaml_variant.h" -#define INIT_NUM_THREAD_ATTRIBUTE 0U - -rcl_thread_attrs_t -rcl_get_zero_initialized_thread_attrs(void) -{ - rcl_thread_attrs_t ret = { - NULL, - }; - return ret; -} - -rcutils_ret_t -rcl_thread_attrs_init( - rcl_thread_attrs_t * thread_attrs, - rcutils_allocator_t allocator) -{ - return rcl_thread_attrs_init_with_capacity( - thread_attrs, allocator, INIT_NUM_THREAD_ATTRIBUTE); -} - -rcutils_ret_t -rcl_thread_attrs_init_with_capacity( - rcl_thread_attrs_t * thread_attrs, - rcutils_allocator_t allocator, - size_t capacity) -{ - RCUTILS_CHECK_ARGUMENT_FOR_NULL(thread_attrs, RCUTILS_RET_INVALID_ARGUMENT); - RCUTILS_CHECK_ALLOCATOR_WITH_MSG( - &allocator, "invalid allocator", return RCUTILS_RET_INVALID_ARGUMENT); - - thread_attrs->allocator = allocator; - thread_attrs->num_attributes = 0U; - thread_attrs->capacity_attributes = capacity; - if (capacity > 0) { - thread_attrs->attributes = - allocator.zero_allocate(capacity, sizeof(rcl_thread_attr_t), allocator.state); - if (NULL == thread_attrs->attributes) { - *thread_attrs = rcl_get_zero_initialized_thread_attrs(); - RCUTILS_SET_ERROR_MSG("Failed to allocate memory for thread attributes"); - return RCUTILS_RET_BAD_ALLOC; - } - } - return RCUTILS_RET_OK; -} - -rcutils_ret_t -rcl_thread_attrs_fini(rcl_thread_attrs_t * thread_attrs) -{ - RCUTILS_CHECK_ARGUMENT_FOR_NULL(thread_attrs, RCUTILS_RET_INVALID_ARGUMENT); - rcutils_allocator_t * allocator = &thread_attrs->allocator; - if (NULL == thread_attrs->attributes) { - return RCUTILS_RET_OK; - } - // check the allocator only if attributes is available to avoid checking after zero-initialized - RCUTILS_CHECK_ALLOCATOR(allocator, return RCUTILS_RET_INVALID_ARGUMENT); - for (size_t i = 0; i < thread_attrs->num_attributes; ++i) { - rcl_thread_attr_t * attr = thread_attrs->attributes + i; - if (NULL != attr->name) { - allocator->deallocate(attr->name, allocator->state); - } - } - allocator->deallocate(thread_attrs->attributes, allocator->state); - *thread_attrs = rcl_get_zero_initialized_thread_attrs(); - - return RCUTILS_RET_OK; -} - /// /// Parse the YAML file and populate thread_attrs /// rcutils_ret_t rcl_parse_yaml_thread_attrs_file( const char * file_path, - rcl_thread_attrs_t * thread_attrs) + rcutils_thread_attrs_t * thread_attrs) { RCUTILS_CHECK_FOR_NULL_WITH_MSG( file_path, "YAML file path is NULL", return RCUTILS_RET_INVALID_ARGUMENT); + RCUTILS_CHECK_ARGUMENT_FOR_NULL(thread_attrs, RCUTILS_RET_INVALID_ARGUMENT); - if (NULL == thread_attrs) { + if (0 < thread_attrs->num_attributes) { RCUTILS_SAFE_FWRITE_TO_STDERR("Pass an initialized thread attr structure"); return RCUTILS_RET_ERROR; } @@ -143,15 +78,12 @@ rcutils_ret_t rcl_parse_yaml_thread_attrs_file( /// rcutils_ret_t rcl_parse_yaml_thread_attrs_value( const char * yaml_value, - rcl_thread_attrs_t * thread_attrs) + rcutils_thread_attrs_t * thread_attrs) { - RCUTILS_CHECK_ARGUMENT_FOR_NULL(yaml_value, false); - - if (0U == strlen(yaml_value)) { - return RCUTILS_RET_ERROR; - } + RCUTILS_CHECK_ARGUMENT_FOR_NULL(yaml_value, RCUTILS_RET_INVALID_ARGUMENT); + RCUTILS_CHECK_ARGUMENT_FOR_NULL(thread_attrs, RCUTILS_RET_INVALID_ARGUMENT); - if (NULL == thread_attrs) { + if (0 < thread_attrs->num_attributes) { RCUTILS_SAFE_FWRITE_TO_STDERR("Pass an initialized thread attr structure"); return RCUTILS_RET_ERROR; } From d8681040a4d7bdb52f254bf6aa1b53fd85808c21 Mon Sep 17 00:00:00 2001 From: Shoji Morita Date: Wed, 12 Jul 2023 17:18:12 +0900 Subject: [PATCH 3/7] Added tests and fixed problems found in the test. Signed-off-by: Shoji Morita --- rcl_yaml_param_parser/CMakeLists.txt | 25 +++ .../src/impl/parse_thread_attr.h | 6 +- rcl_yaml_param_parser/src/parse_thread_attr.c | 35 ++-- .../test/test_parse_thread_attr.cpp | 168 ++++++++++++++++++ .../test/test_parser_thread_attr.cpp | 126 +++++++++++++ .../test/thread_attr_success.yaml | 40 +++++ 6 files changed, 375 insertions(+), 25 deletions(-) create mode 100644 rcl_yaml_param_parser/test/test_parse_thread_attr.cpp create mode 100644 rcl_yaml_param_parser/test/test_parser_thread_attr.cpp create mode 100644 rcl_yaml_param_parser/test/thread_attr_success.yaml diff --git a/rcl_yaml_param_parser/CMakeLists.txt b/rcl_yaml_param_parser/CMakeLists.txt index b14b5de8c..e2cc685fa 100644 --- a/rcl_yaml_param_parser/CMakeLists.txt +++ b/rcl_yaml_param_parser/CMakeLists.txt @@ -182,6 +182,31 @@ if(BUILD_TESTING) ) endif() + ament_add_gtest(test_parse_thread_attr + test/test_parse_thread_attr.cpp + WORKING_DIRECTORY "${CMAKE_CURRENT_SOURCE_DIR}" + ) + if(TARGET test_parse_thread_attr) + target_link_libraries(test_parse_thread_attr + ${PROJECT_NAME} + osrf_testing_tools_cpp::memory_tools + rcutils::rcutils + yaml + ) + endif() + + ament_add_gtest(test_parser_thread_attr + test/test_parser_thread_attr.cpp + WORKING_DIRECTORY "${CMAKE_CURRENT_SOURCE_DIR}" + ) + if(TARGET test_parser_thread_attr) + target_link_libraries(test_parser_thread_attr + ${PROJECT_NAME} + osrf_testing_tools_cpp::memory_tools + rcutils::rcutils + ) + endif() + add_performance_test(benchmark_parse_yaml test/benchmark/benchmark_parse_yaml.cpp WORKING_DIRECTORY "${CMAKE_CURRENT_SOURCE_DIR}") if(TARGET benchmark_parse_yaml) diff --git a/rcl_yaml_param_parser/src/impl/parse_thread_attr.h b/rcl_yaml_param_parser/src/impl/parse_thread_attr.h index 1be1e6883..6a5a3485e 100644 --- a/rcl_yaml_param_parser/src/impl/parse_thread_attr.h +++ b/rcl_yaml_param_parser/src/impl/parse_thread_attr.h @@ -30,18 +30,18 @@ extern "C" { #endif -RCL_YAML_PARAM_PARSER_LOCAL +RCL_YAML_PARAM_PARSER_PUBLIC RCUTILS_WARN_UNUSED rcutils_ret_t parse_thread_attr_key( const char * value, thread_attr_key_type_t * key_type); -RCL_YAML_PARAM_PARSER_LOCAL +RCL_YAML_PARAM_PARSER_PUBLIC RCUTILS_WARN_UNUSED rcutils_thread_scheduling_policy_t parse_thread_attr_scheduling_policy( const char * value); -RCL_YAML_PARAM_PARSER_LOCAL +RCL_YAML_PARAM_PARSER_PUBLIC RCUTILS_WARN_UNUSED rcutils_ret_t parse_thread_attr( yaml_parser_t * parser, diff --git a/rcl_yaml_param_parser/src/parse_thread_attr.c b/rcl_yaml_param_parser/src/parse_thread_attr.c index 2be2c94ee..ce7c43e2e 100644 --- a/rcl_yaml_param_parser/src/parse_thread_attr.c +++ b/rcl_yaml_param_parser/src/parse_thread_attr.c @@ -28,10 +28,10 @@ #include "./impl/parse.h" // to use get_value #include "./impl/parse_thread_attr.h" -#define PARSE_WITH_CHECK_ERROR(out_event) \ +#define PARSE_WITH_CHECK_ERROR() \ do { \ int _parse_ret_; \ - _parse_ret_ = yaml_parser_parse(parser, (out_event)); \ + _parse_ret_ = yaml_parser_parse(parser, &event); \ if (0 == _parse_ret_) { \ RCUTILS_SET_ERROR_MSG("Failed to parse thread attributes"); \ ret = RCUTILS_RET_ERROR; \ @@ -39,17 +39,10 @@ } \ } while (0) -#define PARSE_WITH_EVENT_CHECK(event_type) \ +#define PARSE_WITH_CHECK_EVENT(event_type) \ do { \ - yaml_event_t _event_; \ - int _parse_ret_; \ - _parse_ret_ = yaml_parser_parse(parser, &_event_); \ - if (0 == _parse_ret_) { \ - RCUTILS_SET_ERROR_MSG("Failed to parse thread attributes"); \ - ret = RCUTILS_RET_ERROR; \ - goto error; \ - } \ - if (_event_.type != (event_type)) { \ + PARSE_WITH_CHECK_ERROR(); \ + if (event.type != (event_type)) { \ RCUTILS_SET_ERROR_MSG("Unexpected element in a configuration of thread attributes"); \ ret = RCUTILS_RET_ERROR; \ goto error; \ @@ -132,7 +125,7 @@ rcutils_ret_t parse_thread_attr( rcutils_allocator_t allocator = attrs->allocator; while (1) { - PARSE_WITH_CHECK_ERROR(&event); + PARSE_WITH_CHECK_ERROR(); if (YAML_MAPPING_END_EVENT == event.type) { break; @@ -155,7 +148,7 @@ rcutils_ret_t parse_thread_attr( goto error; } - PARSE_WITH_CHECK_ERROR(&event); + PARSE_WITH_CHECK_EVENT(YAML_SCALAR_EVENT); const char * value = (char *)event.data.scalar.value; yaml_scalar_style_t style = event.data.scalar.style; @@ -243,12 +236,12 @@ rcutils_ret_t parse_thread_attr_events( RCUTILS_CHECK_ARGUMENT_FOR_NULL(parser, RCUTILS_RET_INVALID_ARGUMENT); RCUTILS_CHECK_ARGUMENT_FOR_NULL(thread_attrs, RCUTILS_RET_INVALID_ARGUMENT); - PARSE_WITH_EVENT_CHECK(YAML_STREAM_START_EVENT); - PARSE_WITH_EVENT_CHECK(YAML_DOCUMENT_START_EVENT); - PARSE_WITH_EVENT_CHECK(YAML_SEQUENCE_START_EVENT); + PARSE_WITH_CHECK_EVENT(YAML_STREAM_START_EVENT); + PARSE_WITH_CHECK_EVENT(YAML_DOCUMENT_START_EVENT); + PARSE_WITH_CHECK_EVENT(YAML_SEQUENCE_START_EVENT); while (1) { - PARSE_WITH_CHECK_ERROR(&event); + PARSE_WITH_CHECK_ERROR(); if (YAML_SEQUENCE_END_EVENT == event.type) { break; @@ -262,11 +255,9 @@ rcutils_ret_t parse_thread_attr_events( if (RCUTILS_RET_OK != ret) { goto error; } - - ++thread_attrs->num_attributes; } - PARSE_WITH_EVENT_CHECK(YAML_DOCUMENT_END_EVENT); - PARSE_WITH_EVENT_CHECK(YAML_STREAM_END_EVENT); + PARSE_WITH_CHECK_EVENT(YAML_DOCUMENT_END_EVENT); + PARSE_WITH_CHECK_EVENT(YAML_STREAM_END_EVENT); if (0 == thread_attrs->num_attributes) { RCUTILS_SET_ERROR_MSG("No thread attributes."); diff --git a/rcl_yaml_param_parser/test/test_parse_thread_attr.cpp b/rcl_yaml_param_parser/test/test_parse_thread_attr.cpp new file mode 100644 index 000000000..0ea0fedd7 --- /dev/null +++ b/rcl_yaml_param_parser/test/test_parse_thread_attr.cpp @@ -0,0 +1,168 @@ +// Copyright 2023 eSOL Co.,Ltd. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#include + +#include +#include +#include +#include + +#include "rcutils/thread_attr.h" +#include "rcutils/error_handling.h" + +#include "../src/impl/parse_thread_attr.h" + +struct TestParseThreadAttrs : testing::Test +{ + void SetUp() override + { + rcutils_reset_error(); + + rcutils_ret_t ret; + rcutils_allocator_t allocator; + attrs = rcutils_get_zero_initialized_thread_attrs(); + allocator = rcutils_get_default_allocator(); + ret = rcutils_thread_attrs_init(&attrs, allocator); + ASSERT_EQ(ret, RCUTILS_RET_OK); + + int parser_ret = yaml_parser_initialize(&parser); + ASSERT_NE(0, parser_ret); + } + void TearDown() override + { + yaml_parser_delete(&parser); + rcutils_ret_t ret = rcutils_thread_attrs_fini(&attrs); + ASSERT_EQ(RCUTILS_RET_OK, ret); + } + + void prepare_yaml_parser(char const * yaml_value) + { + yaml_parser_set_input_string(&parser, (const unsigned char *)yaml_value, strlen(yaml_value)); + } + + yaml_parser_t parser; + rcutils_thread_attrs_t attrs; +}; + +struct TestParseThreadAttr : TestParseThreadAttrs +{ + void prepare_yaml_parser(char const * yaml_value) + { + TestParseThreadAttrs::prepare_yaml_parser(yaml_value); + + yaml_event_t event; + int ret; + ret = yaml_parser_parse(&parser, &event); + ASSERT_NE(0, ret); + ret = yaml_parser_parse(&parser, &event); + ASSERT_NE(0, ret); + ret = yaml_parser_parse(&parser, &event); + ASSERT_NE(0, ret); + } +}; + +TEST_F(TestParseThreadAttr, success) { + rcutils_ret_t ret; + yaml_event_t event; + + prepare_yaml_parser( + "{ priority: 10, name: thread-1, core_affinity: 1, scheduling_policy: FIFO }"); + + ret = parse_thread_attr(&parser, &attrs); + EXPECT_EQ(RCUTILS_RET_OK, ret); + + EXPECT_EQ(1, attrs.num_attributes); + EXPECT_EQ(10, attrs.attributes[0].priority); + EXPECT_STREQ("thread-1", attrs.attributes[0].name); + EXPECT_EQ(1, attrs.attributes[0].core_affinity); + EXPECT_EQ(RCUTILS_THREAD_SCHEDULING_POLICY_FIFO, attrs.attributes[0].scheduling_policy); + + int parser_ret; + parser_ret = yaml_parser_parse(&parser, &event); + ASSERT_NE(0, parser_ret); + EXPECT_EQ(YAML_DOCUMENT_END_EVENT, event.type); + parser_ret = yaml_parser_parse(&parser, &event); + ASSERT_NE(0, parser_ret); + EXPECT_EQ(YAML_STREAM_END_EVENT, event.type); +} + +TEST_F(TestParseThreadAttr, unknown_key) { + rcutils_ret_t ret; + + prepare_yaml_parser( + "{ priority: 10, name: thread-1, core_affinity: 1, unknown_key: FIFO }"); + + ret = parse_thread_attr(&parser, &attrs); + EXPECT_EQ(RCUTILS_RET_ERROR, ret); +} + +TEST_F(TestParseThreadAttr, all_valid_keys_with_unknown_key) { + rcutils_ret_t ret; + + prepare_yaml_parser( + "{ priority: 10, name: thread-1, core_affinity: 1, scheduling_policy: FIFO, unknown_key: RR }"); + + ret = parse_thread_attr(&parser, &attrs); + EXPECT_EQ(RCUTILS_RET_ERROR, ret); +} + +TEST_F(TestParseThreadAttr, missing_key_value) { + rcutils_ret_t ret; + prepare_yaml_parser( + "{ priority: 10, name: thread-1 }"); + + ret = parse_thread_attr(&parser, &attrs); + EXPECT_EQ(RCUTILS_RET_ERROR, ret); +} + +TEST_F(TestParseThreadAttr, not_acceptable_mapping) { + rcutils_ret_t ret; + prepare_yaml_parser( + "{ priority: 10, name: thread-1, core_affinity: [1,2,3], scheduling_policy: FIFO }"); + + ret = parse_thread_attr(&parser, &attrs); + EXPECT_EQ(RCUTILS_RET_ERROR, ret); +} + +TEST_F(TestParseThreadAttrs, success) { + rcutils_ret_t ret; + + std::stringstream ss; + ss << "["; + for (std::size_t i = 0; i < 100; ++i) { + ss << "{ priority: " << i * 10; + ss << ", name: thread-" << i; + ss << ", core_affinity: " << i; + ss << ", scheduling_policy: FIFO },"; + } + ss << "]"; + + std::string buf = ss.str(); + prepare_yaml_parser(buf.c_str()); + + ret = parse_thread_attr_events(&parser, &attrs); + EXPECT_EQ(RCUTILS_RET_OK, ret); + ASSERT_EQ(100, attrs.num_attributes); + + for (std::size_t i = 0; i < 100; ++i) { + EXPECT_EQ(i * 10, attrs.attributes[i].priority); + ss.str(""); + ss << "thread-" << i; + buf = ss.str(); + EXPECT_STREQ(buf.c_str(), attrs.attributes[i].name); + EXPECT_EQ(i, attrs.attributes[i].core_affinity); + EXPECT_EQ(RCUTILS_THREAD_SCHEDULING_POLICY_FIFO, attrs.attributes[i].scheduling_policy); + } +} diff --git a/rcl_yaml_param_parser/test/test_parser_thread_attr.cpp b/rcl_yaml_param_parser/test/test_parser_thread_attr.cpp new file mode 100644 index 000000000..e2d604917 --- /dev/null +++ b/rcl_yaml_param_parser/test/test_parser_thread_attr.cpp @@ -0,0 +1,126 @@ +// Copyright 2023 eSOL Co.,Ltd. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#include + +#include +#include +#include + +#include "rcutils/error_handling.h" +#include "rcutils/filesystem.h" +#include "rcutils/thread_attr.h" + +#include "rcl_yaml_param_parser/parser_thread_attr.h" + +struct TestParserThreadAttr : testing::Test +{ + void SetUp() override + { + rcutils_ret_t ret; + path = nullptr; + + rcutils_reset_error(); + attrs = rcutils_get_zero_initialized_thread_attrs(); + alloc = rcutils_get_default_allocator(); + ret = rcutils_thread_attrs_init(&attrs, alloc); + ASSERT_EQ(RCUTILS_RET_OK, ret); + } + + void prepare_thread_attr_file(char const * filename) + { + char buf[1024]; + bool ret = rcutils_get_cwd(buf, sizeof(buf)); + ASSERT_TRUE(ret); + + path = rcutils_join_path(buf, "test/", alloc); + ASSERT_NE(nullptr, path); + path = rcutils_join_path(path, filename, alloc); + ASSERT_NE(nullptr, path); + } + + void TearDown() override + { + rcutils_ret_t ret; + if (path) { + alloc.deallocate(path, alloc.state); + } + ret = rcutils_thread_attrs_fini(&attrs); + EXPECT_EQ(RCUTILS_RET_OK, ret); + } + rcutils_thread_attrs_t attrs; + rcutils_allocator_t alloc; + char * path; +}; + +static const rcutils_thread_scheduling_policy_t expected_policies[] = { + RCUTILS_THREAD_SCHEDULING_POLICY_UNKNOWN, + RCUTILS_THREAD_SCHEDULING_POLICY_FIFO, + RCUTILS_THREAD_SCHEDULING_POLICY_RR, + RCUTILS_THREAD_SCHEDULING_POLICY_SPORADIC, + RCUTILS_THREAD_SCHEDULING_POLICY_OTHER, + RCUTILS_THREAD_SCHEDULING_POLICY_IDLE, + RCUTILS_THREAD_SCHEDULING_POLICY_BATCH, + RCUTILS_THREAD_SCHEDULING_POLICY_DEADLINE, + RCUTILS_THREAD_SCHEDULING_POLICY_UNKNOWN, + RCUTILS_THREAD_SCHEDULING_POLICY_FIFO, +}; + +TEST_F(TestParserThreadAttr, success_file) { + rcutils_ret_t ret; + + prepare_thread_attr_file("thread_attr_success.yaml"); + + ret = rcl_parse_yaml_thread_attrs_file(path, &attrs); + ASSERT_EQ(RCUTILS_RET_OK, ret); + EXPECT_EQ(10, attrs.num_attributes); + + for (int i = 0; i < 10; ++i) { + EXPECT_EQ(attrs.attributes[i].priority, i * 10); + char buf[32]; + snprintf(buf, sizeof(buf), "thread-%d", i); + EXPECT_STREQ(buf, attrs.attributes[i].name); + EXPECT_EQ(i, attrs.attributes[i].core_affinity); + EXPECT_EQ(expected_policies[i], attrs.attributes[i].scheduling_policy); + } +} + +TEST_F(TestParserThreadAttr, success_value) { + rcutils_ret_t ret; + + prepare_thread_attr_file("thread_attr_success.yaml"); + + std::ifstream ifs(path); + std::stringstream ss; + ss << ifs.rdbuf(); + + ret = rcl_parse_yaml_thread_attrs_value(ss.str().c_str(), &attrs); + ASSERT_EQ(RCUTILS_RET_OK, ret); + EXPECT_EQ(10, attrs.num_attributes); + + for (int i = 0; i < 10; ++i) { + EXPECT_EQ(attrs.attributes[i].priority, i * 10); + char buf[32]; + snprintf(buf, sizeof(buf), "thread-%d", i); + EXPECT_STREQ(buf, attrs.attributes[i].name); + EXPECT_EQ(i, attrs.attributes[i].core_affinity); + EXPECT_EQ(expected_policies[i], attrs.attributes[i].scheduling_policy); + } +} + +TEST_F(TestParserThreadAttr, bad_file_path) { + rcutils_ret_t ret = rcl_parse_yaml_thread_attrs_file("not_exist.yaml", &attrs); + + EXPECT_EQ(RCUTILS_RET_ERROR, ret); +} diff --git a/rcl_yaml_param_parser/test/thread_attr_success.yaml b/rcl_yaml_param_parser/test/thread_attr_success.yaml new file mode 100644 index 000000000..a6aae7922 --- /dev/null +++ b/rcl_yaml_param_parser/test/thread_attr_success.yaml @@ -0,0 +1,40 @@ +- priority: 0 + name: thread-0 + core_affinity: 0 + scheduling_policy: BADSCHEDPOLICY1 +- priority: 10 + name: thread-1 + core_affinity: 1 + scheduling_policy: FIFO +- priority: 20 + name: thread-2 + core_affinity: 2 + scheduling_policy: RR +- priority: 30 + name: thread-3 + core_affinity: 3 + scheduling_policy: SPORADIC +- priority: 40 + name: thread-4 + core_affinity: 4 + scheduling_policy: OTHER +- priority: 50 + name: thread-5 + core_affinity: 5 + scheduling_policy: IDLE +- priority: 60 + name: thread-6 + core_affinity: 6 + scheduling_policy: BATCH +- priority: 70 + name: thread-7 + core_affinity: 7 + scheduling_policy: DEADLINE +- priority: 80 + name: thread-8 + core_affinity: 8 + scheduling_policy: BADSCHEDPOLICY2 +- priority: 90 + name: thread-9 + core_affinity: 9 + scheduling_policy: FIFO From 043b11babb3788f1aae6baf5e91cc0e63844bb90 Mon Sep 17 00:00:00 2001 From: Shoji Morita Date: Thu, 3 Aug 2023 17:08:59 +0900 Subject: [PATCH 4/7] Added tests and modified the interface to the upper language binding. The modification of the interface is to go along with the specification described in the draft REP shared in the thread below. https://discourse.ros.org/t/adding-thread-attributes-configuration-in-ros-2-framework/30701/5 Signed-off-by: Shoji Morita --- rcl/include/rcl/context.h | 17 +++---- rcl/src/rcl/arguments.c | 22 ++++++++-- rcl/src/rcl/context.c | 23 +++++----- rcl/test/CMakeLists.txt | 1 + rcl/test/rcl/test_arguments.cpp | 14 +++++- rcl/test/rcl/test_context.cpp | 44 +++++++++++++++++++ .../test_arguments/test_thread_attrs.yaml | 8 ++++ 7 files changed, 100 insertions(+), 29 deletions(-) create mode 100644 rcl/test/resources/test_arguments/test_thread_attrs.yaml 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 From 78ee2816ce9e2f75988877bad3cfd0d079af30df Mon Sep 17 00:00:00 2001 From: Shoji Morita Date: Thu, 26 Oct 2023 12:32:06 +0900 Subject: [PATCH 5/7] Modified to receive multiple core affinity parameters according to the update of REP-2017 below. https://github.com/ros-infrastructure/rep/pull/385 Signed-off-by: Shoji Morita --- rcl/test/rcl/test_arguments.cpp | 4 +- rcl/test/rcl/test_context.cpp | 6 +- .../test_arguments/test_thread_attrs.yaml | 4 +- rcl_yaml_param_parser/src/parse_thread_attr.c | 60 ++++++++++++++++--- .../test/test_parse_thread_attr.cpp | 38 +++++++----- .../test/test_parser_thread_attr.cpp | 16 +++-- .../test/thread_attr_success.yaml | 20 +++---- 7 files changed, 102 insertions(+), 46 deletions(-) diff --git a/rcl/test/rcl/test_arguments.cpp b/rcl/test/rcl/test_arguments.cpp index c3a34897d..e86d215ed 100644 --- a/rcl/test/rcl/test_arguments.cpp +++ b/rcl/test/rcl/test_arguments.cpp @@ -210,7 +210,7 @@ TEST_F(CLASSNAME(TestArgumentsFixture, RMW_IMPLEMENTATION), check_known_vs_unkno EXPECT_TRUE( are_known_ros_args( {"--ros-args", "--thread-attrs-value", - "[{priority: 10, scheduling_policy: FIFO, name: thread-1, core_affinity: 1}]"})); + "[{priority: 10, scheduling_policy: FIFO, name: thread-1, core_affinity: [1,2,3]}]"})); } bool @@ -238,7 +238,7 @@ TEST_F(CLASSNAME(TestArgumentsFixture, RMW_IMPLEMENTATION), check_valid_vs_inval "--params-file", parameters_filepath.c_str(), "--log-level", "INFO", "--log-config-file", "file.config", "--thread-attrs-value", - "[{priority: 10, scheduling_policy: IDLE, name: thread-1, core_affinity: 1}]" + "[{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 925cac57b..a5dd9417f 100644 --- a/rcl/test/rcl/test_context.cpp +++ b/rcl/test/rcl/test_context.cpp @@ -171,11 +171,15 @@ TEST_F(CLASSNAME(TestContextFixture, RMW_IMPLEMENTATION), nominal) { 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; + rcutils_thread_core_affinity_t tmp_affinity = + rcutils_get_zero_initialized_thread_core_affinity(); ret = rcutils_thread_attrs_init(ctx_attrs, rcl_get_default_allocator()); EXPECT_EQ(RCUTILS_RET_OK, ret); + ret = rcutils_thread_core_affinity_init(&tmp_affinity, 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"); + ctx_attrs, RCUTILS_THREAD_SCHEDULING_POLICY_FIFO, &tmp_affinity, 10, "arg"); EXPECT_EQ(RCUTILS_RET_OK, ret); EXPECT_EQ(ctx_attrs, rcl_context_get_thread_attrs(&context)); diff --git a/rcl/test/resources/test_arguments/test_thread_attrs.yaml b/rcl/test/resources/test_arguments/test_thread_attrs.yaml index e784b8cac..af9e89cb8 100644 --- a/rcl/test/resources/test_arguments/test_thread_attrs.yaml +++ b/rcl/test/resources/test_arguments/test_thread_attrs.yaml @@ -1,8 +1,8 @@ - name: thread-1 priority: 10 scheduling_policy: FIFO - core_affinity: 1 + core_affinity: [1] - name: thread-2 priority: 20 scheduling_policy: RR - core_affinity: 2 + core_affinity: [2] diff --git a/rcl_yaml_param_parser/src/parse_thread_attr.c b/rcl_yaml_param_parser/src/parse_thread_attr.c index ce7c43e2e..a922a8599 100644 --- a/rcl_yaml_param_parser/src/parse_thread_attr.c +++ b/rcl_yaml_param_parser/src/parse_thread_attr.c @@ -119,10 +119,12 @@ rcutils_ret_t parse_thread_attr( yaml_event_t event; thread_attr_key_bits_t key_bits = THREAD_ATTR_KEY_BITS_NONE; rcutils_thread_scheduling_policy_t sched_policy; - int core_affinity; + rcutils_thread_core_affinity_t core_affinity = + rcutils_get_zero_initialized_thread_core_affinity(); int priority; char const * name = NULL; rcutils_allocator_t allocator = attrs->allocator; + void * ret_val = NULL; while (1) { PARSE_WITH_CHECK_ERROR(); @@ -148,26 +150,51 @@ rcutils_ret_t parse_thread_attr( goto error; } - PARSE_WITH_CHECK_EVENT(YAML_SCALAR_EVENT); + PARSE_WITH_CHECK_ERROR(); const char * value = (char *)event.data.scalar.value; yaml_scalar_style_t style = event.data.scalar.style; const yaml_char_t * tag = event.data.scalar.tag; data_types_t val_type; - void * ret_val = NULL; switch (key_type) { case THREAD_ATTR_KEY_CORE_AFFINITY: - ret_val = get_value(value, style, tag, &val_type, allocator); - if (DATA_TYPE_INT64 != val_type) { - RCUTILS_SET_ERROR_MSG_WITH_FORMAT_STRING( - "Unrecognized value for thread core affinity: %s", value); + if (event.type != YAML_SEQUENCE_START_EVENT) { ret = RCUTILS_RET_ERROR; goto error; } - core_affinity = ((int)*(int64_t *)(ret_val)); + ret = rcutils_thread_core_affinity_init(&core_affinity, 0, allocator); + if (RCUTILS_RET_OK != ret) { + goto error; + } + while (1) { + PARSE_WITH_CHECK_ERROR(); + if (YAML_SEQUENCE_END_EVENT == event.type) { + break; + } + value = (char *)event.data.scalar.value; + style = event.data.scalar.style; + tag = event.data.scalar.tag; + ret_val = get_value(value, style, tag, &val_type, allocator); + if (DATA_TYPE_INT64 != val_type) { + RCUTILS_SET_ERROR_MSG_WITH_FORMAT_STRING( + "Unrecognized value for thread core affinity: %s", value); + ret = RCUTILS_RET_ERROR; + goto error; + } + size_t core_no = ((size_t)*(int64_t *)(ret_val)); + allocator.deallocate(ret_val, allocator.state); + ret_val = NULL; + ret = rcutils_thread_core_affinity_set(&core_affinity, core_no); + if (RCUTILS_RET_OK != ret) { + goto error; + } + } break; case THREAD_ATTR_KEY_PRIORITY: + if (event.type != YAML_SCALAR_EVENT) { + goto error; + } ret_val = get_value(value, style, tag, &val_type, allocator); if (DATA_TYPE_INT64 != val_type) { RCUTILS_SET_ERROR_MSG_WITH_FORMAT_STRING( @@ -178,9 +205,15 @@ rcutils_ret_t parse_thread_attr( priority = ((int)*(int64_t *)(ret_val)); break; case THREAD_ATTR_KEY_SCHEDULING_POLICY: + if (event.type != YAML_SCALAR_EVENT) { + goto error; + } sched_policy = parse_thread_attr_scheduling_policy(value); break; case THREAD_ATTR_KEY_NAME: + if (event.type != YAML_SCALAR_EVENT) { + goto error; + } if (*value == '\0') { RCUTILS_SET_ERROR_MSG("Empty thread name"); ret = RCUTILS_RET_ERROR; @@ -196,6 +229,7 @@ rcutils_ret_t parse_thread_attr( if (NULL != ret_val) { allocator.deallocate(ret_val, allocator.state); + ret_val = NULL; } key_bits |= (thread_attr_key_bits_t)key_type; @@ -207,7 +241,7 @@ rcutils_ret_t parse_thread_attr( goto error; } - ret = rcutils_thread_attrs_add_attr(attrs, sched_policy, core_affinity, priority, name); + ret = rcutils_thread_attrs_add_attr(attrs, sched_policy, &core_affinity, priority, name); if (RCUTILS_RET_OK != ret) { goto error; } @@ -220,6 +254,14 @@ rcutils_ret_t parse_thread_attr( if (NULL != name) { allocator.deallocate((char *)name, allocator.state); } + if (NULL != ret_val) { + allocator.deallocate(ret_val, allocator.state); + } + if (0 < core_affinity.core_count) { + rcutils_ret_t tmp_ret = + rcutils_thread_core_affinity_fini(&core_affinity); + (void)tmp_ret; + } return ret; } diff --git a/rcl_yaml_param_parser/test/test_parse_thread_attr.cpp b/rcl_yaml_param_parser/test/test_parse_thread_attr.cpp index 0ea0fedd7..1251066c4 100644 --- a/rcl_yaml_param_parser/test/test_parse_thread_attr.cpp +++ b/rcl_yaml_param_parser/test/test_parse_thread_attr.cpp @@ -35,7 +35,7 @@ struct TestParseThreadAttrs : testing::Test attrs = rcutils_get_zero_initialized_thread_attrs(); allocator = rcutils_get_default_allocator(); ret = rcutils_thread_attrs_init(&attrs, allocator); - ASSERT_EQ(ret, RCUTILS_RET_OK); + ASSERT_EQ(RCUTILS_RET_OK, ret); int parser_ret = yaml_parser_initialize(&parser); ASSERT_NE(0, parser_ret); @@ -78,7 +78,7 @@ TEST_F(TestParseThreadAttr, success) { yaml_event_t event; prepare_yaml_parser( - "{ priority: 10, name: thread-1, core_affinity: 1, scheduling_policy: FIFO }"); + "{ priority: 10, name: thread-1, core_affinity: [1], scheduling_policy: FIFO }"); ret = parse_thread_attr(&parser, &attrs); EXPECT_EQ(RCUTILS_RET_OK, ret); @@ -86,7 +86,7 @@ TEST_F(TestParseThreadAttr, success) { EXPECT_EQ(1, attrs.num_attributes); EXPECT_EQ(10, attrs.attributes[0].priority); EXPECT_STREQ("thread-1", attrs.attributes[0].name); - EXPECT_EQ(1, attrs.attributes[0].core_affinity); + EXPECT_TRUE(rcutils_thread_core_affinity_is_set(&attrs.attributes[0].core_affinity, 1)); EXPECT_EQ(RCUTILS_THREAD_SCHEDULING_POLICY_FIFO, attrs.attributes[0].scheduling_policy); int parser_ret; @@ -102,7 +102,7 @@ TEST_F(TestParseThreadAttr, unknown_key) { rcutils_ret_t ret; prepare_yaml_parser( - "{ priority: 10, name: thread-1, core_affinity: 1, unknown_key: FIFO }"); + "{ priority: 10, name: thread-1, core_affinity: [1], unknown_key: FIFO }"); ret = parse_thread_attr(&parser, &attrs); EXPECT_EQ(RCUTILS_RET_ERROR, ret); @@ -112,7 +112,8 @@ TEST_F(TestParseThreadAttr, all_valid_keys_with_unknown_key) { rcutils_ret_t ret; prepare_yaml_parser( - "{ priority: 10, name: thread-1, core_affinity: 1, scheduling_policy: FIFO, unknown_key: RR }"); + "{ priority: 10, name: thread-1, core_affinity: [1], " + "scheduling_policy: FIFO, unknown_key: RR }"); ret = parse_thread_attr(&parser, &attrs); EXPECT_EQ(RCUTILS_RET_ERROR, ret); @@ -127,15 +128,6 @@ TEST_F(TestParseThreadAttr, missing_key_value) { EXPECT_EQ(RCUTILS_RET_ERROR, ret); } -TEST_F(TestParseThreadAttr, not_acceptable_mapping) { - rcutils_ret_t ret; - prepare_yaml_parser( - "{ priority: 10, name: thread-1, core_affinity: [1,2,3], scheduling_policy: FIFO }"); - - ret = parse_thread_attr(&parser, &attrs); - EXPECT_EQ(RCUTILS_RET_ERROR, ret); -} - TEST_F(TestParseThreadAttrs, success) { rcutils_ret_t ret; @@ -144,7 +136,7 @@ TEST_F(TestParseThreadAttrs, success) { for (std::size_t i = 0; i < 100; ++i) { ss << "{ priority: " << i * 10; ss << ", name: thread-" << i; - ss << ", core_affinity: " << i; + ss << ", core_affinity: [" << i << "]"; ss << ", scheduling_policy: FIFO },"; } ss << "]"; @@ -162,7 +154,21 @@ TEST_F(TestParseThreadAttrs, success) { ss << "thread-" << i; buf = ss.str(); EXPECT_STREQ(buf.c_str(), attrs.attributes[i].name); - EXPECT_EQ(i, attrs.attributes[i].core_affinity); + EXPECT_TRUE(rcutils_thread_core_affinity_is_set(&attrs.attributes[i].core_affinity, i)); EXPECT_EQ(RCUTILS_THREAD_SCHEDULING_POLICY_FIFO, attrs.attributes[i].scheduling_policy); } } + +TEST_F(TestParseThreadAttr, affinity_multiple_core) { + rcutils_ret_t ret; + prepare_yaml_parser( + "{ priority: 10, name: thread-1, core_affinity: [1,2,3], scheduling_policy: FIFO }"); + + ret = parse_thread_attr(&parser, &attrs); + EXPECT_EQ(RCUTILS_RET_OK, ret); + EXPECT_FALSE(rcutils_thread_core_affinity_is_set(&attrs.attributes[0].core_affinity, 0)); + EXPECT_TRUE(rcutils_thread_core_affinity_is_set(&attrs.attributes[0].core_affinity, 1)); + EXPECT_TRUE(rcutils_thread_core_affinity_is_set(&attrs.attributes[0].core_affinity, 2)); + EXPECT_TRUE(rcutils_thread_core_affinity_is_set(&attrs.attributes[0].core_affinity, 3)); + EXPECT_FALSE(rcutils_thread_core_affinity_is_set(&attrs.attributes[0].core_affinity, 4)); +} diff --git a/rcl_yaml_param_parser/test/test_parser_thread_attr.cpp b/rcl_yaml_param_parser/test/test_parser_thread_attr.cpp index e2d604917..c50ad9d1b 100644 --- a/rcl_yaml_param_parser/test/test_parser_thread_attr.cpp +++ b/rcl_yaml_param_parser/test/test_parser_thread_attr.cpp @@ -86,12 +86,14 @@ TEST_F(TestParserThreadAttr, success_file) { ASSERT_EQ(RCUTILS_RET_OK, ret); EXPECT_EQ(10, attrs.num_attributes); - for (int i = 0; i < 10; ++i) { + for (size_t i = 0; i < 10; ++i) { EXPECT_EQ(attrs.attributes[i].priority, i * 10); char buf[32]; - snprintf(buf, sizeof(buf), "thread-%d", i); + snprintf(buf, sizeof(buf), "thread-%lu", i); EXPECT_STREQ(buf, attrs.attributes[i].name); - EXPECT_EQ(i, attrs.attributes[i].core_affinity); + EXPECT_TRUE(rcutils_thread_core_affinity_is_set(&attrs.attributes[i].core_affinity, i)); + EXPECT_TRUE(rcutils_thread_core_affinity_is_set(&attrs.attributes[i].core_affinity, i + 10)); + EXPECT_TRUE(rcutils_thread_core_affinity_is_set(&attrs.attributes[i].core_affinity, i * i)); EXPECT_EQ(expected_policies[i], attrs.attributes[i].scheduling_policy); } } @@ -109,12 +111,14 @@ TEST_F(TestParserThreadAttr, success_value) { ASSERT_EQ(RCUTILS_RET_OK, ret); EXPECT_EQ(10, attrs.num_attributes); - for (int i = 0; i < 10; ++i) { + for (size_t i = 0; i < 10; ++i) { EXPECT_EQ(attrs.attributes[i].priority, i * 10); char buf[32]; - snprintf(buf, sizeof(buf), "thread-%d", i); + snprintf(buf, sizeof(buf), "thread-%lu", i); EXPECT_STREQ(buf, attrs.attributes[i].name); - EXPECT_EQ(i, attrs.attributes[i].core_affinity); + EXPECT_TRUE(rcutils_thread_core_affinity_is_set(&attrs.attributes[i].core_affinity, i)); + EXPECT_TRUE(rcutils_thread_core_affinity_is_set(&attrs.attributes[i].core_affinity, i + 10)); + EXPECT_TRUE(rcutils_thread_core_affinity_is_set(&attrs.attributes[i].core_affinity, i * i)); EXPECT_EQ(expected_policies[i], attrs.attributes[i].scheduling_policy); } } diff --git a/rcl_yaml_param_parser/test/thread_attr_success.yaml b/rcl_yaml_param_parser/test/thread_attr_success.yaml index a6aae7922..5e9bdafa2 100644 --- a/rcl_yaml_param_parser/test/thread_attr_success.yaml +++ b/rcl_yaml_param_parser/test/thread_attr_success.yaml @@ -1,40 +1,40 @@ - priority: 0 name: thread-0 - core_affinity: 0 + core_affinity: [0,10,0] scheduling_policy: BADSCHEDPOLICY1 - priority: 10 name: thread-1 - core_affinity: 1 + core_affinity: [1,11,1] scheduling_policy: FIFO - priority: 20 name: thread-2 - core_affinity: 2 + core_affinity: [2,12,4] scheduling_policy: RR - priority: 30 name: thread-3 - core_affinity: 3 + core_affinity: [3,13,9] scheduling_policy: SPORADIC - priority: 40 name: thread-4 - core_affinity: 4 + core_affinity: [4,14,16] scheduling_policy: OTHER - priority: 50 name: thread-5 - core_affinity: 5 + core_affinity: [5,15,25] scheduling_policy: IDLE - priority: 60 name: thread-6 - core_affinity: 6 + core_affinity: [6,16,36] scheduling_policy: BATCH - priority: 70 name: thread-7 - core_affinity: 7 + core_affinity: [7,17,49] scheduling_policy: DEADLINE - priority: 80 name: thread-8 - core_affinity: 8 + core_affinity: [8,18,64] scheduling_policy: BADSCHEDPOLICY2 - priority: 90 name: thread-9 - core_affinity: 9 + core_affinity: [9,19,81] scheduling_policy: FIFO From d94fa547707cbf12862deef579e53a65eaf9c8ca Mon Sep 17 00:00:00 2001 From: Shoji Morita Date: Thu, 14 Dec 2023 14:33:02 +0900 Subject: [PATCH 6/7] Fixed a trivial bug related to core affinity configuration. Signed-off-by: Shoji Morita --- rcl_yaml_param_parser/src/parse_thread_attr.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rcl_yaml_param_parser/src/parse_thread_attr.c b/rcl_yaml_param_parser/src/parse_thread_attr.c index a922a8599..a659d2f4e 100644 --- a/rcl_yaml_param_parser/src/parse_thread_attr.c +++ b/rcl_yaml_param_parser/src/parse_thread_attr.c @@ -163,7 +163,7 @@ rcutils_ret_t parse_thread_attr( ret = RCUTILS_RET_ERROR; goto error; } - ret = rcutils_thread_core_affinity_init(&core_affinity, 0, allocator); + ret = rcutils_thread_core_affinity_init(&core_affinity, allocator); if (RCUTILS_RET_OK != ret) { goto error; } From 97ebc75a0bf86a090c37bd4a30b9855ecaabdfe2 Mon Sep 17 00:00:00 2001 From: Shoji Morita Date: Fri, 26 Jan 2024 19:06:26 +0900 Subject: [PATCH 7/7] Modified the structure of member names to reflect the point made on the thread below. https://github.com/ros-infrastructure/rep/pull/385#discussion_r1350512216 Signed-off-by: Shoji Morita --- rcl/test/rcl/test_arguments.cpp | 4 +-- .../test_arguments/test_thread_attrs.yaml | 4 +-- rcl_yaml_param_parser/src/impl/types.h | 4 +-- rcl_yaml_param_parser/src/parse_thread_attr.c | 32 +++++++++---------- .../test/test_parse_thread_attr.cpp | 18 +++++------ .../test/test_parser_thread_attr.cpp | 8 ++--- .../test/thread_attr_success.yaml | 20 ++++++------ 7 files changed, 45 insertions(+), 45 deletions(-) diff --git a/rcl/test/rcl/test_arguments.cpp b/rcl/test/rcl/test_arguments.cpp index e86d215ed..cbff38e3d 100644 --- a/rcl/test/rcl/test_arguments.cpp +++ b/rcl/test/rcl/test_arguments.cpp @@ -210,7 +210,7 @@ TEST_F(CLASSNAME(TestArgumentsFixture, RMW_IMPLEMENTATION), check_known_vs_unkno EXPECT_TRUE( are_known_ros_args( {"--ros-args", "--thread-attrs-value", - "[{priority: 10, scheduling_policy: FIFO, name: thread-1, core_affinity: [1,2,3]}]"})); + "[{priority: 10, scheduling_policy: FIFO, tag: attr-1, core_affinity: [1,2,3]}]"})); } bool @@ -238,7 +238,7 @@ TEST_F(CLASSNAME(TestArgumentsFixture, RMW_IMPLEMENTATION), check_valid_vs_inval "--params-file", parameters_filepath.c_str(), "--log-level", "INFO", "--log-config-file", "file.config", "--thread-attrs-value", - "[{priority: 10, scheduling_policy: IDLE, name: thread-1, core_affinity: [1]}]" + "[{priority: 10, scheduling_policy: IDLE, tag: attr-1, core_affinity: [1]}]" })); // ROS args unknown to rcl are not (necessarily) invalid diff --git a/rcl/test/resources/test_arguments/test_thread_attrs.yaml b/rcl/test/resources/test_arguments/test_thread_attrs.yaml index af9e89cb8..b1096dcc2 100644 --- a/rcl/test/resources/test_arguments/test_thread_attrs.yaml +++ b/rcl/test/resources/test_arguments/test_thread_attrs.yaml @@ -1,8 +1,8 @@ -- name: thread-1 +- tag: attr-1 priority: 10 scheduling_policy: FIFO core_affinity: [1] -- name: thread-2 +- tag: attr-2 priority: 20 scheduling_policy: RR core_affinity: [2] diff --git a/rcl_yaml_param_parser/src/impl/types.h b/rcl_yaml_param_parser/src/impl/types.h index f7f0c27ee..aae3e384b 100644 --- a/rcl_yaml_param_parser/src/impl/types.h +++ b/rcl_yaml_param_parser/src/impl/types.h @@ -68,7 +68,7 @@ typedef enum thread_attr_key_type_e THREAD_ATTR_KEY_CORE_AFFINITY = 1, THREAD_ATTR_KEY_SCHEDULING_POLICY = 2, THREAD_ATTR_KEY_PRIORITY = 4, - THREAD_ATTR_KEY_NAME = 8 + THREAD_ATTR_KEY_TAG = 8 } thread_attr_key_type_t; typedef enum thread_attr_key_bits_e @@ -78,7 +78,7 @@ typedef enum thread_attr_key_bits_e THREAD_ATTR_KEY_CORE_AFFINITY | THREAD_ATTR_KEY_SCHEDULING_POLICY | THREAD_ATTR_KEY_PRIORITY | - THREAD_ATTR_KEY_NAME + THREAD_ATTR_KEY_TAG } thread_attr_key_bits_t; #ifdef __cplusplus diff --git a/rcl_yaml_param_parser/src/parse_thread_attr.c b/rcl_yaml_param_parser/src/parse_thread_attr.c index a659d2f4e..88eca2bb6 100644 --- a/rcl_yaml_param_parser/src/parse_thread_attr.c +++ b/rcl_yaml_param_parser/src/parse_thread_attr.c @@ -64,10 +64,10 @@ parse_thread_attr_key( *key_type = THREAD_ATTR_KEY_PRIORITY; } else if (strcmp(str, "scheduling_policy") == 0) { *key_type = THREAD_ATTR_KEY_SCHEDULING_POLICY; - } else if (strcmp(str, "name") == 0) { - *key_type = THREAD_ATTR_KEY_NAME; + } else if (strcmp(str, "tag") == 0) { + *key_type = THREAD_ATTR_KEY_TAG; } else if (*str == '\0') { - RCUTILS_SET_ERROR_MSG("empty name for a thread attribute"); + RCUTILS_SET_ERROR_MSG("empty tag for a thread attribute"); ret = RCUTILS_RET_ERROR; goto error; } else { @@ -122,7 +122,7 @@ rcutils_ret_t parse_thread_attr( rcutils_thread_core_affinity_t core_affinity = rcutils_get_zero_initialized_thread_core_affinity(); int priority; - char const * name = NULL; + char const * tag = NULL; rcutils_allocator_t allocator = attrs->allocator; void * ret_val = NULL; @@ -154,7 +154,7 @@ rcutils_ret_t parse_thread_attr( const char * value = (char *)event.data.scalar.value; yaml_scalar_style_t style = event.data.scalar.style; - const yaml_char_t * tag = event.data.scalar.tag; + const yaml_char_t * yaml_tag = event.data.scalar.tag; data_types_t val_type; switch (key_type) { @@ -174,8 +174,8 @@ rcutils_ret_t parse_thread_attr( } value = (char *)event.data.scalar.value; style = event.data.scalar.style; - tag = event.data.scalar.tag; - ret_val = get_value(value, style, tag, &val_type, allocator); + yaml_tag = event.data.scalar.tag; + ret_val = get_value(value, style, yaml_tag, &val_type, allocator); if (DATA_TYPE_INT64 != val_type) { RCUTILS_SET_ERROR_MSG_WITH_FORMAT_STRING( "Unrecognized value for thread core affinity: %s", value); @@ -195,7 +195,7 @@ rcutils_ret_t parse_thread_attr( if (event.type != YAML_SCALAR_EVENT) { goto error; } - ret_val = get_value(value, style, tag, &val_type, allocator); + ret_val = get_value(value, style, yaml_tag, &val_type, allocator); if (DATA_TYPE_INT64 != val_type) { RCUTILS_SET_ERROR_MSG_WITH_FORMAT_STRING( "Unrecognized value for thread priority: %s", value); @@ -210,17 +210,17 @@ rcutils_ret_t parse_thread_attr( } sched_policy = parse_thread_attr_scheduling_policy(value); break; - case THREAD_ATTR_KEY_NAME: + case THREAD_ATTR_KEY_TAG: if (event.type != YAML_SCALAR_EVENT) { goto error; } if (*value == '\0') { - RCUTILS_SET_ERROR_MSG("Empty thread name"); + RCUTILS_SET_ERROR_MSG("Empty thread attribute tag"); ret = RCUTILS_RET_ERROR; goto error; } - name = rcutils_strdup(value, allocator); - if (NULL == name) { + tag = rcutils_strdup(value, allocator); + if (NULL == tag) { ret = RCUTILS_RET_BAD_ALLOC; goto error; } @@ -241,18 +241,18 @@ rcutils_ret_t parse_thread_attr( goto error; } - ret = rcutils_thread_attrs_add_attr(attrs, sched_policy, &core_affinity, priority, name); + ret = rcutils_thread_attrs_add_attr(attrs, sched_policy, &core_affinity, priority, tag); if (RCUTILS_RET_OK != ret) { goto error; } - allocator.deallocate((char *)name, allocator.state); + allocator.deallocate((char *)tag, allocator.state); return RCUTILS_RET_OK; error: - if (NULL != name) { - allocator.deallocate((char *)name, allocator.state); + if (NULL != tag) { + allocator.deallocate((char *)tag, allocator.state); } if (NULL != ret_val) { allocator.deallocate(ret_val, allocator.state); diff --git a/rcl_yaml_param_parser/test/test_parse_thread_attr.cpp b/rcl_yaml_param_parser/test/test_parse_thread_attr.cpp index 1251066c4..058c09ffa 100644 --- a/rcl_yaml_param_parser/test/test_parse_thread_attr.cpp +++ b/rcl_yaml_param_parser/test/test_parse_thread_attr.cpp @@ -78,14 +78,14 @@ TEST_F(TestParseThreadAttr, success) { yaml_event_t event; prepare_yaml_parser( - "{ priority: 10, name: thread-1, core_affinity: [1], scheduling_policy: FIFO }"); + "{ priority: 10, tag: attr-1, core_affinity: [1], scheduling_policy: FIFO }"); ret = parse_thread_attr(&parser, &attrs); EXPECT_EQ(RCUTILS_RET_OK, ret); EXPECT_EQ(1, attrs.num_attributes); EXPECT_EQ(10, attrs.attributes[0].priority); - EXPECT_STREQ("thread-1", attrs.attributes[0].name); + EXPECT_STREQ("attr-1", attrs.attributes[0].tag); EXPECT_TRUE(rcutils_thread_core_affinity_is_set(&attrs.attributes[0].core_affinity, 1)); EXPECT_EQ(RCUTILS_THREAD_SCHEDULING_POLICY_FIFO, attrs.attributes[0].scheduling_policy); @@ -102,7 +102,7 @@ TEST_F(TestParseThreadAttr, unknown_key) { rcutils_ret_t ret; prepare_yaml_parser( - "{ priority: 10, name: thread-1, core_affinity: [1], unknown_key: FIFO }"); + "{ priority: 10, tag: attr-1, core_affinity: [1], unknown_key: FIFO }"); ret = parse_thread_attr(&parser, &attrs); EXPECT_EQ(RCUTILS_RET_ERROR, ret); @@ -112,7 +112,7 @@ TEST_F(TestParseThreadAttr, all_valid_keys_with_unknown_key) { rcutils_ret_t ret; prepare_yaml_parser( - "{ priority: 10, name: thread-1, core_affinity: [1], " + "{ priority: 10, tag: attr-1, core_affinity: [1], " "scheduling_policy: FIFO, unknown_key: RR }"); ret = parse_thread_attr(&parser, &attrs); @@ -122,7 +122,7 @@ TEST_F(TestParseThreadAttr, all_valid_keys_with_unknown_key) { TEST_F(TestParseThreadAttr, missing_key_value) { rcutils_ret_t ret; prepare_yaml_parser( - "{ priority: 10, name: thread-1 }"); + "{ priority: 10, tag: attr-1 }"); ret = parse_thread_attr(&parser, &attrs); EXPECT_EQ(RCUTILS_RET_ERROR, ret); @@ -135,7 +135,7 @@ TEST_F(TestParseThreadAttrs, success) { ss << "["; for (std::size_t i = 0; i < 100; ++i) { ss << "{ priority: " << i * 10; - ss << ", name: thread-" << i; + ss << ", tag: attr-" << i; ss << ", core_affinity: [" << i << "]"; ss << ", scheduling_policy: FIFO },"; } @@ -151,9 +151,9 @@ TEST_F(TestParseThreadAttrs, success) { for (std::size_t i = 0; i < 100; ++i) { EXPECT_EQ(i * 10, attrs.attributes[i].priority); ss.str(""); - ss << "thread-" << i; + ss << "attr-" << i; buf = ss.str(); - EXPECT_STREQ(buf.c_str(), attrs.attributes[i].name); + EXPECT_STREQ(buf.c_str(), attrs.attributes[i].tag); EXPECT_TRUE(rcutils_thread_core_affinity_is_set(&attrs.attributes[i].core_affinity, i)); EXPECT_EQ(RCUTILS_THREAD_SCHEDULING_POLICY_FIFO, attrs.attributes[i].scheduling_policy); } @@ -162,7 +162,7 @@ TEST_F(TestParseThreadAttrs, success) { TEST_F(TestParseThreadAttr, affinity_multiple_core) { rcutils_ret_t ret; prepare_yaml_parser( - "{ priority: 10, name: thread-1, core_affinity: [1,2,3], scheduling_policy: FIFO }"); + "{ priority: 10, tag: attr-1, core_affinity: [1,2,3], scheduling_policy: FIFO }"); ret = parse_thread_attr(&parser, &attrs); EXPECT_EQ(RCUTILS_RET_OK, ret); diff --git a/rcl_yaml_param_parser/test/test_parser_thread_attr.cpp b/rcl_yaml_param_parser/test/test_parser_thread_attr.cpp index c50ad9d1b..6d8668a0a 100644 --- a/rcl_yaml_param_parser/test/test_parser_thread_attr.cpp +++ b/rcl_yaml_param_parser/test/test_parser_thread_attr.cpp @@ -89,8 +89,8 @@ TEST_F(TestParserThreadAttr, success_file) { for (size_t i = 0; i < 10; ++i) { EXPECT_EQ(attrs.attributes[i].priority, i * 10); char buf[32]; - snprintf(buf, sizeof(buf), "thread-%lu", i); - EXPECT_STREQ(buf, attrs.attributes[i].name); + snprintf(buf, sizeof(buf), "attr-%lu", i); + EXPECT_STREQ(buf, attrs.attributes[i].tag); EXPECT_TRUE(rcutils_thread_core_affinity_is_set(&attrs.attributes[i].core_affinity, i)); EXPECT_TRUE(rcutils_thread_core_affinity_is_set(&attrs.attributes[i].core_affinity, i + 10)); EXPECT_TRUE(rcutils_thread_core_affinity_is_set(&attrs.attributes[i].core_affinity, i * i)); @@ -114,8 +114,8 @@ TEST_F(TestParserThreadAttr, success_value) { for (size_t i = 0; i < 10; ++i) { EXPECT_EQ(attrs.attributes[i].priority, i * 10); char buf[32]; - snprintf(buf, sizeof(buf), "thread-%lu", i); - EXPECT_STREQ(buf, attrs.attributes[i].name); + snprintf(buf, sizeof(buf), "attr-%lu", i); + EXPECT_STREQ(buf, attrs.attributes[i].tag); EXPECT_TRUE(rcutils_thread_core_affinity_is_set(&attrs.attributes[i].core_affinity, i)); EXPECT_TRUE(rcutils_thread_core_affinity_is_set(&attrs.attributes[i].core_affinity, i + 10)); EXPECT_TRUE(rcutils_thread_core_affinity_is_set(&attrs.attributes[i].core_affinity, i * i)); diff --git a/rcl_yaml_param_parser/test/thread_attr_success.yaml b/rcl_yaml_param_parser/test/thread_attr_success.yaml index 5e9bdafa2..062c98a97 100644 --- a/rcl_yaml_param_parser/test/thread_attr_success.yaml +++ b/rcl_yaml_param_parser/test/thread_attr_success.yaml @@ -1,40 +1,40 @@ - priority: 0 - name: thread-0 + tag: attr-0 core_affinity: [0,10,0] scheduling_policy: BADSCHEDPOLICY1 - priority: 10 - name: thread-1 + tag: attr-1 core_affinity: [1,11,1] scheduling_policy: FIFO - priority: 20 - name: thread-2 + tag: attr-2 core_affinity: [2,12,4] scheduling_policy: RR - priority: 30 - name: thread-3 + tag: attr-3 core_affinity: [3,13,9] scheduling_policy: SPORADIC - priority: 40 - name: thread-4 + tag: attr-4 core_affinity: [4,14,16] scheduling_policy: OTHER - priority: 50 - name: thread-5 + tag: attr-5 core_affinity: [5,15,25] scheduling_policy: IDLE - priority: 60 - name: thread-6 + tag: attr-6 core_affinity: [6,16,36] scheduling_policy: BATCH - priority: 70 - name: thread-7 + tag: attr-7 core_affinity: [7,17,49] scheduling_policy: DEADLINE - priority: 80 - name: thread-8 + tag: attr-8 core_affinity: [8,18,64] scheduling_policy: BADSCHEDPOLICY2 - priority: 90 - name: thread-9 + tag: attr-9 core_affinity: [9,19,81] scheduling_policy: FIFO