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

Clarify errors when plugins fail to load #1727

Merged
merged 5 commits into from Oct 14, 2022

Conversation

mjcarroll
Copy link
Contributor

@mjcarroll mjcarroll commented Sep 21, 2022

🦟 Bug fix

Summary

I find that it's hard to tell one plugin failure mode from another with the current error messages. This expands the messages and also includes relevant information for helping to diagnose system plugin issues.

Now in the case that it fails to find a library, gz-sim will output the list of paths that were searched for the library.

When a bad (or misspelled!) plugin name is provided, it will output a list of the known plugins and aliases in that shared library.

Example Improved output:

[ RUN      ] SystemLoader.BadLibraryPath
[Err] [SystemLoader.cc:102] Failed to load system plugin: (Reason: Could not find shared library)
- Requested plugin name: [gz::sim::systems::Physics]
- Requested library name: [foo.so]
- Library search paths:
  - /home/mjcarroll/workspaces/gz_garden/build/gz-sim7/lib/
  - /home/mjcarroll/.gz/sim/plugins/
  - /home/mjcarroll/.ignition/gazebo/plugins/
  - /home/mjcarroll/workspaces/gz_garden/install/lib/gz-sim-7/plugins/
[       OK ] SystemLoader.BadLibraryPath (79 ms)
[ RUN      ] SystemLoader.BadPluginName
[Err] [SystemLoader.cc:164] Failed to load system plugin: (Reason: library does not contain requested plugin)
- Requested plugin name: [gz::sim::systems::Foo]
- Requested library name: [libgz-sim7-physics-system.so]
- Resolved library path: [/home/mjcarroll/workspaces/gz_garden/build/gz-sim7/lib/libgz-sim7-physics-system.so]
- Detected Plugins:
  - gz::sim::v7::systems::Physics
    aliases:
      gz::sim::systems::Physics
      ignition::gazebo::systems::Physics
[       OK ] SystemLoader.BadPluginName (43 ms)

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.

Signed-off-by: Michael Carroll <michael@openrobotics.org>
@github-actions github-actions bot added the 🌱 garden Ignition Garden label Sep 21, 2022
@osrf-triage osrf-triage added this to Inbox in Core development Sep 21, 2022
Core development automation moved this from Inbox to In review Sep 21, 2022
Copy link
Contributor

@ahcorde ahcorde left a comment

Choose a reason for hiding this comment

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

linters

Signed-off-by: Michael Carroll <michael@openrobotics.org>
@mjcarroll mjcarroll self-assigned this Sep 21, 2022
Copy link
Contributor

@JoanAguilar JoanAguilar left a comment

Choose a reason for hiding this comment

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

I don't think I can't say much of the unit tests, the rest looks good to me (besides a minor comment).

src/SystemLoader.cc Outdated Show resolved Hide resolved
mjcarroll and others added 2 commits September 21, 2022 16:01
Signed-off-by: Michael Carroll <michael@openrobotics.org>

Co-authored-by: Joan Aguilar Mayans <j.aguilarmayans@gmail.com>
Signed-off-by: Michael Carroll <michael@openrobotics.org>
@mjcarroll
Copy link
Contributor Author

I don't think I can't say much of the unit tests, the rest looks good to me (besides a minor comment).

I updated the tests to actually check the output for the general failure modes.

@codecov
Copy link

codecov bot commented Sep 21, 2022

Codecov Report

Merging #1727 (a0d86ea) into gz-sim7 (0df8e9e) will increase coverage by 0.29%.
The diff coverage is 44.44%.

@@             Coverage Diff             @@
##           gz-sim7    #1727      +/-   ##
===========================================
+ Coverage    63.93%   64.22%   +0.29%     
===========================================
  Files          334      336       +2     
  Lines        26333    26600     +267     
===========================================
+ Hits         16836    17084     +248     
- Misses        9497     9516      +19     
Impacted Files Coverage Δ
src/SystemLoader.cc 59.50% <44.44%> (+4.95%) ⬆️
src/systems/wind_effects/WindEffects.cc 77.82% <0.00%> (-1.09%) ⬇️
src/EntityComponentManager.cc 90.04% <0.00%> (-0.88%) ⬇️
src/systems/scene_broadcaster/SceneBroadcaster.cc 92.21% <0.00%> (-0.27%) ⬇️
src/rendering/RenderUtil.cc 38.80% <0.00%> (-0.14%) ⬇️
src/systems/physics/Physics.cc 65.79% <0.00%> (-0.08%) ⬇️
src/SdfEntityCreator.cc 85.55% <0.00%> (ø)
...stems/joint_state_publisher/JointStatePublisher.hh 100.00% <0.00%> (ø)
src/systems/acoustic_comms/AcousticComms.cc 93.22% <0.00%> (ø)
src/systems/hydrodynamics/Hydrodynamics.cc 89.26% <0.00%> (ø)
... and 8 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@azeey azeey requested a review from ahcorde October 3, 2022 18:37
src/SystemLoader_TEST.cc Outdated Show resolved Hide resolved
src/SystemLoader.cc Show resolved Hide resolved
Signed-off-by: Michael Carroll <michael@openrobotics.org>
@mjcarroll mjcarroll merged commit 752b722 into gz-sim7 Oct 14, 2022
@mjcarroll mjcarroll deleted the mjcarroll/clarify_plugin_errors branch October 14, 2022 13:15
Core development automation moved this from In review to Done Oct 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🌱 garden Ignition Garden
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

None yet

3 participants