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

Update URDF parsing logic and improve R/W operations on files #100

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

flferretti
Copy link
Contributor

@flferretti flferretti commented Oct 11, 2024

This PR updates the parsing logic for URDF files by bounding the check on file existence to the maximum path length allowed by the OS. Moreover, some changes were made to make the RW operations on files safer and some final linting was performed.

Closes ami-iit/comodo#21


📚 Documentation preview 📚: https://adam-docs--100.org.readthedocs.build/en/100/

Comment on lines +25 to +32
## Hack to remove the encoding urdf, see https:/icub-tech-iit/ergocub-gazebo-simulations/issues/49
with open(model_path, "r", encoding="utf-8") as robot_file:
robot_urdf_string = (
robot_file.read()
.replace("<?xml", "")
.replace("version='1.0'", "")
.replace("encoding='UTF-8'?>", "")
)
Copy link
Member

Choose a reason for hiding this comment

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

@traversaro @CarlottaSartore did ros/urdf_parser_py#83 fix ros/urdf_parser_py#82? If yes, maybe these lines that remove the encoding can be removed.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think so, but no release was done, so I guess the issue is still present in the latest release, and the problem is also that the pypi package is abandoned.

Copy link
Member

Choose a reason for hiding this comment

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

Ok I see, so probably the solution for ADAM is to just migrate to something else, at one point in the future. Thanks for the feedback!

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.

OSError when creating RobotModel
3 participants