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.

Sign up for GitHub

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

Jump to bottom

New tutorial on ROS 2 topics #344

Merged
merged 16 commits into from
Oct 11, 2019
Merged

New tutorial on ROS 2 topics #344

merged 16 commits into from
Oct 11, 2019

Conversation

maryaB-osr
Copy link
Contributor

Concerns:

line 116

These attributes, particularly the type, are how nodes know they’re talking about the same information as it moves over topics.

Is that correct/ is there a better way to say it?

line 120 image - debating whether mentioning you can see all those topics is necessary. I figured if they see all of them listed from topic list then they might curious, but it seems like going off on a tangent.

line 157 - having to tell them all these extra steps for seeing certain nodes isn't great, especially since they're all visible by default in ROS 1

line 299 - rqt_plot has some UI issues so that section can't be included until that's fixed

Signed-off-by: maryaB-osr [email protected]

@mjcarroll mjcarroll temporarily deployed to ros2-documentation-pr-344 October 4, 2019 01:00 Inactive
@maryaB-osr
removed random ?
ef51696 
Signed-off-by: maryaB-osr <[email protected]>
@mjcarroll mjcarroll temporarily deployed to ros2-documentation-pr-344 October 4, 2019 01:01 Inactive
@mjcarroll mjcarroll temporarily deployed to ros2-documentation-pr-344 October 4, 2019 01:05 Inactive
clalancette
clalancette requested changes Oct 4, 2019
View reviewed changes
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, I like how the tutorial flows (particularly the latter half). But there are a few things to update and improve here.

source/Tutorials/Topics/UnderstandingROS2Topics.rst Outdated Show resolved Hide resolved
source/Tutorials/Topics/UnderstandingROS2Topics.rst Outdated Show resolved Hide resolved
source/Tutorials/Topics/UnderstandingROS2Topics.rst Outdated Show resolved Hide resolved
source/Tutorials/Topics/UnderstandingROS2Topics.rst Outdated Show resolved Hide resolved
source/Tutorials/Topics/UnderstandingROS2Topics.rst Outdated Show resolved Hide resolved
source/Tutorials/Topics/UnderstandingROS2Topics.rst Outdated Show resolved Hide resolved
wjwwood
wjwwood approved these changes Oct 4, 2019
View reviewed changes
Copy link
Member

@wjwwood wjwwood left a comment

Choose a reason for hiding this comment

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

Looks great to me! I had a few notes and made some cross-references to new issues.

source/Tutorials/Topics/UnderstandingROS2Topics.rst Outdated Show resolved Hide resolved
source/Tutorials/Topics/UnderstandingROS2Topics.rst Outdated Show resolved Hide resolved
source/Tutorials/Topics/UnderstandingROS2Topics.rst Show resolved Hide resolved
source/Tutorials/Topics/UnderstandingROS2Topics.rst Show resolved Hide resolved
source/Tutorials/Topics/UnderstandingROS2Topics.rst Outdated Show resolved Hide resolved
source/Tutorials/Topics/UnderstandingROS2Topics.rst Outdated Show resolved Hide resolved
source/Tutorials/Topics/UnderstandingROS2Topics.rst Outdated Show resolved Hide resolved
source/Tutorials/Topics/UnderstandingROS2Topics.rst Outdated Show resolved Hide resolved
source/Tutorials/Topics/UnderstandingROS2Topics.rst Outdated Show resolved Hide resolved
source/Tutorials/Topics/UnderstandingROS2Topics.rst Show resolved Hide resolved
@maryaB-osr @clalancette
remove "on"
4ee8b91 
Co-Authored-By: Chris Lalancette <[email protected]>
@mjcarroll mjcarroll temporarily deployed to ros2-documentation-pr-344 October 7, 2019 23:42 Inactive
@maryaB-osr @clalancette
clarify topics definition
0d8a4de 
Co-Authored-By: Chris Lalancette <[email protected]>
@mjcarroll mjcarroll temporarily deployed to ros2-documentation-pr-344 October 7, 2019 23:47 Inactive
@maryaB-osr @clalancette
"process" > "topic"
b1ea22b 
Co-Authored-By: Chris Lalancette <[email protected]>
@mjcarroll mjcarroll temporarily deployed to ros2-documentation-pr-344 October 7, 2019 23:47 Inactive
@mjcarroll mjcarroll temporarily deployed to ros2-documentation-pr-344 October 7, 2019 23:51 Inactive
@maryaB-osr @wjwwood
properly name topics and nodes
c61d586 
Co-Authored-By: William Woodall <[email protected]>
@mjcarroll mjcarroll temporarily deployed to ros2-documentation-pr-344 October 7, 2019 23:53 Inactive
@mjcarroll mjcarroll temporarily deployed to ros2-documentation-pr-344 October 7, 2019 23:54 Inactive
@maryaB-osr @wjwwood
typo "you" > "your"
90da1c6 
Co-Authored-By: William Woodall <[email protected]>
@mjcarroll mjcarroll temporarily deployed to ros2-documentation-pr-344 October 7, 2019 23:54 Inactive
@mjcarroll mjcarroll temporarily deployed to ros2-documentation-pr-344 October 8, 2019 00:23 Inactive
wjwwood
wjwwood approved these changes Oct 8, 2019
View reviewed changes
Copy link
Member

