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

Should urdf_parser_py be in a different repo? #70

Closed
jacquelinekay opened this issue Dec 9, 2015 · 12 comments
Closed

Should urdf_parser_py be in a different repo? #70

jacquelinekay opened this issue Dec 9, 2015 · 12 comments
Assignees

Comments

@jacquelinekay
Copy link
Contributor

Why is urdf_parser_py in this repository?

It contains ROS-specific code, e.g. from_parameter_server, which imports rospy when called:

def from_parameter_server(cls, key = 'robot_description'):

It is released in a separate ROS package (ros-indigo-urdf-parser-py).

It seems to me that it should either be moved into robot_model, OR from_parameter_server should be removed and it should be released as a python package (to be installed using pip, for example).

@isucan, @eacousineau, etc. what are your thoughts?

@isucan
Copy link
Contributor

isucan commented Dec 9, 2015

+1 for separate repo

@isucan isucan closed this as completed Dec 9, 2015
@isucan isucan reopened this Dec 9, 2015
@jacquelinekay
Copy link
Contributor Author

should I move it to robot_model or make a new repo entirely for urdf_parser_py?

@isucan
Copy link
Contributor

isucan commented Dec 9, 2015

I would move it to a new urdf_parser_py

@traversaro
Copy link
Contributor

Distributing urdf_parser_py only as a ROS (Catkin) package would make much more difficult to use urdf_parser_py in Windows.
To clarify our use case: mechanical people use Windows (because most CAD modeling software is available on Windows), and then they export their CAD model to URDF using https://github.com/robotology/simmechanics-to-urdf (that clearly does not use from_parameter_server).
Unfortunately it appears that the from_parameter_server method is quite used, so moving urdf_parser_py to a ROS package is the right thing to do.
I would probably need to maintain a fork of urdf_parser_py that does not use Catkin, and keep it in synch with the urdf_parser_py ROS package.
cc @fiorisi

@jacquelinekay
Copy link
Contributor Author

@traversaro I'll keep cross-platform compatibility in mind when I do this.

@jacquelinekay jacquelinekay self-assigned this Jan 29, 2016
@traversaro
Copy link
Contributor

@jacquelinekay I noticed that catkin is also distributed through pip, so perhaps it is not so traumatic to install it on Windows.

@jacquelinekay
Copy link
Contributor Author

Windows development is usually traumatic for me :P

@jacquelinekay
Copy link
Contributor Author

With regards to your concerns, Silvio, my vision is for urdf_parser_py to be cross-platform and ROS-free, and to have a separate catkin package in the robot_model repo to wrap the module with ROS-specific utilities, like communicating with the parameter server. I'm going to start by just separating it out into a different repo (and possibly doing a release).

I confirmed with Ioan that urdf_parser_py is only released as a ROS package right now, but it sounds like your team does your own packaging to use it on Windows, is that correct?

@traversaro
Copy link
Contributor

Yes, we use it by installing through the (pure, non-catkin) CMake code in this repo.

Thanks, what you just proposed is ideal.

@traversaro
Copy link
Contributor

For the sake of precision: I actually noticed that we install the urdf_parser_py script using the classical python setup.py install, see https://github.com/robotology/simmechanics-to-urdf#install-dependencies-2 .

@jacquelinekay
Copy link
Contributor Author

#80
https://github.com/ros/urdf_parser_py
ros/urdf_parser_py#1

todo:

  • Set up Travis for new repo
  • properly set up ROS Wiki pages for respective packages

@jacquelinekay
Copy link
Contributor Author

As a side note, why is the ROS deb called urdfdom-py while the package is called urdf_parser_py in github and in the setup.py? I prefer the name urdf_parser_py myself, but I guess it's more important not to disrupt the configuration of many users by change the name of the debian.

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