Gazebo updates (extensive) #518

gavanderhoorn · 2020-06-24T13:47:46Z

This is a draft PR (for now) to collect ongoing work towards updating the ur_gazebo package.

High-level goals:

isolate Gazebo-specific infrastructure as much as possible so it does not 'leak' into any of the other packages
treat Gazebo (and the simulated robot) as much as possible as just-another-robot, to allow (conceptual) reuse of workflows, .launch and configuration files from other packages

See the commit comments for rationales.

Mainly tested against Gazebo 9.x.

This is -- and will be -- a rather substantial PR, but care has been taken to make sure commits make sense and reviews per-commit should be possible.

Usage is similar to ur_robot_driver: to start a UR10e with its kinematic calibration in Gazebo's empty world with a JointStateController and a position_controllers/JointTrajectoryController loaded and started:

roslaunch ur_gazebo ur10e_bringup.launch kinematics_config:=$(rospack find ur_calibration)/etc/ur10_example_calibration.yaml

gavanderhoorn · 2020-06-24T18:20:19Z

CI is failing as roslaunch in Kinetic does not have ros/ros_comm#1520.

I'm going to ignore the Kinetic failures, as we don't target it with this refactor.

I'll probably update CI to allow Kinetic builds to fail (but still try to not break things on Kinetic for no reason).

gavanderhoorn · 2020-06-25T07:09:53Z

@ipa-hsd: would you be willing to take a look at this?

Not just whether it works, but I'm also wondering whether the structure and distribution of files make sense to ppl.

And I'm curious how straightforward it would be to reuse the Gazebo description in other simulations. One thing that will be problematic is the 'hard-coded' gazebo_ros_control plugin. I'm thinking of making that configurable somehow, but I'm not sure I want to add yet another argument to the _macro.xacro.

gavanderhoorn · 2020-06-25T11:36:26Z

Added a few commits which improve the "public interface" of ur_gazebo:

load the URDF and start the RSP as part of the bringup .launch file (instead of the ros_control part): this should make it easier for users to load custom kinematics configurations, as well as joint limits and altered visual appearances (as the arguments are available directly at the level of the bringup .launch file)
hide all non-entry-point .launch files by giving them a .xml suffix: this prevents them from showing up in roslaunch tab-completion

Also use the dirname substitution arg to avoid crawling the ROS pkg path for local include files.

Edit: dirname is not available on Kinetic, so this increases breakage with Kinetic.

gavanderhoorn · 2020-06-25T11:38:25Z

@ipa-hsd: if you've already pulled, you may want to update, as these updates improve on some issues I encountered while trying to integrate the models provided by ur_gazebo with an existing simulation.

gavanderhoorn · 2020-06-25T11:42:43Z

@fmauch: I'd appreciate it if you could take a look as well.

In essence I've tried to mimic the way ur_robot_driver structures its .launch files (see the OP for rationale).

Related to UniversalRobots/Universal_Robots_ROS_Driver#97 (comment): which parameters/arguments should we/do we want to expose?

gavanderhoorn · 2020-06-25T12:22:47Z

I'm re-arranging some files again, so a proper review would not make sense yet @ipa-hsd and @fmauch (but then this is also still just a draft).

gavanderhoorn · 2020-06-30T10:10:12Z

@ipa-hsd @fmauch: I'm pretty happy with the current state of this, so if you could take a look I'd appreciate it.

I will be adding one or two commits, but those will mostly add things which weren't there before, and will be largely orthogonal to the commits already here.

hsd-dev · 2020-06-30T10:13:36Z

@gavanderhoorn sorry about the delay. I went through the PR and the commits this morning. And it works as expected. For me, it is a go 👍

gavanderhoorn · 2020-06-30T11:41:28Z

In order for all of this to really make sense, we should really include a set of convenience _macro.xacros which provide default paths to the various per-variant .yaml files in ur_description/config. Right now even just including the default ur_robot_gazebo will require users to provide values for all 4 *_parameters_files parameters, which is rather annoying.

I won't do that in this PR though, as it's big enough, and requires changes in ur_description as well.

