Add support for passing arguments to constructors (#291)

- updated `create*Instance` methods to forward arguments to plugin
  constructors using class_loader interface traits

Signed-off-by: pum1k <55055380+pum1k@users.noreply.github.com>

Author: pum1k

pluginlib/include/pluginlib/class_loader.hpp

@@ -34,6 +34,7 @@
 #include <string>
 #include <vector>
 
+#include "class_loader/interface_traits.hpp"
 #include "class_loader/multi_library_class_loader.hpp"
 #include "pluginlib/class_desc.hpp"
 #include "pluginlib/class_loader_base.hpp"
@@ -77,12 +78,16 @@ class ClassLoader : public ClassLoaderBase
    * Deleting the instance and calling unloadLibraryForClass() is automatically
    * handled by the shared pointer.
    * \param lookup_name The name of the class to load
+   * \param args Arguments passed to the constructor of the plugin.
+   *   Plugin parameters must be declared using class_loader::InterfaceTraits.
    * \throws pluginlib::LibraryLoadException when the library associated with
    *   the class cannot be loaded
    * \throws pluginlib::CreateClassException when the class cannot be instantiated
    * \return An instance of the class
    */
-  std::shared_ptr<T> createSharedInstance(const std::string & lookup_name);
+  template<typename ... Args,
+    std::enable_if_t<class_loader::is_interface_constructible_v<T, Args...>, bool> = true>
+  std::shared_ptr<T> createSharedInstance(const std::string & lookup_name, Args && ... args);
 
   /// Create an instance of a desired class.
   /**
@@ -95,12 +100,16 @@ class ClassLoader : public ClassLoaderBase
    * deleter when you want to destroy the released pointer.
    *
    * \param lookup_name The name of the class to load.
+   * \param args Arguments passed to the constructor of the plugin.
+   *   Plugin parameters must be declared using class_loader::InterfaceTraits.
    * \throws pluginlib::LibraryLoadException when the library associated with
    *   the class cannot be loaded.
    * \throws pluginlib::CreateClassException when the class cannot be instantiated
    * \return An instance of the class
    */
-  UniquePtr<T> createUniqueInstance(const std::string & lookup_name);
+  template<typename ... Args,
+    std::enable_if_t<class_loader::is_interface_constructible_v<T, Args...>, bool> = true>
+  UniquePtr<T> createUniqueInstance(const std::string & lookup_name, Args && ... args);
 
   /// Create an instance of a desired class.
   /**
@@ -111,12 +120,16 @@ class ClassLoader : public ClassLoaderBase
    *   (in order to decrement the associated library counter and unloading it
    *   if it is no more used).
    * \param lookup_name The name of the class to load
+   * \param args Arguments passed to the constructor of the plugin.
+   *   Plugin parameters must be declared using class_loader::InterfaceTraits.
    * \throws pluginlib::LibraryLoadException when the library associated with
    *   the class cannot be loaded
    * \throws pluginlib::CreateClassException when the class cannot be instantiated
    * \return An instance of the class
    */
-  T * createUnmanagedInstance(const std::string & lookup_name);
+  template<typename ... Args,
+    std::enable_if_t<class_loader::is_interface_constructible_v<T, Args...>, bool> = true>
+  T * createUnmanagedInstance(const std::string & lookup_name, Args && ... args);
 
   /// Return a list of all available plugin manifest paths for this ClassLoader's base class type.
   /**

pluginlib/include/pluginlib/class_loader_imp.hpp

@@ -45,6 +45,7 @@
 #include "ament_index_cpp/get_package_share_directory.hpp"
 #include "ament_index_cpp/get_resource.hpp"
 #include "ament_index_cpp/get_resources.hpp"
+#include "class_loader/interface_traits.hpp"
 #include "class_loader/class_loader.hpp"
 #include "rcpputils/shared_library.hpp"
 #include "rcutils/logging_macros.h"
@@ -106,14 +107,20 @@ ClassLoader<T>::~ClassLoader()
 }
 
 template<class T>
-std::shared_ptr<T> ClassLoader<T>::createSharedInstance(const std::string & lookup_name)
+template<typename ... Args,
+  std::enable_if_t<class_loader::is_interface_constructible_v<T, Args...>, bool>>
+std::shared_ptr<T> ClassLoader<T>::createSharedInstance(
+  const std::string & lookup_name,
+  Args &&... args)
 /***************************************************************************/
 {
-  return createUniqueInstance(lookup_name);
+  return createUniqueInstance(lookup_name, std::forward<Args>(args)...);
 }
 
 template<class T>
-UniquePtr<T> ClassLoader<T>::createUniqueInstance(const std::string & lookup_name)
+template<typename ... Args,
+  std::enable_if_t<class_loader::is_interface_constructible_v<T, Args...>, bool>>
+UniquePtr<T> ClassLoader<T>::createUniqueInstance(const std::string & lookup_name, Args &&... args)
 {
   RCUTILS_LOG_DEBUG_NAMED("pluginlib.ClassLoader",
     "Attempting to create managed (unique) instance for class %s.",
@@ -128,7 +135,8 @@ UniquePtr<T> ClassLoader<T>::createUniqueInstance(const std::string & lookup_nam
     RCUTILS_LOG_DEBUG_NAMED("pluginlib.ClassLoader", "%s maps to real class type %s",
       lookup_name.c_str(), class_type.c_str());
 
-    UniquePtr<T> obj = lowlevel_class_loader_.createUniqueInstance<T>(class_type);
+    UniquePtr<T> obj = lowlevel_class_loader_.createUniqueInstance<T>(class_type,
+        std::forward<Args>(args)...);
 
     RCUTILS_LOG_DEBUG_NAMED("pluginlib.ClassLoader",
       "std::unique_ptr to object of real type %s created.",
@@ -145,7 +153,9 @@ UniquePtr<T> ClassLoader<T>::createUniqueInstance(const std::string & lookup_nam
 }
 
 template<class T>
-T * ClassLoader<T>::createUnmanagedInstance(const std::string & lookup_name)
+template<typename ... Args,
+  std::enable_if_t<class_loader::is_interface_constructible_v<T, Args...>, bool>>
+T * ClassLoader<T>::createUnmanagedInstance(const std::string & lookup_name, Args &&... args)
 /***************************************************************************/
 {
   RCUTILS_LOG_DEBUG_NAMED("pluginlib.ClassLoader",
@@ -163,7 +173,8 @@ T * ClassLoader<T>::createUnmanagedInstance(const std::string & lookup_name)
     std::string class_type = getClassType(lookup_name);
     RCUTILS_LOG_DEBUG_NAMED("pluginlib.ClassLoader", "%s maps to real class type %s",
       lookup_name.c_str(), class_type.c_str());
-    instance = lowlevel_class_loader_.createUnmanagedInstance<T>(class_type);
+    instance = lowlevel_class_loader_.createUnmanagedInstance<T>(class_type,
+        std::forward<Args>(args)...);
     RCUTILS_LOG_DEBUG_NAMED("pluginlib.ClassLoader",
       "Instance of type %s created.",
       class_type.c_str());

pluginlib/test/include/test_base.hpp

@@ -29,7 +29,10 @@
 #ifndef TEST_BASE_HPP_
 #define TEST_BASE_HPP_
 
+#include <memory>
+
 #include <visibility_control.hpp>
+#include <class_loader/interface_traits.hpp>
 
 namespace test_base
 {
@@ -43,5 +46,33 @@ class TEST_PLUGINLIB_FIXTURE_PUBLIC Fubar
 protected:
   Fubar() {}
 };
+
+class TEST_PLUGINLIB_FIXTURE_PUBLIC FubarWithCtor
+{
+public:
+  virtual double result() = 0;
+  virtual ~FubarWithCtor() = default;
+
+protected:
+  FubarWithCtor() = default;
+};
 }  // namespace test_base
+
+template<>
+struct class_loader::InterfaceTraits<test_base::FubarWithCtor>
+{
+  // Using `std::unique_ptr<double>` to test forwarding of move-only types.
+  using constructor_parameters = class_loader::ConstructorParameters<std::unique_ptr<double>>;
+};
+
+static_assert(
+  class_loader::is_interface_constructible_v<test_base::FubarWithCtor, std::unique_ptr<double>>,
+  "BaseWithInterfaceCtor should be interface constructible with the specifed types."
+);
+
+static_assert(
+  class_loader::is_interface_constructible_v<test_base::FubarWithCtor, std::unique_ptr<double>&&>,
+  "BaseWithInterfaceCtor should be interface constructible with the specifed types."
+);
+
 #endif  // TEST_BASE_HPP_

pluginlib/test/include/test_plugins.hpp

@@ -30,6 +30,7 @@
 #define TEST_PLUGINS_HPP_
 
 #include <cmath>
+#include <memory>
 
 #include <test_base.hpp>
 #include <visibility_control.hpp>
@@ -75,6 +76,21 @@ class TEST_PLUGINLIB_FIXTURE_PUBLIC Foo : public test_base::Fubar
     return foo_ * foo_;
   }
 
+private:
+  double foo_;
+};
+
+class TEST_PLUGINLIB_FIXTURE_PUBLIC FooWithCtor : public test_base::FubarWithCtor
+{
+public:
+  explicit FooWithCtor(std::unique_ptr<double> foo)
+  : foo_(*foo) {}
+
+  double result() override
+  {
+    return foo_ * foo_;
+  }
+
 private:
   double foo_;
 };

pluginlib/test/test_plugins.cpp

@@ -32,3 +32,4 @@
 
 PLUGINLIB_EXPORT_CLASS(test_plugins::Foo, test_base::Fubar)
 PLUGINLIB_EXPORT_CLASS(test_plugins::Bar, test_base::Fubar)
+PLUGINLIB_EXPORT_CLASS(test_plugins::FooWithCtor, test_base::FubarWithCtor)

pluginlib/test/test_plugins.xml

@@ -5,6 +5,9 @@
   <class name="test_pluginlib/bar" type="test_plugins::Bar" base_class_type="test_base::Fubar">
     <description>This is a bar plugin.</description>
   </class>
+  <class name="test_pluginlib/foo_with_ctor" type="test_plugins::FooWithCtor" base_class_type="test_base::FubarWithCtor">
+    <description>This is a foo plugin with constructor.</description>
+  </class>
   <class name="test_pluginlib/none" type="test_plugins::None" base_class_type="test_base::Fubar">
     <description>This is a broken none plugin.</description>
   </class>

pluginlib/test/unique_ptr_test.cpp

@@ -67,6 +67,23 @@ TEST(PluginlibUniquePtrTest, workingPlugin) {
   }
 }
 
+TEST(PluginlibUniquePtrTest, workingPluginCtor) {
+  pluginlib::ClassLoader<test_base::FubarWithCtor> test_loader("test_pluginlib",
+    "test_base::FubarWithCtor");
+
+  try {
+    pluginlib::UniquePtr<test_base::FubarWithCtor> foo =
+      test_loader.createUniqueInstance("test_pluginlib/foo_with_ctor",
+      std::make_unique<double>(10.0));
+    EXPECT_DOUBLE_EQ(100.0, foo->result());
+  } catch (pluginlib::PluginlibException & ex) {
+    FAIL() << "Throwing exception: " << ex.what();
+    return;
+  } catch (...) {
+    FAIL() << "Uncaught exception";
+  }
+}
+
 TEST(PluginlibUniquePtrTest, createUniqueInstanceAndUnloadLibrary) {
   RCUTILS_LOG_INFO("Making the ClassLoader...");
   pluginlib::ClassLoader<test_base::Fubar> pl("test_pluginlib", "test_base::Fubar");

pluginlib/test/utest.cpp

@@ -74,6 +74,23 @@ TEST(PluginlibTest, workingPlugin) {
   }
 }
 
+TEST(PluginlibTest, workingPluginCtor) {
+  pluginlib::ClassLoader<test_base::FubarWithCtor> test_loader("test_pluginlib",
+    "test_base::FubarWithCtor");
+
+  try {
+    std::shared_ptr<test_base::FubarWithCtor> foo =
+      test_loader.createSharedInstance("test_pluginlib/foo_with_ctor",
+      std::make_unique<double>(10.0));
+    EXPECT_DOUBLE_EQ(100.0, foo->result());
+  } catch (pluginlib::PluginlibException & ex) {
+    FAIL() << "Throwing exception: " << ex.what();
+    return;
+  } catch (...) {
+    FAIL() << "Uncaught exception";
+  }
+}
+
 TEST(PluginlibTest, createUnmanagedInstanceAndUnloadLibrary) {
   RCUTILS_LOG_INFO("Making the ClassLoader...");
   pluginlib::ClassLoader<test_base::Fubar> pl("test_pluginlib", "test_base::Fubar");
@@ -99,6 +116,33 @@ TEST(PluginlibTest, createUnmanagedInstanceAndUnloadLibrary) {
   RCUTILS_LOG_INFO("Done.");
 }
 
+TEST(PluginlibTest, createUnmanagedInstanceCtorAndUnloadLibrary) {
+  RCUTILS_LOG_INFO("Making the ClassLoader...");
+  pluginlib::ClassLoader<test_base::FubarWithCtor> pl("test_pluginlib", "test_base::FubarWithCtor");
+
+  RCUTILS_LOG_INFO("Instantiating plugin...");
+  test_base::FubarWithCtor * inst = pl.createUnmanagedInstance("test_pluginlib/foo_with_ctor",
+    std::make_unique<double>(10.0));
+  EXPECT_DOUBLE_EQ(100.0, inst->result());
+
+  RCUTILS_LOG_INFO("Deleting plugin...");
+  delete inst;
+
+  RCUTILS_LOG_INFO("Checking if plugin is loaded with isClassLoaded...");
+  if (pl.isClassLoaded("test_pluginlib/foo_with_ctor")) {
+    RCUTILS_LOG_INFO("Class is loaded");
+  } else {
+    FAIL() << "Library containing class should be loaded but isn't.";
+  }
+  RCUTILS_LOG_INFO("Trying to unload class with unloadLibraryForClass...");
+  try {
+    pl.unloadLibraryForClass("test_pluginlib/foo_with_ctor");
+  } catch (pluginlib::PluginlibException & e) {
+    FAIL() << "Could not unload library when I should be able to. " << e.what();
+  }
+  RCUTILS_LOG_INFO("Done.");
+}
+
 TEST(PluginlibTest, createManagedInstanceAndUnloadLibrary) {
   RCUTILS_LOG_INFO("Making the ClassLoader...");
   pluginlib::ClassLoader<test_base::Fubar> pl("test_pluginlib", "test_base::Fubar");