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

Add Unit Tests for initialize() / Passing in robot model to initialize() is broken #71

Open
DavidB-CMU opened this issue Dec 22, 2015 · 3 comments
Labels

Comments

@DavidB-CMU
Copy link
Contributor

herb.py defines an initialize() function which allows you to optionally pass in the filename of a robot model.

However this is ignored, it looks like it got broken in this commit:
326eed5

@DavidB-CMU DavidB-CMU added the bug label Dec 22, 2015
@mkoval
Copy link
Member

mkoval commented Dec 24, 2015

I think we were right to remove the robot.xml parameter. We load the robot from an OpenRAVE XML file when the robot_xml parameter is present. If it is absent, we load it from .urdf and .srdf files.

I'd prefer to either: (1) remove the robot_xml parameter entirely or (2) replace the parameter with a robot parameter that defaults to None. If the parameter is set to an OpenRAVE robot, then initialize() uses the argument and skips loading the robot. That way the user can load the robot however he or she wishes (e.g. env.LoadRobotXML, using or_urdf).

Thoughts?

@psigen
Copy link
Member

psigen commented Dec 24, 2015

I recently faced a similar issue and ended up going the other way on this parameter, largely because:

(a) changing the robot.xml often requires setting other things that are not specified in the XML to match, such as overriding default link/topic names, meaning this has to be part of a larger code block anyway

(b) because we are moving away from robot.xml as a format: it's likely we'll want our resources to have good interoperability with DART, rviz and other tools, and the OpenRAVE XML formats currently aren't well supported there, in addition to having odd/undocumented transformation and parsing semantics at times.

@DavidB-CMU
Copy link
Contributor Author

Yes I can understand the difficulties of supporting both an xml and a urdf. Perhaps as HERBpy is for HERB, then it only needs to support one or the other.
I don't need this feature in HERBpy, but I just happened to notice it was broken.

I made a WAMpy package which is like PR2py in that it can only load an xml, but incorporates some newer code changes from HERBpy. So that's how I found this problem.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants