search

gazebosim / sdformat

Simulation Description Format (SDFormat) parser and description files.
http://sdformat.org
Apache License 2.0
169 stars 97 forks source link

RFC: Convert xmlschema.rb to c++ #118

Open osrf-migration opened 8 years ago

osrf-migration commented 8 years ago

Original report (archived issue) by Rich Mattes (Bitbucket: richmattes).

I had some spare time a few months ago and managed to convert xmlschema.rb to c++ using tinyxml. I was wondering if there's any interest from you guys in supporting such a switch.

The tinyxml-based C++ xmlschema is basically a line-for-line port of the existing ruby script to C++. It provides the following:

Benefits:

Drawbacks:

The lossy treatment of whitespace seems to be a well-known attribute of tinyxml, and they recommend that tinyxml2 should be used instead (in general, and also because of the whitespace handling.)

I ended up making a second version of xmlschema using tinyxml2, and was able to get identical output to the ruby scripts. It wasn't terribly difficult to go from tinyxml->tinyxml2, but using a different version of tinyxml than sdformat uses seemed to be kind of silly, and I didn't have enough time to try to port sdformat to tinyxml2.

If there's interest in either or both versions of the code I'll clean it up and create PRs, otherwise you can go ahead and close this bug.

osrf-migration commented 8 years ago

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

This sounds great. I'd be in favor of a c++ version of xmlschema.rb. We have started using tinyxml2 in Gazebo, and a port to tinyxml2 in sdformat would be most welcome.

We can do this in two steps: (1) port to tinyxml2 , (2) replace xmlschema.rb with your c++ implementation.

osrf-migration commented 8 years ago

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

@rmattes converting it to c++ sounds great.

As an aside, I would mention that there still may be plans to integrate ruby with sdformat as part of a runtime interpreter, but that would have to go through a pull request review process.

osrf-migration commented 8 years ago

Original comment by Rich Mattes (Bitbucket: richmattes).

Cool. My c++ implementation is located at https://bitbucket.org/rmattes/sdformat/branch/tinyxml2_port

It's currently set up to run both the ruby and c++ generators so I can compare the differences in output. The only difference now is the removal of an errant space with respect to the ruby version, and the fact that the c++ version won't hit "printDocumentation" when there's no description tag (only happens with the root gui.sdf element as it doesn't have a description.)

I can try to tackle moving the rest of sdformat to tinyxml2 as well, but it'll probably take me a while as my free time is pretty limited for the next few months.

osrf-migration commented 8 years ago

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

Sounds great, thanks Rich!