PX4 / PX4-gazebo-models

Model repo in app.gazebosim.org
BSD 3-Clause "New" or "Revised" License
11 stars 34 forks source link

SDF files should be automatically checked for formatting #38

Open slgrobotics opened 2 months ago

slgrobotics commented 2 months ago

At the moment model and world SDF files are not validated when they are checked in, and there is no single convenient tool to format these files. Linux has xmllint command line utility that allows formatting (pretty-printing) such files.

Here are suggested shell scripts that could help with simple checks and formatting (the same way astyle is used for C++ files):

  1. Formatter. format-sdf.sh <file>
    
    #!/bin/bash

xmllint --format $1 | tail -n +2 > /tmp/$1.tmp

mv /tmp/$1.tmp $1


2. Verifier. ```if [ check-sdf.sh <file> ] ...```

!/bin/bash

xmllint --format $1 | tail -n +2 | diff $1 - | head -

xmllint --format $1 | tail -n +2 | diff $1 - &>/dev/null

The ```tail -n +2``` is needed to remove standard XML header (if it is not desirable) 

In verifier the first operand produces visible output, while the second line allows $? variable to be returned from diff
Verifier basically formats the file and compares the result with original

A more elaborated check could be done - **validating SDF files against their XML schema**.

Format of Gazebo SDF files is described here: http://sdformat.org/spec?ver=1.11&elem=sdf

There is a git repository for a C++ code-generating tool https://github.com/gazebosim/sdformat

Schema is available as part of that repository: https://github.com/gazebosim/sdformat/blob/sdf14/sdf/1.9

```xmllint --schema <>``` would perform such validation, if all schema files are pulled locally from the repository.

Pulling a collection of usable XSDs from that repository is a bit complicated - there is an additional step, using an SDF->XSD converter (as part of the _sdformat_ build process):
 https://github.com/gazebosim/sdformat/blob/sdf14/tools/xmlschema.py 

A full _clone / cmake / make_ process is required to produce a directory with actual XSDs usable by _xmllint_ (under _build/sdf/1.9_)

Then this validation works (slowly):

~/gz10/PX4-gazebo-models/worlds$ xmllint --schema sdformat/build/sdf/1.9/root.xsd baylands.sdf ... baylands.sdf:28: element scene: Schemas validity error : Element 'scene': Character content other than whitespace is not allowed because the content type is 'element-only'. baylands.sdf:156: element gravity: Schemas validity error : Element 'gravity': This element is not expected. baylands.sdf fails to validate


I could look into it further, although implementing full schema validation would require having that XSD tree checked in, and validator is a bit slow. I also expect a lot of validation errors in the existing files, and some cleanup to follow.

@dagar - per our recent conversation
slgrobotics commented 2 months ago

@dagar - P.S. - after a closer look at generated XSD files, it looks like "include noise.sdf" statements, for example, are ignored, and the resulting imu.xsd file, for example, does not contain noise elements. Basically, generated XSDs are incomplete.

https://github.com/gazebosim/sdformat/issues/633 - open issue, last mention 2021

It is a long rabbit hole...

slgrobotics commented 2 months ago

Just a note: the command that builds specific XSD file (imu.xsd in this case):

python3 tools/xmlschema.py --sdf-dir sdf/1.9 --input-file sdf/1.9/imu.sdf --output-dir build/sdf/1.9

segment in imu.sdf:

    <element name="x" required="0">
      <description>Angular velocity about the X axis</description>
      <include filename="noise.sdf" required="0"/>
    </element>

the resulting segment in imu.xsd ("include noise.sdf" ignored):

              <xsd:choice  minOccurs='0' maxOccurs='1'>
              <xsd:element name='x'>
                <xsd:annotation>
                  <xsd:documentation xml:lang='en'>
                    <![CDATA[Angular velocity about the X axis]]>
                  </xsd:documentation>
                </xsd:annotation>
                <xsd:complexType>
                  <xsd:choice maxOccurs='unbounded'>
                  </xsd:choice>
                </xsd:complexType>
              </xsd:element>
              </xsd:choice>