search

ros / urdfdom

URDF parser
http://ros.org/wiki/urdf
Other
96 stars 132 forks source link

XML schema invalid #95

Open lexknuther opened 8 years ago

lexknuther commented 8 years ago

The XML schema is currently invalid W.R.T. to the link element. The link uses an xs:all element with some unbounded elements. The XML standard only allows zero or one for the bounds. I'd recommend changing it to a sequence. Changing this would trade off correctness for possible invalidating some existing URDFs.

dljsjr commented 7 years ago

I'm also encountering this issue. I would like to use the standard xjc tooling that comes with JAXB to generate some Java classes for parsing URDF, and the xjc compiler errors out on the "unbounded" value on maxOccurs for link visuals and collisions.

scpeters commented 7 years ago

So the link allows an arbitrary number of collision and visual tags. If we change it to a sequence, I wonder if each type of tag have to be adjacent or can they be mixed? Reading...

dljsjr commented 7 years ago

@lexknuther You seem to know a bit more about XML than I do, what about using choice instead of sequence? Something like this:

  <!-- link node type -->
  <xs:complexType name="link">
    <xs:choice minOccurs="0" maxOccurs="unbounded">
      <xs:element name="inertial"
                  type="inertial" minOccurs="0" maxOccurs="1" />
      <xs:element name="visual"
                  type="visual" minOccurs="0" maxOccurs="unbounded" />
      <xs:element name="collision"
                  type="collision" minOccurs="0" maxOccurs="unbounded" />
    </xs:choice>
    <xs:attribute name="name" type="xs:string" use="required" />

    <!-- FIXME: undocumented but used by PR2 -->
    <xs:attribute name="type" type="xs:string" />
  </xs:complexType>
scpeters commented 7 years ago

Oh, maybe the sequence could contain both collision and visual definitions...

scpeters commented 7 years ago

Ok, I don't know what I'm talking about at all.

dljsjr commented 7 years ago

@scpeters From my limited understanding, an xs:all doesn't enforce an ordering whereas a xs:sequence does. An xs:choice with an unbounded maxOccurs lets the choice show up as many times as you want and you can have additional bound constraints on the possible choices but in the case of code generators it can generate code that isn't type-safe.

For validation purposes, though, I think it should work.

scpeters commented 7 years ago

Could the inertial tag remain in an xs:all block?

dljsjr commented 7 years ago

xs:complexType can only have a single child element.

scpeters commented 7 years ago

choice seems like the right thing to me

dljsjr commented 7 years ago

Using a sequence of choices might make things look nicer for code generators. I'm going to keep playing with it.

dljsjr commented 7 years ago

The plot thickens.

@scpeters Okay so, apparently, using unbounded in the xs:all is actually valid in XSD 1.1…

It's just that almost nothing out in the wild supports XSD 1.1.