Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added gazebo launch file #936

Merged
merged 1 commit into from Aug 9, 2018

Conversation

Projects
None yet
5 participants
@MohmadAyman
Copy link
Member

commented Jun 3, 2018

Description

Added a gazebo launch file template for the setup assistant.
For the ur10 robot it looks like that.

Checklist

  • Required: Code is auto formatted using clang-format
  • Extended the tutorials / documentation, if necessary reference
  • Include a screenshot if changing a GUI
  • Optional: Created tests, which fail without this PR reference
  • Optional: Decide if this should be cherry-picked to other current ROS branches (Indigo, Jade, Kinetic)

@MohmadAyman MohmadAyman referenced this pull request Jun 4, 2018

Closed

Controllers Widget #933

@davetcoleman

This comment has been minimized.

Copy link
Member

commented Jun 8, 2018

Before we merge this can you create a new tutorial that explains how to launch this new file, complete with screenshots of the Franka Emika robot?

https://github.com/ros-planning/moveit_tutorials/

<!-- push robot_description to factory and spawn robot in gazebo -->
<node name="spawn_gazebo_model" pkg="gazebo_ros" type="spawn_model" args="-urdf -param robot_description -model robot -z 0.1" respawn="false" output="screen" />

<include file="$(find [GENERATED_PACKAGE_NAME])/launch/ros_controllers.launch"/>

This comment has been minimized.

Copy link
@jrgnicho

jrgnicho Jun 8, 2018

Contributor

Is the ros_controllers.launch file supposed to be generated automatically as well?

This comment has been minimized.

Copy link
@MohmadAyman

MohmadAyman Jun 9, 2018

Author Member

Yes, ros_controllers.launch can now be generated automatically by the setup assistant.

@MohmadAyman

This comment has been minimized.

Copy link
Member Author

commented Jun 9, 2018

@MohmadAyman

This comment has been minimized.

Copy link
Member Author

commented Jun 9, 2018

@MohmadAyman MohmadAyman referenced this pull request Jun 13, 2018

Merged

Read and write ROS controllers yaml file #951

2 of 4 tasks complete
file.file_name_ = "gazebo.launch";
file.rel_path_ = config_data_->appendPaths(launch_path, file.file_name_);
template_path = config_data_->appendPaths(template_launch_path, "gazebo.launch");
file.description_ = "gazebo launch file";

This comment has been minimized.

Copy link
@davetcoleman

davetcoleman Jun 15, 2018

Member

Please capitalize this description and explain in a sentence what it does, see the other examples in this file.

Also apply the same change to the ros_controllers.launch you added

@@ -0,0 +1,24 @@
<?xml version="1.0"?>
<launch>
<arg name="limited" default="false"/>

This comment has been minimized.

Copy link
@davetcoleman

davetcoleman Jun 15, 2018

Member

can you add a comment here about what limited does?

This comment has been minimized.

Copy link
@gavanderhoorn

gavanderhoorn Jun 15, 2018

Member

This sounds a lot like the limited arg that some of the universal_robot launch files have/use.

I don't believe it should be part of a template file introduced by this contribution.

This comment has been minimized.

Copy link
@MohmadAyman

MohmadAyman Jul 3, 2018

Author Member

I did remove it as @gavanderhoorn suggestion makes sense.

<!-- send robot urdf to param server -->
<param unless="$(arg limited)" name="robot_description" [URDF_LOAD_ATTRIBUTE] />

<param if="$(arg limited)" name="robot_description" [URDF_LOAD_ATTRIBUTE] />

This comment has been minimized.

Copy link
@davetcoleman

davetcoleman Jun 15, 2018

Member

these two lines do the exact same thing, so the limited variable should be removed or the URDF_LOAD_ATTRIBUTE should change for some reason

<param if="$(arg limited)" name="robot_description" [URDF_LOAD_ATTRIBUTE] />

<!-- push robot_description to factory and spawn robot in gazebo -->
<node name="spawn_gazebo_model" pkg="gazebo_ros" type="spawn_model" args="-urdf -param robot_description -model robot -z 0.1" respawn="false" output="screen" />

This comment has been minimized.

Copy link
@davetcoleman

davetcoleman Jun 15, 2018

Member

please break this line into two (line too long - max is 120)

This comment has been minimized.

Copy link
@MohmadAyman

MohmadAyman Jun 15, 2018

Author Member

Thanks for the review, will fix the changes.

Btw I got to get franka to be spawned in gazebo, but I had to do some changes to the urdf file first. What would you recommend, do we make the changes that makes a urdf model gazebo combitable a part of this PR? Or do we better make it a separate one and make this one just for the launch file?

This comment has been minimized.

Copy link
@davetcoleman

davetcoleman Jun 15, 2018

Member

please move this discussion to the main issue or the gdoc, it does not belong inline with a PR code review :-)

@MohmadAyman

This comment has been minimized.

Copy link
Member Author

commented Jun 15, 2018

@davetcoleman
Thanks for the review, will fix the changes.

Btw I got to get franka to be spawned in gazebo, but I had to do some changes to the urdf file first. What would you recommend, do we make the changes that makes a urdf model gazebo combitable a part of this PR? Or do we better make it a separate one and make this one just for the launch file?

@MohmadAyman MohmadAyman referenced this pull request Jun 20, 2018

Merged

Simulation Screen for the SA #956

3 of 5 tasks complete

@MohmadAyman MohmadAyman force-pushed the MohmadAyman:gazebo_config branch from cfb4a63 to 7a8a716 Jul 3, 2018

@MohmadAyman

This comment has been minimized.

Copy link
Member Author

commented Jul 3, 2018

As @davetcoleman and @gavanderhoorn suggested,

gazebo.launch is no longer automatically generated, the simulation screen should be responsible to trigger its generation through a checkbox.

@MohmadAyman MohmadAyman referenced this pull request Jul 9, 2018

Closed

Setup Assistant 2 #894

6 of 6 tasks complete
<param name="robot_description" [URDF_LOAD_ATTRIBUTE] />

<!-- push robot_description to factory and spawn robot in gazebo -->
<node name="spawn_gazebo_model" pkg="gazebo_ros" type="spawn_model" args="-urdf -param robot_description -model robot -z 0.1" respawn="false" output="screen" />

This comment has been minimized.

Copy link
@davetcoleman

davetcoleman Jul 10, 2018

Member

do you understand why -z 0.1 is used?

i believe this may be a custom value for a particular robot that is not very generic. please add a comment in this launch file explaining the purpose of z (changing the start location of the robot). i think it should default to 0

This comment has been minimized.

Copy link
@MohmadAyman

MohmadAyman Jul 10, 2018

Author Member

Yes that is right, 0 as default makes better sense, thanks.

This comment has been minimized.

Copy link
@MohmadAyman

MohmadAyman Jul 12, 2018

Author Member

Changed it, now robot spawn at the origin.

This comment has been minimized.

Copy link
@davetcoleman

davetcoleman Jul 12, 2018

Member

Actually can you leave the -z option there but set it to zero and document its usage?

This comment has been minimized.

Copy link
@MohmadAyman

MohmadAyman Jul 12, 2018

Author Member

If we leave the z, shouldnt we also leave the -x and -y?

This comment has been minimized.

Copy link
@davetcoleman

davetcoleman Jul 12, 2018

Member

great idea!

@MohmadAyman MohmadAyman force-pushed the MohmadAyman:gazebo_config branch from 7a8a716 to 2a73403 Jul 12, 2018

@MohmadAyman MohmadAyman force-pushed the MohmadAyman:gazebo_config branch from 2a73403 to 688fdad Jul 23, 2018

@davetcoleman

This comment has been minimized.

Copy link
Member

commented Aug 5, 2018

@mlautman please review

@davetcoleman davetcoleman merged commit a08d75c into ros-planning:kinetic-devel Aug 9, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

davetcoleman added a commit that referenced this pull request Aug 9, 2018

@davetcoleman

This comment has been minimized.

Copy link
Member

commented Aug 9, 2018

Cherry-picked to melodic

MohmadAyman added a commit to MohmadAyman/moveit that referenced this pull request Aug 25, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.