gazebosim / sdformat

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

Enforce required elements #1002

Open chapulina opened 2 years ago

chapulina commented 2 years ago

The "required" field of the spec accepts 5 different values. They're not documented very prominently (#496 ), but there's a definition for them here:

https://github.com/ignitionrobotics/sdformat/blob/12e77821af7c2810c4d9d532ee8cb833cbb8e318/include/sdf/Element.hh#L109-L113

Current enforcement

Looking through the code, I found some places that make use of them:

  1. Deprecated elements (-1) issue errors:

    https://github.com/ignitionrobotics/sdformat/blob/12e77821af7c2810c4d9d532ee8cb833cbb8e318/src/parser.cc#L1491-L1495

  2. Elements that must have exactly 1 or must have at least one + issue errors:

    https://github.com/ignitionrobotics/sdformat/blob/12e77821af7c2810c4d9d532ee8cb833cbb8e318/src/parser.cc#L1504-L1509 https://github.com/ignitionrobotics/sdformat/blob/12e77821af7c2810c4d9d532ee8cb833cbb8e318/src/parser.cc#L1903

But that 's only part of it.

Missing enforcement

Should these also cause errors (they're all currently valid according to ign sdf -k)?


More than one element when it should have exactly one (1):

SDF

``` ```


The difference between 0 and * should be clarified in the documentation and also in the enforcement. I assume that the existence of * implies that 0 means "either none or one". In that case, this should be an error:

SDF

``` ```

scpeters commented 2 years ago

2. Elements that must have exactly 1 or must have at least one * issue errors: https://github.com/ignitionrobotics/sdformat/blob/12e77821af7c2810c4d9d532ee8cb833cbb8e318/src/parser.cc#L1504-L1509

https://github.com/ignitionrobotics/sdformat/blob/12e77821af7c2810c4d9d532ee8cb833cbb8e318/src/parser.cc#L1903

In the referenced code snippets, it checks for 1 or +, not 1 or *. Is it a typo where you wrote *?

chapulina commented 2 years ago

In the referenced code snippets, it checks for 1 or +, not 1 or . Is it a typo where you wrote ?

Yup, fixed