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

robotology / gym-ignition-models

Collection of robot models compatible with gym-ignition

https://github.com/robotology/gym-ignition

GNU Lesser General Public License v3.0

24 stars 7 forks source link

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

Closed diegoferigo closed 3 years ago

diegoferigo commented 3 years ago

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

https://github.com/robotology/gym-ignition-models/blob/9d3dafe0a7b75ef8af041c6974a2b6f1c1e36f58/gym_ignition_models/__init__.py#L35

https://github.com/robotology/gym-ignition-models/blob/9d3dafe0a7b75ef8af041c6974a2b6f1c1e36f58/gym_ignition_models/__init__.py#L67

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 (https://github.com/osrf/sdformat/issues/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 commented 3 years ago

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

diegoferigo commented 3 years ago

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 commented 3 years ago

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

traversaro commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

Done in 7d79271.