From 56350b6ba9c430242ec77b1d1b01eb17ac5a2a85 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?M=2E=20Fatih=20C=C4=B1r=C4=B1t?= Date: Wed, 8 May 2024 20:45:52 +0300 Subject: [PATCH 01/13] feat: update the naming rules for directories and classes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: M. Fatih Cırıt --- .../ros-nodes/class-design.md | 59 ++++++ .../ros-nodes/directory-structure.md | 177 +++++++++++++----- .../coding-guidelines/ros-nodes/parameters.md | 2 +- 3 files changed, 195 insertions(+), 43 deletions(-) diff --git a/docs/contributing/coding-guidelines/ros-nodes/class-design.md b/docs/contributing/coding-guidelines/ros-nodes/class-design.md index 4ce76c60e80..4e02e886f7a 100644 --- a/docs/contributing/coding-guidelines/ros-nodes/class-design.md +++ b/docs/contributing/coding-guidelines/ros-nodes/class-design.md @@ -1,5 +1,64 @@ # 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. + +## Classes + +### Nodes + +#### `gnss_poser.hpp` + +```cpp +class GNSSPoserNode : public rclcpp::Node +{ + public: + explicit GNSSPoserNode(const rclcpp::NodeOptions & node_options); + ... +} +``` + +#### `gnss_poser.cpp` + +```cpp +GNSSPoserNode::GNSSPoserNode(const rclcpp::NodeOptions & node_options) +: Node("gnss_poser_node", node_options) +{ + ... +} +``` + +- The class name should be in `CamelCase`. +- Node classes should inherit from `rclcpp::Node`. +- The constructor should be explicit. +- The constructor should take `rclcpp::NodeOptions` as an argument. +- Default node name should be `gnss_poser_node`. + +##### Component registration + +```cpp +... +} // namespace autoware::gnss_poser + +#include +RCLCPP_COMPONENTS_REGISTER_NODE(autoware::gnss_poser::GNSSPoserNode) +``` + +- The component should be registered at the end of the `autoware_gnss_poser.cpp` file, outside the namespaces. + +### Libraries + !!! warning Under Construction diff --git a/docs/contributing/coding-guidelines/ros-nodes/directory-structure.md b/docs/contributing/coding-guidelines/ros-nodes/directory-structure.md index acf62ec4bce..6f440b1e5f6 100644 --- a/docs/contributing/coding-guidelines/ros-nodes/directory-structure.md +++ b/docs/contributing/coding-guidelines/ros-nodes/directory-structure.md @@ -1,75 +1,168 @@ # 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 +### 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 - -├─ config -│ ├─ foo_ros.param.yaml -│ └─ foo_non_ros.yaml -├─ doc -│ ├─ foo_document.md -│ └─ foo_diagram.svg -├─ include -│ └─ -│ └─ foo_public.hpp -├─ launch -│ ├─ foo.launch.xml -│ └─ foo.launch.py -├─ schema -│ └─ foo_node.schema.json -├─ src -│ ├─ foo_node.cpp -│ ├─ foo_node.hpp -│ └─ foo_private.hpp -├─ test -│ └─ test_foo.cpp +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` -Place configuration files such as node parameters. -For ROS parameters, use the extension `.param.yaml`. -For non-ROS parameters, use the extension `.yaml`. +- The package name should be entered within the `` tag. + - `autoware_gnss_poser` -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. +#### `CMakeLists.txt` -#### `doc` +- The [`project()`](https://cmake.org/cmake/help/latest/command/project.html) command should call the package name. + - **Example:** `project(autoware_gnss_poser)` -Place document files and link from README. +##### Exporting a component -#### `include` +```cmake +ament_auto_add_library(${PROJECT_NAME} SHARED + src/gnss_poser_node.cpp +) -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. +rclcpp_components_register_node(${PROJECT_NAME} + PLUGIN "autoware::gnss_poser::GNSSPoserNode" + EXECUTABLE ${PROJECT_NAME}_node +) +``` -Reference: +- The component executable should have `_node` suffix. +- The component executable name should start with `${PROJECT_NAME}` -#### `launch` +### `config` and `schema` -Place launch files (`.launch.xml` and `.launch.py`). +```txt +autoware_gnss_poser +│─ config +│ ├─ gnss_poser.param.yaml +│ └─ another_non_ros_config.yaml +└─ schema + └─ gnss_poser.schema.json +``` + +#### `config` + +- ROS parameters uses the extension `.param.yaml`. +- Non-ROS parameters use the extension `.yaml`. + +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). -#### `src` +#### Not exporting headers -Place source files and private header files. +```txt +autoware_gnss_poser +└─ src + ├─ include + │ ├─ gnss_poser_node.hpp + │ └─ foo.hpp + │─ gnss_poser_node.cpp + └─ bar.cpp +``` + +- Put the header files in the `include` directory under the `src` directory. +- 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. -#### `test` +#### Exporting headers + +```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/humble/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:** + +### `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` + +```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 diff --git a/docs/contributing/coding-guidelines/ros-nodes/parameters.md b/docs/contributing/coding-guidelines/ros-nodes/parameters.md index 45c437406dd..38866654529 100644 --- a/docs/contributing/coding-guidelines/ros-nodes/parameters.md +++ b/docs/contributing/coding-guidelines/ros-nodes/parameters.md @@ -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 From 688d90cd11ba2e6dc4853a7db8331558e4691c92 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Wed, 8 May 2024 17:47:53 +0000 Subject: [PATCH 02/13] style(pre-commit): autofix --- .../ros-nodes/directory-structure.md | 24 +++++++++---------- 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/docs/contributing/coding-guidelines/ros-nodes/directory-structure.md b/docs/contributing/coding-guidelines/ros-nodes/directory-structure.md index 6f440b1e5f6..66ff70d79e6 100644 --- a/docs/contributing/coding-guidelines/ros-nodes/directory-structure.md +++ b/docs/contributing/coding-guidelines/ros-nodes/directory-structure.md @@ -12,8 +12,8 @@ We'll use the package `autoware_gnss_poser` as an example. - 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 | -|-----------------------------------|----|------------------------------| +| Package Name | OK | Alternative | +| --------------------------------- | --- | ---------------------------- | | path_smoother | ❌ | autoware_path_smoother | | autoware_trajectory_follower_node | ❌ | autoware_trajectory_follower | | autoware_geography_utils | ✅ | - | @@ -32,12 +32,12 @@ The package folder name should be the same as the package name. #### `package.xml` - The package name should be entered within the `` tag. - - `autoware_gnss_poser` + - `autoware_gnss_poser` #### `CMakeLists.txt` - The [`project()`](https://cmake.org/cmake/help/latest/command/project.html) command should call the package name. - - **Example:** `project(autoware_gnss_poser)` + - **Example:** `project(autoware_gnss_poser)` ##### Exporting a component @@ -108,10 +108,10 @@ autoware_gnss_poser - Put the header files in the `include` directory under the `src` directory. - 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. + - 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. #### Exporting headers @@ -125,9 +125,9 @@ autoware_gnss_poser ``` - `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/humble/include/` directory. This structure is used to avoid conflicts with non-Autoware packages. + - **Rationale:** When installing ROS debian packages, the headers are copied to the `/opt/ros/humble/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. + - **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, @@ -146,9 +146,9 @@ autoware_gnss_poser - 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. + - **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. + - **Rationale:** To avoid verbosity. ### `test` From 234860c4b83d21fa03b95028fabe6a9fd2c77ec1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?M=2E=20Fatih=20C=C4=B1r=C4=B1t?= Date: Wed, 8 May 2024 21:02:53 +0300 Subject: [PATCH 03/13] add entire structure for ease of use MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: M. Fatih Cırıt --- .../ros-nodes/directory-structure.md | 49 +++++++++++++++++-- 1 file changed, 46 insertions(+), 3 deletions(-) diff --git a/docs/contributing/coding-guidelines/ros-nodes/directory-structure.md b/docs/contributing/coding-guidelines/ros-nodes/directory-structure.md index 66ff70d79e6..489c0287c13 100644 --- a/docs/contributing/coding-guidelines/ros-nodes/directory-structure.md +++ b/docs/contributing/coding-guidelines/ros-nodes/directory-structure.md @@ -6,6 +6,49 @@ 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 +autoware_gnss_poser +├─ package.xml +├─ CMakeLists.txt +├─ README.md +│ +├─ config +│ ├─ gnss_poser.param.yaml +│ └─ another_non_ros_config.yaml +│ +├─ schema +│ └─ gnss_poser.schema.json +│ +├─ doc +│ ├─ foo_document.md +│ └─ foo_diagram.svg +│ +├─ include # for exporting headers +│ └─ autoware +│ └─ gnss_poser +│ └─ exported_header.hpp +│ +├─ src +│ ├─ 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_`. @@ -71,7 +114,7 @@ autoware_gnss_poser - ROS parameters uses the extension `.param.yaml`. - Non-ROS parameters use the extension `.yaml`. -Rationale: Different linting rules are used for ROS parameters and non-ROS parameters. +**Rationale:** Different linting rules are used for ROS parameters and non-ROS parameters. #### `schema` @@ -125,7 +168,7 @@ autoware_gnss_poser ``` - `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/humble/include/` directory. This structure is used to avoid conflicts with non-Autoware packages. + - **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. @@ -155,7 +198,7 @@ autoware_gnss_poser ```txt autoware_gnss_poser └─ test - ├─ test_foo.hpp # or place under an `include` folder here + ├─ test_foo.hpp # or place under an `include` folder here └─ test_foo.cpp ``` From 858c9a53018d3ba7fca89044432bf2b22383af9d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?M=2E=20Fatih=20C=C4=B1r=C4=B1t?= Date: Wed, 8 May 2024 21:30:12 +0300 Subject: [PATCH 04/13] clarify node name MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: M. Fatih Cırıt --- .../contributing/coding-guidelines/ros-nodes/class-design.md | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/docs/contributing/coding-guidelines/ros-nodes/class-design.md b/docs/contributing/coding-guidelines/ros-nodes/class-design.md index 4e02e886f7a..23180e0b520 100644 --- a/docs/contributing/coding-guidelines/ros-nodes/class-design.md +++ b/docs/contributing/coding-guidelines/ros-nodes/class-design.md @@ -43,7 +43,10 @@ GNSSPoserNode::GNSSPoserNode(const rclcpp::NodeOptions & node_options) - Node classes should inherit from `rclcpp::Node`. - The constructor should be explicit. - The constructor should take `rclcpp::NodeOptions` as an argument. -- Default node name should be `gnss_poser_node`. +- Default node name: + - should not have `autoware_` prefix. + - should have `_node` suffix. + - **Example:** `gnss_poser_node`. ##### Component registration From c59e128a2dc1482f65e0acc274f34ff409da70c0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?M=2E=20Fatih=20C=C4=B1r=C4=B1t?= Date: Thu, 9 May 2024 11:16:48 +0300 Subject: [PATCH 05/13] update cpp naming to gnss_poser_node.cpp MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: M. Fatih Cırıt --- .../coding-guidelines/ros-nodes/class-design.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/contributing/coding-guidelines/ros-nodes/class-design.md b/docs/contributing/coding-guidelines/ros-nodes/class-design.md index 23180e0b520..6e6ea1c4bb5 100644 --- a/docs/contributing/coding-guidelines/ros-nodes/class-design.md +++ b/docs/contributing/coding-guidelines/ros-nodes/class-design.md @@ -18,7 +18,7 @@ namespace autoware::gnss_poser ### Nodes -#### `gnss_poser.hpp` +#### `gnss_poser_node.hpp` ```cpp class GNSSPoserNode : public rclcpp::Node @@ -29,7 +29,7 @@ class GNSSPoserNode : public rclcpp::Node } ``` -#### `gnss_poser.cpp` +#### `gnss_poser_node.cpp` ```cpp GNSSPoserNode::GNSSPoserNode(const rclcpp::NodeOptions & node_options) @@ -58,7 +58,7 @@ GNSSPoserNode::GNSSPoserNode(const rclcpp::NodeOptions & node_options) RCLCPP_COMPONENTS_REGISTER_NODE(autoware::gnss_poser::GNSSPoserNode) ``` -- The component should be registered at the end of the `autoware_gnss_poser.cpp` file, outside the namespaces. +- The component should be registered at the end of the `gnss_poser_node.cpp` file, outside the namespaces. ### Libraries From 77b3039a7302445d48aee511f7b9eb0a9850dd4b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?M=2E=20Fatih=20C=C4=B1r=C4=B1t?= Date: Thu, 9 May 2024 11:22:55 +0300 Subject: [PATCH 06/13] remove `_node` from node names MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: M. Fatih Cırıt --- .../coding-guidelines/ros-nodes/class-design.md | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/docs/contributing/coding-guidelines/ros-nodes/class-design.md b/docs/contributing/coding-guidelines/ros-nodes/class-design.md index 6e6ea1c4bb5..2d1483913e3 100644 --- a/docs/contributing/coding-guidelines/ros-nodes/class-design.md +++ b/docs/contributing/coding-guidelines/ros-nodes/class-design.md @@ -33,7 +33,7 @@ class GNSSPoserNode : public rclcpp::Node ```cpp GNSSPoserNode::GNSSPoserNode(const rclcpp::NodeOptions & node_options) -: Node("gnss_poser_node", node_options) +: Node("gnss_poser", node_options) { ... } @@ -45,8 +45,9 @@ GNSSPoserNode::GNSSPoserNode(const rclcpp::NodeOptions & node_options) - The constructor should take `rclcpp::NodeOptions` as an argument. - Default node name: - should not have `autoware_` prefix. - - should have `_node` suffix. - - **Example:** `gnss_poser_node`. + - 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 From 52bf7bbb42cb71e5bce9aba520a689ed1296610f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?M=2E=20Fatih=20C=C4=B1r=C4=B1t?= Date: Thu, 9 May 2024 12:01:00 +0300 Subject: [PATCH 07/13] fix autoware_ prefix MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: M. Fatih Cırıt --- .../coding-guidelines/ros-nodes/directory-structure.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/contributing/coding-guidelines/ros-nodes/directory-structure.md b/docs/contributing/coding-guidelines/ros-nodes/directory-structure.md index 489c0287c13..c93f90bbd86 100644 --- a/docs/contributing/coding-guidelines/ros-nodes/directory-structure.md +++ b/docs/contributing/coding-guidelines/ros-nodes/directory-structure.md @@ -153,7 +153,7 @@ autoware_gnss_poser - The source file exporting the node should: - have `_node` suffix. - **Rationale:** To distinguish from other source files. - - **NOT** have `_autoware` prefix. + - **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. From cede324ceeb2a45dbe36f68cb75feec0566b847b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?M=2E=20Fatih=20C=C4=B1r=C4=B1t?= Date: Thu, 9 May 2024 12:01:21 +0300 Subject: [PATCH 08/13] allow devs to arrange inside `src` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: M. Fatih Cırıt --- .../ros-nodes/directory-structure.md | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/docs/contributing/coding-guidelines/ros-nodes/directory-structure.md b/docs/contributing/coding-guidelines/ros-nodes/directory-structure.md index c93f90bbd86..33754159377 100644 --- a/docs/contributing/coding-guidelines/ros-nodes/directory-structure.md +++ b/docs/contributing/coding-guidelines/ros-nodes/directory-structure.md @@ -147,15 +147,25 @@ autoware_gnss_poser │ └─ 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 ``` -- Put the header files in the `include` directory under the `src` directory. - 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 From e38f3a2d556b3d4fa5353bff18ff6b309290abac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?M=2E=20Fatih=20C=C4=B1r=C4=B1t?= Date: Thu, 9 May 2024 12:53:37 +0300 Subject: [PATCH 09/13] take non-composable nodes into account MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: M. Fatih Cırıt --- .../ros-nodes/directory-structure.md | 26 ++++++++++++++++--- 1 file changed, 23 insertions(+), 3 deletions(-) diff --git a/docs/contributing/coding-guidelines/ros-nodes/directory-structure.md b/docs/contributing/coding-guidelines/ros-nodes/directory-structure.md index 33754159377..90597497c6f 100644 --- a/docs/contributing/coding-guidelines/ros-nodes/directory-structure.md +++ b/docs/contributing/coding-guidelines/ros-nodes/directory-structure.md @@ -82,7 +82,7 @@ The package folder name should be the same as the package name. - The [`project()`](https://cmake.org/cmake/help/latest/command/project.html) command should call the package name. - **Example:** `project(autoware_gnss_poser)` -##### Exporting a component +##### Exporting a composable node component executable ```cmake ament_auto_add_library(${PROJECT_NAME} SHARED @@ -95,8 +95,28 @@ rclcpp_components_register_node(${PROJECT_NAME} ) ``` -- The component executable should have `_node` suffix. -- The component executable name should start with `${PROJECT_NAME}` +- The composable node component executable: + - should have `_node` suffix. + - should start with `${PROJECT_NAME}` + +##### Exporting a standalone node executable + +Assuming: +- `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` From deb3a3fc48c0f48742d42a0fd0628ac88da98e70 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Thu, 9 May 2024 09:54:04 +0000 Subject: [PATCH 10/13] style(pre-commit): autofix --- .../coding-guidelines/ros-nodes/directory-structure.md | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/contributing/coding-guidelines/ros-nodes/directory-structure.md b/docs/contributing/coding-guidelines/ros-nodes/directory-structure.md index 90597497c6f..850672160e7 100644 --- a/docs/contributing/coding-guidelines/ros-nodes/directory-structure.md +++ b/docs/contributing/coding-guidelines/ros-nodes/directory-structure.md @@ -102,6 +102,7 @@ rclcpp_components_register_node(${PROJECT_NAME} ##### Exporting a standalone node executable Assuming: + - `src/gnss_poser.cpp` has the `GNSSPoserNode` class. - `src/gnss_poser_node.cpp` has the `main` function. - There is no composable node component registration. From fea457f9918457fb8c5eb22adb2ec92ee89e8928 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?M=2E=20Fatih=20C=C4=B1r=C4=B1t?= Date: Thu, 9 May 2024 13:01:45 +0300 Subject: [PATCH 11/13] s/should/must updates MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: M. Fatih Cırıt --- .../coding-guidelines/ros-nodes/class-design.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/contributing/coding-guidelines/ros-nodes/class-design.md b/docs/contributing/coding-guidelines/ros-nodes/class-design.md index 2d1483913e3..21ae9c36c9a 100644 --- a/docs/contributing/coding-guidelines/ros-nodes/class-design.md +++ b/docs/contributing/coding-guidelines/ros-nodes/class-design.md @@ -12,7 +12,7 @@ namespace autoware::gnss_poser ``` - Everything should be under `autoware::gnss_poser` namespace. -- Closing braces should contain a comment with the namespace name. +- Closing braces must contain a comment with the namespace name. (Automated with `cpplint`) ## Classes @@ -41,8 +41,8 @@ GNSSPoserNode::GNSSPoserNode(const rclcpp::NodeOptions & node_options) - The class name should be in `CamelCase`. - Node classes should inherit from `rclcpp::Node`. -- The constructor should be explicit. -- The constructor should take `rclcpp::NodeOptions` as an argument. +- 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. From d96bd27b2552042c4ff63ca17a5532ac29d132de Mon Sep 17 00:00:00 2001 From: "Takagi, Isamu" Date: Thu, 9 May 2024 19:57:51 +0900 Subject: [PATCH 12/13] update exporting component section Signed-off-by: Takagi, Isamu --- .../ros-nodes/directory-structure.md | 27 +++---------------- 1 file changed, 3 insertions(+), 24 deletions(-) diff --git a/docs/contributing/coding-guidelines/ros-nodes/directory-structure.md b/docs/contributing/coding-guidelines/ros-nodes/directory-structure.md index 850672160e7..78d4ebf2c74 100644 --- a/docs/contributing/coding-guidelines/ros-nodes/directory-structure.md +++ b/docs/contributing/coding-guidelines/ros-nodes/directory-structure.md @@ -82,7 +82,7 @@ The package folder name should be the same as the package name. - 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 +##### Exporting a composable node as a standalone node executable ```cmake ament_auto_add_library(${PROJECT_NAME} SHARED @@ -95,29 +95,8 @@ rclcpp_components_register_node(${PROJECT_NAME} ) ``` -- The composable node component executable: - - should have `_node` suffix. - - should start with `${PROJECT_NAME}` - -##### Exporting a standalone node executable - -Assuming: - -- `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}` +- The composable node executable should have `_node` suffix. +- The composable node executable should start with `${PROJECT_NAME}` ### `config` and `schema` From 9ba71b95d580d47e711653315fd43c8616161241 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?M=2E=20Fatih=20C=C4=B1r=C4=B1t?= Date: Thu, 9 May 2024 14:27:15 +0300 Subject: [PATCH 13/13] handle all executable types MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: M. Fatih Cırıt --- .../ros-nodes/directory-structure.md | 36 +++++++++++++++++-- 1 file changed, 33 insertions(+), 3 deletions(-) diff --git a/docs/contributing/coding-guidelines/ros-nodes/directory-structure.md b/docs/contributing/coding-guidelines/ros-nodes/directory-structure.md index 78d4ebf2c74..794379fbf74 100644 --- a/docs/contributing/coding-guidelines/ros-nodes/directory-structure.md +++ b/docs/contributing/coding-guidelines/ros-nodes/directory-structure.md @@ -82,7 +82,11 @@ The package folder name should be the same as the package name. - 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 as a standalone node executable +##### 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 @@ -95,8 +99,34 @@ rclcpp_components_register_node(${PROJECT_NAME} ) ``` -- The composable node executable should have `_node` suffix. -- The composable node executable should start with `${PROJECT_NAME}` +- 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: + +- `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`