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

urdf_parser_py Exception with new transmission specification #22

Closed
v4hn opened this issue Jan 10, 2014 · 9 comments
Closed

urdf_parser_py Exception with new transmission specification #22

v4hn opened this issue Jan 10, 2014 · 9 comments

Comments

@v4hn
Copy link

v4hn commented Jan 10, 2014

urdf_parser_py fails when processing a URDF containing a element
which adheres to the current specification given in
http://wiki.ros.org/urdf/XML/Transmission .
Because the old specification expected a "type" attribute,
which the new spec includes as a sub-element, the python parser throws "type"-is-missing Exceptions.
Also, the actuator definition became more complex and now includes the mechanicalReduction parameter.

As a temporary fix it's enough to comment the following in urdf.py over here:

xmlr.reflect(Transmission, tag = 'transmission', params = [
        name_attribute,
#       xmlr.Attribute('type', str),
#       xmlr.Element('joint', 'element_name'),
#       xmlr.Element('actuator', 'element_name'),
#       xmlr.Element('mechanicalReduction', float)
        ])

This works over here, because we never access the transmission definitions via urdfdom, but obviously it's just a hot fix, nothing more..

Because it's not possible to quickly overlay this ros-code in a uniform local workspace
(you just recently removed catkin support...),
it's pretty important to me to see this fixed in the official ros (ubuntu) packages soon

  • one way or the other.
@eacousineau
Copy link
Contributor

Good find! I'll add a check when it's parsing the transmission object specifically. I will spend an hour on it tomorrow.
Can you post the URDF you're parsing so I have it to work off of?

And for a manual overlay, maybe you can run CMake such that it installs to your local site packages, something like:

cd urdfdom
mkdir build
cd build
cmake .. -DCMAKE_INSTALL_PREFIX=~/.local
# Install all of urdfdom to local usr setup
make install

It looks like it might install it to dist-packages, though, so I can't speak to it working like that.

@v4hn
Copy link
Author

v4hn commented Jan 11, 2014

Thanks in advance!

We work with calvin[0].
I uploaded the fully generated urdf to [1].


[0] - http://wiki.ros.org/calvin_robot
[1] - http://files.v4hn.de/calvin_description.urdf

@eacousineau
Copy link
Contributor

Started work on it.
You can piggy back off of the devel or install-space for overlaying the install of the package, even though it does not use catkin:

cd urdfdom
mkdir build && cd build
cmake .. -DCMAKE_INSTALL_PREFIX=$(dirname $(catkin_find --first-only))
make install

(This assumes that catkin_find --first-only will return a directory in your devel-space)

Tested it out with the following command:

python -c 'import urdf_parser_py; print urdf_parser_py.__file__'

Should print out to the directory you specified.

@eacousineau
Copy link
Contributor

I've implemented the code, but it is not yet ready. I need to be test and debug it tomorrow: https://github.com/eacousineau/urdfdom/compare/feature;issue-22?expand=1
For the URDF you've posted, it looks like a straight dump from rosparam. Could you regenerate with the following comand?

python -c 'import rospy; open("/tmp/calvin.urdf", "w").write(rospy.get_param("robot_description"))'

Thanks!

@eacousineau
Copy link
Contributor

I apologize, I was unable to work on it today. I will address it either tomorrow or Tuesday.

@v4hn
Copy link
Author

v4hn commented Jan 14, 2014

I'm sorry for not answering in time.

At the moment I'm experiencing technical difficulties
much more severe than a failing python module (failing workstation).

I hope to get back to you soon!

@v4hn
Copy link
Author

v4hn commented Jan 17, 2014

I just uploaded the properly formatted patch to [0].
Thanks for your time.


[0] - http://v4hn.de/files/calvin_description.urdf

@eacousineau
Copy link
Contributor

Thank you! I finished up the tests and submitted the pull requests.

You can see the diff patch when it parses calvin and then spits it back out to a string: gen.patch

isucan added a commit that referenced this issue Jan 19, 2014
@isucan
Copy link
Contributor

isucan commented Jan 19, 2014

merged #23

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

No branches or pull requests

3 participants