Updated coding style documentation.

This commit is contained in:
mikeller 2018-12-29 23:56:04 +13:00 committed by Michael Keller
parent 00e0248988
commit fda4bd2124
4 changed files with 70 additions and 57 deletions

View File

@ -97,7 +97,7 @@ Contributions are welcome and encouraged. You can contribute in many ways:
* Bug reporting & fixes. * Bug reporting & fixes.
* New feature ideas & suggestions. * New feature ideas & suggestions.
The best place to start is the IRC channel on gitter (see above), drop in, say hi. Next place is the github issue tracker: The best place to start is the Betaflight Slack (registration [here](https://slack.betaflight.com/). Next place is the github issue tracker:
https://github.com/betaflight/betaflight/issues https://github.com/betaflight/betaflight/issues
https://github.com/betaflight/betaflight-configurator/issues https://github.com/betaflight/betaflight-configurator/issues
@ -106,7 +106,8 @@ Before creating new issues please check to see if there is an existing one, sear
## Developers ## Developers
Please refer to the development section in the `docs/development` folder. Contribution of bugfixes and new features is encouraged. Please be aware that we have a thorough review process for pull requests, and be prepared to explain what you want to achieve with your pull request.
Before starting to write code, please read our [development guidelines](docs/development/Development.md ) and [coding style definition](docs/development/CodingStyle.md).
TravisCI is used to run automatic builds TravisCI is used to run automatic builds
@ -127,9 +128,9 @@ Betaflight is forked from Cleanflight, so thanks goes to all those whom have con
Origins for this fork (Thanks!): Origins for this fork (Thanks!):
* **Alexinparis** (for MultiWii), * **Alexinparis** (for MultiWii),
* **timecop** (for Baseflight), * **timecop** (for Baseflight),
* **Dominic Clifton** (for Cleanflight), and * **Dominic Clifton** (for Cleanflight),
* **borisbstyle** (for Betaflight), and
* **Sambas** (for the original STM32F4 port). * **Sambas** (for the original STM32F4 port).
* **borisbstyle** (Fork from Cleanflight).
The Betaflight Configurator is forked from Cleanflight Configurator and its origins. The Betaflight Configurator is forked from Cleanflight Configurator and its origins.

View File

@ -1,15 +1,18 @@
# General # General
This document overrides the original Baseflight style that was referenced before. This document overrides the original Baseflight style that was referenced before.
This document has taken inspiration from that style, from Eclipse defaults and from Linux, as well as from some Cleanflight developers and existing code. This document has taken inspiration from that style, from Eclipse defaults and from Linux, as well as from some Cleanflight and Betaflight developers and existing code.
There are not so many changes from the old style, if you managed to find it. There are not so many changes from the old style, if you managed to find it.
# Formatting style # Formatting style
## Indentation ## Indentation
K&R indent style with 4 space indent, NO hard tabs (all tabs replaced by spaces).
[1TBS](https://en.wikipedia.org/wiki/Indentation_style#Variant:_1TBS_(OTBS)) (based K&R) indent style with 4 space indent, NO hard tabs (all tabs replaced by spaces).
## Tool support ## Tool support
Any of these tools can get you pretty close: Any of these tools can get you pretty close:
Eclipse built in "K&R" style, after changing the indent to 4 spaces and change Braces after function declarations to Next line. Eclipse built in "K&R" style, after changing the indent to 4 spaces and change Braces after function declarations to Next line.
@ -27,6 +30,7 @@ Sometimes, for example, you may want other columns and line breaks so it looks l
Note2: The Astyle settings have been tested and will produce a nice result. Many files will be changed, mostly to the better but maybe not always, so use with care. Note2: The Astyle settings have been tested and will produce a nice result. Many files will be changed, mostly to the better but maybe not always, so use with care.
## Curly Braces ## Curly Braces
Functions shall have the opening brace at the beginning of the next line. Functions shall have the opening brace at the beginning of the next line.
``` ```
int function(int x) int function(int x)
@ -72,6 +76,7 @@ if (x is true) {
Omission of "unnecessary" braces in cases where an `if` or `else` block consists only of a single statement is not permissible in any case. These "single statement blocks" are future bugs waiting to happen when more statements are added without enclosing the block in braces. Omission of "unnecessary" braces in cases where an `if` or `else` block consists only of a single statement is not permissible in any case. These "single statement blocks" are future bugs waiting to happen when more statements are added without enclosing the block in braces.
## Spaces ## Spaces
Use a space after (most) keywords. The notable exceptions are sizeof, typeof, alignof, and __attribute__, which look somewhat like functions (and are usually used with parentheses). Use a space after (most) keywords. The notable exceptions are sizeof, typeof, alignof, and __attribute__, which look somewhat like functions (and are usually used with parentheses).
So use a space after these keywords: So use a space after these keywords:
``` ```
@ -108,6 +113,7 @@ and no space around the '.' and "->" structure member operators.
'*' and '&', when used for pointer and reference, shall have no space between it and the following variable name. '*' and '&', when used for pointer and reference, shall have no space between it and the following variable name.
# typedef # typedef
enums that do not have a count or some other form of terminator element shall have a comma after their last element: enums that do not have a count or some other form of terminator element shall have a comma after their last element:
``` ```
@ -131,7 +137,7 @@ typedef enum {
PID_COUNT PID_COUNT
} pidControllerType_e; } pidControllerType_e;
``` ```
It shall not be calculated afterwards, e.g. using PID_CONTROLLER_LUX_FLOAT + 1; It shall not be calculated afterwards, e.g. using PID\_CONTROLLER\_LUX\_FLOAT + 1;
typedef struct definitions should include the struct name, so that the type can be forward referenced, that is in: typedef struct definitions should include the struct name, so that the type can be forward referenced, that is in:
``` ```
@ -141,13 +147,15 @@ typedef struct motorMixer_s {
float yaw; float yaw;
} motorMixer_t; } motorMixer_t;
``` ```
the motorMixer_s name is required. the motorMixer\_s name is required.
# Variables # Variables
## Naming ## Naming
Generally, descriptive lowerCamelCase names are preferred for function names, variables, arguments, etc. Generally, descriptive lowerCamelCase names are preferred for function names, variables, arguments, etc.
For configuration variables that are user accessible via CLI or similar, all_lowercase with underscore is preferred. For configuration variables that are user accessible via CLI or similar, all\_lowercase with underscore is preferred.
Variable names should be nouns. Variable names should be nouns.
@ -156,6 +164,7 @@ Such as "i" as a temporary counter in a `for` loop, like `for (int i = 0; i < 4;
Using "temporaryCounter" in that case would not improve readability. Using "temporaryCounter" in that case would not improve readability.
## Declarations ## Declarations
Avoid global variables. Avoid global variables.
Variables should be declared at the top of the smallest scope where the variable is used. Variables should be declared at the top of the smallest scope where the variable is used.
@ -168,6 +177,7 @@ For example to limit variable scope to a single `case` branch.
Variables with limited use may be declared at the point of first use. It makes PR-review easier (but that point is lost if the variable is used everywhere anyway). Variables with limited use may be declared at the point of first use. It makes PR-review easier (but that point is lost if the variable is used everywhere anyway).
## Initialisation ## Initialisation
The pattern with "lazy initialisation" may be advantageous in the Configurator to speed up the start when the initialisation is "expensive" in some way. The pattern with "lazy initialisation" may be advantageous in the Configurator to speed up the start when the initialisation is "expensive" in some way.
In the FC, however, its always better to use some milliseconds extra before take-off than to use them while flying. In the FC, however, its always better to use some milliseconds extra before take-off than to use them while flying.
@ -176,22 +186,24 @@ So don't use "lazy initialisation".
An explicit "init" function, is preferred. An explicit "init" function, is preferred.
## Data types ## Data types
Be aware of the data types you use and do not trust implicit type casting to be correct. Be aware of the data types you use and do not trust implicit type casting to be correct.
Angles are sometimes represented as degrees in a float. Sometimes as decidegrees in a uint8_t. Angles are sometimes represented as degrees in a float. Sometimes as decidegrees in a uint8\_t.
You have been warned. You have been warned.
Avoid implicit double conversions and only use float-argument functions. Avoid implicit double conversions and only use float-argument functions.
Check .map file to make sure no conversions sneak in, and use -Wdouble-promotion warning for the compiler Check .map file to make sure no conversions sneak in, and use -Wdouble-promotion warning for the compiler
Instead of sin() and cos(), use sin_approx() and cos_approx() from common/math.h. Instead of sin() and cos(), use sin\_approx() and cos\_approx() from common/math.h.
Float constants should be defined with "f" suffix, like 1.0f and 3.1415926f, otherwise double conversion might occur. Float constants should be defined with "f" suffix, like 1.0f and 3.1415926f, otherwise double conversion might occur.
# Functions # Functions
## Naming ## Naming
Methods that return a boolean should be named as a question, and should not change any state. e.g. 'isOkToArm()'. Methods that return a boolean should be named as a question, and should not change any state. e.g. 'isOkToArm()'.
Methods should have verb or verb-phrase names, like deletePage or save. Tell the system to 'do' something 'with' something. e.g. deleteAllPages(pageList). Methods should have verb or verb-phrase names, like deletePage or save. Tell the system to 'do' something 'with' something. e.g. deleteAllPages(pageList).
@ -212,7 +224,8 @@ boolean isBiQuadReady();
``` ```
## Parameter order ## Parameter order
Data should move from right to left, as in memcpy(void *dst, const void *src, size_t size).
Data should move from right to left, as in memcpy(void *dst, const void *src, size\_t size).
This also mimics the assignment operator (e.g. dst = src;) This also mimics the assignment operator (e.g. dst = src;)
When a group of functions act on an 'object' then that object should be the first parameter for all the functions, e.g.: When a group of functions act on an 'object' then that object should be the first parameter for all the functions, e.g.:
@ -227,7 +240,8 @@ void biQuadNewLpf(float filterCutFreq, biquad_t *state, uint32_t refreshRate);
``` ```
## Declarations ## Declarations
Functions not used outside their containing .c file should be declared static (or STATIC_UNIT_TESTED so they can be used in unit tests).
Functions not used outside their containing .c file should be declared static (or STATIC\_UNIT\_TESTED so they can be used in unit tests).
Non-static functions should have their declaration in a single .h file. Non-static functions should have their declaration in a single .h file.
@ -244,6 +258,7 @@ In the module .c file, and in the test file but nowhere else, put `#define MODUL
Note: You can get the same effect by putting the internals in a separate .h file. Note: You can get the same effect by putting the internals in a separate .h file.
## Implementation ## Implementation
Keep functions short and distinctive. Keep functions short and distinctive.
Think about unit test when you define your functions. Ideally you should implement the test cases before implementing the function. Think about unit test when you define your functions. Ideally you should implement the test cases before implementing the function.
@ -270,6 +285,7 @@ rather than relying on the implicit logic and operator priority.
The compiler knows what its doing but it should be easy for people too. The compiler knows what its doing but it should be easy for people too.
# Includes # Includes
All files must include their own dependencies and not rely on includes from the included files or that some other file was included first. All files must include their own dependencies and not rely on includes from the included files or that some other file was included first.
Do not include things you are not using. Do not include things you are not using.
@ -278,6 +294,7 @@ Do not include things you are not using.
# Other details # Other details
No trailing whitespace at the end of lines or at blank lines. No trailing whitespace at the end of lines or at blank lines.
Stay within 120 columns, unless exceeding 120 columns significantly increases readability and does not hide information. Stay within 120 columns, unless exceeding 120 columns significantly increases readability and does not hide information.
@ -285,9 +302,9 @@ Stay within 120 columns, unless exceeding 120 columns significantly increases re
Take maximum possible advantage of compile time checking, so generally warnings should be as strict as possible. Take maximum possible advantage of compile time checking, so generally warnings should be as strict as possible.
Don't call or reference "upwards". That is don't call or use anything in a software layer that is above the current layer. The software layers are not that obvious in Cleanflight, but we can certainly say that device drivers are the bottom layer and so should not call or use anything outside the device drivers. Don't call or reference "upwards". That is don't call or use anything in a software layer that is above the current layer. The software layers are not that obvious in Betaflight, but we can certainly say that device drivers are the bottom layer and so should not call or use anything outside the device drivers.
Target specific code (e.g. #ifdef CC3D) should be absolutely minimised. Target specific code (e.g. #ifdef CC3D) is not permissible outside of the `src/main/target` directory.
`typedef void handlerFunc(void);` is easier to read than `typedef void (*handlerFuncPtr)(void);`. `typedef void handlerFunc(void);` is easier to read than `typedef void (*handlerFuncPtr)(void);`.

View File

@ -1,8 +1,9 @@
# Development # Development
This document is primarily for developers only. This document is primarily for developers.
If you plan to contribute to Betaflight by opening a pull request for a bugfix or feature, please read the following text carefully before you start. This will help you in submitting your contribution in a form that has a good chance of being accepted. Please also read up on the [coding style](/docs/development/CodingStyle.md).
## General principals ## General principles
1. Name everything well. 1. Name everything well.
2. Strike a balance between simplicity and not-repeating code. 2. Strike a balance between simplicity and not-repeating code.
@ -16,8 +17,6 @@ This document is primarily for developers only.
10. Be professional - attempts at humor or slating existing code in the codebase itself is not helpful when you have to change/fix it. 10. Be professional - attempts at humor or slating existing code in the codebase itself is not helpful when you have to change/fix it.
11. Know that there's always more than one way to do something and that code is never final - but it does have to work. 11. Know that there's always more than one way to do something and that code is never final - but it does have to work.
Before making any code contributions, try to comply with the [coding style](CodingStyle.md). It has a lot of sound advice.
It is also advised to read about clean code, here are some useful links: It is also advised to read about clean code, here are some useful links:
* http://cleancoders.com/ * http://cleancoders.com/
@ -59,7 +58,7 @@ make junittest
This will build a set of executable files in the `obj/test` folder, one for each `*_unittest.cc` file. This will build a set of executable files in the `obj/test` folder, one for each `*_unittest.cc` file.
It will stop after first compile/build error. If you want it to continue with the next test module you can use `make -k test`. It will stop after first compile/build error. If you want it to continue with the next test module you can use `make -k test`.
After they have been executed by the make invocation, you can still run them on the command line to execute the tests and to see the test report. Test reports will also be produced in form of junit XML files, if tests are built and run with the "junittest" goal. Junit report files are saved in obj/test directory and has the following naming pattern test_name_results.xml, for example: obj/test/battery_unittest_results.xml After they have been executed by the make invocation, you can still run them on the command line to execute the tests and to see the test report. Test reports will also be produced in form of junit XML files, if tests are built and run with the "junittest" goal. Junit report files are saved in obj/test directory and has the following naming pattern test\_name\_results.xml, for example: obj/test/battery\_unittest\_results.xml
You can also step-debug the tests in eclipse and you can use the GoogleTest test runner to make building and re-running the tests simple. You can also step-debug the tests in eclipse and you can use the GoogleTest test runner to make building and re-running the tests simple.
@ -67,30 +66,6 @@ The tests are currently always compiled with debugging information enabled, ther
Tests are verified and working with GCC 4.9.3 Tests are verified and working with GCC 4.9.3
## Test coverage analysis
There are a number of possibilities to analyse test coverage and produce various reports. There are guides available from many sources, a good overview and link collection to more info can be found on Wikipedia:
https://en.wikipedia.org/wiki/Gcov
A simple report for a single test can for example be made using this command:
```
gcov -s src/main/sensors -o obj/test/ battery_unittest.cc
```
To produce an coverage report in xml format usable by the Cobertura plugin in Jenkins requires installation of a Python script called "gcovr" from github:
https://github.com/gcovr/gcovr/tree/dev
Example usage in Jenkins:
```
/gcovr-install-path/gcovr/scripts/gcovr obj/test --root=src/main -x > coverage.xml
```
There are many other ways to prodice test coverage reports in other formats, like html etc etc.
## Using git and github ## Using git and github
Ensure you understand the github workflow: https://guides.github.com/introduction/flow/index.html Ensure you understand the github workflow: https://guides.github.com/introduction/flow/index.html
@ -103,16 +78,16 @@ https://help.github.com/articles/creating-a-pull-request/
The main flow for a contributing is as follows: The main flow for a contributing is as follows:
1. Login to github, go to the betaflight repository and press `fork`. 1. Login to github, go to the betaflight repository and press `fork`;
2. Then using the command line/terminal on your computer: `git clone <url to YOUR fork>` 2. Then using the command line/terminal on your computer: `git clone <url to YOUR fork>`;
3. `cd betaflight` 3. `cd betaflight`;
4. `git checkout master` 4. `git checkout master`;
5. `git checkout -b my-new-code` 5. `git checkout -b my-new-code`;
6. Make changes 6. Make changes;
7. `git add <files that have changed>` 7. `git add <files that have changed>`;
8. `git commit` 8. `git commit`;
9. `git push origin my-new-code` 9. `git push origin my-new-code`;
10. Create pull request using github UI to merge your changes from your new branch into `betaflight/master` 10. Create pull request using github UI to merge your changes from your new branch into `betaflight/master`;
11. Repeat from step 4 for new other changes. 11. Repeat from step 4 for new other changes.
The primary thing to remember is that separate pull requests should be created for separate branches. Never create a pull request from your `master` branch. The primary thing to remember is that separate pull requests should be created for separate branches. Never create a pull request from your `master` branch.
@ -131,7 +106,4 @@ Later, you can get the changes from the betaflight repo into your `master` branc
4. `git merge betaflight/master` 4. `git merge betaflight/master`
5. `git push origin master` is an optional step that will update your fork on github 5. `git push origin master` is an optional step that will update your fork on github
You can also perform the git commands using the git client inside Eclipse. Refer to the Eclipse git manual. You can also perform the git commands using the git client inside Eclipse. Refer to the Eclipse git manual.

View File

@ -0,0 +1,23 @@
# Test coverage analysis
There are a number of possibilities to analyse test coverage and produce various reports. There are guides available from many sources, a good overview and link collection to more info can be found on Wikipedia:
https://en.wikipedia.org/wiki/Gcov
A simple report for a single test can for example be made using this command:
```
gcov -s src/main/sensors -o obj/test/ battery_unittest.cc
```
To produce a coverage report in XML format usable by the Cobertura plugin in Jenkins requires installation of a Python script called "gcovr" from github:
https://github.com/gcovr/gcovr/tree/dev
Example usage in Jenkins:
```
/gcovr-install-path/gcovr/scripts/gcovr obj/test --root=src/main -x > coverage.xml
```
There are many other ways to produce test coverage reports in other formats, like html etc.