Comments for the first revision

Hello,

First of all, many thanks @fmrico for your effort in creating this book and putting the repository together! Similarly, congratulations on the success it has accomplished so far!

I saw your LinkedIn post about preparing the first revision, so here are my comments after reading the book.

Disclaimers: I have only read the book once — from start to finish. I did not execute/test the code examples myself, nor did I review the code in the repository/appendix. However, I went through all the code snippets that are within the main chapters — although I might have only briefly skimmed through some of them. English is not my first language, so I did not attempt to correct grammar or phrasing — except for obvious typos and missing/extra words. If unsure, take all my comments with a grain of salt and ignore them. I might be incorrect in many of them. :)

General comments

First, some very general comments.

Overall feedback

I like the conciseness of the book. It provides a good overview and guidelines about programming robots with ROS 2 without making the text appear excessively descriptive. It manages to provide extra details about certain concepts, which is welcome. It is great that the book comes with this repository containing code that is actively updated (such as the update now from foxy to humble). Similarly, the accompanying slides are great for providing an overview of each chapter/topic and I believe they will be appreciated by educators.

The book also manages to dive into more complex topics that I would not initially expect from looking at the book cover (at least what I consider to be more complex). Examples of these are software testing, computer vision and behaviour trees — all of which are important topics. However, I suspect that a complete ROS beginner might get overwhelmed. Even though the book states the assumption that "Prior knowledge of ROS/ROS2 is not needed.", I think that prior knowledge (and experience) with ROS might greatly improve understanding and even be necessary for the comprehension of some details. The same applies to the prior knowledge of C++ that is necessary for comprehending the included code snippets, but this is already well-explained in the introduction — together with hyperlinks to external C++ references.

Ultimately, I agree with this statement from the book: "This book is not intended to be a new ROS2 tutorial. The ones on the official website are great!", and I think it is important that readers are aware of that. In this way, the official tutorials could serve as an introduction to ROS for complete beginners, while this book complements it nicely with concrete project examples and suggestions for development guidelines.

While reading the book, I really appreciated these kinds of suggestions and details. Here are two examples that are memorable for me:

QoS suggestion to use reliable publishers and let subscribers decide whether to relax this requirement.
That tf2 does linear interpolation when looking-up transform in time between earlier and later available publications. (I was not aware of this)

Missing epilogue/conclusion

I feel that the book ends abruptly with no final words, which might make the reader wonder what to do after reading the book. I think it might be nice to conclude what the reader has learned and give some pointers for further learning. For example, if the reader is interested in learning more about robotic manipulation (not covered in the book), it might be worth referring the reader to MoveIt 2. And the same could be applied to other concepts, e.g. refer to Gazebo or another robotics simulator if the reader is interested in learning more about simulators.

"ROS2" vs "ROS 2"

This one might be controversial — so I will just insert some links.

Specific comments

Now for some specific comments. Each comment is linked to particular page(s). All markdown quotes > ... refer to text from the book on that page. Please, let me know if something is unclear.

I went through existing/closed issues and tried to remove all comments that were already brought up (but apologies if I missed some).

1 | Page 6: Services

Services: It is a synchronous communication in which a node requests ...

By default, ROS 2 service (client) calls are asynchronous, which I assume to be the recommended paradigm.

From relevant docs: https://docs.ros.org/en/humble/How-To-Guides/Sync-Vs-Async.html ("It is not recommended to implement a synchronous service client. They are susceptible to deadlock ...")

This description of services from docs might be more suitable: https://docs.ros.org/en/humble/Tutorials/Beginner-CLI-Tools/Understanding-ROS2-Services/Understanding-ROS2-Services.html ("Services are based on a call-and-response model")

2 | Page 7: Kobuki

A reader might not know Kobuki while reading this page. Although a reference is provided and it is explained more in the next chapter, a small description might be appreciated. Saying it is a mobile robot might be enough.

3 | Page 7: "probably"

The Kobuki robot driver is a node that communicates with the robot's hardware, probably using a native driver.

The word "probably" sounds unsure here. If there is no reason for it, I would remove the word.

4 | Page 7: `/mobile_base/commands/velocity` used by all robots

/mobile_base/commands/velocity: ... This topic is of type geometry_msgs/msg/Twist. Virtually all robots in ROS2 receive these types of messages to control their speed.

Not sure, but all mobile robots might be more correct. Outside of MoveIt Servo, I don't think I encountered these Twist messages for controlling manipulators. It is also mentioned on page 51.

5 | Page 12: `/.bashrc`

$ echo "source /opt/ros/foxy/setup.bash">> /.bashrc

It should be ~/.bashrc (with ~), "${HOME}/.bashrc" or other alternatives (e.g. $HOME/.bashrc).

Otherwise, the path is incorrect.

6 | Page 26: typo - "Apart of ament_cmake, always needed by colcon, just rclcpp is especified."

Location: second last bullet-point
Suggestion: "Apart of ..." -> "Apart from ..."
Suggestion: "especified" -> "specified"

7 | Page 32: typo - "To compile these program, the relevant..."

Location: first line
Suggestion: "these program" -> "these programs" | "this program"

