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

Embed SDF XML files into library #141

Closed
osrf-migration opened this issue Feb 15, 2017 · 17 comments
Closed

Embed SDF XML files into library #141

osrf-migration opened this issue Feb 15, 2017 · 17 comments

Comments

@osrf-migration
Copy link

Original report (archived issue) by clalancette (Bitbucket: clalancette, GitHub: clalancette).


I'm considering changing the sdformat library so that it directly embeds the SDF XML files (basically all of the contents of the sdf/ subdirectory) into resource strings in the library. They would be separate XML files in the source repository, but at build time we would run an external program to make resource strings out of them, and then build that into the library.

The main benefit to doing so would be to remove any code within the library that goes looking through the filesystem to find these files. This would make it faster, and remove a dependency on boost::filesystem. It would also make it easier for the tests, and potentially easier to deploy the library in general, since you will now only need the .so . It may also be slightly faster, though this is probably a minor benefit.

The potential downsides to doing so are removing visibility of the XML files from the end-user, but I'm not sure that is hugely useful anyway. It also requires a rebuild of the library to "upgrade" the SDF XML files, but that seems pretty minor.

Are there any other objections to this plan?

@osrf-migration
Copy link
Author

Original comment by Shane Loretz (Bitbucket: Shane Loretz, GitHub: sloretz).


I think it could also fix this issue I'm seeing: make test fails because tests can't find the .sdf files until they are installed using sudo make install.

@osrf-migration
Copy link
Author

Original comment by clalancette (Bitbucket: clalancette, GitHub: clalancette).


Yes, exactly. I had that problem too, which I "fixed" by doing an install first, but that is not optimal.

@osrf-migration
Copy link
Author

Original comment by Steve Peters (Bitbucket: Steven Peters, GitHub: scpeters).


Actually, make test should work as of pull request #226, though running individual test binaries directly won't work without installing. Is make test itself not working?

@osrf-migration
Copy link
Author

Original comment by Steve Peters (Bitbucket: Steven Peters, GitHub: scpeters).


I have a higher-level questions that is relevant to this topic:

  • do we want to try to allow 3rd parties to add their own custom schema to sdformat/urdf? how would we do that?

I don't want to block this discussion, but just want to make sure we keep the big picture in mind.

With that said, this is a reasonable way to get around boost::filesystem, and it's not really a great idea to edit the sdf files installed in /usr/share/sdformat, so embedding the files doesn't make you lose much there. I much prefer to make changes with version control and then install, which would still be able to embed. We could also still install the sdf files as a form of documentation, though they wouldn't be parsed by the library.

So I guess my biggest question is about handling 3rd-party schema.

@osrf-migration
Copy link
Author

Original comment by Shane Loretz (Bitbucket: Shane Loretz, GitHub: sloretz).


Ooops my mistake, make test works, but running the binaries does not

@osrf-migration
Copy link
Author

Original comment by Steve Peters (Bitbucket: Steven Peters, GitHub: scpeters).


To further clarify, we configure ctest to set an extra environment variable in cmake/SDFUtils.cmake so that installation isn't necessary, though this doesn't help when directly running test executables without ctest or the make test wrapper.

@osrf-migration
Copy link
Author

Original comment by clalancette (Bitbucket: clalancette, GitHub: clalancette).


Oh, you know, that's probably what happened to me. I was running the unit tests directly, which is probably when I saw this issue.

@osrf-migration
Copy link
Author

Original comment by Steve Peters (Bitbucket: Steven Peters, GitHub: scpeters).


I only remembered this because the same thing happened to me too yesterday, which was puzzling because I distinctly remembered changing one of our CI scripts to perform make test before make install.

I think make test ARGS="-VV -R UNIT_Converter_TEST" will just run a single test (converter unit test in this example)

@osrf-migration
Copy link
Author

Original comment by clalancette (Bitbucket: clalancette, GitHub: clalancette).


@scpeters Your point about 3rd party schemas is a good one, and one that is very relevant for some work with Drake. There, the Drake maintainers have added custom XML to their "fork" of SDF, and are looking for a way to merge some of that back into sdformat. My current (admittedly superficial) thinking is that we would probably want to add some kind of "<include_3rdparty>" tag to the base SDF XML format, which libsdformat would parse for XML consistency, and then expose to the user as a "raw" XML pointer. With that in place, it would still be OK to embed the standard SDF XML into the library.

@osrf-migration
Copy link
Author

Original comment by Steve Peters (Bitbucket: Steven Peters, GitHub: scpeters).


@sloretz you mentioned using xml schemas and stylesheets in #138. Is there a canonical way to handle 3rd party schemas with those tools?

@osrf-migration
Copy link
Author

Original comment by Shane Loretz (Bitbucket: Shane Loretz, GitHub: sloretz).


I think the soap envelope is a good example. It allows custom elements inside it's body using the tag.

If the sdformat schema added a tag (mabye call it <third_party>?) that allowed any xml element inside, it could validate the SDF regardless of the content of the <third_party> tags.

If third party elements specify a namespace, and the schema for that namespace is available (usually the namespace is a URI where the schema can be downloaded, but I recommend adding an API to give custom schemas to sdformat), they can be validated by sdformat too.

@osrf-migration
Copy link
Author

Original comment by Nate Koenig (Bitbucket: Nathan Koenig).


Side note. Using a namespace would also entail ditching tinyxml2. If namespaces make sense here, then we should use them and switch to another library.

@osrf-migration
Copy link
Author

Original comment by Silvio Traversaro (Bitbucket: traversaro).


Regarding 3rd party schemas : there are several parts of the current SDF specification that are quite Gazebo-specific (see [1] or most all the physics parameters). If the long-term idea for SDF is to be a simulation format whose semantics it definite independently from Gazebo, it may be convenient to move Gazebo-specific parameters to a Gazebo-specific 3rd party schema .

[1] https://osrf-migration.github.io/sdformat-gh-pages/#!/osrf/sdformat/issues/93/change-the-sdf-spec-to-make-less-gazebo (#93)

@osrf-migration
Copy link
Author

Original comment by Silvio Traversaro (Bitbucket: traversaro).


A cool solution to embed several files in a binary and still access them with a filesystem-like API is https://github.com/vector-of-bool/cmrc .

Unfortunately it is CMake only, but I guess the same template could be also generate from other build system if necessary.

@osrf-migration
Copy link
Author

Original comment by Eric Cousineau (Bitbucket: eacousineau, GitHub: EricCousineau-TRI).


Er, just to check, has this been resolved by this PR?

https://osrf-migration.github.io/sdformat-gh-pages/#!/osrf/sdformat/pull-requests/434/embed-sdf

@osrf-migration
Copy link
Author

Original comment by clalancette (Bitbucket: clalancette, GitHub: clalancette).


Er, just to check, has this been resolved by this PR?

https://osrf-migration.github.io/sdformat-gh-pages/#!/osrf/sdformat/pull-requests/434/embed-sdf

My understanding is that yes, it has been, but I'll let @nkoenig confirm.

@azeey
Copy link
Collaborator

azeey commented Sep 30, 2021

Resolved by #141 and #270

@azeey azeey closed this as completed Sep 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants