search

ros-industrial / industrial_core

ROS-Industrial core communication packages (http://wiki.ros.org/industrial_core)
154 stars 181 forks source link

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

Closed gavanderhoorn closed 5 years ago

gavanderhoorn commented 6 years ago

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

https://github.com/ros-industrial/industrial_core/blob/9a0fe17aa129fe1857dccf54043ab534379eb924/industrial_utils/src/param_utils.cpp#L122-L129

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 commented 6 years ago

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

gavanderhoorn commented 6 years ago

@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 commented 6 years ago

cool, fixing this as a part of #wrid18

Levi-Armstrong commented 5 years ago

PR #195 has been merged closing.