Add entity and sdf parameters to Server's AddSystem interface

[g-arjones]

🦟 Bug fix

Fixes #2322

Summary

Entity and XML content information are lost when adding systems through the Server::AddSystem(system) API. To reproduce it:

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::systems::DummyPlugin", "<test>foo</test>");
  auto mockSystemPlugin = systemLoader.LoadPlugin(sdfPlugin);
  EXPECT_TRUE(*server.AddSystem(mockSystemPlugin.value()));

Without this patch, the code above will output:

<plugin filename='__default__'/>

After the patch, you will be able to use the extended Server::AddSystem(plugin, entity, sdf) API and get:

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

Checklist

• Signed all commits for DCO
• Added tests
• Updated documentation (as needed)
• Updated migration guide (as needed)
• Consider updating Python bindings (if the library has them)
• codecheck passed (See contributing)
• All tests passed (See test coverage)
• While waiting for a review on your PR, please help review another open pull request to support the maintainers

Note to maintainers: Remember to use Squash-Merge and edit the commit message to match the pull request summary while retaining Signed-off-by messages.

[g-arjones]

Amended to fix linting issues

[azeey]

Looks great! Thanks for the contribution!

I have a few minor comments, but otherwise this is good to go!

One thing I noticed is that adding a plugin through Server is still overly complicated. A follow up PR would be to have a new API:

      public: std::optional<bool> AddSystem(
                  const std::Plugin &_plugin,
                  std::optional<Entity> _entity,
                  const unsigned int _worldIndex = 0);

That would encapsulate loading the plugin and extracting the _sdf element from sdf::Plugin. Thoughts?

[g-arjones]

That would encapsulate loading the plugin and extracting the _sdf element from sdf::Plugin. Thoughts?

Sounds good to me. May I do it in this PR ?

[azeey]

That would encapsulate loading the plugin and extracting the _sdf element from sdf::Plugin. Thoughts?

Sounds good to me. May I do it in this PR ?

Go for it!

[g-arjones]

@azeey Comments have been addressed.

Thank you for the review!

[g-arjones]

@azeey Had to amend again because of another linter issue, sorry about that.

BTW, the contribution guide mentions a codecheck target but the current cmake configuration doesn't seem to be generating that:

$ make codecheck
make: *** No rule to make target 'codecheck'.  Stop.

How should I run cpplint and cppcheck on gz-sim properly?

[azeey]

@azeey Had to amend again because of another linter issue, sorry about that.

BTW, the contribution guide mentions a codecheck target but the current cmake configuration doesn't seem to be generating that:

$ make codecheck
make: *** No rule to make target 'codecheck'.  Stop.

How should I run cpplint and cppcheck on gz-sim properly?

You must install cppcheck first for the codecheck target to be available.

[g-arjones]

You must install cppcheck first for the codecheck target to be available.

Got it. I think we are good to go now.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 65.98%. Comparing base (633ce72) to head (4e739b8).

Additional details and impacted files

@@             Coverage Diff             @@
##           gz-sim8    #2324      +/-   ##
===========================================
+ Coverage    65.97%   65.98%   +0.01%     
===========================================
  Files          327      327              
  Lines        31328    31339      +11     
===========================================
+ Hits         20668    20680      +12     
+ Misses       10660    10659       -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[azeey]

Just one small comment. Thanks!

[g-arjones]

No problem. Amended again.

[iche033]

looks good to me. Thanks for the contribution!

[g-arjones]

@azeey @iche033 Any idea when this might be released?