From f30329fbec79cb699f41a37cc27c0c475de3edc1 Mon Sep 17 00:00:00 2001 From: Chris Lalancette Date: Fri, 28 Feb 2020 15:22:25 -0500 Subject: [PATCH] Switch to using new rcutils_strerror. (#993) * Switch to using new rcutils_strerror. Also increase timeouts for test_logging, which should reduce flakes on Windows. Signed-off-by: Chris Lalancette --- rclcpp/CMakeLists.txt | 3 +++ rclcpp/package.xml | 1 + rclcpp/src/rclcpp/signal_handler.cpp | 25 ++----------------------- rclcpp/test/test_logging.cpp | 6 +++--- 4 files changed, 9 insertions(+), 26 deletions(-) diff --git a/rclcpp/CMakeLists.txt b/rclcpp/CMakeLists.txt index 6420d7a..603e6b0 100644 --- a/rclcpp/CMakeLists.txt +++ b/rclcpp/CMakeLists.txt @@ -8,6 +8,7 @@ find_package(rcl REQUIRED) find_package(rcl_interfaces REQUIRED) find_package(rcl_yaml_param_parser REQUIRED) find_package(rcpputils REQUIRED) +find_package(rcutils REQUIRED) find_package(rmw REQUIRED) find_package(rosgraph_msgs REQUIRED) find_package(rosidl_generator_cpp REQUIRED) @@ -112,6 +113,7 @@ ament_target_dependencies(${PROJECT_NAME} "rcl" "rcl_yaml_param_parser" "rcpputils" + "rcutils" "builtin_interfaces" "rosgraph_msgs" "rosidl_typesupport_cpp" @@ -138,6 +140,7 @@ ament_export_libraries(${PROJECT_NAME}) ament_export_dependencies(ament_cmake) ament_export_dependencies(rcl) ament_export_dependencies(rcpputils) +ament_export_dependencies(rcutils) ament_export_dependencies(builtin_interfaces) ament_export_dependencies(rosgraph_msgs) ament_export_dependencies(rosidl_typesupport_cpp) diff --git a/rclcpp/package.xml b/rclcpp/package.xml index 6c268ba..2ebb8f7 100644 --- a/rclcpp/package.xml +++ b/rclcpp/package.xml @@ -25,6 +25,7 @@ rcl rcl_yaml_param_parser rcpputils + rcutils rmw tracetools diff --git a/rclcpp/src/rclcpp/signal_handler.cpp b/rclcpp/src/rclcpp/signal_handler.cpp index dbdc1ae..44ed628 100644 --- a/rclcpp/src/rclcpp/signal_handler.cpp +++ b/rclcpp/src/rclcpp/signal_handler.cpp @@ -30,6 +30,7 @@ #endif #include "rclcpp/logging.hpp" +#include "rcutils/strerror.h" #include "rmw/impl/cpp/demangle.hpp" using rclcpp::SignalHandler; @@ -159,28 +160,6 @@ SignalHandler::~SignalHandler() } } -RCLCPP_LOCAL -void -__safe_strerror(int errnum, char * buffer, size_t buffer_length) -{ -#if defined(_WIN32) - strerror_s(buffer, buffer_length, errnum); -#elif defined(_GNU_SOURCE) && (!defined(ANDROID) || __ANDROID_API__ >= 23) - /* GNU-specific */ - char * msg = strerror_r(errnum, buffer, buffer_length); - if (msg != buffer) { - strncpy(buffer, msg, buffer_length); - buffer[buffer_length - 1] = '\0'; - } -#else - /* XSI-compliant */ - int error_status = strerror_r(errnum, buffer, buffer_length); - if (error_status != 0) { - throw std::runtime_error("Failed to get error string for errno: " + std::to_string(errnum)); - } -#endif -} - SignalHandler::signal_handler_type SignalHandler::set_signal_handler( int signal_value, @@ -197,7 +176,7 @@ SignalHandler::set_signal_handler( #endif if (signal_handler_install_failed) { char error_string[1024]; - __safe_strerror(errno, error_string, sizeof(error_string)); + rcutils_strerror(error_string, sizeof(error_string)); auto msg = "Failed to set SIGINT signal handler (" + std::to_string(errno) + "): " + error_string; throw std::runtime_error(msg); diff --git a/rclcpp/test/test_logging.cpp b/rclcpp/test/test_logging.cpp index cc8bd62..87e93f9 100644 --- a/rclcpp/test/test_logging.cpp +++ b/rclcpp/test/test_logging.cpp @@ -205,9 +205,9 @@ TEST_F(TestLoggingMacros, test_throttle) { RCLCPP_DEBUG_SKIPFIRST_THROTTLE(g_logger, steady_clock, 1, "Skip first throttling"); EXPECT_EQ(1u, g_log_calls); for (uint64_t i = 0; i < 6; ++i) { - RCLCPP_DEBUG_THROTTLE(g_logger, steady_clock, 10, "Throttling"); - RCLCPP_DEBUG_SKIPFIRST_THROTTLE(g_logger, steady_clock, 40, "Throttling"); - std::this_thread::sleep_for(5ms); + RCLCPP_DEBUG_THROTTLE(g_logger, steady_clock, 100, "Throttling"); + RCLCPP_DEBUG_SKIPFIRST_THROTTLE(g_logger, steady_clock, 400, "Throttling"); + std::this_thread::sleep_for(50ms); } EXPECT_EQ(4u, g_log_calls); rclcpp::Clock ros_clock(RCL_ROS_TIME);