From c971529d0866ffe2bf2eaddbee29cb23f4f82a47 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mathias=20L=C3=BCdtke?= Date: Sat, 21 Oct 2017 17:27:41 +0200 Subject: [PATCH 1/5] test for HW interface without default constructor --- hardware_interface/test/interface_manager_test.cpp | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/hardware_interface/test/interface_manager_test.cpp b/hardware_interface/test/interface_manager_test.cpp index bf7fd184b..51f72917a 100644 --- a/hardware_interface/test/interface_manager_test.cpp +++ b/hardware_interface/test/interface_manager_test.cpp @@ -34,6 +34,7 @@ using namespace hardware_interface; struct FooInterface { + FooInterface(int foo): foo(foo) {} int foo; }; @@ -50,7 +51,7 @@ struct BazInterface TEST(InterfaceManagerTest, InterfaceRegistration) { // Register interfaces - FooInterface foo_iface; + FooInterface foo_iface(0); BarInterface bar_iface; InterfaceManager iface_mgr; @@ -66,11 +67,8 @@ TEST(InterfaceManagerTest, InterfaceRegistration) TEST(InterfaceManagerTest, InterfaceRewriting) { // Two instances of the same interface - FooInterface foo_iface_1; - foo_iface_1.foo = 1; - - FooInterface foo_iface_2; - foo_iface_2.foo = 2; + FooInterface foo_iface_1(1); + FooInterface foo_iface_2(2); // Register first interface and validate it InterfaceManager iface_mgr; @@ -91,4 +89,3 @@ int main(int argc, char** argv) testing::InitGoogleTest(&argc, argv); return RUN_ALL_TESTS(); } - From 148942efae4c299deb684857e3b65170458018c0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mathias=20L=C3=BCdtke?= Date: Sat, 21 Oct 2017 18:21:08 +0200 Subject: [PATCH 2/5] get rid of warnings for functions returning no values --- .../internal/interface_manager.h | 23 +++++++------------ 1 file changed, 8 insertions(+), 15 deletions(-) diff --git a/hardware_interface/include/hardware_interface/internal/interface_manager.h b/hardware_interface/include/hardware_interface/internal/interface_manager.h index 392fb58c0..dad46e09f 100644 --- a/hardware_interface/include/hardware_interface/internal/interface_manager.h +++ b/hardware_interface/include/hardware_interface/internal/interface_manager.h @@ -63,7 +63,7 @@ struct CheckIsResourceManager { // method called if C is a ResourceManager template - static yes& callCM(typename std::vector& managers, C* result, typename C::resource_manager_type*) + static void callCM(typename std::vector& managers, C* result, typename C::resource_manager_type*) { std::vector managers_in; // we have to typecase back to base class @@ -74,7 +74,7 @@ struct CheckIsResourceManager { // method called if C is not a ResourceManager template - static no& callCM(typename std::vector& managers, C* result, ...) {} + static void callCM(typename std::vector& managers, C* result, ...) {} // calls ResourceManager::concatManagers if C is a ResourceManager static const void callConcatManagers(typename std::vector& managers, T* result) @@ -83,18 +83,18 @@ struct CheckIsResourceManager { // method called if C is a ResourceManager template - static std::vector callGR(C* iface, typename C::resource_manager_type*) + static void callGR(std::vector &resources, C* iface, typename C::resource_manager_type*) { - return iface->getNames(); + resources = iface->getNames(); } // method called if C is not a ResourceManager template - static std::vector callGR(T* iface, ...) {} + static void callGR(std::vector &resources, T* iface, ...) { } // calls ResourceManager::concatManagers if C is a ResourceManager - static std::vector callGetResources(T* iface) - { return callGR(iface, 0); } + static void callGetResources(std::vector &resources, T* iface) + { return callGR(resources, iface, 0); } }; class InterfaceManager @@ -118,14 +118,7 @@ class InterfaceManager ROS_WARN_STREAM("Replacing previously registered interface '" << iface_name << "'."); } interfaces_[iface_name] = iface; - - std::vector resources; - if(CheckIsResourceManager::value) - { - // it is a ResourceManager. Get the names of the resources - resources = CheckIsResourceManager::callGetResources(iface); - } - resources_[iface_name] = resources; + CheckIsResourceManager::callGetResources(resources_[iface_name], iface); } void registerInterfaceManager(InterfaceManager* iface_man) From 26bfdc82113c8ef9a826b883b58a3ea0a24e2ae1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mathias=20L=C3=BCdtke?= Date: Sat, 21 Oct 2017 18:17:57 +0200 Subject: [PATCH 3/5] do not require default constructors for HardwareInterface classes ResourceManager-based interfaces still need a default constructor. --- .../internal/interface_manager.h | 34 +++++++++++++++---- 1 file changed, 27 insertions(+), 7 deletions(-) diff --git a/hardware_interface/include/hardware_interface/internal/interface_manager.h b/hardware_interface/include/hardware_interface/internal/interface_manager.h index dad46e09f..b65053747 100644 --- a/hardware_interface/include/hardware_interface/internal/interface_manager.h +++ b/hardware_interface/include/hardware_interface/internal/interface_manager.h @@ -95,6 +95,31 @@ struct CheckIsResourceManager { // calls ResourceManager::concatManagers if C is a ResourceManager static void callGetResources(std::vector &resources, T* iface) { return callGR(resources, iface, 0); } + + template + static T* newCI(boost::ptr_vector &guards, typename C::resource_manager_type*) + { + T* iface_combo = new T; + // save the new interface pointer to allow for its correct destruction + guards.push_back(static_cast(iface_combo)); + return iface_combo; + } + + // method called if C is not a ResourceManager + template + static T* newCI(boost::ptr_vector &guards, ...) { + // it is not a ResourceManager + ROS_ERROR("You cannot register multiple interfaces of the same type which are " + "not of type ResourceManager. There is no established protocol " + "for combining them."); + return NULL; + } + + static T* newCombinedInterface(boost::ptr_vector &guards) + { + return newCI(guards, 0); + } + }; class InterfaceManager @@ -180,13 +205,8 @@ class InterfaceManager iface_combo = static_cast(it_combo->second); } else { // no existing combined interface - if(CheckIsResourceManager::value) { - // it is a ResourceManager - - // create a new combined interface - iface_combo = new T; - // save the new interface pointer to allow for its correct destruction - interface_destruction_list_.push_back(reinterpret_cast(iface_combo)); + iface_combo = CheckIsResourceManager::newCombinedInterface(interface_destruction_list_); + if(iface_combo) { // concat all of the resource managers together CheckIsResourceManager::callConcatManagers(iface_list, iface_combo); // save the combined interface for if this is called again From 50b7790c03ee269c42a897a29e161b92f6c53b3d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mathias=20L=C3=BCdtke?= Date: Sat, 21 Oct 2017 18:19:52 +0200 Subject: [PATCH 4/5] remove unused CheckIsResourceManager::value --- .../internal/interface_manager.h | 15 --------------- 1 file changed, 15 deletions(-) diff --git a/hardware_interface/include/hardware_interface/internal/interface_manager.h b/hardware_interface/include/hardware_interface/internal/interface_manager.h index b65053747..1bb89231f 100644 --- a/hardware_interface/include/hardware_interface/internal/interface_manager.h +++ b/hardware_interface/include/hardware_interface/internal/interface_manager.h @@ -46,21 +46,6 @@ namespace hardware_interface // SFINAE workaround, so that we have reflection inside the template functions template struct CheckIsResourceManager { - // variable definitions for compiler-time logic - typedef char yes[1]; - typedef char no[2]; - - // method called if C is a ResourceManager - template - static yes& testRM(typename C::resource_manager_type*); - - // method called if C is not a ResourceManager - template - static no& testRM(...); - - // CheckIsResourceManager::value == true when T is a ResourceManager - static const bool value = (sizeof(testRM(0)) == sizeof(yes)); - // method called if C is a ResourceManager template static void callCM(typename std::vector& managers, C* result, typename C::resource_manager_type*) From cdaf18e924297fd74ac544cedbcd8cea1df45c12 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mathias=20L=C3=BCdtke?= Date: Tue, 24 Oct 2017 18:07:08 +0200 Subject: [PATCH 5/5] move CheckIsResourceManager into internal namespace --- .../hardware_interface/internal/interface_manager.h | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/hardware_interface/include/hardware_interface/internal/interface_manager.h b/hardware_interface/include/hardware_interface/internal/interface_manager.h index 1bb89231f..acbdfa4f0 100644 --- a/hardware_interface/include/hardware_interface/internal/interface_manager.h +++ b/hardware_interface/include/hardware_interface/internal/interface_manager.h @@ -43,6 +43,10 @@ namespace hardware_interface { + +namespace internal +{ + // SFINAE workaround, so that we have reflection inside the template functions template struct CheckIsResourceManager { @@ -107,6 +111,8 @@ struct CheckIsResourceManager { }; +} // namespace internal + class InterfaceManager { public: @@ -128,7 +134,7 @@ class InterfaceManager ROS_WARN_STREAM("Replacing previously registered interface '" << iface_name << "'."); } interfaces_[iface_name] = iface; - CheckIsResourceManager::callGetResources(resources_[iface_name], iface); + internal::CheckIsResourceManager::callGetResources(resources_[iface_name], iface); } void registerInterfaceManager(InterfaceManager* iface_man) @@ -190,10 +196,10 @@ class InterfaceManager iface_combo = static_cast(it_combo->second); } else { // no existing combined interface - iface_combo = CheckIsResourceManager::newCombinedInterface(interface_destruction_list_); + iface_combo = internal::CheckIsResourceManager::newCombinedInterface(interface_destruction_list_); if(iface_combo) { // concat all of the resource managers together - CheckIsResourceManager::callConcatManagers(iface_list, iface_combo); + internal::CheckIsResourceManager::callConcatManagers(iface_list, iface_combo); // save the combined interface for if this is called again interfaces_combo_[type_name] = iface_combo; num_ifaces_registered_[type_name] = iface_list.size();