-
Notifications
You must be signed in to change notification settings - Fork 276
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
Support adding systems that don't come from a plugin #936
Conversation
Signed-off-by: Louise Poubel <[email protected]>
Signed-off-by: Louise Poubel <[email protected]>
Codecov Report
@@ Coverage Diff @@
## ign-gazebo3 #936 +/- ##
===============================================
+ Coverage 77.78% 77.83% +0.05%
===============================================
Files 219 219
Lines 12566 12593 +27
===============================================
+ Hits 9774 9802 +28
+ Misses 2792 2791 -1
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the addition. This will be really useful. I have a couple of comments about the API.
Signed-off-by: Louise Poubel <[email protected]>
Signed-off-by: Louise Poubel <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great addition! Do you think we should add some documentation/tutorial for developers so that they are aware that this option exists? (Could be done in a follow-up, not going to block this)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see UNIT_ModelCommandAPI_TEST
failed on macOS. Is that a new flake?
It's a new test I believe from the |
I could write up a simple tutorial, but I'm not sure in which context we want to encourage users to use this API directly. The |
Signed-off-by: Louise Poubel <[email protected]>
Signed-off-by: Louise Poubel <[email protected]>
🎉 New feature
Summary
This is broken off #926 for easier review.
When using the
Server
API directly, it's often useful to instantiate systems locally without actually loading a plugin's shared library. This PR adds a newServer::AddSystem
that takes a raw pointer and behaves just like the existingAddSystem
function that takes a plugin.I changed the test
Relay
class to use this approach, which should be a bit faster than loading the shared library.Also added the convenient
gazebo::worldEntity
function that returns the first world found.Test it
Take a look at the unit tests added, and see on #926 how the test fixture uses this.
Checklist
codecheck
passed (See contributing)Note to maintainers: Remember to use Squash-Merge
🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