[ur_description] Allow custom robot

[galou]

The arguments were missing in ur10_macro.xacro and similars, as well as in load_ur.launch

Signed-off-by: Gaël Écorchard gael.ecorchard@cvut.cz

[gavanderhoorn]

I'm not sure I understand the PR.

Did you notice this in the convenience top-level .xacros:

This is a convenience top-level xacro which loads the macro for the UR10 which defines the default values for the various "parameters files" parameters for a UR10.

This file is only useful when loading a stand-alone, completely isolated robot with only default values for all parameters such as the kinematics, visual and physical parameters and joint limits.

This file is not intended to be integrated into a larger scene or other composite xacro.

Instead, xacro:include 'inc/ur10_macro.xacro' and override the defaults for the arguments to that macro.

[galou]

The file ur_robot_driver/ur10_bringup.launch in ec2beb65afd6b (i.e. current master of https://github.com/UniversalRobots/Universal_Robots_ROS_Driver.git) uses ur_description/load_ur10.launch. Previously, If a user wanted to launch a custom kinematics parameter file, he/she has to rewrite all launch files from ur_robot_driver because load_ur10.launch did not support customization.

[galou]

I saw the notice in the top-level xacro. It would need to be removed but I first wanted to discuss about the PR.

[gavanderhoorn]

To start a discussion, posting an issue might have been better.

AFAIU now, the problem is on the Universal_Robots_ROS_Driver side.

The files you change are top-level files, not meant to be parameterised. As the comment explains, they are only there for convenience.

We could consider changing something here, but I'd first want to ask @fmauch to comment on this, as he both maintains ur_robot_driver as well as submitted #562 and #588.

And to explain: the point of not allowing parameterisation everywhere is that it just gets too complex. Files including files including files including files, all the while passing args and params. That has to stop somewhere.

[galou]

This can be considered as an issue of ur_robot_driver as a command documented there does not work:

$ roslaunch ur_robot_driver <robot_type>_bringup.launch robot_ip:=192.168.56.101 \
  kinematics_config:=$(rospack find ur_calibration)/etc/ur10_example_calibration.yaml

This PR should solve the issue there.

[galou]

I agree with you that the argument passing is complicated (this is even exacerbated by the fact that the parameter names change, kinematics_params --> kinematics_config --> kinematics_parameters_file). With this PR, things should be easier for the user as he/she doesn't need to write his/her own xacro file (and launch file?) to get a custom robot description.

[fmauch]

I think that the macro files here should stay as they are, as their purpose is to abstract the parametrization away. I would also vote for fixing this inside the driver. It would probably be best to call the load_ur.launch file by default and passing the robot model.

---

Edit: This should already work reliable. I'll recheck...

---

Edit2: OK, now I understand. With #588 we effectively bypass the xacro specialization. An alternative implementation for fixing this could be https://github.com/fmauch/universal_robot/commit/446e962bdc6ca890ba613ae3ff8331642ddc72dd. With that I can launch ur5e_bringup.launch on a ur10e with passing the kinematics_config from a ur10e resulting in the driver not complaining about wrong kinematics and the TF tree being correct (though, obviously not the meshes, as we still load a ur5e description):

# remember, there's a ur10e simulation connected
roslaunch ur_robot_driver ur5e_bringup.launch robot_ip:=192.168.56.101 kinematics_config:=$(rospack find ur_description)/config/ur10e/default_kinematics.yaml

[danielcranston]

Bump.

FWIW I think these changes make a lot of sense. Intuitively you would expect to be able to run ur_robot_driver/ur5e_bringup.launch, specifying custom kinematics, and have them be applied to the URDF. This was the case up until #588 as mentioned by @fmauch.

So the way I see it #588 introduced a regression by bypassing the xacro specialization, and this PR resolves the issue and re-introduces the original behavior.

Further, even something as isolated as roslaunch ur_description load_ur10.launch kinematics_params:=my_kinematics.yaml will not apply the kinematics to the URDF. The underlying include of load_ur.launch passes the kinematics to ur10.xacro, but there the arg is completely ignored (because it's not exposed). I feel like there is no gain in restricting the individual xacro files as is currently being done. Exposing the xacro args (with suitable defaults, like this PR is doing) feels like a better approach here.

(EDIT: I think I prefer @fmauch's proposed "alternative implementation". That way the convenience urXXX.xacro files don't need to be changed, the changes are fewer, and they more directly revert the introduced regression)

[fmauch]

@galou @danielcranston Would you be fine with closing this in favor of #612?

[danielcranston]

Sounds good to me! Thanks for the ping

[fmauch]

Closing this as mentioned before.