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?

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.

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

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?

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.

@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.

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.

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?

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

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

@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.

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

@fmauch: the last few commits you've pushed are all attributed to an Ubuntu <[email protected]> user, instead of your own account.

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

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.

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.

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.