Add interface to allow systems to declare parameters (#1431)

This adds the ISystemConfigureParameters interface, which
allows systems to declare parameters. This uses a
transport::parameters::ParametersRegistry and creates
services for listing, getting, and setting parameters.

Based on gazebosim/gz-transport#305.

Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>

Author: ivanpauno

CMakeLists.txt

@@ -70,7 +70,7 @@ set(IGN_PLUGIN_VER ${ignition-plugin1_VERSION_MAJOR})
 
 #--------------------------------------
 # Find ignition-transport
-ign_find_package(ignition-transport11 REQUIRED COMPONENTS log)
+ign_find_package(ignition-transport11 REQUIRED COMPONENTS log parameters)
 set(IGN_TRANSPORT_VER ${ignition-transport11_VERSION_MAJOR})
 
 #--------------------------------------

include/ignition/gazebo/System.hh

@@ -25,6 +25,8 @@
 #include <ignition/gazebo/Export.hh>
 #include <ignition/gazebo/Types.hh>
 
+#include <ignition/transport/parameters/Registry.hh>
+
 #include <sdf/Element.hh>
 
 namespace ignition
@@ -100,6 +102,20 @@ namespace ignition
                   EventManager &_eventMgr) = 0;
     };
 
+    /// \class ISystemConfigureParameters ISystem.hh ignition/gazebo/System.hh
+    /// \brief Interface for a system that declares parameters.
+    ///
+    /// ISystemConfigureParameters::ConfigureParameters is called after
+    /// ISystemConfigure::Configure.
+    class ISystemConfigureParameters {
+      /// \brief Configure the parameters of the system.
+      /// \param[in] _registry The parameter registry.
+      public: virtual void ConfigureParameters(
+                  ignition::transport::parameters::ParametersRegistry &
+                    _registry,
+                  EntityComponentManager &_ecm) = 0;
+    };
+
     /// \class ISystemPreUpdate ISystem.hh ignition/gazebo/System.hh
     /// \brief Interface for a system that uses the PreUpdate phase
     class ISystemPreUpdate {

src/SimulationRunner.cc

@@ -61,6 +61,10 @@ SimulationRunner::SimulationRunner(const sdf::World *_world,
   // Keep world name
   this->worldName = _world->Name();
 
+  this->parametersRegistry = std::make_unique<
+    ignition::transport::parameters::ParametersRegistry>(
+      std::string{"world/"} + this->worldName);
+
   // Get the physics profile
   // TODO(luca): remove duplicated logic in SdfEntityCreator and LevelManager
   auto physics = _world->PhysicsByIndex(0);
@@ -132,8 +136,9 @@ SimulationRunner::SimulationRunner(const sdf::World *_world,
   this->node = std::make_unique<transport::Node>(opts);
 
   // Create the system manager
-  this->systemMgr = std::make_unique<SystemManager>(_systemLoader,
-      &this->entityCompMgr, &this->eventMgr, validNs);
+  this->systemMgr = std::make_unique<SystemManager>(
+      _systemLoader, &this->entityCompMgr, &this->eventMgr, validNs,
+      this->parametersRegistry.get());
 
   this->pauseConn = this->eventMgr.Connect<events::Pause>(
       std::bind(&SimulationRunner::SetPaused, this, std::placeholders::_1));

src/SimulationRunner.hh

@@ -396,6 +396,11 @@ namespace ignition
       /// Note: must be before EntityComponentManager
       private: EventManager eventMgr;
 
+      /// \brief Manager all parameters
+      private: std::unique_ptr<
+        ignition::transport::parameters::ParametersRegistry
+      > parametersRegistry;
+
       /// \brief Manager of all components.
       private: EntityComponentManager entityCompMgr;
 

src/SystemInternal.hh

@@ -45,6 +45,8 @@ namespace ignition
               : systemPlugin(std::move(_systemPlugin)),
                 system(systemPlugin->QueryInterface<System>()),
                 configure(systemPlugin->QueryInterface<ISystemConfigure>()),
+                configureParameters(
+                  systemPlugin->QueryInterface<ISystemConfigureParameters>()),
                 preupdate(systemPlugin->QueryInterface<ISystemPreUpdate>()),
                 update(systemPlugin->QueryInterface<ISystemUpdate>()),
                 postupdate(systemPlugin->QueryInterface<ISystemPostUpdate>()),
@@ -60,6 +62,8 @@ namespace ignition
               : systemShared(_system),
                 system(_system.get()),
                 configure(dynamic_cast<ISystemConfigure *>(_system.get())),
+                configureParameters(
+                  dynamic_cast<ISystemConfigureParameters *>(_system.get())),
                 preupdate(dynamic_cast<ISystemPreUpdate *>(_system.get())),
                 update(dynamic_cast<ISystemUpdate *>(_system.get())),
                 postupdate(dynamic_cast<ISystemPostUpdate *>(_system.get())),
@@ -83,6 +87,11 @@ namespace ignition
       /// Will be nullptr if the System doesn't implement this interface.
       public: ISystemConfigure *configure = nullptr;
 
+      /// \brief Access this system via the ISystemConfigureParameters
+      ///   interface.
+      /// Will be nullptr if the System doesn't implement this interface.
+      public: ISystemConfigureParameters *configureParameters = nullptr;
+
       /// \brief Access this system via the ISystemPreUpdate interface
       /// Will be nullptr if the System doesn't implement this interface.
       public: ISystemPreUpdate *preupdate = nullptr;

src/SystemManager.cc

@@ -28,13 +28,16 @@ using namespace ignition;
 using namespace gazebo;
 
 //////////////////////////////////////////////////
-SystemManager::SystemManager(const SystemLoaderPtr &_systemLoader,
-                             EntityComponentManager *_entityCompMgr,
-                             EventManager *_eventMgr,
-                             const std::string &_namespace)
+SystemManager::SystemManager(
+  const SystemLoaderPtr &_systemLoader,
+  EntityComponentManager *_entityCompMgr,
+  EventManager *_eventMgr,
+  const std::string &_namespace,
+  ignition::transport::parameters::ParametersRegistry *_parametersRegistry)
   : systemLoader(_systemLoader),
     entityCompMgr(_entityCompMgr),
-    eventMgr(_eventMgr)
+    eventMgr(_eventMgr),
+    parametersRegistry(_parametersRegistry)
 {
   transport::NodeOptions opts;
   opts.SetNameSpace(_namespace);
@@ -102,6 +105,9 @@ size_t SystemManager::ActivatePendingSystems()
     if (system.configure)
       this->systemsConfigure.push_back(system.configure);
 
+    if (system.configureParameters)
+      this->systemsConfigureParameters.push_back(system.configureParameters);
+
     if (system.preupdate)
       this->systemsPreupdate.push_back(system.preupdate);
 
@@ -169,6 +175,16 @@ void SystemManager::AddSystemImpl(
                                  *this->eventMgr);
   }
 
+  // Configure the system parameters, if necessary
+  if (
+    _system.configureParameters && this->entityCompMgr &&
+    this->parametersRegistry)
+  {
+    _system.configureParameters->ConfigureParameters(
+      *this->parametersRegistry,
+      *this->entityCompMgr);
+  }
+
   // Update callbacks will be handled later, add to queue
   std::lock_guard<std::mutex> lock(this->pendingSystemsMutex);
   this->pendingSystems.push_back(_system);
@@ -180,6 +196,13 @@ const std::vector<ISystemConfigure *>& SystemManager::SystemsConfigure()
   return this->systemsConfigure;
 }
 
+//////////////////////////////////////////////////
+const std::vector<ISystemConfigureParameters *>&
+SystemManager::SystemsConfigureParameters()
+{
+  return this->systemsConfigureParameters;
+}
+
 //////////////////////////////////////////////////
 const std::vector<ISystemPreUpdate *>& SystemManager::SystemsPreUpdate()
 {

src/SystemManager.hh

@@ -52,10 +52,13 @@ namespace ignition
       /// \param[in] _eventMgr Pointer to the event manager to be used when
       ///  configuring new systems
       /// \param[in] _namespace Namespace to use for the transport node
-      public: explicit SystemManager(const SystemLoaderPtr &_systemLoader,
-                            EntityComponentManager *_entityCompMgr = nullptr,
-                            EventManager *_eventMgr = nullptr,
-                            const std::string &_namespace = std::string());
+      public: explicit SystemManager(
+        const SystemLoaderPtr &_systemLoader,
+        EntityComponentManager *_entityCompMgr = nullptr,
+        EventManager *_eventMgr = nullptr,
+        const std::string &_namespace = std::string(),
+        ignition::transport::parameters::ParametersRegistry *
+          _parametersRegistry = nullptr);
 
       /// \brief Load system plugin for a given entity.
       /// \param[in] _entity Entity
@@ -99,6 +102,12 @@ namespace ignition
       /// \return Vector of systems's configure interfaces.
       public: const std::vector<ISystemConfigure *>& SystemsConfigure();
 
+      /// \brief Get an vector of all active systems implementing
+      ///   "ConfigureParameters"
+      /// \return Vector of systems's configure interfaces.
+      public: const std::vector<ISystemConfigureParameters *>&
+      SystemsConfigureParameters();
+
       /// \brief Get an vector of all active systems implementing "PreUpdate"
       /// \return Vector of systems's pre-update interfaces.
       public: const std::vector<ISystemPreUpdate *>& SystemsPreUpdate();
@@ -162,6 +171,10 @@ namespace ignition
       /// \brief Systems implementing Configure
       private: std::vector<ISystemConfigure *> systemsConfigure;
 
+      /// \brief Systems implementing ConfigureParameters
+      private: std::vector<ISystemConfigureParameters *>
+        systemsConfigureParameters;
+
       /// \brief Systems implementing PreUpdate
       private: std::vector<ISystemPreUpdate *> systemsPreupdate;
 
@@ -191,6 +204,10 @@ namespace ignition
 
       /// \brief Node for communication.
       private: std::unique_ptr<transport::Node> node{nullptr};
+
+      /// \brief Pointer to associated parameters registry
+      private: ignition::transport::parameters::ParametersRegistry *
+        parametersRegistry;
     };
     }
   }  // namespace gazebo

