Fetch use lower limit

[tonypan1995]

The joint limits file was not correctly loaded because moveit does not load it when urdf and srdf is available. Now we check and load the joint limits correctly when a file is provided.

[ChamzasKonstantinos]

Ok i Looked into this and it seems as you correctly pointed out, that the joint_limits file is completely ignored!, so definitely a good catch that needs fixing.

I have the following suggestions:
A) The yaml loading code should be moved to yaml.cpp and use the encode/decode system to load the join_limits file. Basically to read the file you will use this generic templated argument function, which is much cleaner. For this function to automatically work you need to create the decode/encode functions for a JointLimitsMsg, like here. B) I would create an internal jointlimits member variable that is a JointLimitsMsg to eliminate the extra argument in initializeInternal(). Then to use it there are two options:

1. Directly set it to the param server, similar to here. and then load the (RobotModel)[http://docs.ros.org/en/noetic/api/moveit_ros_planning/html/robot__model__loader_8cpp_source.html]. From my understanding this would work, since line 105 reads from the param server when the urdf exists.
2. The other solution is to do it manually like now, with setJointVariableBounds but I would still set the param server afterwards just so the loaded params are available and consistent.

I am sorry if I am being annoying, I just think this is much cleaner and will make the code more readable/maintainable. This is definitely an important bug that needs fixing, so very good catch!

[tonypan1995]

Ok i Looked into this and it seems as you correctly pointed out, that the joint_limits file is completely ignored!, so definitely a good catch that needs fixing.

I have the following suggestions: A) The yaml loading code should be moved to yaml.cpp and use the encode/decode system to load the join_limits file. Basically to read the file you will use this generic templated argument function, which is much cleaner. For this function to automatically work you need to create the decode/encode functions for a JointLimitsMsg, like here. B) I would create an internal jointlimits member variable that is a JointLimitsMsg to eliminate the extra argument in initializeInternal(). Then to use it there are two options:

1. Directly set it to the param server, similar to here. and then load the (RobotModel)[http://docs.ros.org/en/noetic/api/moveit_ros_planning/html/robot__model__loader_8cpp_source.html]. From my understanding this would work, since line 105 reads from the param server when the urdf exists.
2. The other solution is to do it manually like now, with setJointVariableBounds but I would still set the param server afterwards just so the loaded params are available and consistent.

I am sorry if I am being annoying, I just think this is much cleaner and will make the code more readable/maintainable. This is definitely an important bug that needs fixing, so very good catch!

Don't worry! I think this is good suggestions and I'll try to address them later when I have time.

One thing to point out about loading the joint limits as params from the param server though, is that robowflex is already loading the joint limits file to the ros server. However, if urdf exists, that line 105 would not load robot description from ros server. So the problem is not that the params in the server are inconsistent, but that the joint limits in the joint_limits.yaml file and the urdf file are different. When URDF exists, MoveIt only reads joint limits from the urdf.

So in general when URDF exists, we need some post-processing after loading model using MoveIt, which loads the joint limits either from server or file (I did it from file as above because I faced with some bugs reading from server - I can look into this because as you suggested reading from the server is more consistent).