@wjwwood wjwwood left a comment

Choose a reason for hiding this comment

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

lgtm!

clalancette
clalancette reviewed Oct 8, 2019
View reviewed changes
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.

I have one more comment on the opening section. Besides that this looks good to me.

source/Tutorials/Topics/UnderstandingROS2Topics.rst Outdated Show resolved Hide resolved
@maryaB-osr
Rearrange background info
2042668 
Signed-off-by: maryaB-osr <[email protected]>
@mjcarroll mjcarroll temporarily deployed to ros2-documentation-pr-344 October 10, 2019 14:19 Inactive
@mjcarroll mjcarroll temporarily deployed to ros2-documentation-pr-344 October 10, 2019 18:10 Inactive
clalancette
clalancette approved these changes Oct 10, 2019
View reviewed changes
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.

I like the flow of that opening paragraph a lot more, thanks. I found one typo in it, but I'll approve anyway and you can fix right before merging.

source/Tutorials/Topics/UnderstandingROS2Topics.rst Outdated Show resolved Hide resolved
jacobperron
jacobperron reviewed Oct 10, 2019
View reviewed changes
source/Tutorials/Topics/UnderstandingROS2Topics.rst
Publisher count: 1
Subscriber count: 2

6 ros2 msg show
Copy link
Member

Choose a reason for hiding this comment

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

I left a similar comment for the services tutorial.

What are peoples thoughts on using ros2 interface instead?

Though, ros2 msg and ros2 srv can be a little more concise, perhaps we should be encouraging people to use ros2 interface since it also supports actions and IDL interfaces. The main difference in usage for the show verb is that you need to provide the .msg or .srv suffix in order to see the ROS definition instead of the generated IDL (perhaps this could be improved in the tool if we can infer that it is a legacy type).

Copy link
Member

Choose a reason for hiding this comment

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

I would support moving in this direction, but you should probably bring it up at a ROS 2 meeting. At the very least we could mention here that ros2 interface will also work and will work for all kinds of things too.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm also fine with moving that direction, though I'd argue for a follow-up task to change all of them later so that @maryaB-osr can continue on with this PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah I'm not sure how many times this will come up by the end of all the tutorials, so I'll add it to the list of things that I'll follow up on once everything's merged into staging.

Thanks @jacobperron

