Code style guide

[Legion2]

I'm always frustrated when is see the code of OMG and can't read it because the indentation is not correct or variable names are only one letter.

Can you define a code style guide for this project and apply this style guide to all existing code. This style guide should be defined in CONTRIBUTING.md and should have IDE support, so you can easily apply indentation rules to new code.

This would increase readability and maintainability of the code.

The main point of this issue is to make the indentation of conditional blocks uniform.

[1technophile]

Can you define a code style guide for this project and apply this style guide to all existing code. This style guide should be defined in CONTRIBUTING.md and should have IDE support, so you can easily apply indentation rules to new code.

Do you have some code style guide to suggest?

[Legion2]

Google C++ Style Guide

[1technophile]

Hello, Could you indicate your opinion about macro indentation. VSC put all the #ifdef, #else, #endif at the same level. Making these statements imbrications difficult to read.

[Legion2]

This is difficult, because some part of this project (main.ino) make heavy use of these marcos and indenting them helps the improve the readability. But there are also part where macros are only used for include once (*.h) and or to enable a sensors or gateways (*.ino) and no indent is currently used in this cases (or not used correctly).

Normally you would not indent macros, but we can define a rule that files with many macros should be indented like normal control flow.

[1technophile]

I have applied auto format to the code from VSC, remaining:

• variable naming convention
• method naming convention

[Legion2]

@1technophile You can define the code format with clang-format, so it can be applied with other IDEs and from command line tools. This also enables the automatic format checking in the CI pipeline for Pull Requests.

See Legion2/CorsairLightingProtocol#104 for an example. There I used clang-format on my arduino library.

[1technophile]

Great, thanks for the tip!

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[Legion2]

@1technophile I can create a clang-format file for OpenMQTTGateway, which can be used to automatically apply and check the code style. Should I create a PR?

[1technophile]

Yes it would be great!

[Legion2]

@1technophile sorry for the late response, I created a clang-format config which preserve most of the current formatting. Clang-format can not handle different file extensions or patterns, so the same style applies to all files in the main directory (.h and .ino). But currently they are styled differently, the .h files mostly consist of preprocessor directives which are indented. In the .ino files the preprocessor directives (#include) are not indented. This kind of styling can not be done with a single clang-format file and can also not be automatically applied with IDE clang-format extensions.

[1technophile]

We may indent the preprocessor directives into the .ino also as it improves the readability in my point of view. Do you think it is relevant?

[Legion2]

The indent of preprocessor directives blocks does not apply to the normal code in that block. For example look at the following code, the preprocessor directives are indented after the #ifdef ZgatewayIR but the code is not.

#include "User_config.h"

#ifdef ZgatewayIR

  #if defined(ESP8266) || defined(ESP32)
    #include <IRrecv.h> // Needed if you want to receive IR commands.
    #include <IRremoteESP8266.h>
    #include <IRsend.h> // Needed if you want to send IR commands.
    #include <IRutils.h>
    #ifdef DumpMode // in dump mode we increase the size of the buffer to catch big codes
IRrecv irrecv(IR_RECEIVER_PIN, 1024, 15U, true);
    #else
IRrecv irrecv(IR_RECEIVER_PIN);
    #endif
IRsend irsend(IR_EMITTER_PIN);
  #else
    #include <IRremote.h>
IRrecv irrecv(IR_RECEIVER_PIN);
IRsend irsend; //connect IR emitter pin to D9 on arduino, you need to comment #define IR_USE_TIMER2 and uncomment #define IR_USE_TIMER1 on library IRremote.h so as to free pin D3 for RF RECEIVER PIN
  #endif
...

This looks weird when the #ifdef is used instead of if in the normal program flow.

One option is to use IndentPPDirectives: AfterHash like in the google style guide which looks like this:

#include "User_config.h"

#ifdef ZgatewayIR

#  if defined(ESP8266) || defined(ESP32)
#    include <IRrecv.h> // Needed if you want to receive IR commands.
#    include <IRremoteESP8266.h>
#    include <IRsend.h> // Needed if you want to send IR commands.
#    include <IRutils.h>
#    ifdef DumpMode // in dump mode we increase the size of the buffer to catch big codes
IRrecv irrecv(IR_RECEIVER_PIN, 1024, 15U, true);
#    else
IRrecv irrecv(IR_RECEIVER_PIN);
#    endif
IRsend irsend(IR_EMITTER_PIN);
#  else
#    include <IRremote.h>
IRrecv irrecv(IR_RECEIVER_PIN);
IRsend irsend; //connect IR emitter pin to D9 on arduino, you need to comment #define IR_USE_TIMER2 and uncomment #define IR_USE_TIMER1 on library IRremote.h so as to free pin D3 for RF RECEIVER PIN
#  endif

[1technophile]

One option is to use IndentPPDirectives: AfterHash like in the google style guide which looks like this:

I didn't know that, we could apply it

[Legion2]

should I add format check to travis?

[1technophile]

yes please

[Legion2]

I can also create a GitHub Action instead?

[1technophile]

Thanks for the PR!

Yes I have never used it for the moment.