From 9e5187341a95d028751f83b2919f721af7e39dd0 Mon Sep 17 00:00:00 2001 From: Audrow Nash Date: Tue, 13 Oct 2020 21:41:28 -0700 Subject: [PATCH 1/9] Add warnings Signed-off-by: Audrow Nash --- rclcpp_components/CMakeLists.txt | 5 +- .../test/test_component_manager.cpp | 4 +- .../test/test_component_manager_api.cpp | 86 +++++++++---------- 3 files changed, 49 insertions(+), 46 deletions(-) diff --git a/rclcpp_components/CMakeLists.txt b/rclcpp_components/CMakeLists.txt index bd8ffddee8..5098fd8631 100644 --- a/rclcpp_components/CMakeLists.txt +++ b/rclcpp_components/CMakeLists.txt @@ -7,7 +7,10 @@ if(NOT CMAKE_CXX_STANDARD) set(CMAKE_CXX_STANDARD 14) endif() if(CMAKE_COMPILER_IS_GNUCXX OR CMAKE_CXX_COMPILER_ID MATCHES "Clang") - add_compile_options(-Wall -Wextra -Wpedantic -Wnon-virtual-dtor -Woverloaded-virtual) + add_compile_options( + -Wall -Wextra -Wpedantic -Wnon-virtual-dtor -Woverloaded-virtual + -Wformat=2 -Wconversion -Wshadow -Wsign-conversion -Wold-style-cast -Wcast-qual + ) endif() find_package(ament_cmake_ros REQUIRED) diff --git a/rclcpp_components/test/test_component_manager.cpp b/rclcpp_components/test/test_component_manager.cpp index a0515094cf..bed7aad54e 100644 --- a/rclcpp_components/test/test_component_manager.cpp +++ b/rclcpp_components/test/test_component_manager.cpp @@ -85,8 +85,8 @@ TEST_F(TestComponentManager, create_component_factory_invalid) rclcpp_components::ComponentManagerException); // Test valid library with invalid class - auto resources = manager->get_component_resources("rclcpp_components"); - auto factory = manager->create_component_factory({"foo_class", resources[0].second}); + auto component_resources = manager->get_component_resources("rclcpp_components"); + auto factory = manager->create_component_factory({"foo_class", component_resources[0].second}); EXPECT_EQ(nullptr, factory); // Test improperly formed resources file diff --git a/rclcpp_components/test/test_component_manager_api.cpp b/rclcpp_components/test/test_component_manager_api.cpp index 138fc38124..73d4bc3687 100644 --- a/rclcpp_components/test/test_component_manager_api.cpp +++ b/rclcpp_components/test/test_component_manager_api.cpp @@ -45,10 +45,10 @@ TEST_F(TestComponentManager, components_api) exec->add_node(manager); exec->add_node(node); - auto client = node->create_client( + auto composition_client = node->create_client( "/ComponentManager/_container/load_node"); - if (!client->wait_for_service(20s)) { + if (!composition_client->wait_for_service(20s)) { ASSERT_TRUE(false) << "service not available after waiting"; } @@ -57,7 +57,7 @@ TEST_F(TestComponentManager, components_api) request->package_name = "rclcpp_components"; request->plugin_name = "test_rclcpp_components::TestComponentFoo"; - auto result = client->async_send_request(request); + auto result = composition_client->async_send_request(request); auto ret = exec->spin_until_future_complete(result, 5s); // Wait for the result. EXPECT_EQ(ret, rclcpp::FutureReturnCode::SUCCESS); EXPECT_EQ(result.get()->success, true); @@ -71,7 +71,7 @@ TEST_F(TestComponentManager, components_api) request->package_name = "rclcpp_components"; request->plugin_name = "test_rclcpp_components::TestComponentBar"; - auto result = client->async_send_request(request); + auto result = composition_client->async_send_request(request); auto ret = exec->spin_until_future_complete(result, 5s); // Wait for the result. EXPECT_EQ(ret, rclcpp::FutureReturnCode::SUCCESS); EXPECT_EQ(result.get()->success, true); @@ -87,7 +87,7 @@ TEST_F(TestComponentManager, components_api) request->plugin_name = "test_rclcpp_components::TestComponentFoo"; request->node_name = "test_component_baz"; - auto result = client->async_send_request(request); + auto result = composition_client->async_send_request(request); auto ret = exec->spin_until_future_complete(result, 5s); // Wait for the result. EXPECT_EQ(ret, rclcpp::FutureReturnCode::SUCCESS); EXPECT_EQ(result.get()->success, true); @@ -104,7 +104,7 @@ TEST_F(TestComponentManager, components_api) request->node_namespace = "/ns"; request->node_name = "test_component_bing"; - auto result = client->async_send_request(request); + auto result = composition_client->async_send_request(request); auto ret = exec->spin_until_future_complete(result, 5s); // Wait for the result. EXPECT_EQ(ret, rclcpp::FutureReturnCode::SUCCESS); EXPECT_EQ(result.get()->success, true); @@ -119,7 +119,7 @@ TEST_F(TestComponentManager, components_api) request->package_name = "rclcpp_components"; request->plugin_name = "test_rclcpp_components::TestComponent"; - auto result = client->async_send_request(request); + auto result = composition_client->async_send_request(request); auto ret = exec->spin_until_future_complete(result, 5s); // Wait for the result. EXPECT_EQ(ret, rclcpp::FutureReturnCode::SUCCESS); EXPECT_EQ(result.get()->success, false); @@ -134,7 +134,7 @@ TEST_F(TestComponentManager, components_api) request->package_name = "rclcpp_components_foo"; request->plugin_name = "test_rclcpp_components::TestComponentFoo"; - auto result = client->async_send_request(request); + auto result = composition_client->async_send_request(request); auto ret = exec->spin_until_future_complete(result, 5s); // Wait for the result. EXPECT_EQ(ret, rclcpp::FutureReturnCode::SUCCESS); EXPECT_EQ(result.get()->success, false); @@ -151,7 +151,7 @@ TEST_F(TestComponentManager, components_api) request->node_name = "test_component_remap"; request->remap_rules.push_back("alice:=bob"); - auto result = client->async_send_request(request); + auto result = composition_client->async_send_request(request); auto ret = exec->spin_until_future_complete(result, 5s); // Wait for the result. EXPECT_EQ(ret, rclcpp::FutureReturnCode::SUCCESS); EXPECT_EQ(result.get()->success, true); @@ -170,7 +170,7 @@ TEST_F(TestComponentManager, components_api) rclcpp::ParameterValue(true)); request->extra_arguments.push_back(use_intraprocess_comms.to_parameter_msg()); - auto result = client->async_send_request(request); + auto result = composition_client->async_send_request(request); auto ret = exec->spin_until_future_complete(result, 5s); // Wait for the result. EXPECT_EQ(ret, rclcpp::FutureReturnCode::SUCCESS); EXPECT_EQ(result.get()->success, true); @@ -191,7 +191,7 @@ TEST_F(TestComponentManager, components_api) rclcpp::ParameterValue("hello")); request->extra_arguments.push_back(use_intraprocess_comms.to_parameter_msg()); - auto result = client->async_send_request(request); + auto result = composition_client->async_send_request(request); auto ret = exec->spin_until_future_complete(result, 5s); // Wait for the result. EXPECT_EQ(ret, rclcpp::FutureReturnCode::SUCCESS); EXPECT_EQ(result.get()->success, false); @@ -226,23 +226,23 @@ TEST_F(TestComponentManager, components_api) auto result = client->async_send_request(request); auto ret = exec->spin_until_future_complete(result, 5s); // Wait for the result. EXPECT_EQ(ret, rclcpp::FutureReturnCode::SUCCESS); - auto node_names = result.get()->full_node_names; - auto unique_ids = result.get()->unique_ids; - - EXPECT_EQ(node_names.size(), 6u); - EXPECT_EQ(node_names[0], "/test_component_foo"); - EXPECT_EQ(node_names[1], "/test_component_bar"); - EXPECT_EQ(node_names[2], "/test_component_baz"); - EXPECT_EQ(node_names[3], "/ns/test_component_bing"); - EXPECT_EQ(node_names[4], "/test_component_remap"); - EXPECT_EQ(node_names[5], "/test_component_intra_process"); - EXPECT_EQ(unique_ids.size(), 6u); - EXPECT_EQ(unique_ids[0], 1u); - EXPECT_EQ(unique_ids[1], 2u); - EXPECT_EQ(unique_ids[2], 3u); - EXPECT_EQ(unique_ids[3], 4u); - EXPECT_EQ(unique_ids[4], 5u); - EXPECT_EQ(unique_ids[5], 6u); + auto result_node_names = result.get()->full_node_names; + auto result_unique_ids = result.get()->unique_ids; + + EXPECT_EQ(result_node_names.size(), 6u); + EXPECT_EQ(result_node_names[0], "/test_component_foo"); + EXPECT_EQ(result_node_names[1], "/test_component_bar"); + EXPECT_EQ(result_node_names[2], "/test_component_baz"); + EXPECT_EQ(result_node_names[3], "/ns/test_component_bing"); + EXPECT_EQ(result_node_names[4], "/test_component_remap"); + EXPECT_EQ(result_node_names[5], "/test_component_intra_process"); + EXPECT_EQ(result_unique_ids.size(), 6u); + EXPECT_EQ(result_unique_ids[0], 1u); + EXPECT_EQ(result_unique_ids[1], 2u); + EXPECT_EQ(result_unique_ids[2], 3u); + EXPECT_EQ(result_unique_ids[3], 4u); + EXPECT_EQ(result_unique_ids[4], 5u); + EXPECT_EQ(result_unique_ids[5], 6u); } } @@ -290,21 +290,21 @@ TEST_F(TestComponentManager, components_api) auto result = client->async_send_request(request); auto ret = exec->spin_until_future_complete(result, 5s); // Wait for the result. EXPECT_EQ(ret, rclcpp::FutureReturnCode::SUCCESS); - auto node_names = result.get()->full_node_names; - auto unique_ids = result.get()->unique_ids; - - EXPECT_EQ(node_names.size(), 5u); - EXPECT_EQ(node_names[0], "/test_component_bar"); - EXPECT_EQ(node_names[1], "/test_component_baz"); - EXPECT_EQ(node_names[2], "/ns/test_component_bing"); - EXPECT_EQ(node_names[3], "/test_component_remap"); - EXPECT_EQ(node_names[4], "/test_component_intra_process"); - EXPECT_EQ(unique_ids.size(), 5u); - EXPECT_EQ(unique_ids[0], 2u); - EXPECT_EQ(unique_ids[1], 3u); - EXPECT_EQ(unique_ids[2], 4u); - EXPECT_EQ(unique_ids[3], 5u); - EXPECT_EQ(unique_ids[4], 6u); + auto result_node_names = result.get()->full_node_names; + auto result_unique_ids = result.get()->unique_ids; + + EXPECT_EQ(result_node_names.size(), 5u); + EXPECT_EQ(result_node_names[0], "/test_component_bar"); + EXPECT_EQ(result_node_names[1], "/test_component_baz"); + EXPECT_EQ(result_node_names[2], "/ns/test_component_bing"); + EXPECT_EQ(result_node_names[3], "/test_component_remap"); + EXPECT_EQ(result_node_names[4], "/test_component_intra_process"); + EXPECT_EQ(result_unique_ids.size(), 5u); + EXPECT_EQ(result_unique_ids[0], 2u); + EXPECT_EQ(result_unique_ids[1], 3u); + EXPECT_EQ(result_unique_ids[2], 4u); + EXPECT_EQ(result_unique_ids[3], 5u); + EXPECT_EQ(result_unique_ids[4], 6u); } } } From 6bc4a6babb0e6d79e7a5e9264c1361f33b35e9c8 Mon Sep 17 00:00:00 2001 From: Audrow Nash Date: Thu, 5 Nov 2020 15:32:46 -0800 Subject: [PATCH 2/9] Fix nonliteral string warnings Signed-off-by: Audrow Nash --- rclcpp/resource/logging.hpp.em | 5 ++-- rclcpp/test/rclcpp/test_logger.cpp | 42 +++++++++++++++-------------- rclcpp/test/rclcpp/test_logging.cpp | 4 +++ 3 files changed, 28 insertions(+), 23 deletions(-) diff --git a/rclcpp/resource/logging.hpp.em b/rclcpp/resource/logging.hpp.em index 3e99619c79..8caf61597d 100644 --- a/rclcpp/resource/logging.hpp.em +++ b/rclcpp/resource/logging.hpp.em @@ -151,10 +151,9 @@ def get_rclcpp_suffix_from_features(features): @[ end if]@ logger.get_name(), \ @[ if 'stream' not in feature_combination]@ - rclcpp::get_c_string(RCLCPP_FIRST_ARG(__VA_ARGS__, "")), \ - RCLCPP_ALL_BUT_FIRST_ARGS(__VA_ARGS__,"")); \ + __VA_ARGS__); \ @[ else]@ - "%s", rclcpp::get_c_string(ss.str())); \ + "%s", ss.str()); \ @[ end if]@ } while (0) diff --git a/rclcpp/test/rclcpp/test_logger.cpp b/rclcpp/test/rclcpp/test_logger.cpp index 34e4e7a4fb..b83a2d4037 100644 --- a/rclcpp/test/rclcpp/test_logger.cpp +++ b/rclcpp/test/rclcpp/test_logger.cpp @@ -80,75 +80,77 @@ TEST(TestLogger, set_level) { rcutils_logging_set_output_handler(rcutils_logging_console_output_handler); // default - RCLCPP_DEBUG(logger, std::string("message %s"), "debug"); + RCLCPP_DEBUG(logger, "message %s", "debug"); EXPECT_FALSE(g_last_log_event.console_output_handler_called); - RCLCPP_INFO(logger, std::string("message %s"), "info"); + RCLCPP_INFO(logger, "message %s", "info"); EXPECT_TRUE(g_last_log_event.console_output_handler_called); EXPECT_EQ("message info", g_last_log_event.message); // unset g_last_log_event.console_output_handler_called = false; logger.set_level(rclcpp::Logger::Level::Unset); - RCLCPP_DEBUG(logger, std::string("message %s"), "debug"); + RCLCPP_DEBUG(logger, "message"); + RCLCPP_DEBUG(logger, "message %s", "debug"); EXPECT_FALSE(g_last_log_event.console_output_handler_called); - RCLCPP_INFO(logger, std::string("message %s"), "info"); + RCLCPP_INFO(logger, "message"); + RCLCPP_INFO(logger, "message %s", "info"); EXPECT_TRUE(g_last_log_event.console_output_handler_called); EXPECT_EQ("message info", g_last_log_event.message); // debug g_last_log_event.console_output_handler_called = false; logger.set_level(rclcpp::Logger::Level::Debug); - RCLCPP_DEBUG(logger, std::string("message %s"), "debug"); + RCLCPP_DEBUG(logger, "message %s", "debug"); EXPECT_TRUE(g_last_log_event.console_output_handler_called); EXPECT_EQ("message debug", g_last_log_event.message); - RCLCPP_INFO(logger, std::string("message %s"), "info"); + RCLCPP_INFO(logger, "message %s", "info"); EXPECT_EQ("message info", g_last_log_event.message); // info g_last_log_event.console_output_handler_called = false; logger.set_level(rclcpp::Logger::Level::Info); - RCLCPP_DEBUG(logger, std::string("message %s"), "debug"); + RCLCPP_DEBUG(logger, "message %s", "debug"); EXPECT_FALSE(g_last_log_event.console_output_handler_called); - RCLCPP_INFO(logger, std::string("message %s"), "info"); + RCLCPP_INFO(logger, "message %s", "info"); EXPECT_TRUE(g_last_log_event.console_output_handler_called); EXPECT_EQ("message info", g_last_log_event.message); // warn g_last_log_event.console_output_handler_called = false; logger.set_level(rclcpp::Logger::Level::Warn); - RCLCPP_DEBUG(logger, std::string("message %s"), "debug"); + RCLCPP_DEBUG(logger, "message %s", "debug"); EXPECT_FALSE(g_last_log_event.console_output_handler_called); - RCLCPP_INFO(logger, std::string("message %s"), "info"); + RCLCPP_INFO(logger, "message %s", "info"); EXPECT_FALSE(g_last_log_event.console_output_handler_called); - RCLCPP_WARN(logger, std::string("message %s"), "warn"); + RCLCPP_WARN(logger, "message %s", "warn"); EXPECT_TRUE(g_last_log_event.console_output_handler_called); EXPECT_EQ("message warn", g_last_log_event.message); // error g_last_log_event.console_output_handler_called = false; logger.set_level(rclcpp::Logger::Level::Error); - RCLCPP_DEBUG(logger, std::string("message %s"), "debug"); + RCLCPP_DEBUG(logger, "message %s", "debug"); EXPECT_FALSE(g_last_log_event.console_output_handler_called); - RCLCPP_INFO(logger, std::string("message %s"), "info"); + RCLCPP_INFO(logger, "message %s", "info"); EXPECT_FALSE(g_last_log_event.console_output_handler_called); - RCLCPP_WARN(logger, std::string("message %s"), "warn"); + RCLCPP_WARN(logger, "message %s", "warn"); EXPECT_FALSE(g_last_log_event.console_output_handler_called); - RCLCPP_ERROR(logger, std::string("message %s"), "error"); + RCLCPP_ERROR(logger, "message %s", "error"); EXPECT_TRUE(g_last_log_event.console_output_handler_called); EXPECT_EQ("message error", g_last_log_event.message); // fatal g_last_log_event.console_output_handler_called = false; logger.set_level(rclcpp::Logger::Level::Fatal); - RCLCPP_DEBUG(logger, std::string("message %s"), "debug"); + RCLCPP_DEBUG(logger, "message %s", "debug"); EXPECT_FALSE(g_last_log_event.console_output_handler_called); - RCLCPP_INFO(logger, std::string("message %s"), "info"); + RCLCPP_INFO(logger, "message %s", "info"); EXPECT_FALSE(g_last_log_event.console_output_handler_called); - RCLCPP_WARN(logger, std::string("message %s"), "warn"); + RCLCPP_WARN(logger, "message %s", "warn"); EXPECT_FALSE(g_last_log_event.console_output_handler_called); - RCLCPP_ERROR(logger, std::string("message %s"), "error"); + RCLCPP_ERROR(logger, "message %s", "error"); EXPECT_FALSE(g_last_log_event.console_output_handler_called); - RCLCPP_FATAL(logger, std::string("message %s"), "fatal"); + RCLCPP_FATAL(logger, "message %s", "fatal"); EXPECT_TRUE(g_last_log_event.console_output_handler_called); EXPECT_EQ("message fatal", g_last_log_event.message); diff --git a/rclcpp/test/rclcpp/test_logging.cpp b/rclcpp/test/rclcpp/test_logging.cpp index 87e93f9fe3..878c4b2547 100644 --- a/rclcpp/test/rclcpp/test_logging.cpp +++ b/rclcpp/test/rclcpp/test_logging.cpp @@ -111,6 +111,7 @@ TEST_F(TestLoggingMacros, test_logging_named) { } TEST_F(TestLoggingMacros, test_logging_string) { + /* for (std::string i : {"one", "two", "three"}) { RCLCPP_DEBUG(g_logger, "message " + i); } @@ -128,9 +129,11 @@ TEST_F(TestLoggingMacros, test_logging_string) { RCLCPP_DEBUG(g_logger, "message seven"); EXPECT_EQ("message seven", g_last_log_event.message); + */ } TEST_F(TestLoggingMacros, test_logging_stream) { + /* for (std::string i : {"one", "two", "three"}) { RCLCPP_DEBUG_STREAM(g_logger, "message " << i); } @@ -142,6 +145,7 @@ TEST_F(TestLoggingMacros, test_logging_stream) { RCLCPP_DEBUG_STREAM(g_logger, "message " << 5); EXPECT_EQ("message 5", g_last_log_event.message); + */ } TEST_F(TestLoggingMacros, test_logging_once) { From abd223e32e4aa1daad0051d6a9ec18b172654009 Mon Sep 17 00:00:00 2001 From: Audrow Nash Date: Thu, 5 Nov 2020 16:17:14 -0800 Subject: [PATCH 3/9] Fix compile error from new logging functions in rclcpp_components Signed-off-by: Audrow Nash --- rclcpp_components/src/component_manager.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/rclcpp_components/src/component_manager.cpp b/rclcpp_components/src/component_manager.cpp index bfb1b5cf89..c344676a37 100644 --- a/rclcpp_components/src/component_manager.cpp +++ b/rclcpp_components/src/component_manager.cpp @@ -244,7 +244,9 @@ ComponentManager::OnUnloadNode( std::stringstream ss; ss << "No node found with unique_id: " << request->unique_id; response->error_message = ss.str(); - RCLCPP_WARN(get_logger(), ss.str()); + const std::string tmp = ss.str(); + const char * cstr = tmp.c_str(); + RCLCPP_WARN(get_logger(), cstr); } else { if (auto exec = executor_.lock()) { exec->remove_node(wrapper->second.get_node_base_interface()); From 9cf60bd471aae20508d18cc08d5d644be14bf08e Mon Sep 17 00:00:00 2001 From: Audrow Nash Date: Tue, 10 Nov 2020 09:08:58 -0800 Subject: [PATCH 4/9] Use "%s" to make log string evaluation secure Signed-off-by: Audrow Nash --- rclcpp_components/src/component_manager.cpp | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/rclcpp_components/src/component_manager.cpp b/rclcpp_components/src/component_manager.cpp index c344676a37..0eff0b67c2 100644 --- a/rclcpp_components/src/component_manager.cpp +++ b/rclcpp_components/src/component_manager.cpp @@ -223,7 +223,7 @@ ComponentManager::OnLoadNode( response->error_message = "Failed to find class with the requested plugin name."; response->success = false; } catch (const ComponentManagerException & ex) { - RCLCPP_ERROR(get_logger(), ex.what()); + RCLCPP_ERROR(get_logger(), "%s", ex.what()); response->error_message = ex.what(); response->success = false; } @@ -244,9 +244,7 @@ ComponentManager::OnUnloadNode( std::stringstream ss; ss << "No node found with unique_id: " << request->unique_id; response->error_message = ss.str(); - const std::string tmp = ss.str(); - const char * cstr = tmp.c_str(); - RCLCPP_WARN(get_logger(), cstr); + RCLCPP_WARN(get_logger(), "%s", ss.str().c_str()); } else { if (auto exec = executor_.lock()) { exec->remove_node(wrapper->second.get_node_base_interface()); From 96286cc5ff8a939e3c5ce22547eb10360d5382c8 Mon Sep 17 00:00:00 2001 From: Audrow Nash Date: Tue, 10 Nov 2020 11:39:39 -0800 Subject: [PATCH 5/9] Fix logging warnings in rclcpp Signed-off-by: Audrow Nash --- rclcpp/src/rclcpp/logger.cpp | 3 ++- rclcpp/src/rclcpp/signal_handler.cpp | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/rclcpp/src/rclcpp/logger.cpp b/rclcpp/src/rclcpp/logger.cpp index 421de7275e..7cf73a40a9 100644 --- a/rclcpp/src/rclcpp/logger.cpp +++ b/rclcpp/src/rclcpp/logger.cpp @@ -38,7 +38,8 @@ get_node_logger(const rcl_node_t * node) const char * logger_name = rcl_node_get_logger_name(node); if (nullptr == logger_name) { auto logger = rclcpp::get_logger("rclcpp"); - RCLCPP_ERROR(logger, "failed to get logger name from node at address %p", node); + RCLCPP_ERROR(logger, "failed to get logger name from node at address %p", + static_cast(const_cast(node))); return logger; } return rclcpp::get_logger(logger_name); diff --git a/rclcpp/src/rclcpp/signal_handler.cpp b/rclcpp/src/rclcpp/signal_handler.cpp index 44ed628591..cb2fa075d4 100644 --- a/rclcpp/src/rclcpp/signal_handler.cpp +++ b/rclcpp/src/rclcpp/signal_handler.cpp @@ -243,7 +243,7 @@ SignalHandler::deferred_signal_handler() get_logger(), "deferred_signal_handler(): " "shutting down rclcpp::Context @ %p, because it had shutdown_on_sigint == true", - context_ptr.get()); + static_cast(context_ptr.get())); context_ptr->shutdown("signal handler"); } } From 6b8392d1caf98701f0fe155b521d1f8d5ed3899e Mon Sep 17 00:00:00 2001 From: Audrow Nash Date: Tue, 10 Nov 2020 12:02:56 -0800 Subject: [PATCH 6/9] Fix coding style to pass linter Signed-off-by: Audrow Nash --- rclcpp/src/rclcpp/logger.cpp | 5 +++-- rclcpp/src/rclcpp/signal_handler.cpp | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/rclcpp/src/rclcpp/logger.cpp b/rclcpp/src/rclcpp/logger.cpp index 7cf73a40a9..af65b84623 100644 --- a/rclcpp/src/rclcpp/logger.cpp +++ b/rclcpp/src/rclcpp/logger.cpp @@ -38,8 +38,9 @@ get_node_logger(const rcl_node_t * node) const char * logger_name = rcl_node_get_logger_name(node); if (nullptr == logger_name) { auto logger = rclcpp::get_logger("rclcpp"); - RCLCPP_ERROR(logger, "failed to get logger name from node at address %p", - static_cast(const_cast(node))); + RCLCPP_ERROR( + logger, "failed to get logger name from node at address %p", + static_cast(const_cast(node))); return logger; } return rclcpp::get_logger(logger_name); diff --git a/rclcpp/src/rclcpp/signal_handler.cpp b/rclcpp/src/rclcpp/signal_handler.cpp index cb2fa075d4..eda8585002 100644 --- a/rclcpp/src/rclcpp/signal_handler.cpp +++ b/rclcpp/src/rclcpp/signal_handler.cpp @@ -243,7 +243,7 @@ SignalHandler::deferred_signal_handler() get_logger(), "deferred_signal_handler(): " "shutting down rclcpp::Context @ %p, because it had shutdown_on_sigint == true", - static_cast(context_ptr.get())); + static_cast(context_ptr.get())); context_ptr->shutdown("signal handler"); } } From c63283774f40cf7bfa5c8f3d2a60a77d61c95aa5 Mon Sep 17 00:00:00 2001 From: Audrow Nash Date: Thu, 10 Dec 2020 19:56:29 -0800 Subject: [PATCH 7/9] Get the c_str of streams Signed-off-by: Audrow Nash --- rclcpp/resource/logging.hpp.em | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rclcpp/resource/logging.hpp.em b/rclcpp/resource/logging.hpp.em index 8caf61597d..e8009678c7 100644 --- a/rclcpp/resource/logging.hpp.em +++ b/rclcpp/resource/logging.hpp.em @@ -153,7 +153,7 @@ def get_rclcpp_suffix_from_features(features): @[ if 'stream' not in feature_combination]@ __VA_ARGS__); \ @[ else]@ - "%s", ss.str()); \ + "%s", ss.str().c_str()); \ @[ end if]@ } while (0) From 774f72ba63d26256bd5bc8a04f680e9078f12fa9 Mon Sep 17 00:00:00 2001 From: Audrow Nash Date: Mon, 14 Dec 2020 11:57:54 -0800 Subject: [PATCH 8/9] Remove test to check string logging Signed-off-by: Audrow Nash --- rclcpp/test/rclcpp/test_logging.cpp | 22 ---------------------- 1 file changed, 22 deletions(-) diff --git a/rclcpp/test/rclcpp/test_logging.cpp b/rclcpp/test/rclcpp/test_logging.cpp index 878c4b2547..245db26f4d 100644 --- a/rclcpp/test/rclcpp/test_logging.cpp +++ b/rclcpp/test/rclcpp/test_logging.cpp @@ -110,28 +110,6 @@ TEST_F(TestLoggingMacros, test_logging_named) { EXPECT_EQ("message 3", g_last_log_event.message); } -TEST_F(TestLoggingMacros, test_logging_string) { - /* - for (std::string i : {"one", "two", "three"}) { - RCLCPP_DEBUG(g_logger, "message " + i); - } - EXPECT_EQ(3u, g_log_calls); - EXPECT_EQ("message three", g_last_log_event.message); - - RCLCPP_DEBUG(g_logger, "message " "four"); - EXPECT_EQ("message four", g_last_log_event.message); - - RCLCPP_DEBUG(g_logger, std::string("message " "five")); - EXPECT_EQ("message five", g_last_log_event.message); - - RCLCPP_DEBUG(g_logger, std::string("message %s"), "six"); - EXPECT_EQ("message six", g_last_log_event.message); - - RCLCPP_DEBUG(g_logger, "message seven"); - EXPECT_EQ("message seven", g_last_log_event.message); - */ -} - TEST_F(TestLoggingMacros, test_logging_stream) { /* for (std::string i : {"one", "two", "three"}) { From e71004c33f267c416cdc3314ce339a3cdae38b51 Mon Sep 17 00:00:00 2001 From: Audrow Nash Date: Mon, 14 Dec 2020 11:58:23 -0800 Subject: [PATCH 9/9] Add in stream logging tests Signed-off-by: Audrow Nash --- rclcpp/test/rclcpp/test_logging.cpp | 2 -- 1 file changed, 2 deletions(-) diff --git a/rclcpp/test/rclcpp/test_logging.cpp b/rclcpp/test/rclcpp/test_logging.cpp index 245db26f4d..7fce7db39a 100644 --- a/rclcpp/test/rclcpp/test_logging.cpp +++ b/rclcpp/test/rclcpp/test_logging.cpp @@ -111,7 +111,6 @@ TEST_F(TestLoggingMacros, test_logging_named) { } TEST_F(TestLoggingMacros, test_logging_stream) { - /* for (std::string i : {"one", "two", "three"}) { RCLCPP_DEBUG_STREAM(g_logger, "message " << i); } @@ -123,7 +122,6 @@ TEST_F(TestLoggingMacros, test_logging_stream) { RCLCPP_DEBUG_STREAM(g_logger, "message " << 5); EXPECT_EQ("message 5", g_last_log_event.message); - */ } TEST_F(TestLoggingMacros, test_logging_once) {