@maryaB-osr @clalancette
typo
cd9131a 
Co-Authored-By: Chris Lalancette <[email protected]>
@mjcarroll mjcarroll temporarily deployed to ros2-documentation-pr-344 October 11, 2019 09:21 Inactive
@maryaB-osr maryaB-osr merged commit 406999e into staging Oct 11, 2019
@delete-merged-branch delete-merged-branch bot deleted the tutorial4 branch October 11, 2019 09:53
maryaB-osr added a commit that referenced this pull request Oct 25, 2019
@maryaB-osr
Complete tutorials set (#386)
d313ba7 
* Environment config tutorial (#334)

* Environment config tutorial

Signed-off-by: maryaB-osr <[email protected]>

* Added pages to some empty links for CI check

Signed-off-by: maryaB-osr <[email protected]>

* Add requirements.txt and app.json for Heroku build (#336)

This enables heroku "review apps"

Signed-off-by: Michael Carroll <[email protected]>

* Addressed reviews and implemented tabs

Signed-off-by: maryaB-osr <[email protected]>

* Added tabs install instructions for travis

Signed-off-by: maryaB-osr <[email protected]>

* Fixed Travis warnings

Signed-off-by: maryaB-osr <[email protected]>

* Prereqs note about using debs. for linux

Signed-off-by: maryaB-osr <[email protected]>

* Added reference for linking

Signed-off-by: maryaB-osr <[email protected]>

* Update Goal statement

Co-Authored-By: Jacob Perron <[email protected]>

* Remove leading arrowhead from Windows command

Co-Authored-By: Jacob Perron <[email protected]>

* Empty line before .. todo:

Co-Authored-By: Jacob Perron <[email protected]>

* "value" to "integer" for ROS_DOMAIN_ID

Co-Authored-By: Jacob Perron <[email protected]>

* Added ROS_DOMAIN_ID explanation

Signed-off-by: maryaB-osr <[email protected]>

* Link to ROS Answers

Also left a note to test Windows domain ID command

Signed-off-by: maryaB-osr <[email protected]>

* Syntax error on ROS answers link

Signed-off-by: maryaB-osr <[email protected]>

* Changed domainID tabs to include script command

Signed-off-by: maryaB-osr <[email protected]>

* Fixed too much indentation

Signed-off-by: maryaB-osr <[email protected]>

* Update Windows ROS_DOMAIN_ID config section (#341)

* Update Windows ROS_DOMAIN_ID config section

Signed-off-by: Jacob Perron <[email protected]>

* Update source/Tutorials/ConfiguringROS2Environment.rst

Co-Authored-By: Chris Lalancette <[email protected]>

* New tutorial for introducing turtlesim for ROS 2 (#342)

* New tutorial for introducing turtlesim for ROS 2

Signed-off-by: maryaB-osr <[email protected]>

* Trying one image

Signed-off-by: maryaB-osr <[email protected]>

* New file not included in toctree

Signed-off-by: maryaB-osr <[email protected]>

* All images and fix toctree (?)

Signed-off-by: maryaB-osr <[email protected]>

* More concise goal statement

Co-Authored-By: Chris Lalancette <[email protected]>

* Update Windows ROS_DOMAIN_ID config section (#341)

* Update Windows ROS_DOMAIN_ID config section

Signed-off-by: Jacob Perron <[email protected]>

* Update source/Tutorials/ConfiguringROS2Environment.rst

Co-Authored-By: Chris Lalancette <[email protected]>

* Removed ros2 run explanation (for next tutorial)

Signed-off-by: maryaB-osr <[email protected]>

* "turtled" to "turtle"

Co-Authored-By: Chris Lalancette <[email protected]>

* clalancette's requested changes

Signed-off-by: maryaB-osr <[email protected]>

* dirk-thomas's requested changes

Signed-off-by: maryaB-osr <[email protected]>

* forgot to remove "2" from file name

Signed-off-by: maryaB-osr <[email protected]>

* fixed backticks and added sentence about pen

Signed-off-by: maryaB-osr <[email protected]>

* corrections from dirk-thomas

Signed-off-by: maryaB-osr <[email protected]>

* New tutorial on ROS 2 topics (#344)

* New tutorial on ROS 2 topics

Signed-off-by: maryaB-osr <[email protected]>

* removed random ?

Signed-off-by: maryaB-osr <[email protected]>

* comment out empty reference

Signed-off-by: maryaB-osr <[email protected]>

* remove "on"

Co-Authored-By: Chris Lalancette <[email protected]>

* clarify topics definition

Co-Authored-By: Chris Lalancette <[email protected]>

* "process" > "topic"

Co-Authored-By: Chris Lalancette <[email protected]>

* reword "command line introspection"

Co-Authored-By: William Woodall <[email protected]>

* properly name topics and nodes

Co-Authored-By: William Woodall <[email protected]>

* clarify not all robots need a data stream

Co-Authored-By: William Woodall <[email protected]>

* typo "you" > "your"

Co-Authored-By: William Woodall <[email protected]>

* address comments on definition of topics

Signed-off-by: maryaB-osr <[email protected]>

* Rearrange background info

Signed-off-by: maryaB-osr <[email protected]>

* typo

Co-Authored-By: Chris Lalancette <[email protected]>

* minor edits

Signed-off-by: maryaB-osr <[email protected]>

* New tutorial on ROS 2 services (#346)

* New tutorial on ROS 2 services

Signed-off-by: maryaB-osr <[email protected]>

* adding links on turtlesim, config, topics

Signed-off-by: maryaB-osr <[email protected]>

* removed cluttered parameter services

Signed-off-by: maryaB-osr <[email protected]>

* YAML syntax only on cmd line

Co-Authored-By: Chris Lalancette <[email protected]>

* specified synchronous services will wait

Signed-off-by: maryaB-osr <[email protected]>

* single quotes inside double quotes

Co-Authored-By: Scott K Logan <[email protected]>

* addressed cottsay's reviews

Signed-off-by: maryaB-osr <[email protected]>

* more intuitive topics/services description

Signed-off-by: maryaB-osr <[email protected]>

* more concise wording

Co-Authored-By: Chris Lalancette <[email protected]>

* missing comma

Co-Authored-By: Chris Lalancette <[email protected]>

* typo

Signed-off-by: maryaB-osr <[email protected]>

* New tutorial for ROS 2 parameters (#347)

* New tutorial for ROS 2 parameters

Signed-off-by: maryaB-osr <[email protected]>

* Forgot toctree and goal statement

Signed-off-by: maryaB-osr <[email protected]>

* italics > bold

Signed-off-by: maryaB-osr <[email protected]>

* small edits based on reviews

Signed-off-by: maryaB-osr <[email protected]>

* Change the main README to say to use pip for sphinx and required sphinx-tab installation. (#363)

Add sphinx and sphinx-tab into requirements.txt

Clarify to use pip3 so people implicitly know that python3 is required

Further clarify sphinx requirements

* New tutorial for ROS 2 nodes (#343)

* New tutorial for ROS 2 nodes

Signed-off-by: maryaB-osr <[email protected]>

* commented out empty reference (for now)

Also added link target to concept overview page

Signed-off-by: maryaB-osr <[email protected]>

* removed pic link, added code blocks

Signed-off-by: maryaB-osr <[email protected]>

* "Hopefully" sounds silly

Co-Authored-By: Chris Lalancette <[email protected]>

* "multiple nodes" to "one or more"

Co-Authored-By: Chris Lalancette <[email protected]>

* added node info with actions

Signed-off-by: maryaB-osr <[email protected]>

* added links for prereqs and next steps

Signed-off-by: maryaB-osr <[email protected]>

* fixed node def & added design doc link

Signed-off-by: maryaB-osr <[email protected]>

* New tutorial for ROS 2 actions (#350)

* New tutorial for ROS 2 actions

Signed-off-by: maryaB-osr <[email protected]>

* travis warning

Signed-off-by: maryaB-osr <[email protected]>

* explicitly describe action goal process

Co-Authored-By: Jacob Perron <[email protected]>

* correct services definition

Co-Authored-By: Jacob Perron <[email protected]>

* correct implemenation details

Co-Authored-By: Jacob Perron <[email protected]>

* cancel > preempt

Co-Authored-By: Jacob Perron <[email protected]>

* minor updates

Signed-off-by: maryaB-osr <[email protected]>

* jacobperron's review

Signed-off-by: maryaB-osr <[email protected]>

* Changed `action show` to `interface show`

Signed-off-by: maryaB-osr <[email protected]>

* updated hidden actions in node info & removed bash

Signed-off-by: maryaB-osr <[email protected]>

* New tutorial for ROS 2 rqt_console (#351)

* New tutorial for ROS 2 rqt_console

Signed-off-by: maryaB-osr <[email protected]>

* missing spaces in topic pub yaml

Co-Authored-By: Michel Hidalgo <[email protected]>

* change tense

Co-Authored-By: Michel Hidalgo <[email protected]>

* first > top

Co-Authored-By: Chris Lalancette <[email protected]>

* specify new terminal

Co-Authored-By: Chris Lalancette <[email protected]>

* change file names and add links to prereqs

Signed-off-by: maryaB-osr <[email protected]>

* minor changes from reviews

Signed-off-by: maryaB-osr <[email protected]>

* addressed reviews

Signed-off-by: maryaB-osr <[email protected]>

* typo + updated remapping syntax

Signed-off-by: maryaB-osr <[email protected]>

* Updated remapping syntax (#354)

* Updated remapping syntax

Signed-off-by: maryaB-osr <[email protected]>

* tabs for eloquent/dashing remap syntax

Signed-off-by: maryaB-osr <[email protected]>

* New tutorial for ROS 2 bag files (#355)

* New tutorial for ROS 2 bag files

Signed-off-by: maryaB-osr <[email protected]>

* trying to make alt-text for internal links work

Signed-off-by: maryaB-osr <[email protected]>

* added available links

Signed-off-by: maryaB-osr <[email protected]>

* addressed reviews

Signed-off-by: maryaB-osr <[email protected]>

* New tutorial for creating a workspace (#359)

* New tutorial for creating a workspace

Signed-off-by: maryaB-osr <[email protected]>

* typo

Signed-off-by: maryaB-osr <[email protected]>

* typo

Co-Authored-By: Chris Lalancette <[email protected]>

* addressed reviews (dirk-thomas & clalancette)

Signed-off-by: maryaB-osr <[email protected]>

* revert source paths until PR 380 merges

Signed-off-by: maryaB-osr <[email protected]>

* underlay != main installation

Co-Authored-By: Tully Foote <[email protected]>

* main source not mandatory

Co-Authored-By: Tully Foote <[email protected]>

* underlay != main installation (again)

Co-Authored-By: Tully Foote <[email protected]>

* tfoote and clalancette suggestions

Signed-off-by: maryaB-osr <[email protected]>

* trailing whitespace

Signed-off-by: maryaB-osr <[email protected]>

* New tutorial for creating a package in ROS 2 (#362)

* New tutorial for creating a package in ROS 2

Signed-off-by: maryaB-osr <[email protected]>

* addressed review from ivanpauno & sloretz

Signed-off-by: maryaB-osr <[email protected]>

* ivan's final review

Signed-off-by: maryaB-osr <[email protected]>

* New tutorial for ros2doctor (#378)

* New tutorial for ros2doctor

Signed-off-by: maryaB-osr <[email protected]>

* linter

Signed-off-by: maryaB-osr <[email protected]>

* claireyywang's review

Signed-off-by: maryaB-osr <[email protected]>

* New tutorial for service and client using C++ (#370)

* new tutorial for service and client using C++

Signed-off-by: maryaB-osr <[email protected]>

* linter fix

Signed-off-by: maryaB-osr <[email protected]>

* linter

Signed-off-by: maryaB-osr <[email protected]>

* forgot package name in pkg create

Signed-off-by: maryaB-osr <[email protected]>

* improve sentence structure

Co-Authored-By: Michel Hidalgo <[email protected]>

* minor changes

Signed-off-by: maryaB-osr <[email protected]>

* target > targets

* Update parameters tutorial (#384)

* Update parameters tutorial

Signed-off-by: maryaB-osr <[email protected]>

* linter

Signed-off-by: maryaB-osr <[email protected]>

* New tutorial for Python pub/sub (#364)

* New tutorial for Python pub/sub

Signed-off-by: maryaB-osr <[email protected]>

* fix linter issue

Signed-off-by: maryaB-osr <[email protected]>

* removed mentions of line numbers, plus other reviews

Signed-off-by: maryaB-osr <[email protected]>

* removed comments from long code blocks

Signed-off-by: maryaB-osr <[email protected]>

* removed parenthesis from file names

Signed-off-by: maryaB-osr <[email protected]>

* final nits

Signed-off-by: maryaB-osr <[email protected]>

* macOS users already have dependencies installed

Signed-off-by: maryaB-osr <[email protected]>

* Consistent indentation in code blocks

Signed-off-by: Shane Loretz <[email protected]>

* unneeded codeblock line

Signed-off-by: maryaB-osr <[email protected]>

* New tutorial for service/client with Python (#372)

* New tutorial for service/client with Python

Signed-off-by: maryaB-osr <[email protected]>

* title underline too short

Signed-off-by: maryaB-osr <[email protected]>

* trailing whitespace

Signed-off-by: maryaB-osr <[email protected]>

* fix indentation and other nits

Signed-off-by: maryaB-osr <[email protected]>

* Consistent indentation in code blocks

Signed-off-by: Shane Loretz <[email protected]>

* wrong section

Signed-off-by: maryaB-osr <[email protected]>

* New tutorial for ROS 2 launch files (#353)

* New tutorial for ROS 2 launch files

Signed-off-by: maryaB-osr <[email protected]>

* Forgot to include in toctree

Signed-off-by: maryaB-osr <[email protected]>

* trailing white space

Signed-off-by: maryaB-osr <[email protected]>

* missing period

Co-Authored-By: Chris Lalancette <[email protected]>

* inside pkg > provided by pkg

Co-Authored-By: Michel Hidalgo <[email protected]>

* python modules explanation

Co-Authored-By: Michel Hidalgo <[email protected]>

* handful of reviews

Signed-off-by: maryaB-osr <[email protected]>

* linter & added next steps to rqt_console

Signed-off-by: maryaB-osr <[email protected]>

* more trailing whitespaces...

Signed-off-by: maryaB-osr <[email protected]>

* trying linenos

Signed-off-by: maryaB-osr <[email protected]>

* adjust indented space on linenos

Signed-off-by: maryaB-osr <[email protected]>

* another attempt at linenos indentation

Signed-off-by: maryaB-osr <[email protected]>

* change indentation

Signed-off-by: maryaB-osr <[email protected]>

* fix indentation on python block

Signed-off-by: maryaB-osr <[email protected]>

* give up on linenos for now

Signed-off-by: maryaB-osr <[email protected]>

* fixed indentation...?

Signed-off-by: maryaB-osr <[email protected]>

* New tutorial for C++ pub/sub (#366)

* New tutorial for C++ pub/sub

Signed-off-by: maryaB-osr <[email protected]>

* try C++ code highlighting

Signed-off-by: maryaB-osr <[email protected]>

* one incorrect directive

Signed-off-by: maryaB-osr <[email protected]>

* fixed some indentation

Signed-off-by: maryaB-osr <[email protected]>

* changed entry points to executables

Signed-off-by: maryaB-osr <[email protected]>

* removed parenthesis from file name

Signed-off-by: maryaB-osr <[email protected]>

* New branch for cleaning up staging

This commit organizes the order of the tutorials,
adds an introduction to the beginner tutorials,
and hyphenates all the page names

Signed-off-by: maryaB-osr <[email protected]>

* added next steps sections and other internal links

Signed-off-by: maryaB-osr <[email protected]>

* change msg show to interface show

Signed-off-by: maryaB-osr <[email protected]>

* linter

Signed-off-by: maryaB-osr <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jacobperron jacobperron jacobperron left review comments

@clalancette clalancette clalancette approved these changes

@wjwwood wjwwood wjwwood approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@maryaB-osr @wjwwood @clalancette @jacobperron @mjcarroll