diff --git a/rclcpp/resource/logging.hpp.em b/rclcpp/resource/logging.hpp.em index 3e99619c79..e8009678c7 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().c_str()); \ @[ end if]@ } while (0) diff --git a/rclcpp/src/rclcpp/logger.cpp b/rclcpp/src/rclcpp/logger.cpp index 421de7275e..af65b84623 100644 --- a/rclcpp/src/rclcpp/logger.cpp +++ b/rclcpp/src/rclcpp/logger.cpp @@ -38,7 +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", 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..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", - context_ptr.get()); + static_cast(context_ptr.get())); context_ptr->shutdown("signal handler"); } } 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..7fce7db39a 100644 --- a/rclcpp/test/rclcpp/test_logging.cpp +++ b/rclcpp/test/rclcpp/test_logging.cpp @@ -110,26 +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"}) { RCLCPP_DEBUG_STREAM(g_logger, "message " << i); 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/src/component_manager.cpp b/rclcpp_components/src/component_manager.cpp index bfb1b5cf89..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,7 +244,7 @@ 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()); + RCLCPP_WARN(get_logger(), "%s", ss.str().c_str()); } else { if (auto exec = executor_.lock()) { exec->remove_node(wrapper->second.get_node_base_interface()); 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); } } }