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

Support passing additional search paths via package_dirs argument #13

Merged
merged 6 commits into from
May 14, 2024

Conversation

flferretti
Copy link
Contributor

@flferretti flferretti commented May 14, 2024

This pull request adds support for custom environment variables as additional search paths when resolving a robotics URI. The resolve_robotics_uri function now accepts an optional extra_path parameter, which allows users to specify the name of an environment variable associated with a search path. If extra_path is provided, the function will include the path associated with the environment variable to the search paths.

Example usage to find meshes path:

import os
import pathlib

import robot_descriptions.ur10_description
from robot_descriptions._package_dirs import get_package_dirs

from resolve_robotics_uri_py import resolve_robotics_uri

model_urdf_path = pathlib.Path(robot_descriptions.ur10_description.URDF_PATH)

package_dirs = get_package_dirs(robot_descriptions.ur10_description)
mesh_path = resolve_robotics_uri(
    uri="package://example-robot-data/robots/ur_description/meshes/ur10/collision/shoulder.stl",
    package_dirs=package_dirs,
)

print(f"Mesh path: {mesh_path}")

Solves #12

@traversaro
Copy link
Collaborator

traversaro commented May 14, 2024

Can't we just add a package_dirs vector of actual package directories in which to search?

In this way we can do:

mesh_path = resolve_robotics_uri(
    uri="package://example-robot-data/robots/ur_description/meshes/ur10/collision/shoulder.stl",
    package_dirs=get_package_dirs(import_module(f"robot_descriptions.ur10_description")),
)

as done in the rest of the robot_descriptions loaders in https://github.com/robot-descriptions/robot_descriptions.py/tree/d0dc260953a459450e73d9a4b456fd0b02ace59d/robot_descriptions/loaders ?

I think I know the answer to my question, but I want to understand your rationale.

- `GZ_SIM_RESOURCE_PATH`
- `IGN_GAZEBO_RESOURCE_PATH`
- `ROS_PACKAGE_PATH`
- `SDF_PATH`
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we should specify how the extra variable is interpreter. For example, if the uri is package://StrangeModel/Nested/mesh.st, and the actual mesh is in /usr/local/share/StrangeModel/Nested/mesh.stl, what should contain the extra path? /usr/local/share, /usr/local or /usr/local/share/StrangeModel. In theory we should also explicitly document how we interpret all the env variables listed here, but that is a more mitigated problems as they are documented in their respective projects, while the extra_path is something we add only here.

Copy link
Contributor Author

@flferretti flferretti May 14, 2024

Choose a reason for hiding this comment

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

Addressed in 2633149

@flferretti
Copy link
Contributor Author

flferretti commented May 14, 2024

Yes it totally makes sense, I'll do the modification

@flferretti flferretti force-pushed the feature/custom_path branch 2 times, most recently from 38ad20d to 7fa1139 Compare May 14, 2024 11:00
@flferretti
Copy link
Contributor Author

flferretti commented May 14, 2024

Ready for review after applying your suggestion in c1000b7. Thanks a lot!

flferretti and others added 2 commits May 14, 2024 13:09
Co-authored-by: Silvio Traversaro <silvio.traversaro@iit.it>
@flferretti flferretti force-pushed the feature/custom_path branch from 7fa1139 to 2633149 Compare May 14, 2024 11:10
Co-authored-by: Silvio Traversaro <silvio@traversaro.it>
@traversaro traversaro merged commit 6997992 into ami-iit:main May 14, 2024
3 checks passed
@traversaro
Copy link
Collaborator

Thanks @flferretti ! I release v0.3.0 .

@traversaro traversaro changed the title Support passing the name of custom env vars as additional search paths Support passing additional search paths May 14, 2024
@traversaro traversaro changed the title Support passing additional search paths Support passing additional search paths via package_dirs argument May 14, 2024
@flferretti flferretti deleted the feature/custom_path branch May 14, 2024 13:33
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.

2 participants