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

industrial_utils: using 'default joint names' when parameter cannot be found is a bad idea #180

Closed
gavanderhoorn opened this issue Oct 2, 2017 · 4 comments
Labels

Comments

@gavanderhoorn
Copy link
Member

In industrial_utils/param_utils::getJointNames(..):

// 3) Use default joint-names
const int NUM_JOINTS = 6; //Most robots have 6 joints
for (int i=0; i<NUM_JOINTS; ++i)
{
std::stringstream tmp;
tmp << "joint_" << i+1;
joint_names.push_back(tmp.str());
}

If either controller_joint_names cannot be found, or the urdf cannot be parsed for joint names, getJointNames(..) returns a set of 'default names'.

This is not a good behaviour: the function should fail if it cannot find any joint names. Making some names up (even though they follow ros-i conventions) and then returning successfully makes for poor reproducibility and complicates debugging.

@Ridhwanluthra
Copy link
Contributor

hi, is this as straightforward as raising as error and returning?

@gavanderhoorn
Copy link
Member Author

@Ridhwanluthra: yes, it should fail:

This is not a good behaviour: the function should fail if it cannot find any joint names.

a suitable message printed using ROS_ERROR(..) and returning false would be good.

@Ridhwanluthra
Copy link
Contributor

cool, fixing this as a part of #wrid18

@Levi-Armstrong
Copy link
Member

PR #195 has been merged closing.

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

No branches or pull requests

3 participants