src/SystemManager_TEST.cc

@@ -31,7 +31,8 @@ using namespace ignition::gazebo;
 /////////////////////////////////////////////////
 class SystemWithConfigure:
   public System,
-  public ISystemConfigure
+  public ISystemConfigure,
+  public ISystemConfigureParameters
 {
   // Documentation inherited
   public: void Configure(
@@ -40,7 +41,12 @@ class SystemWithConfigure:
                 EntityComponentManager &,
                 EventManager &) override { configured++; };
 
+  public: void ConfigureParameters(
+                ignition::transport::parameters::ParametersRegistry &,
+                EntityComponentManager &) override { configuredParameters++; }
+
   public: int configured = 0;
+  public: int configuredParameters = 0;
 };
 
 /////////////////////////////////////////////////
@@ -99,6 +105,7 @@ TEST(SystemManager, AddSystemNoEcm)
 
   // SystemManager without an ECM/EventmManager will mean no config occurs
   EXPECT_EQ(0, configSystem->configured);
+  EXPECT_EQ(0, configSystem->configuredParameters);
 
   EXPECT_EQ(0u, systemMgr.ActiveCount());
   EXPECT_EQ(1u, systemMgr.PendingCount());
@@ -150,7 +157,10 @@ TEST(SystemManager, AddSystemEcm)
   auto ecm = EntityComponentManager();
   auto eventManager = EventManager();
 
-  SystemManager systemMgr(loader, &ecm, &eventManager);
+  auto paramRegistry = std::make_unique<
+    ignition::transport::parameters::ParametersRegistry>("SystemManager_TEST");
+  SystemManager systemMgr(
+    loader, &ecm, &eventManager, std::string(), paramRegistry.get());
 
   EXPECT_EQ(0u, systemMgr.ActiveCount());
   EXPECT_EQ(0u, systemMgr.PendingCount());
@@ -165,6 +175,7 @@ TEST(SystemManager, AddSystemEcm)
 
   // Configure called during AddSystem
   EXPECT_EQ(1, configSystem->configured);
+  EXPECT_EQ(1, configSystem->configuredParameters);
 
   EXPECT_EQ(0u, systemMgr.ActiveCount());
   EXPECT_EQ(1u, systemMgr.PendingCount());