Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

URDF included in SDF 1.6 is silently corrupted in Gazebo 11 when spawned from the GUI #2728

Closed
traversaro opened this issue May 7, 2020 · 11 comments · Fixed by #2734
Closed

Comments

@traversaro
Copy link
Collaborator

traversaro commented May 7, 2020

I have one simple URDF file:

Click to see URDF content
<?xml version="1.0"?>
<robot name="model_urdf">
  <link name="base_link">
    <inertial>
      <mass value="1"/>
      <origin xyz="0 0 0" rpy="0 -0 0"/>
      <inertia ixx="0.01" ixy="0" ixz="0" iyy="0.01" iyz="0" izz="0.01"/>
    </inertial>
   <visual>
     <origin xyz="0 0 0.5" rpy="0 0 0" />
     <geometry>
       <box size="1 1 1" />
     </geometry>
     <material name="Red">
       <color rgba="1.0 0.0 0.0 1.0"/>
     </material>
   </visual>
   <collision>
     <origin xyz="0 0 0.5" rpy="0 0 0"/>
     <geometry>
       <box size="1 1 1" />
     </geometry>
   </collision>
  </link>
  <joint name="joint_a1" type="continuous">
    <origin xyz="0 0 1.0" rpy="0 -0 0"/>
    <axis xyz="0 0 -1"/>
    <parent link="base_link"/>
    <child link="link_1"/>
  </joint>
  <link name="link_1">
    <inertial>
      <mass value="1"/>
      <origin xyz="0 0 0" rpy="0 -0 0"/>
      <inertia ixx="0.01" ixy="0" ixz="0" iyy="0.01" iyz="0" izz="0.01"/>
    </inertial>
   <visual>
     <origin xyz="0 0 0.5" rpy="0 0 0" />
     <geometry>
       <cylinder radius="0.6" length="1.0"/>
     </geometry>
     <material name="Green">
       <color rgba="0.0 1.0 0.0 1.0"/>
     </material>
   </visual>
   <collision>
     <origin xyz="0 0 0.5" rpy="0 0 0"/>
     <geometry>
       <cylinder radius="0.6" length="1.0"/>
     </geometry>
   </collision>
  </link>
</robot>

that is included in two almost identical SDF files, one with version 1.6:

<?xml version='1.0'?>
<sdf version='1.6'>
    <model name='model_sdf_1_6'>
        <include>
             <uri>model://model_urdf</uri>
             <pose>0.0 0.0 0.0 0.0 0 0.0</pose>
        </include>
    </model>
</sdf>

and another with version 1.7 :

<?xml version='1.0'?>
<sdf version='1.7'>
    <model name='model_sdf_1_7'>
        <include>
             <uri>model://model_urdf</uri>
             <pose>0.0 0.0 0.0 0.0 0 0.0</pose>
        </include>
    </model>
</sdf>

when I spawn the models from the GUI, the SDF 1.7 is loaded correctly, while the kinematics of the SDF 1.6 model are silently loaded incorrectly, see the following GIF:
urdf_nested_in_sdf_1_6

Interestingly, the urdf included models is loaded correctly if I load it in a world model, both if the world is a SDF 1.6 or 1.7 model.
I reproduced the problem on Gazebo 11 on both Windows and Linux.
To reproduce it locally, you can use this models: urdf_included_in_sdf_1_6_mwe.zip .

Short setup bash script to reproduce the GUI spawning:

wget https://github.com/osrf/gazebo/files/4594695/urdf_included_in_sdf_1_6_mwe.zip
unzip urdf_included_in_sdf_1_6_mwe.zip
cd urdf_included_in_sdf_1_6_mwe
export GAZEBO_MODEL_PATH=`pwd`
gazebo

on the same terminal, you can see that loading the models from a world file is working correctly:

gazebo world_sdf_1_6.world
gazebo world_sdf_1_7.world
@traversaro
Copy link
Collaborator Author