gavanderhoorn · 2020-06-30T11:41:55Z

@ipa-hsd wrote:

@gavanderhoorn sorry about the delay. I went through the PR and the commits this morning. And it works as expected. For me, it is a go +1

thanks @ipa-hsd.

I'll wait a bit for @fmauch's review and then I'll merge.

gavanderhoorn · 2020-06-30T12:50:57Z

And something to consider for a later PR: should ur_gazebo be switched to hardware_interface/EffortJointInterface?

Right now a hardware_interface/PositionJointInterface is used, but this has several disadvantages (interaction with objects in the rest of the simulation is really bad fi).

@ipa-hsd @ipa-nhg @miguelprada: thoughts?

Rename file to reflect name of macro or top-level entity. Include 'gazebo' reference as these files host content specific to Gazebo.

Only the ros_control elements are retained. The UR doesn't have a battery, so no need to include that plugin.

Align with filenames and other ROS-Industrial Gazebo support packages.

This is most likely a relic from the PR2 simulation.

So merge contents into 'ur_common.launch' and remove the file.

These mimic the files with the same names as in ur_description, but load the Gazebo model instead of the real robot onto the parameter server.

These files will serve the same purpose as those provided by a/the driver (but instead of a driver, they will launch Gazebo). Give them the same name to make them recognisable.

Top-level xacro should not default to any robot.

Use the same set of controllers for all robots, use the same names for controllers as used by `ur_robot_driver` and include the JointStateController in the same file. NOTE: this is all position control only (and thus open-loop or 'forward command' control (using ros_control vernacular)).

These launch files are not meant to be started directly, so 'hide' them roslaunch's auto-complete.

Use 'dirname' substitution arg to express relative include.

Over: - which world to load - whether to start Gazebo or not - from which parameter to (attempt to) load the urdf - what name to give to the spawned robot model (inside the simulation)

By lifting some of the nodes and parameters to the bringup files (from ur_common.launch.xml), they should be easier to use (and maintain). Anything specific to the use of a robot in a particular configuration (ie: a workcell or simulation) should exist at the bringup level, not in a common (ie: shared) file, as that will make it harder to change (as changes will be shared by many other setups). This restructuring makes ur_common.launch.xml redundant, so we also remove it.

Same rate as the real controller.

Prevent potential clashes with symbols outside of that scope.

gavanderhoorn · 2020-07-01T08:42:20Z

@fmauch: I've just force-pushed the latest version (rebased on-top of #520).

Would you perhaps have time to take a look today or tomorrow?

I'm mostly interested to know whether someone with a fresh eye can make sense of the structure of ur_gazebo, or whether it's completely illogical.

Note: I will add a more extensive readme to the package to explain which files should be used for which purpose and in which case (ie: top-level xacro should not be used in case a UR is to be integrated into a larger scene, etc).

So include that in the name.

fmauch · 2020-07-01T09:24:19Z

Would you perhaps have time to take a look today or tomorrow?

I'll try to fit it in this afternoon / evening.

ipa-nhg

Tested on:

Ubuntu 18.04
Rosdistro: Melodic
Gazebo version: 9.0.0

I started the launch for the simulation and tested the controllers using the rqt_joint_trajectory_controller plugin, it works as expected.

General review: I checked the filesystem structure and it makes completely sense to me.

@gavanderhoorn Thanks for the contribution

gavanderhoorn · 2020-07-01T11:50:52Z

Thanks for the review @ipa-nhg 👍

Would you have any thoughts on this:

And something to consider for a later PR: should ur_gazebo be switched to hardware_interface/EffortJointInterface?

Right now a hardware_interface/PositionJointInterface is used, but this has several disadvantages (interaction with objects in the rest of the simulation is really bad fi).

Perhaps I should post a new issue about it, so as to not discuss this in the comments under a PR review.

Edit: I've posted #521 for the discussion about the default hardware_interface.

ipa-nhg · 2020-07-01T12:00:56Z

@gavanderhoorn As you suggest, we should open a new issue for the controllers type that is a perfect candidate to be solved during the WRID20. In principle, I am agree with the change but I want to evaluate and test it separately.

