From f3a7e396aef0c84da4de58f3f9c87281298fec17 Mon Sep 17 00:00:00 2001 From: Ivan Santiago Paunovic Date: Tue, 3 Mar 2020 13:24:53 -0300 Subject: [PATCH 1/3] Use 10sec lifespan in rosout publisher qos Signed-off-by: Ivan Santiago Paunovic --- rcl/src/rcl/logging_rosout.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/rcl/src/rcl/logging_rosout.c b/rcl/src/rcl/logging_rosout.c index 9929c00b5..776dbdc3a 100644 --- a/rcl/src/rcl/logging_rosout.c +++ b/rcl/src/rcl/logging_rosout.c @@ -173,8 +173,11 @@ rcl_ret_t rcl_logging_rosout_init_publisher_for_node( const rosidl_message_type_support_t * type_support = rosidl_typesupport_c__get_message_type_support_handle__rcl_interfaces__msg__Log(); rcl_publisher_options_t options = rcl_publisher_get_default_options(); + // Late joining subscriptions get the last 10 seconds of logs, up to 1000 logs. options.qos.depth = 1000; options.qos.durability = RMW_QOS_POLICY_DURABILITY_TRANSIENT_LOCAL; + options.qos.lifespan.nsec = 0; + options.qos.lifespan.sec = 10; new_entry.publisher = rcl_get_zero_initialized_publisher(); status = rcl_publisher_init(&new_entry.publisher, node, type_support, ROSOUT_TOPIC_NAME, &options); From c8c251cb202d331d7210d8c30b9c8fc8a750bc08 Mon Sep 17 00:00:00 2001 From: Ivan Santiago Paunovic Date: Tue, 3 Mar 2020 13:57:56 -0300 Subject: [PATCH 2/3] Address peer review comments Signed-off-by: Ivan Santiago Paunovic --- rcl/include/rcl/expand_and_remap.h | 91 +++++++++++++++++++++++++++ rcl/src/expand_and_remap.c | 99 ++++++++++++++++++++++++++++++ rcl/src/rcl/logging_rosout.c | 2 +- rcl/test/rcl/test_logging.cpp | 47 ++++++++++++++ 4 files changed, 238 insertions(+), 1 deletion(-) create mode 100644 rcl/include/rcl/expand_and_remap.h create mode 100644 rcl/src/expand_and_remap.c create mode 100644 rcl/test/rcl/test_logging.cpp diff --git a/rcl/include/rcl/expand_and_remap.h b/rcl/include/rcl/expand_and_remap.h new file mode 100644 index 000000000..be1be69c8 --- /dev/null +++ b/rcl/include/rcl/expand_and_remap.h @@ -0,0 +1,91 @@ +// Copyright 2020 Open Source Robotics Foundation, Inc. +// +// 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__EXPAND_AND_REMAP_TOPIC_NAME_H_ +#define RCL__EXPAND_AND_REMAP_TOPIC_NAME_H_ + +#ifdef __cplusplus +extern "C" +{ +#endif + +#include "rcl/node.h" +#include "rcl/visibility_control.h" + +/// Expand and apply remapping rules in a given name. +/** + * The input_topic_name, node_name, and node_namespace arguments must all be + * vaid, null terminated c strings. + * The output_topic_name will not be assigned a value in the event of an error. + * + * The output_topic_name will be null terminated. + * It is also allocated, so it needs to be deallocated, when it is no longer + * needed, with the same allocator given to this function. + * Make sure the `char *` which is passed for the output_topic_name does not + * point to allocated memory before calling this function, because it will be + * overwritten and therefore leaked if this function is successful. + * + * The input topic name is validated using rcl_validate_topic_name(), + * but if it fails validation RCL_RET_TOPIC_NAME_INVALID is returned. + * + * The input node name is validated using rmw_validate_node_name(), + * but if it fails validation RCL_RET_NODE_INVALID_NAME is returned. + * + * The input node namespace is validated using rmw_validate_namespace(), + * but if it fails validation RCL_RET_NODE_INVALID_NAMESPACE is returned. + * + * /sa rcl_expand_topic_name + * /sa rcl_remap_topic_name + * /sa rmw_remap_service_name + * + *
+ * Attribute | Adherence + * ------------------ | ------------- + * Allocates Memory | Yes + * Thread-Safe | No + * Uses Atomics | No + * Lock-Free | Yes + * + * \param[in] input_topic_name topic name to be expanded + * \param[in] node_name name of the node associated with the topic + * \param[in] node_namespace namespace of the node associated with the topic + * \param[in] allocator the allocator to be used when creating the output topic + * \param[in] is_service indicates that a service name should be expanded when `true`. + * If not, a topic name is expanded. + * \param[out] output_topic_name output char * pointer + * \return `RCL_RET_OK` if the topic name was expanded successfully, or + * \return `RCL_RET_INVALID_ARGUMENT` if any arguments are invalid, or + * \return `RCL_RET_BAD_ALLOC` if allocating memory failed, or + * \return `RCL_RET_TOPIC_NAME_INVALID` if the given topic name is invalid, or + * \return `RCL_RET_NODE_INVALID_NAME` if the name is invalid, or + * \return `RCL_RET_NODE_INVALID_NAMESPACE` if the namespace_ is invalid, or + * \return `RCL_RET_UNKNOWN_SUBSTITUTION` for unknown substitutions in name, or + * \return `RCL_RET_ERROR` if an unspecified error occurs. + */ +RCL_PUBLIC +RCL_WARN_UNUSED +rcl_ret_t +rcl_expand_and_remap_name( + const char * input_topic_name, + const char * node_name, + const char * node_namespace, + rcl_allocator_t allocator, + bool is_service, + char ** output_topic_name); + +#ifdef __cplusplus +} +#endif + +#endif // RCL__EXPAND_AND_REMAP_TOPIC_NAME_H_ diff --git a/rcl/src/expand_and_remap.c b/rcl/src/expand_and_remap.c new file mode 100644 index 000000000..221152c5f --- /dev/null +++ b/rcl/src/expand_and_remap.c @@ -0,0 +1,99 @@ +// Copyright 2020 Open Source Robotics Foundation, Inc. +// +// 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 "rcl/expand_and_remap.h" + +#include "rcutils/logging_macros.h" + +#include "rcl/expand_topic_name.h" +#include "rcl/remap.h" + +rcl_ret_t +rcl_expand_and_remap_name( + const char * input_topic_name, + const char * node_name, + const char * node_namespace, + rcl_allocator_t * allocator, + bool is_service, + char ** output_topic_name) +{ + // Expand the given topic name. + rcutils_allocator_t rcutils_allocator = *allocator; // implicit conversion to rcutils version + rcutils_string_map_t substitutions_map = rcutils_get_zero_initialized_string_map(); + rcutils_ret_t rcutils_ret = rcutils_string_map_init(&substitutions_map, 0, rcutils_allocator); + if (rcutils_ret != RCUTILS_RET_OK) { + RCL_SET_ERROR_MSG(rcutils_get_error_string().str); + if (rcutils_ret == RCUTILS_RET_BAD_ALLOC) { + return RCL_RET_BAD_ALLOC; + } + return RCL_RET_ERROR; + } + rcl_ret_t ret = rcl_get_default_topic_name_substitutions(&substitutions_map); + if (ret != RCL_RET_OK) { + rcutils_ret = rcutils_string_map_fini(&substitutions_map); + if (rcutils_ret != RCUTILS_RET_OK) { + RCUTILS_LOG_ERROR_NAMED( + ROS_PACKAGE_NAME, + "failed to fini string_map (%d) during error handling: %s", + rcutils_ret, + rcutils_get_error_string().str); + } + if (ret == RCL_RET_BAD_ALLOC) { + return ret; + } + return RCL_RET_ERROR; + } + char * expanded_topic_name = NULL; + ret = rcl_expand_topic_name( + input_topic_name, + node_name, + node_namespace, + &substitutions_map, + *allocator, + &expanded_topic_name); + rcutils_ret = rcutils_string_map_fini(&substitutions_map); + if (rcutils_ret != RCUTILS_RET_OK) { + RCL_SET_ERROR_MSG(rcutils_get_error_string().str); + ret = RCL_RET_ERROR; + goto cleanup; + } + if (ret != RCL_RET_OK) { + if (ret == RCL_RET_TOPIC_NAME_INVALID || ret == RCL_RET_UNKNOWN_SUBSTITUTION) { + ret = RCL_RET_TOPIC_NAME_INVALID; + } else { + ret = RCL_RET_ERROR; + } + goto cleanup; + } + RCUTILS_LOG_DEBUG_NAMED(ROS_PACKAGE_NAME, "Expanded topic name '%s'", expanded_topic_name); + + const rcl_node_options_t * node_options = rcl_node_get_options(node); + if (NULL == node_options) { + ret = RCL_RET_ERROR; + goto cleanup; + } + rcl_arguments_t * global_args = NULL; + if (node_options->use_global_arguments) { + global_args = &(node->context->global_arguments); + } + ret = rcl_remap_topic_name( + &(node_options->arguments), global_args, expanded_topic_name, + rcl_node_get_name(node), rcl_node_get_namespace(node), *allocator, &remapped_topic_name); + if (RCL_RET_OK != ret) { + goto fail; + } else if (NULL == remapped_topic_name) { + remapped_topic_name = expanded_topic_name; + expanded_topic_name = NULL; + } +} diff --git a/rcl/src/rcl/logging_rosout.c b/rcl/src/rcl/logging_rosout.c index 776dbdc3a..b078f126d 100644 --- a/rcl/src/rcl/logging_rosout.c +++ b/rcl/src/rcl/logging_rosout.c @@ -176,8 +176,8 @@ rcl_ret_t rcl_logging_rosout_init_publisher_for_node( // Late joining subscriptions get the last 10 seconds of logs, up to 1000 logs. options.qos.depth = 1000; options.qos.durability = RMW_QOS_POLICY_DURABILITY_TRANSIENT_LOCAL; - options.qos.lifespan.nsec = 0; options.qos.lifespan.sec = 10; + options.qos.lifespan.nsec = 0; new_entry.publisher = rcl_get_zero_initialized_publisher(); status = rcl_publisher_init(&new_entry.publisher, node, type_support, ROSOUT_TOPIC_NAME, &options); diff --git a/rcl/test/rcl/test_logging.cpp b/rcl/test/rcl/test_logging.cpp new file mode 100644 index 000000000..04b107087 --- /dev/null +++ b/rcl/test/rcl/test_logging.cpp @@ -0,0 +1,47 @@ +// Copyright 2020 Open Source Robotics Foundation, Inc. +// +// 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/logging.h" + +#ifdef RMW_IMPLEMENTATION +# define CLASSNAME_(NAME, SUFFIX) NAME ## __ ## SUFFIX +# define CLASSNAME(NAME, SUFFIX) CLASSNAME_(NAME, SUFFIX) +#else +# define CLASSNAME(NAME, SUFFIX) NAME +#endif + +class CLASSNAME (TestLoggingFixture, RMW_IMPLEMENTATION) : public ::testing::Test {}; + +#define CONTAINS_LOGGING_ARGUMENTS(argv) \ + rcl_contains_logging_arguments(sizeof(argv) / sizeof(const char *), argv) + +TEST_F(CLASSNAME(TestLoggingFixture, RMW_IMPLEMENTATION), contains_logging_arguments) { + const char * with_logging_arguments[] = { + "--log-level", "debug", "--log-config-file", "asd.config" + }; + const char * with_one_logging_argument[] = {"--enable-stdout-logs", "bsd"}; + const char * with_mixed_arguments[] = { + "asd", "--log-level", "debug", "bsd", "--log-config-file", "asd.config" + }; + const char * with_other_argument[] = {"--asd"}; + const char * with_other_arguments[] = {"--asd", "bsd"}; + + EXPECT_TRUE(CONTAINS_LOGGING_ARGUMENTS(with_logging_arguments)); + EXPECT_TRUE(CONTAINS_LOGGING_ARGUMENTS(with_one_logging_argument)); + EXPECT_TRUE(CONTAINS_LOGGING_ARGUMENTS(with_mixed_arguments)); + EXPECT_FALSE(CONTAINS_LOGGING_ARGUMENTS(with_other_argument)); + EXPECT_FALSE(CONTAINS_LOGGING_ARGUMENTS(with_other_arguments)); +} From f611bbe447e10787d8e7df6892cde323e8444050 Mon Sep 17 00:00:00 2001 From: Ivan Santiago Paunovic Date: Tue, 3 Mar 2020 14:04:31 -0300 Subject: [PATCH 3/3] Delete unrelated files Signed-off-by: Ivan Santiago Paunovic --- rcl/include/rcl/expand_and_remap.h | 91 --------------------------- rcl/src/expand_and_remap.c | 99 ------------------------------ rcl/test/rcl/test_logging.cpp | 47 -------------- 3 files changed, 237 deletions(-) delete mode 100644 rcl/include/rcl/expand_and_remap.h delete mode 100644 rcl/src/expand_and_remap.c delete mode 100644 rcl/test/rcl/test_logging.cpp diff --git a/rcl/include/rcl/expand_and_remap.h b/rcl/include/rcl/expand_and_remap.h deleted file mode 100644 index be1be69c8..000000000 --- a/rcl/include/rcl/expand_and_remap.h +++ /dev/null @@ -1,91 +0,0 @@ -// Copyright 2020 Open Source Robotics Foundation, Inc. -// -// 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__EXPAND_AND_REMAP_TOPIC_NAME_H_ -#define RCL__EXPAND_AND_REMAP_TOPIC_NAME_H_ - -#ifdef __cplusplus -extern "C" -{ -#endif - -#include "rcl/node.h" -#include "rcl/visibility_control.h" - -/// Expand and apply remapping rules in a given name. -/** - * The input_topic_name, node_name, and node_namespace arguments must all be - * vaid, null terminated c strings. - * The output_topic_name will not be assigned a value in the event of an error. - * - * The output_topic_name will be null terminated. - * It is also allocated, so it needs to be deallocated, when it is no longer - * needed, with the same allocator given to this function. - * Make sure the `char *` which is passed for the output_topic_name does not - * point to allocated memory before calling this function, because it will be - * overwritten and therefore leaked if this function is successful. - * - * The input topic name is validated using rcl_validate_topic_name(), - * but if it fails validation RCL_RET_TOPIC_NAME_INVALID is returned. - * - * The input node name is validated using rmw_validate_node_name(), - * but if it fails validation RCL_RET_NODE_INVALID_NAME is returned. - * - * The input node namespace is validated using rmw_validate_namespace(), - * but if it fails validation RCL_RET_NODE_INVALID_NAMESPACE is returned. - * - * /sa rcl_expand_topic_name - * /sa rcl_remap_topic_name - * /sa rmw_remap_service_name - * - *
- * Attribute | Adherence - * ------------------ | ------------- - * Allocates Memory | Yes - * Thread-Safe | No - * Uses Atomics | No - * Lock-Free | Yes - * - * \param[in] input_topic_name topic name to be expanded - * \param[in] node_name name of the node associated with the topic - * \param[in] node_namespace namespace of the node associated with the topic - * \param[in] allocator the allocator to be used when creating the output topic - * \param[in] is_service indicates that a service name should be expanded when `true`. - * If not, a topic name is expanded. - * \param[out] output_topic_name output char * pointer - * \return `RCL_RET_OK` if the topic name was expanded successfully, or - * \return `RCL_RET_INVALID_ARGUMENT` if any arguments are invalid, or - * \return `RCL_RET_BAD_ALLOC` if allocating memory failed, or - * \return `RCL_RET_TOPIC_NAME_INVALID` if the given topic name is invalid, or - * \return `RCL_RET_NODE_INVALID_NAME` if the name is invalid, or - * \return `RCL_RET_NODE_INVALID_NAMESPACE` if the namespace_ is invalid, or - * \return `RCL_RET_UNKNOWN_SUBSTITUTION` for unknown substitutions in name, or - * \return `RCL_RET_ERROR` if an unspecified error occurs. - */ -RCL_PUBLIC -RCL_WARN_UNUSED -rcl_ret_t -rcl_expand_and_remap_name( - const char * input_topic_name, - const char * node_name, - const char * node_namespace, - rcl_allocator_t allocator, - bool is_service, - char ** output_topic_name); - -#ifdef __cplusplus -} -#endif - -#endif // RCL__EXPAND_AND_REMAP_TOPIC_NAME_H_ diff --git a/rcl/src/expand_and_remap.c b/rcl/src/expand_and_remap.c deleted file mode 100644 index 221152c5f..000000000 --- a/rcl/src/expand_and_remap.c +++ /dev/null @@ -1,99 +0,0 @@ -// Copyright 2020 Open Source Robotics Foundation, Inc. -// -// 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 "rcl/expand_and_remap.h" - -#include "rcutils/logging_macros.h" - -#include "rcl/expand_topic_name.h" -#include "rcl/remap.h" - -rcl_ret_t -rcl_expand_and_remap_name( - const char * input_topic_name, - const char * node_name, - const char * node_namespace, - rcl_allocator_t * allocator, - bool is_service, - char ** output_topic_name) -{ - // Expand the given topic name. - rcutils_allocator_t rcutils_allocator = *allocator; // implicit conversion to rcutils version - rcutils_string_map_t substitutions_map = rcutils_get_zero_initialized_string_map(); - rcutils_ret_t rcutils_ret = rcutils_string_map_init(&substitutions_map, 0, rcutils_allocator); - if (rcutils_ret != RCUTILS_RET_OK) { - RCL_SET_ERROR_MSG(rcutils_get_error_string().str); - if (rcutils_ret == RCUTILS_RET_BAD_ALLOC) { - return RCL_RET_BAD_ALLOC; - } - return RCL_RET_ERROR; - } - rcl_ret_t ret = rcl_get_default_topic_name_substitutions(&substitutions_map); - if (ret != RCL_RET_OK) { - rcutils_ret = rcutils_string_map_fini(&substitutions_map); - if (rcutils_ret != RCUTILS_RET_OK) { - RCUTILS_LOG_ERROR_NAMED( - ROS_PACKAGE_NAME, - "failed to fini string_map (%d) during error handling: %s", - rcutils_ret, - rcutils_get_error_string().str); - } - if (ret == RCL_RET_BAD_ALLOC) { - return ret; - } - return RCL_RET_ERROR; - } - char * expanded_topic_name = NULL; - ret = rcl_expand_topic_name( - input_topic_name, - node_name, - node_namespace, - &substitutions_map, - *allocator, - &expanded_topic_name); - rcutils_ret = rcutils_string_map_fini(&substitutions_map); - if (rcutils_ret != RCUTILS_RET_OK) { - RCL_SET_ERROR_MSG(rcutils_get_error_string().str); - ret = RCL_RET_ERROR; - goto cleanup; - } - if (ret != RCL_RET_OK) { - if (ret == RCL_RET_TOPIC_NAME_INVALID || ret == RCL_RET_UNKNOWN_SUBSTITUTION) { - ret = RCL_RET_TOPIC_NAME_INVALID; - } else { - ret = RCL_RET_ERROR; - } - goto cleanup; - } - RCUTILS_LOG_DEBUG_NAMED(ROS_PACKAGE_NAME, "Expanded topic name '%s'", expanded_topic_name); - - const rcl_node_options_t * node_options = rcl_node_get_options(node); - if (NULL == node_options) { - ret = RCL_RET_ERROR; - goto cleanup; - } - rcl_arguments_t * global_args = NULL; - if (node_options->use_global_arguments) { - global_args = &(node->context->global_arguments); - } - ret = rcl_remap_topic_name( - &(node_options->arguments), global_args, expanded_topic_name, - rcl_node_get_name(node), rcl_node_get_namespace(node), *allocator, &remapped_topic_name); - if (RCL_RET_OK != ret) { - goto fail; - } else if (NULL == remapped_topic_name) { - remapped_topic_name = expanded_topic_name; - expanded_topic_name = NULL; - } -} diff --git a/rcl/test/rcl/test_logging.cpp b/rcl/test/rcl/test_logging.cpp deleted file mode 100644 index 04b107087..000000000 --- a/rcl/test/rcl/test_logging.cpp +++ /dev/null @@ -1,47 +0,0 @@ -// Copyright 2020 Open Source Robotics Foundation, Inc. -// -// 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/logging.h" - -#ifdef RMW_IMPLEMENTATION -# define CLASSNAME_(NAME, SUFFIX) NAME ## __ ## SUFFIX -# define CLASSNAME(NAME, SUFFIX) CLASSNAME_(NAME, SUFFIX) -#else -# define CLASSNAME(NAME, SUFFIX) NAME -#endif - -class CLASSNAME (TestLoggingFixture, RMW_IMPLEMENTATION) : public ::testing::Test {}; - -#define CONTAINS_LOGGING_ARGUMENTS(argv) \ - rcl_contains_logging_arguments(sizeof(argv) / sizeof(const char *), argv) - -TEST_F(CLASSNAME(TestLoggingFixture, RMW_IMPLEMENTATION), contains_logging_arguments) { - const char * with_logging_arguments[] = { - "--log-level", "debug", "--log-config-file", "asd.config" - }; - const char * with_one_logging_argument[] = {"--enable-stdout-logs", "bsd"}; - const char * with_mixed_arguments[] = { - "asd", "--log-level", "debug", "bsd", "--log-config-file", "asd.config" - }; - const char * with_other_argument[] = {"--asd"}; - const char * with_other_arguments[] = {"--asd", "bsd"}; - - EXPECT_TRUE(CONTAINS_LOGGING_ARGUMENTS(with_logging_arguments)); - EXPECT_TRUE(CONTAINS_LOGGING_ARGUMENTS(with_one_logging_argument)); - EXPECT_TRUE(CONTAINS_LOGGING_ARGUMENTS(with_mixed_arguments)); - EXPECT_FALSE(CONTAINS_LOGGING_ARGUMENTS(with_other_argument)); - EXPECT_FALSE(CONTAINS_LOGGING_ARGUMENTS(with_other_arguments)); -}