traversaro commented May 7, 2020

I think this is the actual bug related to URDF included in SDF 1.4 files that I thought was due to something in sdformat and I reported (but without being able to reproduce it) in robotology-legacy/icub-gazebo-wholebody#30 @EricCousineau-TRI @scpeters @azeey @ale-git .

@traversaro traversaro changed the title URDF included in SDF 1.6 is corrupted in Gazebo 11 when spawned from the GUI URDF included in SDF 1.6 is silently corrupted in Gazebo 11 when spawned from the GUI May 7, 2020
@scpeters
Copy link
Member

which version of libsdformat9 do you have installed?

@traversaro
Copy link
Collaborator Author

traversaro commented May 12, 2020

gazebosim/sdformat@f7dcdd7 on Windows (see https://github.com/iit-danieli-joint-lab/idjl-software-dependencies-vcpkg/blob/master/gazebo-repos.yaml#L37), I don't have the setup in which I reproduced it on Ubuntu but I had installed Gazebo 11 from the OSRF Ubuntu Repo that day.

@traversaro
Copy link
Collaborator Author

I tested again on Ubuntu, and the problem is present with SDFormat 9.2.0-1~bionic .

@azeey
Copy link
Collaborator

azeey commented May 12, 2020

I'm able to reproduce this. I think the main issue is that the included URDF is always converted to SDFormat 1.7, but we're using readFileWithoutConversion in Gazebo for the GUI. I'm trying to remember why and if we still need to use it instead of readFile.

@azeey
Copy link
Collaborator

azeey commented May 12, 2020

We use readFileWithoutConversion because we need to preserve the original SDFormat version when serializing and sending to gzserver, i.e, if we read in a model that's SDFormat 1.6, we want to send it to gzserver as 1.6 not as the upconverted version. The solution I came up with is to use readFile for the SDFPtr used for the visual preview and readFileWithoutConversion for another SDFPtr used for sending the model to gzserver.

@traversaro
Copy link
Collaborator Author

traversaro commented May 14, 2020

We use readFileWithoutConversion because we need to preserve the original SDFormat version when serializing and sending to gzserver, i.e, if we read in a model that's SDFormat 1.6, we want to send it to gzserver as 1.6 not as the upconverted version.

I was a bit confused by this sentence, I am trying to understand the rationale from the code, I guess this comment https://osrf-migration.github.io/gazebo-gh-pages/#!/osrf/gazebo/pull-requests/3133/page/1#comment-131669268 is related, even if I still miss which problem there is in sending SDF 1.7 files to ~/factory.

@EricCousineau-TRI
Copy link

EricCousineau-TRI commented May 14, 2020

Per f2f, possible next steps:

  • Confirm that this is still an issue with Ignition Gazebo - is the error obvious at least?
  • Prevent a SDFormat 1.6 file to include SDFormat 1.7?
  • If an URDF is incorporated in an SDFormat 1.6, then use the "old" transforming code. Can still have the option to keep nominal 1.7 conversion, and have explicit option for --urdf_to_sdformat_version=1.6, which is only valid for *.urdf file.

@traversaro
Copy link
Collaborator Author

traversaro commented May 20, 2020

If anyone needs to maintain a codebase with models that need to be compatible with both Gazebo 10 and 11, for which then just bumping the SDF version to 1.7 is not a viable solution, the following CMake snippet may be useful as a temporary workaround:

# Before this, copy the SDF files in ${CMAKE_BINARY_DIR} for processing
# ...

# List all the .sdf files in  ${CMAKE_BINARY_DIR}
file(GLOB_RECURSE sdf_files ${CMAKE_BINARY_DIR}/*.sdf)

# Workaround for the following osrf/gazebo issues:
# * https://github.com/osrf/gazebo/issues/2728
# * https://github.com/osrf/gazebo/issues/2739
if(GAZEBO_VERSION VERSION_GREATER_EQUAL 11.0)
  message(STATUS "Gazebo >= 11 detected, patching SDF models to version 1.7")
  foreach(sdf_file IN LISTS sdf_files)
    file(READ ${sdf_file} SDF_CONTENT)
    string(REPLACE "<sdf version='1.6'>" "<sdf version='1.7'>" SDF_CONTENT_CONVERT_TO_1_7 ${SDF_CONTENT})
    file(WRITE ${sdf_file} ${SDF_CONTENT_CONVERT_TO_1_7})
  endforeach()
endif()

# After that, install the SDF files from  ${CMAKE_BINARY_DIR}
# ...

Note that this will not work if your SDF 1.6 use some features that changed meaning/is not supported in 1.7, i.e. simply changing <sdf version='1.6'> to <sdf version='1.7'> is not a replacement for proper SDF 1.6 --> 1.7 conversion for general SDF files.

@azeey
Copy link
Collaborator

azeey commented May 21, 2020

I was a bit confused by this sentence, I am trying to understand the rationale from the code, I guess this comment https://osrf-migration.github.io/gazebo-gh-pages/#!/osrf/gazebo/pull-requests/3133/page/1#comment-131669268 is related, even if I still miss which problem there is in sending SDF 1.7 files to ~/factory.

When an SDFormat file is parsed, it automatically gets up-converted to the latest spec. In this case 1.7. The original version of the document is kept and it can be retrieved by calling sdf::Element::OriginalVersion. The code in Gazebo classic looks at the original version for deciding whether to resolve poses using frame semantics. If the original version is 1.6, it doesn't use frame semantics. When using ~/factory in the GUI, we parse the SDFormat document for creating a visual preview and once the model is placed in the world, we serialize the document again to publish to ~/factory. This serialization loses the fact that the original document was 1.6. When the server receives this document, it will treat it as though it was originally a 1.7 file.

The fact that Gazebo doesn't use frame semantics if the original version is 1.6 is also a problem with the addition of Bitbucket PR #668 and Bitbucket PR #676. Bitbucket PR #668 uses frame semantics for handling pose transforms of nested models. This now means frames have to be resolved for nesting to work properly.

@EricCousineau-TRI

If an URDF is incorporated in an SDFormat 1.6, then use the "old" transforming code. Can still have the option to keep nominal 1.7 conversion, and have explicit option for --urdf_to_sdformat_version=1.6, which is only valid for *.urdf file.

Even if we made the changes from Bitbucket PR #676 optional, the problem will persist because the converted URDF is nested inside a parent model and that nesting uses frames.

My suggestion is to fix Gazebo to resolve frames even if the original version is not 1.7. Otherwise, we'll have to revert both PR 668 and 676 or come up with a way to make them optional.

The caveat with my suggested approach is that if the model containing the nested URDF has an error, frames may not be resolved properly. Whereas in the past Gazebo might have ignored those errors and loaded the model anyway. We can try to resolve poses even if Load returns an error though. This would allow files that have errors in, say, the material tag to still work because that doesn't affect the creation of the frame graph.

@traversaro
Copy link
Collaborator Author

traversaro commented May 22, 2020

Thanks for the nice explanation @azeey , I had completely overlooked that sdf::Element::OriginalVersion is used to decide whether to resolve poses using frame semantics.

azeey added a commit to azeey/gazebo that referenced this issue Jun 26, 2020
Signed-off-by: Addisu Z. Taddese <addisu@openrobotics.org>
traversaro added a commit to robotology/icub-models that referenced this issue Jul 7, 2020
traversaro added a commit to robotology/icub-models that referenced this issue Jul 7, 2020
j-rivero pushed a commit that referenced this issue Jul 20, 2020
…#2734)

* Fix corruption when a URDF file is included from a SDFormat 1.6 model
* Update changelog
* Add regression test for #2728

Signed-off-by: Addisu Z. Taddese <addisu@openrobotics.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants