Fail to parse URDF from file/string

[fjulian]

Hello, I am trying to get a KDL tree from a URDF file. For this, I am calling the function kdl_parser_py.urdf.treeFromFile. Unfortunately, when calling this, I get the error:

Traceback (most recent call last):
  File "/opt/ros/noetic/lib/python3/dist-packages/kdl_parser_py/urdf.py", line 14, in treeFromFile
    return treeFromUrdfModel(urdf.URDF.from_xml_string(urdf_file.read()))
  File "/opt/ros/noetic/lib/python3/dist-packages/kdl_parser_py/urdf.py", line 122, in treeFromUrdfModel
    if not _add_children_to_tree(robot_model, robot_model.link_map[child], tree):
  File "/opt/ros/noetic/lib/python3/dist-packages/kdl_parser_py/urdf.py", line 84, in _add_children_to_tree
    _toKdlJoint(parent_joint),
  File "/opt/ros/noetic/lib/python3/dist-packages/kdl_parser_py/urdf.py", line 67, in _toKdlJoint
    return type_map[jnt.type](jnt, _toKdlPose(jnt.origin))
  File "/opt/ros/noetic/lib/python3/dist-packages/kdl_parser_py/urdf.py", line 53, in <lambda>
    fixed = lambda j,F: kdl.Joint(j.name, getattr(kdl.Joint, 'None'))
AttributeError: type object 'PyKDL.Joint' has no attribute 'None'

Indeed, when just checking the PyKDL.Joint object, it turns out that it has no 'None' attribute. Does someone here know how to fix this?

I am running ROS Noetic and installed the packages ros-noetic-kdl-parser (version 1.14.1-1focal.20200825.225101) and ros-noetic-kdl-parser-py (version 1.14.1-1focal.20200825.225208) using apt. I saw that in this repo that the critical line, File "/opt/ros/noetic/lib/python3/dist-packages/kdl_parser_py/urdf.py", line 53, is the same in the version that I installed through apt and on the master branch here.

Thanks in advance for any help.

[clalancette]

Hm, it is hard to say. I'm not exactly sure what is meant to be found there.

Is there any chance you can share your URDF file? That would help us track it down.

[fjulian]

Thanks for your swift response. In the mean time, I managed to solve the issue by changing this line to

fixed = lambda j,F: kdl.Joint(j.name, kdl.Joint.Fixed)

Shall I open a PR with this fix?

[clalancette]

Shall I open a PR with this fix?

It's really unclear to me. The original code from when kdl_parser_py was first developed is this:

    fixed = lambda j,F: kdl.Joint(j.name, kdl.Joint.None)

Back in 2018, for Python 3 compatibility, we changed it to this:

    fixed = lambda j,F: kdl.Joint(j.name, getattr(kdl.Joint, 'None'))

But now you are proposing changing it to a completely different attribute. So I'm honestly not sure which is the right one.

[fjulian]

In the documentation of the C++ version of KDL, which PyKDL is a wrapper of, it says that kdl.Joint has both attributes None and Fixed and it seems that they are synonym to each other.

I just tried with

fixed = lambda j,F: kdl.Joint(j.name, kdl.Joint.None)

and it says

Traceback (most recent call last):
  File "<frozen importlib._bootstrap>", line 991, in _find_and_load
  File "<frozen importlib._bootstrap>", line 975, in _find_and_load_unlocked
  File "<frozen importlib._bootstrap>", line 671, in _load_unlocked
  File "<frozen importlib._bootstrap_external>", line 783, in exec_module
  File "<frozen importlib._bootstrap>", line 219, in _call_with_frames_removed
  File "<some path>/sim/robot_arm.py", line 14, in <module>
    from kdl_parser_py.urdf import treeFromFile
  File "<some path>/kdl_parser_py/kdl_parser_py/urdf.py", line 53
    fixed = lambda j,F: kdl.Joint(j.name, kdl.Joint.None)
                                                    ^
SyntaxError: invalid syntax

So maybe python3 doesn't allow having None as an attribute, but using Fixed, the synonym, works fine. Does this make sense?

[clalancette]

In the documentation of the C++ version of KDL, which PyKDL is a wrapper of, it says that kdl.Joint has both attributes None and Fixed and it seems that they are synonym to each other.

Thanks for looking that up.

So maybe python3 doesn't allow having None as an attribute, but using Fixed, the synonym, works fine. Does this make sense?

Yeah, I do agree that Python 3 doesn't like kdl.Joint.None. But that was the reason we changed it to getattr(kdl.Joint, 'None'). I'm not opposed to changing it to Fixed if that makes sense, but I'm wondering why the existing code didn't work for you.

Maybe for maximum compatibility the thing to do here is:

  fixed = lambda j,F: kdl.Joint(j.name, getattr(kdl.Joint, 'Fixed') if hasattr(kdl.Joint, 'Fixed') else getattr(kdl.Joint, 'None'))

This will fail if a Joint has both Fixed and None, and they are different values, but I have no idea how common that is. What do you think?

[fjulian]

This will fail if a Joint has both Fixed and None, and they are different values

Yes then it would just use Fixed and ignore None, right? I also don't know if it can happen that they have different values. Although in the C++ documentation line 47, it is really just hard-coded that they are synonym, so maybe it is safe to rely on that.

[clalancette]

Yes then it would just use Fixed and ignore None, right?

Yeah, exactly. We are essentially giving "precedence" to Fixed.

Looking a bit deeper, I think I see what happened. https://github.com/orocos/orocos_kinematics_dynamics/commit/d518c8b86e2be19fc1389fda91c9f566c21c0b8c removed the None joint from Python 3, so that's probably what is happening to you. I will say that it looks like Ubuntu Focal ships with Orocos 1.4.0, and this commit is after that. So the only way your original problem could happen is if you are using a custom-built Orocos. Is that the case?

In any case, I think it is safe to go with my previous suggestion. That is, we'll use the Fixed joint if it is available, and fallback to None if it isn't. This is probably overkill, but I'll feel a little better using that solution.

[fjulian]

Is that the case?

Yes it is, I just built Orocos from the latest commit on the master branch.

I think it is safe to go with my previous suggestion.

Sounds good to me. Thanks for looking into this!

[fjulian]

Am I opening the PR or are you?

[clalancette]

Am I opening the PR or are you?

I'd appreciate it if you could do it, thanks.