Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add interface to allow systems to declare parameters #1431

Merged
merged 4 commits into from
Dec 21, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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})

#--------------------------------------
Expand Down
16 changes: 16 additions & 0 deletions include/ignition/gazebo/System.hh
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 {
Expand Down
9 changes: 7 additions & 2 deletions src/SimulationRunner.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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));
Expand Down
5 changes: 5 additions & 0 deletions src/SimulationRunner.hh
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
9 changes: 9 additions & 0 deletions src/SystemInternal.hh
Original file line number Diff line number Diff line change
Expand Up @@ -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>()),
Expand All @@ -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())),
Expand All @@ -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;
Expand Down
33 changes: 28 additions & 5 deletions src/SystemManager.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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);

Expand Down Expand Up @@ -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);
Expand All @@ -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()
{
Expand Down
25 changes: 21 additions & 4 deletions src/SystemManager.hh
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -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
Expand Down
15 changes: 13 additions & 2 deletions src/SystemManager_TEST.cc
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,8 @@ using namespace ignition::gazebo;
/////////////////////////////////////////////////
class SystemWithConfigure:
public System,
public ISystemConfigure
public ISystemConfigure,
public ISystemConfigureParameters
{
// Documentation inherited
public: void Configure(
Expand All @@ -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;
};

/////////////////////////////////////////////////
Expand Down Expand Up @@ -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());
Expand Down Expand Up @@ -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());
Expand All @@ -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());
Expand Down