Update MoveIt! support

[fmauch]

MoveIt! support has not been migrated to the new description structure. So, this PR aims at implementing #430.

As raised in #448 (comment) this is a first draft version find out what the desired state should be.

Basically, I recreated the package from scratch using the MSA and replaced the files in the existing package.

I removed all gazebo references and straightened a couple of minor issues.

Things I would like to specifically discuss about:

• Our current xacro file generates a robot named "ur". Therefore, the moveit_config also contains files for ur_<...> instead of ur16e_<...> which I adapted manually in most cases. It might be a good idea to add the actual robot name as a parameter to ur.xacro
• How should we handle the default expected controller? In the old configuration the controller name was set to "" effectively waiting for an action server coming up at /follow_joint_trajectory. This raised a lot of issues in the ur_robot_driver about people expecting this to work together out of the box. I left this at manipulator for now as a reminder that we should resolve this. In my local setup I set it to scaled_pos_joint_traj_controller in order to work with the ur_robot_driver out of the box.
• The file ur10_e_moveit_planning_execution.launch was not created by the MSA. I don't know whether I did something wrong or whether this has changed nowadays.

[Victorlouisdg]

Hi, I was wondering how you generate the MoveIt config files, do you have to manually walk through the MSA GUI for each UR robot? Is there a fixed procedure? Also, what is the .setup_assistant file used for?

[fmauch]

Hi, I was wondering how you generate the MoveIt config files, do you have to manually walk through the MSA GUI for each UR robot? Is there a fixed procedure? Also, what is the .setup_assistant file used for?

I currently created the files using the MSA GUI and after that did some manual modifications as mentioned above. From my point of view, all other moveit configurations could be generated by this one using search&replace mechanisms once we consented on a final version of this package. @gavanderhoorn can probably say more about this.

[fmauch]

@gavanderhoorn Any comments on this, so I can push this forward?

[lianghongzhuo]

The urdf files for the robot are probably missing in the melodic-devel-staging branch. As a result, the moveit config you edited here cannot find a robot description file. Any plan to push this branch forward?

[fmauch]

The urdf files for the robot are probably missing in the melodic-devel-staging branch. As a result, the moveit config you edited here cannot find a robot description file. Any plan to push this branch forward?

If you are referring to the failing tests this is because of the tests not being updated. This is also related to the third of my questions above.

[ipa-nhg]

@fmauch Thanks a lot for this contribution, I made a quick review of the changes and it looks good to me! 👍

A test on a the real hardware is pending!

Some answers/opinions to your questions:

• Our current xacro file generates a robot named "ur". Therefore, the moveit_config also contains files for ur_<...> instead of ur16e_<...> which I adapted manually in most cases. It might be a good idea to add the actual robot name as a parameter to ur.xacro

I am agree, we should update the ur.xacro to get the robot "name" as parameter. But we need a consensus here, @gavanderhoorn what do you think?

• How should we handle the default expected controller? In the old configuration the controller name was set to "" effectively waiting for an action server coming up at /follow_joint_trajectory. This raised a lot of issues in the ur_robot_driver about people expecting this to work together out of the box. I left this at manipulator for now as a reminder that we should resolve this. In my local setup I set it to scaled_pos_joint_traj_controller in order to work with the ur_robot_driver out of the box.

I think for the simulation the follow_joint_trajectory action is mapped to /arm_controller/follow_joint_trajectory . I am not sure if there is a clean way to set the namespace as argument of a top launch file, then should be ok if we keep the name empty.

• The file ur10_e_moveit_planning_execution.launch was not created by the MSA. I don't know whether I did something wrong or whether this has changed nowadays.

If you check the ur10_e_moveit_planning_execution.launch it just include the move_group.launch and the file is only used by the test, then I vote for modifying the test file to include directly move_group. NOTE: For the simulation case the remap of the follow_joint_trajectory controller is required.

[fmauch]

I am agree, we should update the ur.xacro to get the robot "name" as parameter. But we need a consensus here, @gavanderhoorn what do you think?

As @gavanderhoorn mentioned in #556 (comment) he already prepared a commit for this in gavanderhoorn@4187b12.

[fmauch]

I've updated the PR building ontop of the changes from @gavanderhoorn mentioned above. It might be cleaner to merge the description changes in a separate PR first and then rebase this ontop of that. (Edit: Especially, since those changes break other packages like ur_gazebo)

