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

Implement system Reset interface for Sensors and SceneBroadcaster #1355

Merged
merged 55 commits into from
Jun 30, 2022
Merged
Show file tree
Hide file tree
Changes from 51 commits
Commits
Show all changes
55 commits
Select commit Hold shift + click to select a range
11e029a
SystemInternal and WorldControl into own headers
mjcarroll Feb 15, 2022
56f28bd
Move System interaction to SystemManager
mjcarroll Feb 15, 2022
c35e3ce
Adjustments for Fortress
mjcarroll Feb 16, 2022
8266e73
Address reviewer feedback
mjcarroll Feb 17, 2022
ca91023
Introduce System::Reset API
mjcarroll Feb 8, 2022
85cb36e
SimulationRunner Reset implementation
mjcarroll Feb 8, 2022
144556d
Simulation Reset implementation in Physics system
mjcarroll Feb 8, 2022
4b023fa
Simulation reset example plugin
mjcarroll Feb 8, 2022
75e57fb
Add integration test for resetting physics
mjcarroll Feb 8, 2022
e3368e5
Fix diff test and add documentation
mjcarroll Feb 15, 2022
63ec622
Fix build and test errors after merge, some documentation and cleanup.
azeey Feb 24, 2022
222695d
Merge branch 'reset' into reset_physics
azeey Feb 24, 2022
e5f01bc
Merge branch 'main' into reset_physics
mjcarroll Mar 23, 2022
859df32
Bump year
mjcarroll Mar 29, 2022
8c669b0
Merge branch 'main' into reset_physics
mjcarroll Mar 29, 2022
749daca
Fix merge from main
mjcarroll Mar 29, 2022
371d8ce
Fix CI platform
mjcarroll Mar 29, 2022
d1fabbe
Update test/integration/reset.cc
mjcarroll Mar 30, 2022
8ddb739
Merge branch 'main' into reset_physics
mjcarroll Apr 4, 2022
32d21f5
Merge branch 'main' into reset_physics
chapulina Apr 8, 2022
a766933
Merge branch 'main' into reset_physics
mjcarroll Apr 18, 2022
c8ab4d5
Address reviewer feedback
mjcarroll Apr 18, 2022
0c52ab3
Reset sensors and scenebroadcaster implementation
mjcarroll Feb 22, 2022
825c280
Remove debug outputs
mjcarroll Mar 31, 2022
68f6f85
Address reviewer feedback
mjcarroll Mar 31, 2022
0bd1d6b
Merge branch 'main' into reset_physics
mjcarroll May 2, 2022
3799ff3
Adjust randomized range to account for limits
mjcarroll May 3, 2022
fbf6d0e
Fix header for new location
mjcarroll May 3, 2022
383006f
Merge branch 'reset_physics' into reset_sensors
mjcarroll May 3, 2022
dc0cdbb
Merge branch 'main' into reset_sensors
mjcarroll May 17, 2022
e1acbb0
Address reviewer feedback
mjcarroll May 17, 2022
60ea603
Merge branch 'main' into reset_sensors
mjcarroll May 24, 2022
093ca37
Merge branch 'main' into reset_sensors
mjcarroll May 31, 2022
4770dc7
Updates for main and add topic test
mjcarroll May 31, 2022
2286ec5
Fix interaction with log playback static variable
mjcarroll May 31, 2022
7e9245a
Also disable on Mac where OGRE2 doesn't load
mjcarroll Jun 1, 2022
0c4a4c5
Address reviewer feedback.
mjcarroll Jun 7, 2022
f717c96
Merge branch 'main' into reset_sensors
mjcarroll Jun 7, 2022
e096731
Merge branch 'main' into reset_sensors
mjcarroll Jun 13, 2022
d22f655
Merge branch 'main' into reset_sensors
chapulina Jun 14, 2022
ae3d076
Apply suggestions from code review
mjcarroll Jun 14, 2022
df6cc36
Better logging for reset in-memory systems
mjcarroll Jun 14, 2022
768aee2
Impove sensor reset logging and drop cast
mjcarroll Jun 14, 2022
26d7396
More informative warning on systems not implementing reset
mjcarroll Jun 14, 2022
b211196
Prune headers in reset_sensors test
mjcarroll Jun 14, 2022
101654a
Update LogPlayback reset functionality
mjcarroll Jun 14, 2022
c1b3e4e
Merge branch 'main' into reset_sensors
mjcarroll Jun 27, 2022
ec890ef
Fixed SystemPlugin loader
ahcorde Jun 28, 2022
e6c08e4
Improvements to sensor reset test
mjcarroll Jun 29, 2022
e5922cb
Lint
mjcarroll Jun 29, 2022
c7f5964
Merge branch 'main' into reset_sensors
mjcarroll Jun 29, 2022
6c8853c
Drop detail from header
mjcarroll Jun 29, 2022
61b57c9
Reviewer feedback
mjcarroll Jun 29, 2022
c961edf
Update scene_broadcaster
mjcarroll Jun 29, 2022
86c2f1d
Strip trailing whitespace
mjcarroll Jun 29, 2022
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
3 changes: 1 addition & 2 deletions src/SimulationRunner.cc
Original file line number Diff line number Diff line change
Expand Up @@ -538,8 +538,7 @@ void SimulationRunner::UpdateSystems()
if (this->resetInitiated)
{
GZ_PROFILE("Reset");
for (auto &system : this->systemMgr->SystemsReset())
system->Reset(this->currentInfo, this->entityCompMgr);
this->systemMgr->Reset(this->currentInfo, this->entityCompMgr);
return;
}

