SystemLoader ignores XML content when loading plugins through LoadPlugin(...)

[g-arjones]

Environment

• OS Version: Ubuntu 22.04
• Source or binary build? Binary (version 8.1.0)

``` ```

Description

• Expected behavior:
• Actual behavior:

Steps to reproduce

1. Create a dummy system plugin:

   class DummyPlugin : public gz::sim::System,
   public gz::sim::ISystemConfigure,
   {
   public:
   void Configure(
   const gz::sim::Entity &_entity,
   const std::shared_ptr<const sdf::Element> &_sdf,
   gz::sim::EntityComponentManager &_ecm,
   gz::sim::EventManager &_eventMgr) override
   {
   std::cout << _sdf->ToString("") << std::endl;
   }
   };

2. Load it with the SystemLoader (extracted from here):

   sim::SystemLoader systemLoader;
   sdf::Plugin sdfPlugin("dummy_plugin", "gz::sim::DummyPlugin", "<test>foo</test>");
   auto mockSystemPlugin = systemLoader.LoadPlugin(sdfPlugin);
   EXPECT_TRUE(*server.AddSystem(mockSystemPlugin.value()));

Output

You should get on your console:

<plugin filename='__default__'/>

When you should actually be getting something like:

<plugin filename='dummy_plugin' name='gz::sim::DummyPlugin'>
  <test>foo</test>
</plugin>

[azeey]

It looks like we need a new API for adding systems programmatically with XML content. The current System::AddSystem API calls SimulationRunner::AddSystem with the default arguments, which has sdf::ElementPtr set to std::nullopt.

https://github.com/gazebosim/gz-sim/blob/8a56f0293d8f3da0ea160a4ecd25ee2e704f59d8/src/SimulationRunner.hh#L117-L120

[g-arjones]

Unfortunately, both SystemManager and SimulationRunner are private classes so I don't see any way around this. Is there a reason why this wasn't implemented? Would you guys accept a patch?

[azeey]

Yes, I think it would be great to have a Server::AddSystem that mirrors SimulationRunner::AddSystem but has the additional _worldIndex parameter. A patch for that would be awesome!

Another thought is to extend ServerConfig to allow adding plugins in addition to the ones loaded by default or listed in the SDFormat file. This would be useful for https://github.com/gazebosim/ros_gz/pull/500

[g-arjones]

Another thought is to extend ServerConfig to allow adding plugins in addition to the ones loaded by default or listed in the SDFormat file. This would be useful for https://github.com/gazebosim/ros_gz/pull/500

Not sure I get this one... Isn't this possible already through these interfaces:

https://github.com/gazebosim/gz-sim/blob/633ce72171e27e83bf2e0292c9998e036d5da3fc/include/gz/sim/ServerConfig.hh#L391-L398

[azeey]

Unfortunately, those APIs don't work if the SDF file doesn't have any plugins and instead wants to just use the default set. When calling ServerConfig::AddPlugin, just the plugins in the ServerConfig get loaded, but the default set of plugins doesn't. I haven't actually what happens if the SDF file actually has plugins listed and we call ServerConfig::AddPlugin.

[g-arjones]

I see. OK, I will give it a try.

[g-arjones]

@azeey I have submitted a patch to fix the original issue. The feature you proposed seemed a bit more involved and is worth a separate PR IMO. I will work on the other one if we can merge #2324 .