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

feat: update the naming rules for directories and classes #547

Merged
merged 13 commits into from
May 9, 2024
63 changes: 63 additions & 0 deletions docs/contributing/coding-guidelines/ros-nodes/class-design.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,68 @@
# Class design

We'll use the `autoware_gnss_poser` package as an example.

## Namespaces

```cpp
namespace autoware::gnss_poser
{
...
} // namespace autoware::gnss_poser
```

- Everything should be under `autoware::gnss_poser` namespace.
- Closing braces should contain a comment with the namespace name.
esteve marked this conversation as resolved.
Show resolved Hide resolved

## Classes

### Nodes

#### `gnss_poser_node.hpp`

```cpp
class GNSSPoserNode : public rclcpp::Node
{
public:
explicit GNSSPoserNode(const rclcpp::NodeOptions & node_options);
...
}
```

#### `gnss_poser_node.cpp`

```cpp
GNSSPoserNode::GNSSPoserNode(const rclcpp::NodeOptions & node_options)
: Node("gnss_poser", node_options)
{
...
}
```

- The class name should be in `CamelCase`.
- Node classes should inherit from `rclcpp::Node`.
- The constructor should be explicit.
esteve marked this conversation as resolved.
Show resolved Hide resolved
- The constructor should take `rclcpp::NodeOptions` as an argument.
esteve marked this conversation as resolved.
Show resolved Hide resolved
- Default node name:
- should not have `autoware_` prefix.
- should **NOT** have `_node` suffix.
- **Rationale:** Node names are useful in the runtime. And output of `ros2 node list` will show only nodes anyway. Having `_node` is redundant.
- **Example:** `gnss_poser`.

##### Component registration

```cpp
...
} // namespace autoware::gnss_poser

#include <rclcpp_components/register_node_macro.hpp>
RCLCPP_COMPONENTS_REGISTER_NODE(autoware::gnss_poser::GNSSPoserNode)
```

- The component should be registered at the end of the `gnss_poser_node.cpp` file, outside the namespaces.

### Libraries

!!! warning

Under Construction
241 changes: 204 additions & 37 deletions docs/contributing/coding-guidelines/ros-nodes/directory-structure.md
Original file line number Diff line number Diff line change
@@ -1,75 +1,242 @@
# Directory structure

!!! warning
This document describes the directory structure of ROS nodes within Autoware.

Under Construction
We'll use the package `autoware_gnss_poser` as an example.

## C++ package

### Entire structure

- This is a reference on how the entire package might be structured.
- A package may not have all the directories shown here.

```txt
<package_name>
autoware_gnss_poser
├─ package.xml
├─ CMakeLists.txt
├─ README.md
├─ config
│ ├─ foo_ros.param.yaml
│ └─ foo_non_ros.yaml
│ ├─ gnss_poser.param.yaml
│ └─ another_non_ros_config.yaml
├─ schema
│ └─ gnss_poser.schema.json
├─ doc
│ ├─ foo_document.md
│ └─ foo_diagram.svg
├─ include
│ └─ <package_name>
│ └─ foo_public.hpp
├─ launch
│ ├─ foo.launch.xml
│ └─ foo.launch.py
├─ schema
│ └─ foo_node.schema.json
├─ include # for exporting headers
│ └─ autoware
│ └─ gnss_poser
│ └─ exported_header.hpp
├─ src
│ ├─ foo_node.cpp
│ ├─ foo_node.hpp
│ └─ foo_private.hpp
├─ test
│ └─ test_foo.cpp
│ ├─ include
esteve marked this conversation as resolved.
Show resolved Hide resolved
│ │ ├─ gnss_poser_node.hpp
│ │ └─ foo.hpp
│ ├─ gnss_poser_node.cpp
│ └─ bar.cpp
├─ launch
│ ├─ gnss_poser.launch.xml
│ └─ gnss_poser.launch.py
└─ test
├─ test_foo.hpp # or place under an `include` folder here
└─ test_foo.cpp
```

### Package name

