From 8effe7d4d03d5ad9dc716f700dcb1132eb3a591b Mon Sep 17 00:00:00 2001 From: Thomas Stastny Date: Wed, 30 Mar 2022 16:55:17 +0200 Subject: [PATCH 1/7] Contribute: update style guide to google + (minimal) exceptions --- en/contribute/code.md | 69 ++++++++++++++++++++++++++++++++++--------- 1 file changed, 55 insertions(+), 14 deletions(-) diff --git a/en/contribute/code.md b/en/contribute/code.md index 410b3900ae1a..98acdb67032a 100644 --- a/en/contribute/code.md +++ b/en/contribute/code.md @@ -16,26 +16,66 @@ Changes will be merged when they pass our [continuous integration](https://en.wi All code contributions have to be under the permissive [BSD 3-clause license](https://opensource.org/licenses/BSD-3-Clause) and all code must not impose any further constraints on the use. -## Code Style Formatting +## Code Style -PX4 uses [astyle](http://astyle.sourceforge.net/) for code formatting. Valid versions are -* [astyle 2.06](https://sourceforge.net/projects/astyle/files/astyle/astyle%202.06/) (deprecated) -* [astyle 3.0](https://sourceforge.net/projects/astyle/files/astyle/astyle%203.0/) -* [astyle 3.01](https://sourceforge.net/projects/astyle/files/) (recommended) +PX4 uses the [Google C++ style guide](https://google.github.io/styleguide/cppguide.html), with the following (minimal) modifications: -Once installed, formatting can be checked with `./Tools/astyle/check_code_style_all.sh`. The output should be `Format checks passed` on a clean master. If that worked, `make format` can be used in the future to check and format all files automatically. +### Tabs -## File name conventions +- Tabs are used for indentation (equivalent to 8 spaces). +- Spaces are used for alignment. -Going forward we aim to follow these file naming conventions: +### Line Length -- C++ source files should be named in CamelCase and match the class name. E.g. A C++ file containing a class named `FooThing` should be named `FooThing.cpp`. -- C++ header files should be named the same as source files except have the suffix `.hpp`. -- C++ header files that are required to be C compatible, should have the suffix `.h`. -- Folder names are `snake_case` for the first level inside `modules`/`drivers`/`systemcmds`/etc. but should be named CamelCase when more deeply nested to match the source and header files. -- Test files must have a `Test` suffix as shown: `FooThingTest.cpp`. +- Maximum line length is 120 characters. -- One exception to the rules above are the MAVLink streams in [src/modules/mavlink/streams](https://github.com/PX4/PX4-Autopilot/tree/main/src/modules/mavlink/streams) which are ALL_UPPERCASE.hpp matching the MAVLink message name. +### File Extensions + +- Source files use extension `*.cpp` instead of `*.cc` + +- An exception to the rule above are the MAVLink streams in [src/modules/mavlink/streams](https://github.com/PX4/PX4-Autopilot/tree/main/src/modules/mavlink/streams) which are ALL_UPPERCASE.hpp matching the MAVLink message name. + +### Function and Method Names + +- `lowerCamelCase()` is used for functions and methods to *visually* distinguish them from `ClassConstructors()` and `ClassNames`. + +### Class Privacy Keywords + +- *zero* spaces before `public:`, `private:`, or `protected:` keywords. + +### Example Code Snippet + +``` +class MyClass { +public: + + /** + * @breif Description of what this function does. + * + * @param[in] input_param Clear description of the input [units] + * @return Whatever we are returning [units] + */ + float doSomething(const float input_param) const { + const float in_scope_variable = input_param + kConstantFloat; + return in_scope_variable * private_member_variable_; + } + + void setPrivateMember(const float private_member_variable) { private_member_variable_ = private_member_variable; } + + /** + * @return Whatever we are "getting" [units] + */ + float getPrivateMember() const { return private_member_variable_; } + +private: + + // Clear description of the constant if not completely obvious from the name [units] + static constexpr float kConstantFloat = ...; + + // Clear description of the variable if not completely obvious from the name [units] + float private_member_variable_{...}; +}; +``` ## In-Source Documentation @@ -57,6 +97,7 @@ Currently we have two types of source-based documentation: ::: - Do not add documentation that can trivially be assumed from C++ entity names. + - ALWAYS specify units of variables, constants, and input/return parameters where they are defined - Commonly you may want to add information about corner cases and error handling. - [Doxgyen](http://www.doxygen.nl/) tags should be used if documentation is needed: `@class`, `@file`, `@param`, `@return`, `@brief`, `@var`, `@see`, `@note`. A good example of usage is [src/modules/events/send_event.h](https://github.com/PX4/PX4-Autopilot/blob/main/src/modules/events/send_event.h). From 28d96802cf37f4a900c09e283c24ffe7e4218429 Mon Sep 17 00:00:00 2001 From: Hamish Willee Date: Fri, 1 Jul 2022 09:51:22 +1000 Subject: [PATCH 2/7] Update en/contribute/code.md Co-authored-by: Matthias Grob --- en/contribute/code.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/en/contribute/code.md b/en/contribute/code.md index 98acdb67032a..0d365e21c21f 100644 --- a/en/contribute/code.md +++ b/en/contribute/code.md @@ -50,7 +50,7 @@ class MyClass { public: /** - * @breif Description of what this function does. + * @brief Description of what this function does. * * @param[in] input_param Clear description of the input [units] * @return Whatever we are returning [units] From 71b1e7e6a5afd7edec5a92952624d965bfd9fb51 Mon Sep 17 00:00:00 2001 From: Thomas Stastny Date: Fri, 1 Jul 2022 09:32:09 +0200 Subject: [PATCH 3/7] en/contribute/code.md: use cpp style for code block --- en/contribute/code.md | 40 ++++++++++++++++++++-------------------- 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/en/contribute/code.md b/en/contribute/code.md index 0d365e21c21f..5c878cdb1586 100644 --- a/en/contribute/code.md +++ b/en/contribute/code.md @@ -45,35 +45,35 @@ PX4 uses the [Google C++ style guide](https://google.github.io/styleguide/cppgui ### Example Code Snippet -``` +```cpp class MyClass { public: - /** - * @brief Description of what this function does. - * - * @param[in] input_param Clear description of the input [units] - * @return Whatever we are returning [units] - */ - float doSomething(const float input_param) const { - const float in_scope_variable = input_param + kConstantFloat; - return in_scope_variable * private_member_variable_; - } + /** + * @brief Description of what this function does. + * + * @param[in] input_param Clear description of the input [units] + * @return Whatever we are returning [units] + */ + float doSomething(const float input_param) const { + const float in_scope_variable = input_param + kConstantFloat; + return in_scope_variable * private_member_variable_; + } - void setPrivateMember(const float private_member_variable) { private_member_variable_ = private_member_variable; } + void setPrivateMember(const float private_member_variable) { private_member_variable_ = private_member_variable; } - /** - * @return Whatever we are "getting" [units] - */ - float getPrivateMember() const { return private_member_variable_; } + /** + * @return Whatever we are "getting" [units] + */ + float getPrivateMember() const { return private_member_variable_; } private: - // Clear description of the constant if not completely obvious from the name [units] - static constexpr float kConstantFloat = ...; + // Clear description of the constant if not completely obvious from the name [units] + static constexpr float kConstantFloat = ...; - // Clear description of the variable if not completely obvious from the name [units] - float private_member_variable_{...}; + // Clear description of the variable if not completely obvious from the name [units] + float private_member_variable_{...}; }; ``` From 11b39560057186f7ae9281a76f785328abbcd19c Mon Sep 17 00:00:00 2001 From: Thomas Stastny Date: Fri, 1 Jul 2022 10:44:49 +0200 Subject: [PATCH 4/7] en/contribute/code.md: grammar --- en/contribute/code.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/en/contribute/code.md b/en/contribute/code.md index 5c878cdb1586..2721047e2a47 100644 --- a/en/contribute/code.md +++ b/en/contribute/code.md @@ -97,7 +97,7 @@ Currently we have two types of source-based documentation: ::: - Do not add documentation that can trivially be assumed from C++ entity names. - - ALWAYS specify units of variables, constants, and input/return parameters where they are defined + - ALWAYS specify units of variables, constants, and input/return parameters where they are defined. - Commonly you may want to add information about corner cases and error handling. - [Doxgyen](http://www.doxygen.nl/) tags should be used if documentation is needed: `@class`, `@file`, `@param`, `@return`, `@brief`, `@var`, `@see`, `@note`. A good example of usage is [src/modules/events/send_event.h](https://github.com/PX4/PX4-Autopilot/blob/main/src/modules/events/send_event.h). From 20afdba72505cfd4dda9121f7fe1356f30660326 Mon Sep 17 00:00:00 2001 From: Thomas Stastny Date: Fri, 1 Jul 2022 10:45:53 +0200 Subject: [PATCH 5/7] en/contribute/code.md: chastise magic numbers --- en/contribute/code.md | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/en/contribute/code.md b/en/contribute/code.md index 2721047e2a47..d6e055b4c221 100644 --- a/en/contribute/code.md +++ b/en/contribute/code.md @@ -101,6 +101,33 @@ Currently we have two types of source-based documentation: - Commonly you may want to add information about corner cases and error handling. - [Doxgyen](http://www.doxygen.nl/) tags should be used if documentation is needed: `@class`, `@file`, `@param`, `@return`, `@brief`, `@var`, `@see`, `@note`. A good example of usage is [src/modules/events/send_event.h](https://github.com/PX4/PX4-Autopilot/blob/main/src/modules/events/send_event.h). +Please avoid "magic numbers", for example, where does this number in the conditional come from? What about the multiplier on yaw stick input? + ```cpp + if (fabsf(yaw_stick_normalized_input) < 0.1f) { + yaw_rate_setpoint = 0.0f; + } + else { + yaw_rate_setpoint = 0.52f * yaw_stick_normalized_input; + } + ``` + Instead, define the numbers as named constants with appropriate context in the header + ```cpp + // Deadzone threshold for normalized yaw stick input + static constexpr float kYawStickDeadzone = 0.1f; + + // [rad/s] Deadzone threshold for normalized yaw stick input + static constexpr float kMaxYawRate = math::radians(30.0f); + ``` + and update the source implementation. + ```cpp + if (fabsf(yaw_stick_normalized_input) < kYawStickDeadzone) { + yaw_rate_setpoint = 0.0f; + } + else { + yaw_rate_setpoint = kMaxYawRate * yaw_stick_normalized_input; + } + ``` + ## Commits and Commit Messages Please use descriptive, multi-paragraph commit messages for all non-trivial changes. Structure them well so they make sense in the one-line summary but also provide full detail. From 33fa25ab43519b4b23867ef8c4231382f7ada268 Mon Sep 17 00:00:00 2001 From: Thomas Stastny Date: Fri, 1 Jul 2022 10:46:26 +0200 Subject: [PATCH 6/7] en/contribute/code.md: remove mavlink header exception --- en/contribute/code.md | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/en/contribute/code.md b/en/contribute/code.md index d6e055b4c221..9829e79b15a3 100644 --- a/en/contribute/code.md +++ b/en/contribute/code.md @@ -31,9 +31,7 @@ PX4 uses the [Google C++ style guide](https://google.github.io/styleguide/cppgui ### File Extensions -- Source files use extension `*.cpp` instead of `*.cc` - -- An exception to the rule above are the MAVLink streams in [src/modules/mavlink/streams](https://github.com/PX4/PX4-Autopilot/tree/main/src/modules/mavlink/streams) which are ALL_UPPERCASE.hpp matching the MAVLink message name. +- Source files use extension `*.cpp` instead of `*.cc`. ### Function and Method Names From fb83cae171217927c70f3ad2ba5062b0814eae6f Mon Sep 17 00:00:00 2001 From: Hamish Willee Date: Thu, 7 Jul 2022 09:43:26 +1000 Subject: [PATCH 7/7] Minor subedit for indentation --- en/contribute/code.md | 64 +++++++++++++++++++++++-------------------- 1 file changed, 35 insertions(+), 29 deletions(-) diff --git a/en/contribute/code.md b/en/contribute/code.md index 9829e79b15a3..578c992e15e7 100644 --- a/en/contribute/code.md +++ b/en/contribute/code.md @@ -88,47 +88,53 @@ Currently we have two types of source-based documentation: - `PRINT_MODULE_*` methods are used for both module run time usage instructions and for the [Modules & Commands Reference](../modules/modules_main.md) in this guide. - The API is documented [in the source code here](https://github.com/PX4/PX4-Autopilot/blob/v1.8.0/src/platforms/px4_module.h#L381). - Good examples of usage include the [Application/Module Template](../modules/module_template.md) and the files linked from the modules reference. -* We encourage other in-source documentation *where it adds value/is not redundant*. +- We encourage other in-source documentation *where it adds value/is not redundant*. :::tip Developers should name C++ entities (classes, functions, variables etc.) such that their purpose can be inferred - reducing the need for explicit documentation. ::: - - Do not add documentation that can trivially be assumed from C++ entity names. + - Do not add documentation that can trivially be inferred from C++ entity names. - ALWAYS specify units of variables, constants, and input/return parameters where they are defined. - Commonly you may want to add information about corner cases and error handling. - - [Doxgyen](http://www.doxygen.nl/) tags should be used if documentation is needed: `@class`, `@file`, `@param`, `@return`, `@brief`, `@var`, `@see`, `@note`. A good example of usage is [src/modules/events/send_event.h](https://github.com/PX4/PX4-Autopilot/blob/main/src/modules/events/send_event.h). + - [Doxgyen](http://www.doxygen.nl/) tags should be used if documentation is needed: `@class`, `@file`, `@param`, `@return`, `@brief`, `@var`, `@see`, `@note`. + A good example of usage is [src/modules/events/send_event.h](https://github.com/PX4/PX4-Autopilot/blob/main/src/modules/events/send_event.h). Please avoid "magic numbers", for example, where does this number in the conditional come from? What about the multiplier on yaw stick input? - ```cpp - if (fabsf(yaw_stick_normalized_input) < 0.1f) { - yaw_rate_setpoint = 0.0f; - } - else { - yaw_rate_setpoint = 0.52f * yaw_stick_normalized_input; - } - ``` - Instead, define the numbers as named constants with appropriate context in the header - ```cpp - // Deadzone threshold for normalized yaw stick input - static constexpr float kYawStickDeadzone = 0.1f; - - // [rad/s] Deadzone threshold for normalized yaw stick input - static constexpr float kMaxYawRate = math::radians(30.0f); - ``` - and update the source implementation. - ```cpp - if (fabsf(yaw_stick_normalized_input) < kYawStickDeadzone) { - yaw_rate_setpoint = 0.0f; - } - else { - yaw_rate_setpoint = kMaxYawRate * yaw_stick_normalized_input; - } - ``` + +```cpp +if (fabsf(yaw_stick_normalized_input) < 0.1f) { + yaw_rate_setpoint = 0.0f; +} +else { + yaw_rate_setpoint = 0.52f * yaw_stick_normalized_input; +} +``` + +Instead, define the numbers as named constants with appropriate context in the header: + +```cpp +// Deadzone threshold for normalized yaw stick input +static constexpr float kYawStickDeadzone = 0.1f; + +// [rad/s] Deadzone threshold for normalized yaw stick input +static constexpr float kMaxYawRate = math::radians(30.0f); +``` + +and update the source implementation. +```cpp +if (fabsf(yaw_stick_normalized_input) < kYawStickDeadzone) { + yaw_rate_setpoint = 0.0f; +} +else { + yaw_rate_setpoint = kMaxYawRate * yaw_stick_normalized_input; +} +``` ## Commits and Commit Messages -Please use descriptive, multi-paragraph commit messages for all non-trivial changes. Structure them well so they make sense in the one-line summary but also provide full detail. +Please use descriptive, multi-paragraph commit messages for all non-trivial changes. +Structure them well so they make sense in the one-line summary but also provide full detail. ``` Component: Explain the change in one sentence. Fixes #1234