-
Notifications
You must be signed in to change notification settings - Fork 92
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
Comments
Original comment by Shane Loretz (Bitbucket: Shane Loretz, GitHub: sloretz). I think it could also fix this issue I'm seeing: |
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. |
Original comment by Steve Peters (Bitbucket: Steven Peters, GitHub: scpeters). Actually, |
Original comment by Steve Peters (Bitbucket: Steven Peters, GitHub: scpeters). I have a higher-level questions that is relevant to this topic:
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 So I guess my biggest question is about handling 3rd-party schema. |
Original comment by Shane Loretz (Bitbucket: Shane Loretz, GitHub: sloretz). Ooops my mistake, |
Original comment by Steve Peters (Bitbucket: Steven Peters, GitHub: scpeters). To further clarify, we configure |
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. |
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 I think |
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. |
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? |
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 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. |
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. |
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) |
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. |
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 |
Original comment by clalancette (Bitbucket: clalancette, GitHub: clalancette).
My understanding is that yes, it has been, but I'll let @nkoenig confirm. |
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?
The text was updated successfully, but these errors were encountered: