Custom robot model loader does not respect joint_limits.yaml files

[jarvisschultz]

Many tools in MoveIt! use a RobotModelLoader create a moveit::core::RobotModel. In bio_ik a custom helper function is used. Unfortunately, this custom loader does not look at a joint_limits.yaml file (like the RobotModelLoader does). What this means is that if you have a URDF that defines one set of joint limits, and you use joint_limits.yaml file to set a different set of joint limits for the purposes of planning (a setup that is commonly used in MoveIt, including packages generated with the setup assistant). Then, when using bio_ik it's possible to get IK solutions (for example from robot_state::RobotState::setFromIK) that are outside of the motion planning limits defined by the joint_limits.yaml file, but inside of the limits defined in the URDF.

[v4hn]

which version of MoveIt do you use?

The helper function you mention should not actually be called anymore because MoveIt prefers an initializer that passes the robot model from MoveIt directly. So yes, there is (at least one) issue with this code, but it also should not matter and the code could be deleted from what I can see.

[jarvisschultz]

I'm using ROS Kinetic and 0.9.18-1xenial-20210503-175051-0800 version of ros-kinetic-moveit. On this version, I believe the KinematicsPluginLoader uses a plugin initialize method that reads the robot_description parameter. See relevant source code.

I can confirm on my version of moveit, with bio_ik it's possible to find IK solutions "between" the joint limits set by the joint_limits.yaml file and the limits set in the URDF. Updating bio_ik source code to use the RobotModelLoader instead of the custom helper function fixes this issue.

Your point does seem correct that for newer versions of moveit (melodic for example) a different version of initialize is used that would likely avoid this problem.

[v4hn]

This repository has a kinetic-support git tag which is the best support for ROS kinetic we had.
I'm surprised master apparently still compiles on kinetic for you.
We do not maintain branches for better support with old distributions.
Feel free to create a pull request, but I would personally prefer a pull-request that removes this broken loader altogether over adding more code that nobody will use anymore with recent software releases.

Other group members might disagree.

[jarvisschultz]

FWIW, I'm not using the master branch. As you guessed, that won't compile. I'm using the kinetic-support tag.

In my opinion, the custom loader is incorrect regardless of which version of moveit is being used. As a result, I think you're completely correct in asserting that the ideal PR would remove that loader.