From 2a6d7a5ab4ab2885daf83bb4503bf89f1d5bc51d Mon Sep 17 00:00:00 2001 From: Joshua Wallace Date: Thu, 8 Jun 2023 11:16:08 -0400 Subject: [PATCH 1/6] added compile flags --- nav2_common/cmake/nav2_package.cmake | 8 +++++++- nav2_core/include/nav2_core/behavior_tree_navigator.hpp | 2 +- .../include/costmap_queue/map_based_queue.hpp | 5 +++++ .../include/nav2_mppi_controller/critic_manager.hpp | 5 +++-- nav2_mppi_controller/test/critic_manager_test.cpp | 4 ++++ 5 files changed, 20 insertions(+), 4 deletions(-) diff --git a/nav2_common/cmake/nav2_package.cmake b/nav2_common/cmake/nav2_package.cmake index 82a0fe92b1..8dc025dec3 100644 --- a/nav2_common/cmake/nav2_package.cmake +++ b/nav2_common/cmake/nav2_package.cmake @@ -37,8 +37,14 @@ macro(nav2_package) endif() if(CMAKE_COMPILER_IS_GNUCXX OR CMAKE_CXX_COMPILER_ID MATCHES "Clang") - add_compile_options(-Wall -Wextra -Wpedantic -Werror -Wdeprecated -fPIC) + add_compile_options(-Wall -Wextra -Wpedantic -Werror -Wdeprecated -fPIC -Woverloaded-virtual -Wnon-virtual-dtor -Wcast-align -Wunused -Wmisleading-indentation -Wduplicated-cond -Wduplicated-branches) endif() + # Wold-style-cast used in Throttle + # Wconversion breaks alot of things + # Wuseless cast + # double promotation + # Wnull-dereference denosie layer + option(COVERAGE_ENABLED "Enable code coverage" FALSE) if(COVERAGE_ENABLED) diff --git a/nav2_core/include/nav2_core/behavior_tree_navigator.hpp b/nav2_core/include/nav2_core/behavior_tree_navigator.hpp index 9a101f20ef..1a35aa6686 100644 --- a/nav2_core/include/nav2_core/behavior_tree_navigator.hpp +++ b/nav2_core/include/nav2_core/behavior_tree_navigator.hpp @@ -117,7 +117,7 @@ class NavigatorBase { public: NavigatorBase() = default; - ~NavigatorBase() = default; + virtual ~NavigatorBase() = default; /** * @brief Configuration of the navigator's backend BT and actions diff --git a/nav2_dwb_controller/costmap_queue/include/costmap_queue/map_based_queue.hpp b/nav2_dwb_controller/costmap_queue/include/costmap_queue/map_based_queue.hpp index 3a723e9c95..97db6c1c07 100644 --- a/nav2_dwb_controller/costmap_queue/include/costmap_queue/map_based_queue.hpp +++ b/nav2_dwb_controller/costmap_queue/include/costmap_queue/map_based_queue.hpp @@ -69,6 +69,11 @@ class MapBasedQueue { reset(); } + + /** + * @brief Default virtual Destructor + */ + virtual ~MapBasedQueue() = default; /** * @brief Clear the queue diff --git a/nav2_mppi_controller/include/nav2_mppi_controller/critic_manager.hpp b/nav2_mppi_controller/include/nav2_mppi_controller/critic_manager.hpp index d8eeb87a3a..b7902dba5b 100644 --- a/nav2_mppi_controller/include/nav2_mppi_controller/critic_manager.hpp +++ b/nav2_mppi_controller/include/nav2_mppi_controller/critic_manager.hpp @@ -47,6 +47,8 @@ class CriticManager */ CriticManager() = default; + virtual ~CriticManager() = default; + /** * @brief Configure critic manager on bringup and load plugins * @param parent WeakPtr to node @@ -73,14 +75,13 @@ class CriticManager /** * @brief Load the critic plugins */ - virtual void loadCritics(); + void loadCritics(); /** * @brief Get full-name namespaced critic IDs */ std::string getFullName(const std::string & name); -protected: rclcpp_lifecycle::LifecycleNode::WeakPtr parent_; std::shared_ptr costmap_ros_; std::string name_; diff --git a/nav2_mppi_controller/test/critic_manager_test.cpp b/nav2_mppi_controller/test/critic_manager_test.cpp index 0ed1fc811f..180809a9eb 100644 --- a/nav2_mppi_controller/test/critic_manager_test.cpp +++ b/nav2_mppi_controller/test/critic_manager_test.cpp @@ -46,6 +46,8 @@ class CriticManagerWrapper : public CriticManager CriticManagerWrapper() : CriticManager() {} + virtual ~CriticManagerWrapper() = default; + virtual void loadCritics() { critics_.clear(); @@ -78,6 +80,8 @@ class CriticManagerWrapperEnum : public CriticManager CriticManagerWrapperEnum() : CriticManager() {} + // virtual ~CriticManagerWrapperEnum() = default; + unsigned int getCriticNum() { return critics_.size(); From e5006caf0150675eb194652766ff82889cf6e1db Mon Sep 17 00:00:00 2001 From: Joshua Wallace Date: Thu, 8 Jun 2023 11:17:31 -0400 Subject: [PATCH 2/6] cleanup --- nav2_common/cmake/nav2_package.cmake | 6 ------ 1 file changed, 6 deletions(-) diff --git a/nav2_common/cmake/nav2_package.cmake b/nav2_common/cmake/nav2_package.cmake index 8dc025dec3..4bc38265c7 100644 --- a/nav2_common/cmake/nav2_package.cmake +++ b/nav2_common/cmake/nav2_package.cmake @@ -39,12 +39,6 @@ macro(nav2_package) if(CMAKE_COMPILER_IS_GNUCXX OR CMAKE_CXX_COMPILER_ID MATCHES "Clang") add_compile_options(-Wall -Wextra -Wpedantic -Werror -Wdeprecated -fPIC -Woverloaded-virtual -Wnon-virtual-dtor -Wcast-align -Wunused -Wmisleading-indentation -Wduplicated-cond -Wduplicated-branches) endif() - # Wold-style-cast used in Throttle - # Wconversion breaks alot of things - # Wuseless cast - # double promotation - # Wnull-dereference denosie layer - option(COVERAGE_ENABLED "Enable code coverage" FALSE) if(COVERAGE_ENABLED) From ac00416c76c8cf18cb03f87c16be1af4d6668664 Mon Sep 17 00:00:00 2001 From: Joshua Wallace Date: Thu, 8 Jun 2023 11:24:00 -0400 Subject: [PATCH 3/6] cleanup --- nav2_common/cmake/nav2_package.cmake | 2 +- .../include/nav2_mppi_controller/critic_manager.hpp | 7 ++++++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/nav2_common/cmake/nav2_package.cmake b/nav2_common/cmake/nav2_package.cmake index 4bc38265c7..a49fc15d7d 100644 --- a/nav2_common/cmake/nav2_package.cmake +++ b/nav2_common/cmake/nav2_package.cmake @@ -37,7 +37,7 @@ macro(nav2_package) endif() if(CMAKE_COMPILER_IS_GNUCXX OR CMAKE_CXX_COMPILER_ID MATCHES "Clang") - add_compile_options(-Wall -Wextra -Wpedantic -Werror -Wdeprecated -fPIC -Woverloaded-virtual -Wnon-virtual-dtor -Wcast-align -Wunused -Wmisleading-indentation -Wduplicated-cond -Wduplicated-branches) + add_compile_options(-Wall -Wextra -Wpedantic -Werror -Wdeprecated -fPIC -Wnon-virtual-dtor) endif() option(COVERAGE_ENABLED "Enable code coverage" FALSE) diff --git a/nav2_mppi_controller/include/nav2_mppi_controller/critic_manager.hpp b/nav2_mppi_controller/include/nav2_mppi_controller/critic_manager.hpp index b7902dba5b..2ae8a38513 100644 --- a/nav2_mppi_controller/include/nav2_mppi_controller/critic_manager.hpp +++ b/nav2_mppi_controller/include/nav2_mppi_controller/critic_manager.hpp @@ -47,6 +47,10 @@ class CriticManager */ CriticManager() = default; + + /** + * @brief Virtual Destructor for mppi::CriticManager + */ virtual ~CriticManager() = default; /** @@ -75,13 +79,14 @@ class CriticManager /** * @brief Load the critic plugins */ - void loadCritics(); + virtual void loadCritics(); /** * @brief Get full-name namespaced critic IDs */ std::string getFullName(const std::string & name); +protected: rclcpp_lifecycle::LifecycleNode::WeakPtr parent_; std::shared_ptr costmap_ros_; std::string name_; From 41eb11bad2e592e0faff4a5c1867754c8239bd7e Mon Sep 17 00:00:00 2001 From: Joshua Wallace Date: Thu, 8 Jun 2023 11:25:05 -0400 Subject: [PATCH 4/6] cleanup --- nav2_mppi_controller/test/critic_manager_test.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nav2_mppi_controller/test/critic_manager_test.cpp b/nav2_mppi_controller/test/critic_manager_test.cpp index 180809a9eb..5a931de92f 100644 --- a/nav2_mppi_controller/test/critic_manager_test.cpp +++ b/nav2_mppi_controller/test/critic_manager_test.cpp @@ -80,7 +80,7 @@ class CriticManagerWrapperEnum : public CriticManager CriticManagerWrapperEnum() : CriticManager() {} - // virtual ~CriticManagerWrapperEnum() = default; + virtual ~CriticManagerWrapperEnum() = default; unsigned int getCriticNum() { From 128a752d394bc4c0ebb7bec4ea881b321d9cde80 Mon Sep 17 00:00:00 2001 From: Joshua Wallace Date: Thu, 8 Jun 2023 14:17:04 -0400 Subject: [PATCH 5/6] add virtual destructors --- nav2_system_tests/src/behavior_tree/dummy_action_server.hpp | 1 + nav2_system_tests/src/behavior_tree/dummy_service.hpp | 2 ++ 2 files changed, 3 insertions(+) diff --git a/nav2_system_tests/src/behavior_tree/dummy_action_server.hpp b/nav2_system_tests/src/behavior_tree/dummy_action_server.hpp index d478885eaf..c16bfebff3 100644 --- a/nav2_system_tests/src/behavior_tree/dummy_action_server.hpp +++ b/nav2_system_tests/src/behavior_tree/dummy_action_server.hpp @@ -55,6 +55,7 @@ class DummyActionServer std::bind(&DummyActionServer::handle_accepted, this, _1)); } + virtual ~DummyActionServer() = default; void setFailureRanges(const Ranges & failureRanges) { failure_ranges_ = failureRanges; diff --git a/nav2_system_tests/src/behavior_tree/dummy_service.hpp b/nav2_system_tests/src/behavior_tree/dummy_service.hpp index ff5d742dd9..e24244d113 100644 --- a/nav2_system_tests/src/behavior_tree/dummy_service.hpp +++ b/nav2_system_tests/src/behavior_tree/dummy_service.hpp @@ -44,6 +44,8 @@ class DummyService std::bind(&DummyService::handle_service, this, _1, _2, _3)); } + virtual ~DummyService() = default; + void disable() { service_.reset(); From 7cd2b5240e348fd8b260e12e190edcb1f220b4f5 Mon Sep 17 00:00:00 2001 From: Joshua Wallace Date: Thu, 8 Jun 2023 20:01:10 -0400 Subject: [PATCH 6/6] uncrusify --- .../costmap_queue/include/costmap_queue/map_based_queue.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nav2_dwb_controller/costmap_queue/include/costmap_queue/map_based_queue.hpp b/nav2_dwb_controller/costmap_queue/include/costmap_queue/map_based_queue.hpp index 97db6c1c07..ba206fca7e 100644 --- a/nav2_dwb_controller/costmap_queue/include/costmap_queue/map_based_queue.hpp +++ b/nav2_dwb_controller/costmap_queue/include/costmap_queue/map_based_queue.hpp @@ -69,7 +69,7 @@ class MapBasedQueue { reset(); } - + /** * @brief Default virtual Destructor */