Skip to content

Commit

Permalink
Create STYLE_GUIDE.md (#122)
Browse files Browse the repository at this point in the history
  • Loading branch information
JafarAbdi authored and henningkayser committed Dec 7, 2019
1 parent 233b7cf commit f10e0ce
Show file tree
Hide file tree
Showing 2 changed files with 50 additions and 0 deletions.
3 changes: 3 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ The MoveIt Motion Planning Framework for ROS 2
We are currently working on the upcoming Beta version of MoveIt 2.
See the [MoveIt's website homepage](https://moveit.ros.org) for release dates and [this document](JAFAR's LINK) for our immediate migration progress.

Implementation instructions for the ROS 2 migration process can be found in our [Migration Guidelines](docs/MIGRATION_GUIDE.md).

### Build from Source

> Note: Currently, only the moveit\_core packages are being compiled.
Expand Down Expand Up @@ -45,6 +47,7 @@ The MoveIt Motion Planning Framework **for ROS 2.0**
- [Installation Instructions](http://moveit.ros.org/install/)
- [Documentation](http://moveit.ros.org/documentation/)
- [Get Involved](http://moveit.ros.org/documentation/contributing/)
- [Migration Guidelines](docs/MIGRATION_GUIDE.md)


## Continuous Integration Status
Expand Down
47 changes: 47 additions & 0 deletions doc/MIGRATION_GUIDE.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
# MoveIt 2 Migration Guide

## Logging

In MoveIt 1 we commonly use named ROS logging macros (i.e. `ROS_INFO_NAMED`) with file-specific logger names defined as `constexpr char LOGNAME[]="logger_name"`.
ROS 2 provides similar macros (i.e. `RCLCPP_INFO`) that instead of an optional name always require a `rclcpp::Logger` instance for scoping the namespace.
All source files that use ROS logging should now define a file-specific `static const rclcpp::Logger` named `LOGGER`, located at the top of the file and inside the namespace with the narrowest scope (if there is one).
This leads to the following basic migration steps:

1. Replace `LOGNAME` with `rclcpp::Logger` instance:

<b>Old:</b>

constexpr char LOGNAME[] = "logger_name";

<b>New:</b>

static const rclcpp::Logger LOGGER = rclcpp::get_logger("logger_name");

2. Replace logging macros:

<b>Old:</b>

ROS_INFO_NAMED(LOGNAME, "Very important info message");

<b>New:</b>

RCLCPP_INFO(LOGGER, "Very important info message");
Currently, there is no direct replacement for the `ROS_*_STREAM` macros in dashing, however, a first implementation is available in eloquent (https://github.com/ros2/rclcpp/pull/926). Until we switch to eloquent, stream macros should be implemented using C formatting or commented with an adequate TODO and substitute log message.

### Logger naming convention

Migrating the loggers is a good opportunity to make logger names more consistent.
In order to create unique and descriptive logger names we encourage the following naming pattern: general `LIBRARY_NAME.SOURCE_FILE_NAME`.
For instance, the file `joint_model_group.cpp` inside the library `moveit_robot_model` contains the following logger:

static const rclcpp::Logger LOGGER = rclcpp::get_logger("moveit_robot_model.joint_model_group");

For finding the `LIBRARY_NAME` refer to the line `set(MOVEIT_LIB_NAME LIBRARY_NAME)` at the top of the library's `CMakeLists.txt`.
If the source file name is the same or very similar to the library name it is sufficient to only use the source file name.

### Logging in header files

Some classes declared in header files may contain log messages, for instance to warn about not-implemented virtual functions in abstract classes.
A logger defined in the header file would not tell us what derived class is missing the implementation, since the source name would be resolved from the header file.
For this case, the base class should declare a private member variable `static const rclcpp::Logger LOGGER` which is to be defined in the implementing class using the corresponding source file name.

0 comments on commit f10e0ce

Please sign in to comment.