- All the packages in Autoware should be prefixed with `autoware_`.
- Even if the package is exports a node, the package name **should NOT** have the `_node` suffix.
- The package name should be in `snake_case`.

| Package Name | OK | Alternative |
| --------------------------------- | --- | ---------------------------- |
| path_smoother | ❌ | autoware_path_smoother |
| autoware_trajectory_follower_node | ❌ | autoware_trajectory_follower |
| autoware_geography_utils | ✅ | - |

### Package folder

```txt
autoware_gnss_poser
├─ package.xml
├─ CMakeLists.txt
└─ README.md
```

### Directory descriptions
The package folder name should be the same as the package name.

#### `config`
#### `package.xml`

- The package name should be entered within the `<name>` tag.
- `<name>autoware_gnss_poser</name>`

Place configuration files such as node parameters.
For ROS parameters, use the extension `.param.yaml`.
For non-ROS parameters, use the extension `.yaml`.
#### `CMakeLists.txt`

Rationale: Since ROS parameters files are type-sensitive, they should not be the target of some code formatters and linters. In order to distinguish the file type, we use different file extensions.
- The [`project()`](https://cmake.org/cmake/help/latest/command/project.html) command should call the package name.
- **Example:** `project(autoware_gnss_poser)`

##### Exporting a composable node component executable

```cmake
ament_auto_add_library(${PROJECT_NAME} SHARED
src/gnss_poser_node.cpp
)

rclcpp_components_register_node(${PROJECT_NAME}
PLUGIN "autoware::gnss_poser::GNSSPoserNode"
EXECUTABLE ${PROJECT_NAME}_node
)
```

#### `doc`
- The composable node component executable:
- should have `_node` suffix.
- should start with `${PROJECT_NAME}`

Place document files and link from README.
##### Exporting a standalone node executable
esteve marked this conversation as resolved.
Show resolved Hide resolved

#### `include`
Assuming:

Place header files exposed to other packages. Do not place files directly under the `include` directory, but place files under the directory with the package name.
This directory is used for mostly library headers. Note that many headers do not need to be placed here. It is enough to place the headers under the `src` directory.
- `src/gnss_poser.cpp` has the `GNSSPoserNode` class.
- `src/gnss_poser_node.cpp` has the `main` function.
- There is no composable node component registration.

```cmake
ament_auto_add_library(${PROJECT_NAME} SHARED
src/gnss_poser.cpp
)

ament_auto_add_executable(${PROJECT_NAME}_node src/gnss_poser_node.cpp)
```

- The node executable:
- should have `_node` suffix.
- should start with `${PROJECT_NAME}`

### `config` and `schema`

```txt
autoware_gnss_poser
│─ config
│ ├─ gnss_poser.param.yaml
│ └─ another_non_ros_config.yaml
└─ schema
└─ gnss_poser.schema.json
```

Reference: <https://docs.ros.org/en/humble/How-To-Guides/Ament-CMake-Documentation.html#adding-targets>
#### `config`

#### `launch`
- ROS parameters uses the extension `.param.yaml`.
- Non-ROS parameters use the extension `.yaml`.

Place launch files (`.launch.xml` and `.launch.py`).
**Rationale:** Different linting rules are used for ROS parameters and non-ROS parameters.

#### `schema`

Place parameter definition files. See [parameters](./parameters.md) for details.
Place parameter definition files. See [Parameters](./parameters.md) for details.

### `doc`

```txt
autoware_gnss_poser
└─ doc
├─ foo_document.md
└─ foo_diagram.svg
```

Place documentation files and link them from the README file.

### `include` and `src`

- Unless you specifically need to export headers, you shouldn't have a `include` directory under the package directory.
- For most cases, follow [Not exporting headers](#not-exporting-headers).
- Library packages that export headers may follow [Exporting headers](#exporting-headers).

#### Not exporting headers

```txt
autoware_gnss_poser
└─ src
├─ include
│ ├─ gnss_poser_node.hpp
│ └─ foo.hpp
│─ gnss_poser_node.cpp
└─ bar.cpp

OR

autoware_gnss_poser
└─ src
├─ gnss_poser_node.hpp
├─ gnss_poser_node.cpp
├─ foo.hpp
└─ bar.cpp
```

- The source file exporting the node should:
- have `_node` suffix.
- **Rationale:** To distinguish from other source files.
- **NOT** have `autoware_` prefix.
- **Rationale:** To avoid verbosity.
- See [Classes](../../class-design.md) for more details on how to construct `gnss_poser_node.hpp` and `gnss_poser_node.cpp` files.
- It is up to developer how to organize the source files under `src`.
- **Note:** The `include` folder under `src` is optional.

#### Exporting headers

#### `src`
```txt
autoware_gnss_poser
└─ include
└─ autoware
└─ gnss_poser
└─ exported_header.hpp
```

- `autoware_gnss_poser/include` folder should contain **ONLY** the `autoware` folder.
- **Rationale:** When installing ROS debian packages, the headers are copied to the `/opt/ros/$ROS_DISTRO/include/` directory. This structure is used to avoid conflicts with non-Autoware packages.
- `autoware_gnss_poser/include/autoware` folder should contain **ONLY** the `gnss_poser` folder.
- **Rationale:** Similarly, this structure is used to avoid conflicts with other packages.
- `autoware_gnss_poser/include/autoware/gnss_poser` folder should contain the header files to be exported.

**Note:** If `ament_auto_package()` command is used in the `CMakeLists.txt` file and `autoware_gnss_poser/include` folder exists,
this `include` folder will be exported to the `install` folder as part of [ament_auto_package.cmake](https://github.com/ament/ament_cmake/blob/79cc237f8eb819edf4c1c624b56451e0a05a45f8/ament_cmake_auto/cmake/ament_auto_package.cmake#L62-L66)

Place source files and private header files.
**Reference:** <https://docs.ros.org/en/humble/How-To-Guides/Ament-CMake-Documentation.html#adding-targets>

### `launch`

```txt
autoware_gnss_poser
└─ launch
├─ gnss_poser.launch.xml
└─ gnss_poser.launch.py
```

#### `test`
- You may have multiple launch files here.
- Unless you have a specific reason, use the `.launch.xml` extension.
- **Rationale:** While the `.launch.py` extension is more flexible, it comes with a readability cost.
- Avoid `autoware_` prefix in the launch file names.
- **Rationale:** To avoid verbosity.

### `test`

```txt
autoware_gnss_poser
└─ test
├─ test_foo.hpp # or place under an `include` folder here
└─ test_foo.cpp
```

Place source files for testing. See [unit testing](../../testing-guidelines/unit-testing.md) for details.

## Python package

T.B.D.
!!! warning

Under Construction
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ Find more information on parameters from the official ROS documentation:

A ROS package which uses the [declare_parameter(...)](https://docs.ros.org/en/ros2_packages/humble/api/rclcpp/generated/classrclcpp_1_1Node.html#_CPPv4N6rclcpp4Node17declare_parameterERKNSt6stringERKN6rclcpp14ParameterValueERKN14rcl_interfaces3msg19ParameterDescriptorEb) function should:

- use the [declare_parameter(...)](https://docs.ros.org/en/ros2_packages/humble/api/rclcpp/generated/classrclcpp_1_1Node.html#_CPPv4N6rclcpp4Node17declare_parameterERKNSt6stringERKN6rclcpp14ParameterValueERKN14rcl_interfaces3msg19ParameterDescriptorEb) with out a default value
- use the [declare_parameter(...)](https://docs.ros.org/en/ros2_packages/humble/api/rclcpp/generated/classrclcpp_1_1Node.html#_CPPv4N6rclcpp4Node17declare_parameterERKNSt6stringERKN6rclcpp14ParameterValueERKN14rcl_interfaces3msg19ParameterDescriptorEb) without a default value
- create a parameter file
- create a schema file

Expand Down
Loading