Expand Down
10 changes: 9 additions & 1 deletion src/SystemInternal.hh
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@

#include <chrono>
#include <memory>
#include <string>
#include <utility>
#include <vector>

Expand Down Expand Up @@ -105,6 +106,14 @@ namespace gz
/// system during the `Configure` call.
public: Entity parentEntity = {kNullEntity};

/// \brief Cached filename of the plugin used when system was loaded.
/// Used for reloading a system at runtime.
public: std::string fname = "";

/// \brief Cached plugin name of the plugin used when system was loaded.
/// Used for reloading a system at runtime.
public: std::string name = "";

/// \brief Cached sdf that was used to call `Configure` on the system
/// Useful for if a system needs to be reconfigured at runtime
public: std::shared_ptr<const sdf::Element> configureSdf = nullptr;
Expand All @@ -116,4 +125,3 @@ namespace gz
} // namespace sim
} // namespace gz
#endif // GZ_SIM_SYSTEMINTERNAL_HH_

2 changes: 0 additions & 2 deletions src/SystemLoader.cc
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,6 @@ class gz::sim::SystemLoaderPrivate
return false;
}

this->systemPluginsAdded.insert(_plugin);
return true;
}

Expand All @@ -151,7 +150,6 @@ class gz::sim::SystemLoaderPrivate
public: std::unordered_set<std::string> systemPluginPaths;

/// \brief System plugins that have instances loaded via the manager.
public: std::unordered_set<SystemPluginPtr> systemPluginsAdded;
};

//////////////////////////////////////////////////
Expand Down
82 changes: 81 additions & 1 deletion src/SystemManager.cc
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,11 @@ void SystemManager::LoadPlugin(const Entity _entity,
// System correctly loaded from library
if (system)
{
this->AddSystem(system.value(), _entity, _sdf);
SystemInternal ss(system.value(), _entity);
ss.fname = _fname;
ss.name = _name;
ss.configureSdf = _sdf;
this->AddSystemImpl(ss, ss.configureSdf);
gzdbg << "Loaded system [" << _name
<< "] for entity [" << _entity << "]" << std::endl;
}
Expand Down Expand Up @@ -103,6 +107,82 @@ size_t SystemManager::ActivatePendingSystems()
return count;
}

//////////////////////////////////////////////////
/// \brief Structure to temporarily store plugin information for reset
struct PluginInfo {
/// \brief Entity plugin is attached to
Entity entity;
/// \brief Filename of the plugin library
std::string fname;
/// \brief Name of the plugin
std::string name;
/// \brief SDF element (content of the plugin tag)
sdf::ElementPtr sdf;
};