fmauch

Also looks really clean to me. I only had a couple of small suggestions, but nothing critical.

Tested against gazebo 11 using ros-melodic and it seems to work finr.

ur_gazebo/launch/inc/load_ur.launch.xml

ur_gazebo/launch/inc/ur_control.launch.xml

For some reason this was not yet documented.

And we've broken Kinetic-compatibility quite a few commits ago.

So explain this in the arg's documentation.

gavanderhoorn · 2020-07-01T16:09:35Z

Ok.

Given the nr of reviews, the green CI and the state of this I'm going to merge the PR.

It's not perfect yet, but I believe it's a good set of changes which gets us where we want to go with ur_gazebo.

Further improvements and fixes can be done in follow-up PRs.

gavanderhoorn · 2020-07-01T16:13:23Z

Thanks for the reviews @ipa-hsd @ipa-nhg and @fmauch 👍

gavanderhoorn added this to the melodic-devel-staging milestone Jun 24, 2020

gavanderhoorn force-pushed the gazebo_updates branch from 7c681be to 759803e Compare June 24, 2020 19:53

gavanderhoorn mentioned this pull request Jun 30, 2020

Consider renaming '*_traj_controller' to *_joint_traj_controller' UniversalRobots/Universal_Robots_ROS_Driver#213

Closed

gavanderhoorn force-pushed the gazebo_updates branch from 6406258 to abf438d Compare June 30, 2020 10:08

gavanderhoorn marked this pull request as ready for review June 30, 2020 10:10

gavanderhoorn mentioned this pull request Jun 30, 2020

Load yaml files in read_model_data(..), nowhere else #520

Merged

gavanderhoorn changed the title ~~WIP: Gazebo updates~~ Gazebo updates (extensive) Jun 30, 2020

gavanderhoorn added 12 commits July 1, 2020 10:29

gazebo: follow description xacro macro naming.

92d1dd7

Rename file to reflect name of macro or top-level entity. Include 'gazebo' reference as these files host content specific to Gazebo.

gazebo: use new filenames.

3157374

gazebo: merge relevant content from common.gazebo.xacro into macro.

8222d6b

Only the ros_control elements are retained. The UR doesn't have a battery, so no need to include that plugin.

gazebo: use new robot and macro name.

3147a01

Align with filenames and other ROS-Industrial Gazebo support packages.

gazebo: formatting and comments of xacros.

a74b7ca

gazebo: 2-spaces per indent level.

a0477c2

gazebo: nothing to calibrate.

e8f4929

This is most likely a relic from the PR2 simulation.

gazebo: controller_utils isn't re-used anywhere else.

907058f

So merge contents into 'ur_common.launch' and remove the file.

gazebo: introduce 'load_*.launch' helpers.

3bdad68

These mimic the files with the same names as in ur_description, but load the Gazebo model instead of the real robot onto the parameter server.

gazebo: use 'bringup' launch file name.

aac8d2d

These files will serve the same purpose as those provided by a/the driver (but instead of a driver, they will launch Gazebo). Give them the same name to make them recognisable.

gazebo: don't default to UR3 parameter files.

eb7b1f3

Top-level xacro should not default to any robot.

gavanderhoorn added 12 commits July 1, 2020 10:34

gazebo: hide non-entry-point launch files.

5255796

These launch files are not meant to be started directly, so 'hide' them roslaunch's auto-complete.

gazebo: update paths to included launch files.

ab94498

Use 'dirname' substitution arg to express relative include.

gazebo: hide loaders as well.

5a8add1

gazebo: include load launch file with relative include.

225b772

gazebo: update tests to load non-entry-point loaders.

b48781e

gazebo: rostopic is no longer used.

a08b565

gazebo: ur_control: allow more control over launch args.

17235da

Over: - which world to load - whether to start Gazebo or not - from which parameter to (attempt to) load the urdf - what name to give to the spawned robot model (inside the simulation)

gazebo: give e-Series variants their own controller configs.

4ebb070

gazebo: run e-Series controllers at 500 Hz.

8732d0f

Same rate as the real controller.

