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

Static plugins #97

Merged
merged 15 commits into from
Jul 29, 2022
Merged

Conversation

shameekganguly
Copy link
Contributor

🎉 New feature

Summary

This change adds support to register plugins statically and load them from a library that is directly linked into the main program. Two new macros are added for static plugin registration: GZ_ADD_STATIC_PLUGIN and GZ_ADD_STATIC_PLUGIN_ALIAS which are functionally identical to their runtime-loading equivalents GZ_ADD_PLUGIN and GZ_ADD_PLUGIN_ALIAS respectively. Users can use this feature by invoking the static registration macros to register their plugin, and force-linking the plugin library into the program where they wish to load the plugin. The existing Loader class can be used to load plugins that have been linked in statically (see test/integration/static_plugins.cc for example).

Note that this change refactors the implementation of the following:

  • register/include/gz/plugin/detail/Register.hh to share code with register/include/gz/plugin/detail/RegisterStatic.hh, and
  • loader/src/Loader.cc to share code between querying and instantiating plugins from libraries which are loaded from files and linked-in libraries. A Registry class is introduced which acts as a store for a list of plugin Infos. A singleton StaticRegistry class is derived from Registry to maintain the store of statically registered plugin Infos.

Test it

Checklist

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.

@github-actions github-actions bot added the 🌱 garden Ignition Garden label Jul 21, 2022
@chapulina chapulina requested a review from mjcarroll July 21, 2022 21:25
@chapulina chapulina changed the title Shameek/static plugins Static plugins Jul 23, 2022
@chapulina chapulina added the enhancement New feature or request label Jul 23, 2022
@codecov
Copy link

codecov bot commented Jul 25, 2022

Codecov Report

Merging #97 (87012c8) into main (9ddd731) will decrease coverage by 1.09%.
The diff coverage is 96.73%.

@@            Coverage Diff             @@
##             main      #97      +/-   ##
==========================================
- Coverage   99.35%   98.25%   -1.10%     
==========================================
  Files          17       23       +6     
  Lines         617      745     +128     
==========================================
+ Hits          613      732     +119     
- Misses          4       13       +9     
Impacted Files Coverage Δ
core/include/gz/plugin/PluginPtr.hh 100.00% <ø> (ø)
loader/include/gz/plugin/detail/StaticRegistry.hh 33.33% <33.33%> (ø)
core/src/Plugin.cc 97.70% <71.42%> (-2.30%) ⬇️
loader/src/detail/StaticRegistry.cc 90.62% <90.62%> (ø)
loader/include/gz/plugin/detail/Loader.hh 93.75% <90.90%> (-6.25%) ⬇️
loader/src/detail/Registry.cc 99.09% <99.09%> (ø)
core/include/gz/plugin/detail/PluginPtr.hh 100.00% <100.00%> (ø)
loader/include/gz/plugin/detail/Registry.hh 100.00% <100.00%> (ø)
loader/src/Loader.cc 100.00% <100.00%> (ø)
register/include/gz/plugin/detail/Common.hh 100.00% <100.00%> (ø)
... and 2 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9ddd731...87012c8. Read the comment docs.

@mjcarroll
Copy link
Contributor

I am continuing to investigate the windows failures here in a local VM.

@shameekganguly
Copy link
Contributor Author

Thanks for the clean up and Windows fixes Michael!

Signed-off-by: Michael Carroll <[email protected]>
Copy link
Contributor

@mjcarroll mjcarroll left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm happy with this. Thanks for following style as much as possible.

I think this will be a very helpful improvement, interested to try it specifically with testing in gz-sim testing plugins.

@mjcarroll
Copy link
Contributor

I made a sketch of how this can be used in gz-sim, to be fleshed out after feature freeze (for first Garden minor release): gazebosim/gz-sim#1623

@mjcarroll mjcarroll merged commit a49b9f4 into gazebosim:main Jul 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request 🌱 garden Ignition Garden
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants