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 basic Ignition Gazebo tutorial and restructure simulation index #2356

Merged
merged 4 commits into from
Mar 22, 2022

Conversation

ahcorde
Copy link
Contributor

@ahcorde ahcorde commented Mar 18, 2022

Signed-off-by: ahcorde ahcorde@gmail.com

Added basic Ignition Gazebo tutorial and restructure simulation index

@ahcorde ahcorde requested a review from chapulina March 18, 2022 14:19
@ahcorde ahcorde self-assigned this Mar 18, 2022
Copy link
Collaborator

@fujitatomoya fujitatomoya left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

overall lgtm, just minor fixes.

Signed-off-by: ahcorde <ahcorde@gmail.com>
Copy link
Contributor

@clalancette clalancette left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall, the tutorial is good. I've left a bunch of comments inline on things to fix to make the language a bit more consistent. Once that is in, I'll do another review pass.


**Goal:** Launch a Simulation with Ignition Gazebo and ROS 2

**Tutorial level:** Medium
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Our tutorial levels are of the form "Beginner", "Intermediate", and "Advanced". I'll suggest that this should be an "Intermediate" tutorial.


.. code-block:: console

source /opt/ros/<ROS_DISTRO>/setup.bash
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
source /opt/ros/<ROS_DISTRO>/setup.bash
source /opt/ros/{ROS_DISTRO}/setup.bash


.. code-block:: console

source /opt/ros/<ROS_DISTRO>/setup.bash
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
source /opt/ros/<ROS_DISTRO>/setup.bash
source /opt/ros/{ROS_DISTRO}/setup.bash

Summary
-------

In this tutorial, you launch a robot simulation with Ignition Gazebo, launch bridges with actuators and sensors, visualize data from a sensor and move a diff drive robot.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
In this tutorial, you launch a robot simulation with Ignition Gazebo, launch bridges with actuators and sensors, visualize data from a sensor and move a diff drive robot.
In this tutorial, you launched a robot simulation with Ignition Gazebo, launched bridges with actuators and sensors, visualized data from a sensor, and moved a diff drive robot.

ahcorde and others added 2 commits March 22, 2022 10:15
Signed-off-by: ahcorde <ahcorde@gmail.com>
Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
Copy link
Contributor

@clalancette clalancette left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Apologies, I made a small mistake in my previous review. I suggested to use {ROS_DISTRO}, when it really should have been {DISTRO}. I fixed that up, as well as made a few more minor fixes to the text and formatting. Now this looks good to me, so I'll approve. @ahcorde should this also be backported to Foxy and Galactic?

Copy link
Contributor

@chapulina chapulina left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, @ahcorde , LGTM 👍

@chapulina
Copy link
Contributor

@ahcorde should this also be backported to Foxy and Galactic?

Yeah that should work with those versions too, and I believe the content can remain the same.

@clalancette
Copy link
Contributor

Yeah that should work with those versions too, and I believe the content can remain the same.

All right, thanks. Merging, then.

@clalancette clalancette added the backport-all backport at reviewers discretion; from rolling to all versions label Mar 22, 2022
@clalancette clalancette merged commit ef89d38 into rolling Mar 22, 2022
@clalancette clalancette deleted the ahcorde/ignition/basic_tutorial branch March 22, 2022 16:32
mergify bot pushed a commit that referenced this pull request Mar 22, 2022
…#2356)

* Added basic Ignition Gazebo tutorial and restructure simulation index

Signed-off-by: ahcorde <ahcorde@gmail.com>
Co-authored-by: Chris Lalancette <clalancette@openrobotics.org>
(cherry picked from commit ef89d38)
mergify bot pushed a commit that referenced this pull request Mar 22, 2022
…#2356)

* Added basic Ignition Gazebo tutorial and restructure simulation index

Signed-off-by: ahcorde <ahcorde@gmail.com>
Co-authored-by: Chris Lalancette <clalancette@openrobotics.org>
(cherry picked from commit ef89d38)
clalancette pushed a commit that referenced this pull request Mar 22, 2022
…#2356) (#2362)

* Added basic Ignition Gazebo tutorial and restructure simulation index

Signed-off-by: ahcorde <ahcorde@gmail.com>
Co-authored-by: Chris Lalancette <clalancette@openrobotics.org>
(cherry picked from commit ef89d38)

Co-authored-by: Alejandro Hernández Cordero <ahcorde@gmail.com>
clalancette pushed a commit that referenced this pull request Mar 22, 2022
…#2356) (#2363)

* Added basic Ignition Gazebo tutorial and restructure simulation index

Signed-off-by: ahcorde <ahcorde@gmail.com>
Co-authored-by: Chris Lalancette <clalancette@openrobotics.org>
(cherry picked from commit ef89d38)

Co-authored-by: Alejandro Hernández Cordero <ahcorde@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-all backport at reviewers discretion; from rolling to all versions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants