[parsing][urdf] Damping element silently ignored

[dmcconachie-tri]

What happened?

While debugging @amcastro-tri and I stumbled across Drake's URDF parser silently ignoring an entry in a URDF; he suggested I post this as a GitHub issue. In the following code below, note the

    <dynamics>
      <damping>100</damping>
    </dynamics>

section. This entry is silently ignored, however

    <dynamics damping="100"/>

is correctly parsed. The difference is an XML attribute vs an XML element; are we deliberately silently ignoring all elements that are not recognized, or is this something we have not considered before?

Example Code

import os

from pydrake.multibody.parsing import *
from pydrake.multibody.plant import MultibodyPlant

finger_example = """
<?xml version="1.0" ?>
<robot name="example_damping" xmlns:drake="https://drake.mit.edu">
  <link name="finger">
    <inertial>
      <mass value="1"/>
      <origin rpy="0 0 0" xyz="0 0 0"/>
      <inertia ixx="1" ixy="0.0" ixz="0.0" iyy="1" iyz="0" izz="1"/>
    </inertial>
    <collision>
      <origin rpy="0 0 0" xyz="0 0 0"/>
      <geometry>
        <box>
          <size>0.1 0.1 0.1</size>
        </box>
      </geometry>
      <drake:proximity_properties>
        <drake:compliant_hydroelastic/>
        <drake:hydroelastic_modulus value="1e6"/>
        <drake:hunt_crossley_dissipation value="5"/>
        <drake:mu_static value="8.0"/>
        <drake:mu_dynamic value="3.0"/>
      </drake:proximity_properties>
    </collision>
  </link>
  <joint name="finger_joint" type="prismatic">
    <parent link="world"/>
    <child link="finger"/>
    <origin rpy="0 0 0" xyz="0 0 0"/>
    <axis xyz="1 0 0"/>
    <limit drake:acceleration="1.0" effort="100" lower="-1" upper="1.0" velocity="1.0"/>
    <dynamics>
      <damping>100</damping>
    </dynamics>
  </joint>
  <transmission name="transmission">
    <type>transmission_interface/SimpleTransmission</type>
    <joint name="finger_joint">
      <hardwareInterface>PositionJointInterface</hardwareInterface>
    </joint>
    <actuator name="moter">
      <hardwareInterface>PositionJointInterface</hardwareInterface>
      <mechanicalReduction>1</mechanicalReduction>
    </actuator>
  </transmission>
</robot>
"""

folder = os.path.join(os.getcwd(), "tmp")
try:
    os.mkdir(folder)
except OSError:
    pass
with open(os.path.join(folder, "finger.urdf"), "w") as f:
    f.write(finger_example)

directives = """
directives:
  - add_model:
      name: new_model
      file: model://tmp/finger.urdf
"""

plant = MultibodyPlant(time_step=0.01)
parser = Parser(plant=plant)
parser.package_map().Add("tmp", folder)

directives = LoadModelDirectivesFromString(model_directives=directives)
ProcessModelDirectives(directives=directives, parser=parser)
plant.Finalize()
plant.GetJointByName("finger_joint").damping()

Version

af77662

What operating system are you using?

Ubuntu 22.04

What installation option are you using?

compiled from source code using Bazel

Relevant log output

No response

[SeanCurtis-TRI]

It's worth noting that SDF and URDF fundamentally go in different directions. SDF prefers elements and URDF prefers attributes.

As documented for URDF, <dynamics damping="123"/> is the only correct form for URDF. Spelling it as a value is not equivalent and should not produce the same result. The nature of XML is to allow inclusion of arbitrary extra tags and to simply ignore them.

Short conclusion: <damping>100</damping> is 100% incorrect in a URDF file and should not contribute to a correctly configured joint. However, silently ignoring cruft (of any sort) in the URDF is unfortunate.

Looking at the URDF parsing code, it uses the strategy of looking for known elements and attributes, implicitly ignoring everything it isn't explicitly targeting. Short of validating the XML with a schema, this strategy precludes giving feedback on extraneous data.

[jwnimmer-tri]

I think it was worth noting this as a bug (since it gives us practical feedback), but probably we should resolve this as a duplicate of #16997. WDYT @rpoyner-tri?

[jwnimmer-tri]

Closing as duplicate of #16997.