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

Resource env var, with transport interface #172

Merged
merged 25 commits into from
Aug 3, 2020

Conversation

chapulina
Copy link
Contributor

@chapulina chapulina commented Jun 3, 2020


This allows models and meshes to be found according to the the IGN_GAZEBO_RESOURCE_PATH environment variable.

  • It's the Ignition Gazebo equivalent of both GAZEBO_MODEL_PATH and GAZEBO_RESOURCE_PATH.
  • That variable is already being used to find worlds, now it's extended to <include>s and mesh <uri>s.
  • The name "resources" is better fitting than "model", since it can load different types of resources.
  • We've already been supporting the SDF_PATH and IGN_FILE_PATH variables with the same behaviour, so this implementation is backwards compatible and keeps those variables in sync with IGN_GAZEBO_RESOURCE_PATH.
  • I added a transport interface so these paths can be queried and updated.

Try it

Get paths:

ign service -s /gazebo/get_resource_paths --reqtype ignition.msgs.Empty --reptype ignition.msgs.StringMsg_V --timeout 5000 --req "unused: false"

Add path:

ign service -s /gazebo/add_resource_paths --reqtype ignition.msgs.StringMsg_V --reptype ignition.msgs.Empty --timeout 5000 --req "data: '/tmp/lala'"


TODOs

I'll leave this in draft state until:

@chapulina chapulina requested a review from nkoenig June 3, 2020 02:53
@chapulina chapulina self-assigned this Jun 3, 2020
@github-actions github-actions bot added the 📜 blueprint Ignition Blueprint label Jun 3, 2020
@chapulina chapulina marked this pull request as ready for review June 4, 2020 17:06
@chapulina chapulina force-pushed the chapulina/resource_path branch from d9bc0c7 to 840b173 Compare June 4, 2020 22:03
@chapulina chapulina changed the base branch from ign-gazebo2 to chapulina/relay_tests June 4, 2020 22:04
Base automatically changed from chapulina/relay_tests to ign-gazebo2 June 8, 2020 16:52
Copy link
Contributor

@maryaB-osr maryaB-osr left a comment

Choose a reason for hiding this comment

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

lgtm!

Signed-off-by: Louise Poubel <[email protected]>
@codecov
Copy link

codecov bot commented Jun 12, 2020

Codecov Report

Merging #172 into ign-gazebo2 will increase coverage by 8.92%.
The diff coverage is 72.72%.

Impacted file tree graph

@@               Coverage Diff               @@
##           ign-gazebo2     #172      +/-   ##
===============================================
+ Coverage        53.78%   62.70%   +8.92%     
===============================================
  Files              121      124       +3     
  Lines             5838     6200     +362     
===============================================
+ Hits              3140     3888     +748     
+ Misses            2698     2312     -386     
Impacted Files Coverage Δ
include/ignition/gazebo/Server.hh 100.00% <ø> (ø)
src/gui/Gui.cc 5.93% <0.00%> (+5.93%) ⬆️
src/gui/PathManager.cc 0.00% <0.00%> (ø)
src/systems/physics/Physics.cc 15.98% <ø> (ø)
src/systems/scene_broadcaster/SceneBroadcaster.cc 36.24% <0.00%> (-0.12%) ⬇️
src/systems/user_commands/UserCommands.hh 100.00% <ø> (ø)
src/systems/user_commands/UserCommands.cc 12.29% <16.66%> (ø)
src/Util.cc 84.74% <97.82%> (+8.35%) ⬆️
src/LevelManager.cc 90.71% <100.00%> (+49.10%) ⬆️
src/SdfGenerator.cc 95.61% <100.00%> (+0.07%) ⬆️
... and 27 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 4a1e1b5...3d035b3. Read the comment docs.

@chapulina
Copy link
Contributor Author

@osrf-jenkins run tests please

@chapulina
Copy link
Contributor Author

@JShep1, I think this is ready to be used by #173 . Let me know if you run into any problems.

Signed-off-by: Louise Poubel <[email protected]>
src/ServerPrivate.cc Outdated Show resolved Hide resolved
src/Util.cc Outdated Show resolved Hide resolved
@JShep1
Copy link

JShep1 commented Jul 8, 2020

I just ran this and am running into an issue with ign gazebo only showing a black screen with the message

[GUI] [Dbg] [Gui.cc:144] GUI requesting list of world names. The server may be busy downloading resources. Please be patient.

being printed every few seconds. I replicated this issue on my laptop as well, are you running into this? @chapulina

I believe the changes that broke things are in this diff 00d9235 - those are the changes I merged into the insert local models branch that made it give this error.

Signed-off-by: Louise Poubel <[email protected]>
@chapulina
Copy link
Contributor Author

@JShep1 , a086574 has a temporary fix to that issue. For some reason the global node works when doing ign gazebo -s and ign gazebo -g separately, but not for ign gazebo 🤔 I'll look better into it tomorrow. For now, be warned that if you add any paths through the add service, the GUI will not know about them automatically, which may cause some models to fail.

@chapulina
Copy link
Contributor Author

Ok, this should be good to be reviewed again. @JShep1 , let me know if you need help using it on #173 .

tutorials/resources.md Outdated Show resolved Hide resolved
src/gui/PathManager.cc Outdated Show resolved Hide resolved
src/gui/PathManager.cc Outdated Show resolved Hide resolved
chapulina and others added 2 commits July 15, 2020 11:14
Signed-off-by: Louise Poubel <[email protected]>

Co-authored-by: Nate Koenig <[email protected]>
Signed-off-by: Louise Poubel <[email protected]>
@chapulina chapulina linked an issue Jul 20, 2020 that may be closed by this pull request
@chapulina
Copy link
Contributor Author

The 3 test failures are not there on ign-gazebo2, I'll take a look: https://build.osrfoundation.org/job/ignition_gazebo-ci-pr_any-ubuntu_auto-amd64/3765/testReport/

@chapulina
Copy link
Contributor Author

The 3 test failures are not there on ign-gazebo2

Not true, they're there on ign-gazebo2 now.

@chapulina chapulina merged commit fb8ece7 into ign-gazebo2 Aug 3, 2020
@chapulina chapulina deleted the chapulina/resource_path branch August 3, 2020 15:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
📜 blueprint Ignition Blueprint
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Insert models from local paths
5 participants