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

Add Python helper that returns path, file, or string of URDF and SDF models (and, if needed, performs conversion) #32

Merged
merged 2 commits into from May 12, 2021

Conversation

diegoferigo
Copy link
Member

@diegoferigo diegoferigo commented May 12, 2021

This repository contains a mixed collection of SDF and URDF models. Until now, we offered the following two functions to get the resources:

def get_model_file(robot_name: str) -> str:

def get_model_string(robot_name: str) -> str:

This PR adds a new get_model_resource function that allows specifying the desired resource type.

It implements the current logic:

  • URDF models can be converted to SDF if scenario is installed.
  • SDF models cannot be converted yet to SDF (Should support SDFormat to URDF conversion gazebosim/sdformat#273).
  • If the model is converted, a temporary file is created.
    • If the resource type is *_FILE, the temp file is automatically deleted when it goes out of scope.
    • If the resource type is *_PATH, the caller is responsible to delete the temp file.

@traversaro
Copy link
Member

Which version of SDF is used? The latest one supported by the underlying sdformat libraries?

@diegoferigo
Copy link
Member Author

Which version of SDF is used? The latest one supported by the underlying sdformat libraries?

The conversion uses the public helper functions of scenario, therefore the sdformat version depends on which Ignition distribution is supported. In this moment, sdformat10 for the stable 1.2.2 release (master branch, Dome), and sdformat11 for the pre-releases (devel branch, Edifice). Both convert by default to SDFormat 1.8.

Disclaimer: I just tried the pre-releases of scenario, however what I wrote above should be correct also for the stable release.

@diegoferigo
Copy link
Member Author

Renamed type argument to resource_type in force push since I always forget that type is a builtin.

@traversaro
Copy link
Member

If the resource type is *_PATH, the caller is responsible to delete the temp file.

Sorry, I had missed this part. Do you think it make sense to place this somewhere in the method docs? If it is just in the PR, it will be lost in the sands of time.

@diegoferigo
Copy link
Member Author

diegoferigo commented May 12, 2021

I am planning to give a refresh to both README and setup.py in a new PR. Regardless, I think it's worth adding a Note in the docstring :)

@diegoferigo
Copy link
Member Author

Done in 7d79271.

@diegoferigo diegoferigo merged commit 978a84a into master May 12, 2021
@diegoferigo diegoferigo deleted the feature/conversion branch May 12, 2021 10:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants