gazebosim / gz-sim

Open source robotics simulator. The latest version of Gazebo.
https://gazebosim.org
Apache License 2.0
621 stars 251 forks source link

Enable StoreResolvedURIs when loading SDF #2349

Closed iche033 closed 2 months ago

iche033 commented 3 months ago

🎉 New feature

Summary

Replaces #2323 - see description in that PR.

This PR is needed so that bullet-featherstone plugin in gz-physics can load meshes with relative path URIs.

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.

iche033 commented 3 months ago

The ServerFixture.LoadSdfModelRelativeUri test in UNIT_Server_Rendering_TEST.cc was failing because it contains a mesh uri pointing to a non-existent dae file. With SetStoreResolvedURIs set to true, it returns an error and so the server stops loading the model. This breaks current behavior. To resolve this, I added logic in ddf7f46 to allow the server to proceed with presence of URI_LOOKUP errors.

iche033 commented 2 months ago

The ServerFixture.LoadSdfModelRelativeUri test in UNIT_Server_Rendering_TEST.cc was failing because it contains a mesh uri pointing to a non-existent dae file. With SetStoreResolvedURIs set to true, it returns an error and so the server stops loading the model. This breaks current behavior. To resolve this, I added logic in ddf7f46 to allow the server to proceed with presence of URI_LOOKUP errors.

Looking for feedback on this. Any objections to allowing Server to continue loading with presence of URI errors?

azeey commented 2 months ago

I'm not sure this is a good idea. The URI_LOOKUP error is not just emitted for missing mesh files, but also when an included model is not found, which should be a blocker error IMO.

iche033 commented 2 months ago

The other option is to not return the URI_LOOKUP error in sdformat when resolving URIs but that's also not ideal.

So with SetStoreResolvedURIs now set to true, we'll just be more strict and stop the Server from loading if an URI is not found. How does that sound?

azeey commented 2 months ago

Maybe we can define a parameter in ServerConfig that determines if a SDF errors prevent the Server from loading. Or it could be an environment variable. Either way, we can just set that in the test without affecting end users.

iche033 commented 2 months ago

Maybe we can define a parameter in ServerConfig that determines if a SDF errors prevent the Server from loading.?

Added a new parameter in ServerConfig, blockOnSdfErrors to stop server from loading in presence of SDF errors. It can also be passed from the command line, gz sim -v 4 shapes.sdf --block-on-sdf-errors 1. fbddcb7

iche033 commented 2 months ago

DCO needs to be fixed.

fixed and force pushed

iche033 commented 2 months ago

fixed a couple of windows failures in 7488f7e.