-
Notifications
You must be signed in to change notification settings - Fork 158
feat: update the naming rules for directories and classes #547
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
Merged
Merged
Changes from all commits
Commits
Show all changes
13 commits
Select commit
Hold shift + click to select a range
56350b6
feat: update the naming rules for directories and classes
688d90c
style(pre-commit): autofix
pre-commit-ci[bot] 234860c
add entire structure for ease of use
858c9a5
clarify node name
c59e128
update cpp naming to gnss_poser_node.cpp
77b3039
remove `_node` from node names
52bf7bb
fix autoware_ prefix
cede324
allow devs to arrange inside `src`
e38f3a2
take non-composable nodes into account
deb3a3f
style(pre-commit): autofix
pre-commit-ci[bot] fea457f
s/should/must updates
d96bd27
update exporting component section
isamu-takagi 9ba71b9
handle all executable types
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 hidden or 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 hidden or 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 hidden or 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
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.