stack-of-tasks / pinocchio

A fast and flexible implementation of Rigid Body Dynamics algorithms and their analytical derivatives
http://stack-of-tasks.github.io/pinocchio/
BSD 2-Clause "Simplified" License
1.85k stars 386 forks source link

URDF relative paths that don't start with "./" #1747

Closed stephane-caron closed 2 years ago

stephane-caron commented 2 years ago

The update from https://github.com/stack-of-tasks/pinocchio/pull/1742 addressed https://github.com/stack-of-tasks/pinocchio/issues/1741, but people are inventive and there is another relative-path syntax out there :wink:

There are currently 5 / 45 descriptions configured in robot_descriptions.py that use relative paths:

Description Pinocchio 2.6.4 Pinocchio 2.6.10
cf2_description :heavy_check_mark:
laikago_description :heavy_check_mark:
mini_cheetah_description
minitaur_description
pr2_description

https://github.com/stack-of-tasks/pinocchio/pull/1742 solved descriptions that do <mesh filename="./foo/bar">, but it doesn't work on the remaining 3 because they do <mesh filename="foo/bar">.

jcarpent commented 2 years ago

@fabinsch COuld you handle it?

stephane-caron commented 2 years ago

I don't have a Pinocchio build environment to test it right now, but the fix may be simple.

In:

          if ( bf::exists( bf::path(package_dirs[i] + "/" + string.substr(2))))
          {
            result_path = std::string( package_dirs[i] + "/" + string.substr(2));
            break;
          }

Can the calls to substr(2) be removed? Substrings like "/./" (or "/./.././../") shouldn't hurt the path.

Also note that a URDF may do <mesh filename="../foo/bar">.

fabinsch commented 2 years ago

Hey @stephane-caron , thanks for raising the issue. You are right, and the solution that you're proposing works. I created a PR #1748 which should fix your loading issues in https://github.com/stephane-caron/robot_descriptions.py/issues/6.

jcarpent commented 2 years ago

@stephane-caron I have merged #1748. Could you check if it is now working well on your side?

stephane-caron commented 1 year ago

Could you check if it is now working well on your side?

Sorry this one went out of my radar. I've opened a follow up at https://github.com/stack-of-tasks/pinocchio/issues/1786