//////////////////////////////////////////////////
void SystemManager::Reset(const UpdateInfo &_info, EntityComponentManager &_ecm)
{
{
std::lock_guard<std::mutex> lock(this->pendingSystemsMutex);
this->pendingSystems.clear();
}

// Clear all iterable collections of systems
this->systemsConfigure.clear();
this->systemsReset.clear();
this->systemsPreupdate.clear();
this->systemsUpdate.clear();
this->systemsPostupdate.clear();

std::vector<PluginInfo> pluginsToBeLoaded;

for (auto &system : this->systems)
{
if (nullptr != system.reset)
{
// If implemented, call reset and add to pending systems.
system.reset->Reset(_info, _ecm);

{
std::lock_guard<std::mutex> lock(this->pendingSystemsMutex);
this->pendingSystems.push_back(system);
}
}
else
{
// Cannot reset systems that were created in memory rather than
// from a plugin, because there isn't access to the constructor.
if (nullptr != system.systemShared)
{
ignwarn << "In-memory without ISystemReset detected: ["
<< system.name << "]\n"
<< "Systems created without plugins that do not implement Reset"
<< " will not be reloaded. Reset may not work correctly\n";
continue;
}
PluginInfo info = {
system.parentEntity, system.fname, system.name,
system.configureSdf->Clone()
};

pluginsToBeLoaded.push_back(info);
}
}

this->systems.clear();

// Load plugins which do not implement reset after clearing this->systems
// to ensure the previous instance is destroyed before the new one is created
// and configured.
for (const auto &pluginInfo : pluginsToBeLoaded)
{
this->LoadPlugin(pluginInfo.entity, pluginInfo.fname, pluginInfo.name,
pluginInfo.sdf);
}
this->ActivatePendingSystems();
}

//////////////////////////////////////////////////
void SystemManager::AddSystem(const SystemPluginPtr &_system,
Entity _entity,
Expand Down
20 changes: 17 additions & 3 deletions src/SystemManager.hh
Original file line number Diff line number Diff line change
Expand Up @@ -92,12 +92,26 @@ namespace gz
/// \return The number of newly-active systems
public: size_t ActivatePendingSystems();

/// \brief Get an vector of all active systems implementing "Configure"
/// \return Vector of systems's configure interfaces.
/// \brief Perform a reset on all systems
///
/// If a system implements the ISystemReset interface, it will be called.
//
/// Otherwise, if a system does not have the ISystemReset interface
/// implemented, and was created via loading a plugin,
/// that plugin will be reloaded.
///
/// Otherwise, if a system is created from in-memory rather than a plugin,
/// that system will remain unaffected.
/// \param[in] _info Update info corresponding to the update time
/// \param[in] _ecm Version of the ECM reset to an initial state
public: void Reset(const UpdateInfo &_info, EntityComponentManager &_ecm);

/// \brief Get a vector of all systems implementing "Configure"
/// \return Vector of systems' configure interfaces.
public: const std::vector<ISystemConfigure *>& SystemsConfigure();

/// \brief Get an vector of all active systems implementing "Reset"
/// \return Vector of systems's reset interfaces.
/// \return Vector of systems' reset interfaces.
public: const std::vector<ISystemReset *>& SystemsReset();

/// \brief Get an vector of all active systems implementing "PreUpdate"
Expand Down
1 change: 0 additions & 1 deletion src/rendering/RenderUtil.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1661,7 +1661,6 @@ void RenderUtilPrivate::CreateEntitiesFirstUpdate(
return true;
});


_ecm.Each<components::Model, components::Name, components::Pose,
components::ParentEntity>(
[&](const Entity &_entity,
Expand Down
11 changes: 11 additions & 0 deletions src/systems/log/LogPlayback.cc
Original file line number Diff line number Diff line change
Expand Up @@ -453,6 +453,16 @@ bool LogPlaybackPrivate::ExtractStateAndResources()
}
}

//////////////////////////////////////////////////
void LogPlayback::Reset(const UpdateInfo &, EntityComponentManager &)
{
// In this case, Reset is a noop
// LogPlayback already has handling for jumps in time as part of the
// Update method.
// Leaving this function implemented but empty prevents the SystemManager
// from trying to destroy and recreate the plugin.
}