8 | Page 32: incorrect `add_executable(logger_class src/logger.cpp)`

In CMakeLists.txt snippet:

add_executable(logger_class src/logger.cpp)

This first line should be add_executable(logger src/logger.cpp). Otherwise, logger executable does not exist and logger_class is added twice.

9 | Page 36: typo - "Last ROS2 distros lets to create launchers written in Yaml and XML"

Location: footnote 1
Suggestion: "lets to" -> "let us" | "enable creating" | something else?

10 | Page 36: consistency of "Yaml" vs "YAML"

In footnote 1, "Yaml" is used. However, I think that most of the book uses "YAML"?

11 | Page 36: examples of Yaml/XML launchers

It might be nice to include examples of Yaml/XML launchers (same nodes/structure used by the already included Python launcher).

On a side note, this page mentions that Python launchers typically use the extension _launch.py (starting with _ underscore). However, there is a mixed-use with .launch.py (starting with . dot) within the book.

12 | Page 44: pre-configured RViz2 interface?

For beginners, it might be difficult to figure out how to add panels and configure some options in RViz (even though it is explained in depth within this section). To simplify things, using a pre-configured .rviz config and loading it with rviz2 -d ... could be an alternative. This is just a thought, the current approach might work well.

13 | Page 48: angles grow left vs counter-clockwise

Angles grow by turning to the left (right part of Figure 3.2).

I suggest replacing left with counter-clockwise.

14 | Page 56: typo - "going forward when deteting an obstacle"

Location: comment in the second snippet (src/bump go cpp/BumpGoNode.cpp)
Suggestion: "deteting" -> "detecting"

15 | Pages 56/57: use of constants

Although using constants is a good practice for values like OBSTACLE_DISTANCE and SCAN_TIMEOUT, I think it might be beneficial to use an exact value when it is used as an example snippet within chapters of books.

For example, instead of using SCAN_TIMEOUT in the following snippet, I would suggest using the concrete value like 1.0 because I think it might improve comprehension for readers and let them know what data type SCAN_TIMEOUT is while reading through the chapter. Currently, SCAN_TIMEOUT is not defined anywhere within the chapter, so a reader would need to go to the code to see what data type these constants have.

// Stop if no sensor readings for 1 second
auto elapsed = now() - rclcpp::Time(last_scan_->header.stamp);
return elapsed > SCAN_TIMEOUT;

16 | Page 57: suggestion to rephrase explanation of `use_sim_time`

Maybe it is just me, but I think the current explanation below could be misunderstood like there is always a /clock topic, which contains messages that are either published by the simulator or the computer.

When using a simulator, set the use_sim_time parameter to true. This causes the time to be taken from the topic /clock, published by the simulator, instead of the current one from the computer.

Therefore, here is a small rephrasing suggestion:

"..., instead of the current one from the computer." -> "..., instead of using the system clock."

17 | Page 62: typo - "... first launch the simuladtor by typing ..."

Location: sentence before the second snippet
Suggestion: "simuladtor" -> "simulator"

18 | Pages 64/65: incorrect slashes before frame names

In this section, all frames (/base_footprint, /base_link, /odom in bullet points) are prefixed with a slash (/). However, I do not think that is correct. Shouldn't it be without the namespace-like leading slash?

19 | Page 78: remove redundant "use"

Sentence: "To get odom2laser, we need to ~~use~~ query the TF subsystem with ..."
Location: second bullet point

20 | Page 83: missing "of"

Sentence: "See that the organization of the package, in the next box, is already standard in our packages"
Location: First sentence of 5.1.2

21 | Page 85: typo - "This is the obstacle radious in which ..."

Location: comment in the second snippet (src/br2_vff_avoidance/AvoidanceNode.cpp)
Suggestion: "radious" -> "radius"

22 | Page 86: use of an incorrect word "close" -> "further"

Sentence: Closer obstacles have to generate more repulse than those ~~close~~.
Location: third bullet point
Suggestion: Closer obstacles have to generate more repulse than those futher.

23 | Page 110: merge the sentence with the previous paragraph?

Figure 5.10 shows a diagram of this PID controller.

This sentence is the entire second-last paragraph, so I think it could be merged with its previous paragraph.

24 | Page 118: typo - "... they use the input of the blackborad whose key ..."

Location: second-last paragraph
Suggestion: "blackborad" -> "blackboard"

25 | Page 120: missing whitespace between words "timesbefore"

Location: table, 3rd column, 9th item row
Suggestion: "N timesbefore" -> "N times before"

26 | Page 120: missing whitespace after "Sequence:"

Location: beginning of last bullet point
Suggestion: "Sequence:As ..." -> "Sequence: As ..."

27 | Page 136: remove redundant "one"

Suggestion: "Since ~~one~~ frames should not have two parents, ..."
Location: First paragraph/bullet point
Alternative: Replace "one" with "every"

28 | Page 149: typos in comment "Get the state of the controlled node. Ot can fail, ..."

Location: code snipped, last comment (the corresponding function is a setter, not a getter)
Original: "Get the state of the controlled node. Ot can fail, if not transition [?] possible"
Suggestion: -> "Set the state of the controlled node. It can fail, if no transition is possible"

fmrico / book_ros2