gazebo: update comments in xacros.

36b2eae

gazebo: move include to scope of ur_robot_gazebo macro.

3f1f034

Prevent potential clashes with symbols outside of that scope.

gavanderhoorn force-pushed the gazebo_updates branch from 5062f4f to 3f1f034 Compare July 1, 2020 08:40

gazebo: clarify we load a position joint trajectory controller.

ea0b900

So include that in the name.

ipa-nhg approved these changes Jul 1, 2020

View reviewed changes

gavanderhoorn mentioned this pull request Jul 1, 2020

Change ur_gazebo to EffortJointInterface [discussion] #521

Closed

gavanderhoorn mentioned this pull request Jul 1, 2020

How to use position_controllers/JointTrajectoryController when simulate UR in gazebo #492

Closed

fmauch reviewed Jul 1, 2020

View reviewed changes

gavanderhoorn added 3 commits July 1, 2020 16:56

gazebo: list valid values for 'transmission_hw_interface'.

61a1aed

For some reason this was not yet documented.

gazebo: in-order is the default on Melodic.

be7d392

And we've broken Kinetic-compatibility quite a few commits ago.

gazebo: values less than 0.1 for spawn_z will be problematic.

467042b

So explain this in the arg's documentation.

gavanderhoorn merged commit 8e5cc30 into ros-industrial:melodic-devel-staging Jul 1, 2020

gavanderhoorn deleted the gazebo_updates branch July 1, 2020 16:13

gavanderhoorn mentioned this pull request Jul 1, 2020

Breaking changes in Melodic release #448

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Gazebo updates (extensive) #518

Gazebo updates (extensive) #518

gavanderhoorn commented Jun 24, 2020 •

edited

Loading

gavanderhoorn commented Jun 24, 2020

gavanderhoorn commented Jun 25, 2020

gavanderhoorn commented Jun 25, 2020 •

edited

Loading

gavanderhoorn commented Jun 25, 2020

gavanderhoorn commented Jun 25, 2020

gavanderhoorn commented Jun 25, 2020

gavanderhoorn commented Jun 30, 2020

hsd-dev commented Jun 30, 2020

gavanderhoorn commented Jun 30, 2020

gavanderhoorn commented Jun 30, 2020

gavanderhoorn commented Jun 30, 2020

gavanderhoorn commented Jul 1, 2020

fmauch commented Jul 1, 2020

ipa-nhg left a comment •

edited

Loading

gavanderhoorn commented Jul 1, 2020 •

edited

Loading

ipa-nhg commented Jul 1, 2020

fmauch left a comment

gavanderhoorn commented Jul 1, 2020

gavanderhoorn commented Jul 1, 2020

Gazebo updates (extensive) #518

Gazebo updates (extensive) #518

Conversation

gavanderhoorn commented Jun 24, 2020 • edited Loading

gavanderhoorn commented Jun 24, 2020

gavanderhoorn commented Jun 25, 2020

gavanderhoorn commented Jun 25, 2020 • edited Loading

gavanderhoorn commented Jun 25, 2020

gavanderhoorn commented Jun 25, 2020

gavanderhoorn commented Jun 25, 2020

gavanderhoorn commented Jun 30, 2020

hsd-dev commented Jun 30, 2020

gavanderhoorn commented Jun 30, 2020

gavanderhoorn commented Jun 30, 2020

gavanderhoorn commented Jun 30, 2020

gavanderhoorn commented Jul 1, 2020

fmauch commented Jul 1, 2020

ipa-nhg left a comment • edited Loading

Choose a reason for hiding this comment

gavanderhoorn commented Jul 1, 2020 • edited Loading

ipa-nhg commented Jul 1, 2020

fmauch left a comment

Choose a reason for hiding this comment

gavanderhoorn commented Jul 1, 2020

gavanderhoorn commented Jul 1, 2020

gavanderhoorn commented Jun 24, 2020 •

edited

Loading

gavanderhoorn commented Jun 25, 2020 •

edited

Loading

ipa-nhg left a comment •

edited

Loading

gavanderhoorn commented Jul 1, 2020 •

edited

Loading