//////////////////////////////////////////////////
void LogPlayback::Update(const UpdateInfo &_info, EntityComponentManager &_ecm)
{
Expand Down Expand Up @@ -621,6 +631,7 @@ void LogPlayback::Update(const UpdateInfo &_info, EntityComponentManager &_ecm)
GZ_ADD_PLUGIN(gz::sim::systems::LogPlayback,
gz::sim::System,
LogPlayback::ISystemConfigure,
LogPlayback::ISystemReset,
LogPlayback::ISystemUpdate)

GZ_ADD_PLUGIN_ALIAS(LogPlayback,
Expand Down
5 changes: 5 additions & 0 deletions src/systems/log/LogPlayback.hh
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ namespace systems
class LogPlayback:
public System,
public ISystemConfigure,
public ISystemReset,
public ISystemUpdate
{
/// \brief Constructor
Expand All @@ -53,6 +54,10 @@ namespace systems
EntityComponentManager &_ecm,
EventManager &_eventMgr) final;

/// Documentation inherited
public: void Reset(const UpdateInfo &_info,
EntityComponentManager &_ecm) final;

/// Documentation inherited
public: void Update(const UpdateInfo &_info,
EntityComponentManager &_ecm) final;
Expand Down
25 changes: 24 additions & 1 deletion src/systems/scene_broadcaster/SceneBroadcaster.cc
Original file line number Diff line number Diff line change
Expand Up @@ -399,6 +399,28 @@ void SceneBroadcaster::PostUpdate(const UpdateInfo &_info,
}
}

//////////////////////////////////////////////////
void SceneBroadcaster::Reset(const UpdateInfo &_info,
EntityComponentManager &_manager)
{
// Update scene graph with added entities before populating pose message
this->dataPtr->SceneGraphAddEntities(_manager);

this->dataPtr->PoseUpdate(_info, _manager);

// call SceneGraphRemoveEntities at the end of this update cycle so that
// removed entities are removed from the scene graph for the next update cycle
this->dataPtr->SceneGraphRemoveEntities(_manager);

std::unique_lock<std::mutex> lock(this->dataPtr->stateMutex);
this->dataPtr->stepMsg.Clear();

set(this->dataPtr->stepMsg.mutable_stats(), _info);
_manager.State(*this->dataPtr->stepMsg.mutable_state(), {}, {}, true);
this->dataPtr->statePub.Publish(this->dataPtr->stepMsg);
this->dataPtr->lastStatePubTime = std::chrono::system_clock::now();
}

//////////////////////////////////////////////////
void SceneBroadcasterPrivate::PoseUpdate(const UpdateInfo &_info,
const EntityComponentManager &_manager)
Expand Down Expand Up @@ -1201,7 +1223,8 @@ void SceneBroadcasterPrivate::RemoveFromGraph(const Entity _entity,
GZ_ADD_PLUGIN(SceneBroadcaster,
gz::sim::System,
SceneBroadcaster::ISystemConfigure,
SceneBroadcaster::ISystemPostUpdate)
SceneBroadcaster::ISystemPostUpdate,
SceneBroadcaster::ISystemReset)

// Add plugin alias so that we can refer to the plugin without the version
// namespace
Expand Down
7 changes: 6 additions & 1 deletion src/systems/scene_broadcaster/SceneBroadcaster.hh
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,8 @@ namespace systems
class SceneBroadcaster final:
public System,
public ISystemConfigure,
public ISystemPostUpdate
public ISystemPostUpdate,
public ISystemReset
{
/// \brief Constructor
public: SceneBroadcaster();
Expand All @@ -58,6 +59,10 @@ namespace systems
public: void PostUpdate(const UpdateInfo &_info,
const EntityComponentManager &_ecm) final;

// Documentation inherited
public: void Reset(const UpdateInfo &_info,
EntityComponentManager &_ecm) final;

/// \brief Private data pointer
private: std::unique_ptr<SceneBroadcasterPrivate> dataPtr;
};
Expand Down
Loading