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

Enhance processing and URDF export of //frame elements #34

Merged
merged 16 commits into from
May 17, 2024

Conversation

diegoferigo
Copy link
Member

@diegoferigo diegoferigo commented May 16, 2024

This PR:

  • Fixes the computation of the //frame/pose element when not explicitly defined in the model description following the documented semantics (pay attention how the pose is defined when there is no explicit //frame/pose, and what's the default when also //frame/attached_to is not defined).
  • Enhances the process of converting //frame elements to compatible chains in URDF.

In the last two year, sdformat received different updates on how URDF models are converted to SDF. In particular, few of them altered how kinematic chains involving massless links (or with tiny masses) are processed. Here below some references for those that want to dig deeper:

TL;DR

  • Before, links with mass smaller than 1e-6 were completely ignored, now instead they become frames.
  • Before, child link of fixed joints (without additional tags for keeping such joints) were lumped into the parent link of the joint, therefore disappearing in the output model. Now, when a link is lumped, instead of disappear, becomes a SDF frame.

This PR bumps the minimum version of sdformat to 12.x such that the processing of such cases is handled in a unique way. Then, in order to prevent the need to install a full Gazebo suite, the minimum sdformat version is raised to 13.x. This version allows to have the gz sdf command line just by installing a small subset of the Gazebo libraries.


Some further details on the logic introduced by this PR:

  • Switching frame convention has now an additional attach_frames_to_links option to update the //frame/attached_to element to be a link, following recursively the hierarchy. In this way, downstream projects can post-process easily frames attached to other frames knowing their real parent link.
  • The inverse of a transform is now computed without inverting the original transform.

@diegoferigo diegoferigo self-assigned this May 16, 2024
@diegoferigo
Copy link
Member Author

Ok I guess it's now time to remove support for the sdformat version included in Ubuntu 22.04. cc @traversaro

@traversaro
Copy link
Collaborator

Ok I guess it's now time to remove support for the sdformat version included in Ubuntu 22.04. cc @traversaro

That kinds of mean enforcing the installation of gz sim via openrobotics's apt repo in apt and gz-sim with conda-forge on conda-forge, right? Fortunatly both at the moment are side-by-side installable with gazebo classic thanks to:

@diegoferigo
Copy link
Member Author

diegoferigo commented May 16, 2024

That kinds of mean enforcing the installation of gz sim via openrobotics's apt repo in apt and gz-sim with conda-forge on conda-forge, right?

Yep!

I don't want to install the whole Gazebo Sim suite just for having the /usr/bin/gz sdf command. I tried to get it using Fortress (sdformat 12.x and gz-tools 1.x), but I couldn't find a way to install only a subset of the libraries. Instead, things work fine with sdformat 13.x and gz-tools 2.x. I propose to support them as minimum versions.

Btw, the working versions are the first ones after the transition from Ignition Gazebo to Gazebo Sim.


Edit: on Fortress it's actually possible to have a minimum installation with only libsdformat12 gz-tools, but the right command line is ign sdf. However, as discussed below, this sdformat supports only the 1.9 SDF specification that at the time of writing does not have all the necessary features to support what ROD require.

@diegoferigo diegoferigo marked this pull request as ready for review May 16, 2024 14:45
@traversaro
Copy link
Collaborator

Can you add a few lines on this runtime dependency on sdformat + gz-tools ? As long as the dependency was just any gz command (either gz-tools+sdformat or gazebo classic) I guess it was quite common for users to just have in the system and/or environemnt, while now the the requirement is more specific, it is quite probably that users will get it wrong. By the way, what is the error in case gz from gazebo classic is installed?

@diegoferigo
Copy link
Member Author

diegoferigo commented May 16, 2024

Can you add a few lines on this runtime dependency on sdformat + gz-tools?

I plan to update the main README with the new dependencies in another PR. I will also bump Python to 3.10. Furthermore, I'd also like to add these dependencies to the conda-forge recipe of rod so that they get automatically installed.

By the way, what is the error in case gz from gazebo classic is installed?

You can see the test failing here. The sdformat of Gazebo Classic 1) does not convert chains of dummy links to frames and 2) links with zero mass are silently removed.

@traversaro
Copy link
Collaborator

I plan to update the main README with the new dependencies in another PR.

Ok.

@traversaro
Copy link
Collaborator

You can see the test failing here. The sdformat of Gazebo Classic 1) does not convert chains of dummy links to frames and 2) links with zero mass are silently removed.

So it does not fail on gz invocation, but silently produce the wrong result? Do you think we can make that an hard failure?

Copy link
Collaborator

@flferretti flferretti left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thank you Diego! I just left a comment on a minor thing

src/rod/urdf/exporter.py Outdated Show resolved Hide resolved
Co-authored-by: Filippo Luca Ferretti <102977828+flferretti@users.noreply.github.com>
@diegoferigo
Copy link
Member Author

So it does not fail on gz invocation, but silently produce the wrong result? Do you think we can make that an hard failure?

I gave it a thought. Initially I was thinking how to check the installed sdformat version either by checking the output of gz sdf --versions or by inspecting the OS. I didn't like both solutions as they are quite fragile.

Then, I realized that the sdformat version is not really relevant. What really matters is the SDF specification. This can be checked more easily by processing a SDF string <sdf version='1.4'> and checking the version of the output (I used 1.4 as the minimum specification listed in the sdformat website).


  • On my setup based on conda-forge with libsdformat14 and libgz-tools2, the check does not raise any exception.
  • On a clean Ubuntu 22.04 docker image with a Gazebo classic installation (and libsdformat9-9), I get the following exception:
    RuntimeError: The found sdformat installation only supports the '1.7' specification, while ROD requires at least the '1.10' specification.
    
  • On a clean Ubuntu 22.04 docker image with libsdformat13 gz-tools2 installed from the OSRF repo, the check does not raise any exception.
  • On a clean Ubuntu 22.04 docker image with libsdformat12 gz-tools installed from the OSRF repo (on this setting, the command line is the old ign sdf instead of gz sdf), the check raises the following exception:
    RuntimeError: The found sdformat installation only supports the '1.9' specification, while ROD requires at least the '1.10' specification.
    
    On this setup, I also verified that the rod tests fail since I was not sure that this specific sdformat version got the necessary features required by the new //frame handling.

@diegoferigo
Copy link
Member Author

diegoferigo commented May 17, 2024

@traversaro last check? Apparently on Windows files need to properly closed before being opened by another process, therefore I had to pass through a temporary directory when a model description string has to be processed by sdformat.

if output_sdf_version < packaging.version.Version(specification_version):
msg = "The found sdformat installation only supports the '{}' specification, "
msg += "while ROD requires at least the '{}' specification."
raise RuntimeError(msg.format(output_sdf_version, specification_version))
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great!

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 this pull request may close these issues.

3 participants