If no objections rise, I can make this a full PR asap, so we can start the real review process (although other robot models will be pretty much copy&paste so reviewing one robot model might be simpler. What do you think @ipa-nhg?

[ipa-nhg]

I am agree, we have to merge first the description changes.

You can go ahead with the rest of the robots. I made today a quick test on simulation and the planner worked as expected 👍🏽

Test commands:

roslaunch ur_gazebo ur10_bringup.launch
roslaunch ur10_moveit_config move_group.launch

Then using the MoveIt! RVIZ plugin I gave different goal positions, also adding a ground and some boxes as collisions, and the planner was able to find and execute trajectories without problems.

I made a couple of minor changes to make this version working on simulation: https://github.com/fmauch/universal_robot/compare/moveit...ipa-nhg:moveit_sim_test?expand=1

[fmauch]

Thanks for testing with gazebo. I did a test with URSim and the ur_robot_driver and it also worked as expected.

[fmauch]

@ipa-nhg Concerning the changes you needed to make to move_group.launch it might be beneficial to indeed keep the ur10_e_moveit_planning_execution.launch file, so it can be used with both, a real robot and a gazebo robot.

[fmauch]

I went ahead and finished the moveit configurations for all robots. The branch will be cleaned up once #562 is merged.

A couple of remarks to decisions I took on the way:

• I set the default controller of ur_robot_driver into the default controller for the moveit configurations to enable users an easy start with the real robot.
• I've renamed the e-series packages so that they match to the notation we use everywhere else (ur10e instead of ur10_e)

For the record: All packages are copied from the ur10_moveit_config package using the following commands inside the copied package:

export ROBOT=ur10e # just as an example
find . -type f -print0 | xargs -0 sed -i -e "s/ur10/${ROBOT}/g"
mv ./config/ur10.srdf ./config/${ROBOT}.srdf && \
  mv ./launch/ur10_moveit_controller_manager.launch.xml ./launch/${ROBOT}_moveit_controller_manager.launch.xml && \
  mv ./launch/ur10_robot_moveit_sensor_manager.launch.xml ./launch/${ROBOT}_robot_moveit_sensor_manager.launch.xml && \
  mv ./launch/ur10_robot_moveit_controller_manager.launch.xml ./launch/${ROBOT}_robot_moveit_controller_manager.launch.xml && \
  mv ./launch/ur10_moveit_planning_execution.launch ./launch/${ROBOT}_moveit_planning_execution.launch && \
  mv ./launch/ur10_moveit_sensor_manager.launch.xml ./launch/${ROBOT}_moveit_sensor_manager.launch.xml

As of writing this: The joint limits should probably be changed inside the individual packages, so that remains a ToDo.

Edit: Updated joint limits in a4237e8

[RobertWilbrandt]

Why does this not just include the corresponding load_*.launchfile?

[gavanderhoorn]

Because that would take this file further away from what is generated by the MSA, which would increase maintenance effort.

See ros-industrial/fanuc#170 (and ros-industrial/fanuc#208 for the replacement) for a PR which first tried to do what you suggest here (for a different OEM / in a different repository, but I had the same idea).

[gavanderhoorn]

@fmauch: the last few commits you've pushed are all attributed to an Ubuntu <ubuntu@rosmelodic.lxd> user, instead of your own account.

[fmauch]

Should we address #431 also in this PR? @gavanderhoorn @RobertWilbrandt opinions?

[RobertWilbrandt]

Should we address #431 also in this PR? @gavanderhoorn @RobertWilbrandt opinions?

I would prefer to integrate this here, merging completely reworked moveit configs that don't follow this standard doesn't seem consistent to me.

[fmauch]

I've tested this PR on real hardware. For using it with gazebo, @RobertWilbrandt is currently working on a fix of the gazebo xacro files.

As I am the author of this PR, it would be good if another maintainer could review this PR before merging.

[RobertWilbrandt]

I think this is now at a stage that it can get merged:

• The demo.launch files work for each robot type
• Planning and executing trajectories using ur_gazebo works with each robot type (i got PATH_TOLERANCE_VIOLATED errors for ur16e when planning wtih full velocity, but i don't think this is caused by this PR)
• catkin_lint -W2 universal_robot only shows 3 warnings, and those are in ur_kinematics and not related to this PR
• rosdep has no problems with unknown dependencies
• I only found the one remaining copy-paste error while going through the commits (and checking for wrong robot types in each config repo)
• I tested the version on a ur3e, ur5e and a ur10. I could plan and execute trajectories without problems.

I will aprove this after this last suggestion is fixed.

[fmauch]

Merging, as the failed testing build on noetic seems to be a buildfarm glitch. It complains about a missing ros-noetic-moveit-ros-visualization package, which is indeed not part of the package list atm. as ros-noetic-moveit-ros, ros-noetic-moveit-ros-planning-interface and ros-noetic-moveit-ros-visualization are missing, I suspect, this is due to failing builds of ros-noetic-moveit-ros-planning-interface.