URDF to SDF conversion silently ignores <gazebo> tag with non-existing reference

[traversaro]

Desired behavior

If I try to convert the following URDF model, saved in a file called oneLink.urdf:

<?xml version='1.0' encoding='UTF-8'?>
<robot name="oneLink">
    <link name="link1">
        <inertial>
            <mass value="1" />
            <inertia ixx="0.01" ixy="0.0" ixz="0.0" iyy="0.01" iyz="0.0" izz="0.01" />
        </inertial>
    </link>
    <gazebo reference="lіnk1">
        <sensor name="link1_imu" type="imu">
            <always_on>1</always_on>
            <update_rate>100</update_rate>
            <pose>0.13525 0 -0.07019999999999993 0.0 -0.0 -2.0943952105869315</pose>
            <plugin name="sensor_plugin" filename="example_plugin.so" />
        </sensor>
    </gazebo>
</robot>

to SDF with sdformat 14.0.0 (i.e. via gz sdf -p ./oneLink.urdf > oneLink.sdf), I obtaine the following result, without any additional output:

(gzsdf) traversaro@IITBMP014LW012:~/test34$ gz sdf -p ./oneLink.urdf
<sdf version='1.11'>
  <model name='oneLink'>
    <link name='link1'>
      <inertial>
        <pose>0 0 0 0 0 0</pose>
        <mass>1</mass>
        <inertia>
          <ixx>0.01</ixx>
          <ixy>0</ixy>
          <ixz>0</ixz>
          <iyy>0.01</iyy>
          <iyz>0</iyz>
          <izz>0.01</izz>
        </inertia>
      </inertial>
    </link>
  </model>
</sdf>

Note how the imu sensors is completely ignored, without any warning message or error. Why is this happening? Because, despite the apperance, the link1 string in <link name='link1'> is not the same string as lіnk1 in <gazebo reference="lіnk1">. The second letter of the string in the first case is i , i.e. unicode character U+0069 (Latin Small Letter I), while in the second case is unicode character U+0456 (Cyrillic Small Letter Byelorussian-Ukrainian I).

Clearly I crafted this example to make it explicitly tricky to spot (see https://gist.github.com/StevenACoffman/a5f6f682d94e38ed804182dc2693ed4b if you want to have fun), but in general it would be convenient if the URDF --> SDF parser detected <gazebo reference=".."> that referenced to non-existing elements, and printed at least a warning, to help in case of typos.

Alternatives considered

Continue not to print any warning if a <gazebo> tag refers to a non existing element.

Implementation suggestion

I checked the code, and indeed first the parser parsers all the <gazebo> tags in

sdformat/src/parser_urdf.cc

Line 3372 in f360776

this->ParseSDFExtension(urdfXml);

, and then only later when it parses links, collisions, visuals and joints, it uses the related sdf extensions if any is found (see for example https://github.com/gazebosim/sdformat/blob/f3607761d3846dba0b660880484ff1e5eb1e9425/src/parser_urdf.cc#L2870C3-L2870C12 or

sdformat/src/parser_urdf.cc

Line 3182 in f360776

InsertSDFExtensionJoint(joint, _link->parent_joint->name);

).

Probably we should add a boolean flag or similar to each SDF extension, that is set to true once a SDF extension is consumed. If an SDF extension is not used at the end of the parsing, a warning should be printed.

Additional context

We experienced this with @Gio-DS .

[azeey]

Ouch! That's a tough one to debug. +1 for a warning message if an extension references an unknown link.

[aagrawal05]

Hi, I have attempted an implementation on my personal fork aagrawal05/sdformat on the parser_ref_warning_message branch.

Currently it only checks in the InsertSDFExtensionJoint method, but similar functionality can be easily implemented for the other functions (namely InsertSDFExtensionRobot, InsertSDFExtensionCollision, InsertSDFExtensionVisual, and InsertSDFExtensionJoint).

My solution currently works by adding a boolean check in the linear search of the extensions in the insertion function and adding the warning if not found.

I've also written a test ParseGazeboRefDoesntExistWarningMessage that ensures the functionality of this check.

Please let me know if this approach is suitable and if any changes would need to be made. If not, I'd be happy to open the pull request.

[azeey]

Thanks for looking into it @aagrawal05. I think it would be easier to check your approach if you just open a draft PR.