-
Notifications
You must be signed in to change notification settings - Fork 145
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
feat: update the naming rules for directories and classes (#547)
Signed-off-by: M. Fatih Cırıt <[email protected]>
- Loading branch information
Showing
3 changed files
with
277 additions
and
38 deletions.
There are no files selected for viewing
63 changes: 63 additions & 0 deletions
63
docs/contributing/coding-guidelines/ros-nodes/class-design.md
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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 must contain a comment with the namespace name. (Automated with `cpplint`) | ||
|
||
## 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 must be marked be explicit. | ||
- The constructor must take `rclcpp::NodeOptions` as an argument. | ||
- 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 |
250 changes: 213 additions & 37 deletions
250
docs/contributing/coding-guidelines/ros-nodes/directory-structure.md
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,75 +1,251 @@ | ||
# 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 | ||
│ │ ├─ 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>` | ||
|
||
#### `CMakeLists.txt` | ||
|
||
- 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 executables | ||
|
||
For best practices and system efficiency, it is recommended to primarily use composable node components. | ||
|
||
This method facilitates easier deployment and maintenance within ROS environments. | ||
|
||
```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 | ||
) | ||
``` | ||
|
||
- If you are building: | ||
- **only a single composable node component,** the executable name should start with `${PROJECT_NAME}` | ||
- **multiple composable node components,** the executable name is up to the developer. | ||
- All composable node component executables should have the `_node` suffix. | ||
|
||
##### Exporting a standalone node executable without composition _(discouraged for most cases)_ | ||
|
||
Use of standalone executables **should be limited** to cases where specific needs such as debugging or tooling are required. | ||
|
||
[Exporting a composable node component executables](#exporting-a-composable-node-component-executables) is generally preferred for standard operational use due its flexibility and scalability within the ROS ecosystem. | ||
|
||
Assuming: | ||
|
||
Place configuration files such as node parameters. | ||
For ROS parameters, use the extension `.param.yaml`. | ||
For non-ROS parameters, use the extension `.yaml`. | ||
- `src/gnss_poser.cpp` has the `GNSSPoserNode` class. | ||
- `src/gnss_poser_node.cpp` has the `main` function. | ||
- There is no composable node component registration. | ||
|
||
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. | ||
```cmake | ||
ament_auto_add_library(${PROJECT_NAME} SHARED | ||
src/gnss_poser.cpp | ||
) | ||
#### `doc` | ||
ament_auto_add_executable(${PROJECT_NAME}_node src/gnss_poser_node.cpp) | ||
``` | ||
|
||
Place document files and link from README. | ||
- The node executable: | ||
- should have `_node` suffix. | ||
- should start with `${PROJECT_NAME} | ||
|
||
#### `include` | ||
### `config` and `schema` | ||
|
||
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. | ||
```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. | ||
|
||
#### `src` | ||
#### Exporting headers | ||
|
||
Place source files and private header files. | ||
```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) | ||
|
||
**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 | ||
``` | ||
|
||
- 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` | ||
### `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 |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters