Programming Style Guide

This document provides guidelines to improve code consistency across files worked on by different people on the team.

File layout

All new files should be based on the templates in the Templates folder. This keeps the layout of files consistent, which improves organization. Also, the templates contain some basic fields at the top which must be there for Doxygen to parse the file
Lines should not exceed 80 characters. Function invocations can be split across several lines, and temporary variables can be introduced instead of chaining methods. There are many different styles when it comes to splitting function invocations across lines, so check through our code, look online, and stick with a style you love. Example:

// Bad
bool success = unnecessarilyLongFunctionNameForNoGoodReason(whatAmIEvenThinking, 42, BIT_MASK, A_FOURTH_ARG, WHOA);
...
// Good - doesn't exceed 80 characters on any line, and is easier to read
bool success = unnecessarilyLongFunctionNameForNoGoodReason(
    whatAmIEvenThinking,
    42,
    BIT_MASK,
    A_FOURTH_ARG,
    WHOA
);

Indentation

Indentations must consist of 4 spaces (no tabs!). You can configure Eclipse (and indeed, most text editors) to automatically insert 4 spaces when you press the tab key; this is what is suggested

Comments

Each file must contain a Doxygen-style comment at the top with the @file tag. The @author statement should be filled, and the @brief should be filled too except when it is completely obvious
Each file should correspond to some module, and thus should define a Doxygen group (@defgroup) encompassing the file contents. The group definition should be in the module's header file, and any other files inside that module should add themselves to the group via the @ingroup tag
Functions must contain a Doxygen-style comment above them. @brief statements should be no more than 2 lines, and further details should be in @details. All parameters must be documented using @param, and the return value must be documented using @retval or @return. C++ only: anything related to the interface, that is, how to use the function (so @brief, @param, @return, etc.) should be documented in the header file. Any further details (@details) should be in the implementation file (i.e. the .cpp file).
Code examples embedded within Doxygen comments must either use ticks "`", or @code @endcode to denote code.

Example in C: file.h

/**
 * @brief  Reads the motor supply voltage
 * @param  hdynamixel pointer to a Dynamixel_HandleTypeDef structure that
 *         contains the configuration information for the motor
 * @return The voltage in volts if the last read is valid, otherwise
 *         INFINITY
 */
float Dynamixel_GetVoltage(Dynamixel_HandleTypeDef* hdynamixel);

file.c

/**
 * @details Reads address 0x2A in the motor RAM to see what the current voltage
 *          is. The value retrieved from motor is 10 times the actual voltage.
 *          The results are written to `hdynamixel -> _lastLoad`,
 *          and `hdynamixel -> _lastReadIsValid` is set if the
 *          checksum is verified, and is cleared otherwise
 */
float Dynamixel_GetVoltage(Dynamixel_HandleTypeDef* hdynamixel){
	// Read data from motor
	uint8_t retVal = (uint8_t)Dynamixel_DataReader(
            hdynamixel,
            REG_CURRENT_VOLTAGE,
            1
        );

	if(hdynamixel->_lastReadIsValid){
		return((float)(retVal / 10.0));
	}
	return INFINITY;
}

Doxygen-style comments must be started as a C-style comment block with two *'s. Example:

/**
 * @brief  Gets the day of the week
 * @return The day of the week
 */
days_t Days_GetDay(){
    return theDay;
}

All structs and enums must have their own documentation, and as a rule of thumb their members should be documented too. The documentation of a struct or enum should precede the definition of the struct or enum, and should use the @brief tag. For documenting members of a struct or enum, the comment must be added right after the member using the < marker after opening the comment block. Example:

/** @brief Enumerates the names of the week */
typedef enum{
    MONDAY,    /**< The first day of the week     */
    TUESDAY,   /**< The second day of the week    */
    WEDNESDAY, /**< The third day of the week     */
    THURSDAY,  /**< The fourth day of the week    */
    FRIDAY,    /**< The fifth day of the week     */
    SATURDAY,  /**< The first day of the weekend  */
    SUNDAY     /**< The second day of the weekend */
}days_e;

In this case it would have been okay to omit the member documentation since it does not add anything. However, if the enum was used to enumerate states of a controller, this omission would not have been acceptable.

Naming

Function and variable names must be descriptive
Never begin the name of anything with an underscore. Such identifiers are generally reserved, so we don't want to take any risks
A name should only be all caps when it is for a constant or macro. The exception is for when the name is very short and rule 0 is satisfied better by having all caps
If you read the name of a bool out loud, it should essentially sound like a claim. For example:

bool isInitialized = mySpecialFunction();
if(!isInitialized){
    return;
}

(C only) Function names should be preceded by the name of the module they are associated with followed by an underscore. Example: Dynamixel_GetVoltage is a function from the Dynamixel module that gets the voltage of an actuator

Memory

Heap usage is disallowed. Objects may either have static storage duration (i.e. allocated at compile time), or be allocated on the stack

Namespaces (C++ only)

The using directive must not be used, but using declarations may be used. Using directives introduce all the names from a namespace into the current scope, while using declarations introduce only a specific name.

using namespace i2c; // Bad (using directive)
using i2c::MPU6050; // Good (using declaration)

Header files must not have anonymous namespaces
All functions and data structures besides main should not reside in the global namespace. Instead, they should be in named or anonymous namespaces

Object-oriented (C++ only)

Constructors must initialize their objects to a valid internal state
Constructors must not perform any I/O
The layout of class and struct members/methods based on visibility must be like so:

class Example{
public:
...
protected:
...
private:
...
};

Private class members start with the prefix "m_". Example:

class Example{
public:
...
private:
    uint8_t m_secret;
};

Unit testing

All hardware and OS-related functions must be in wrapper classes which inherit from an interface class (i.e. abstract class) to facilitate mocking. These hardware-facing classes should not have data members, only functions.
The test driver for a component must follow a name of the form <component>_test.cpp

Const-qualified function arguments and class methods (C++ only)

Any class method which does not modify members of that class should be const-qualified
Any function arguments which are not modified by the function should be const-qualified

Binary data

Binary data should be represented using the stdint types (e.g. uint8_t, etc.)

TODOs

Whenever you think of something that needs to be done, write a "TODO" comment so that we won't forget about it. Any comment beginning with "TODO" catches Eclipse's attention, so it is easy to track these and come back to them later. Also, put your name after the TODO so that if someone sees it in the future they'll know who to ask if they have any questions. Example:

uint8_t foo = -1; // TODO(tyler): Foo should not be assigned a negative value if it is unsigned

Code generation

The configuration of all peripherals must be done from within Cube
Pin names should be labelled in Cube once their functionality has been decided
All FreeRTOS things that can be configured in Cube should be configured in Cube. Examples of things that cannot be configured in Cube are event groups and queue sets. This rule can be bypassed for other things (e.g. mutexes), as long as there is a sufficiently good reason that the team supports, and the scenario is documented

Interrupts and concurrency

All things that may be written to inside an ISR must be declared volatile
Task notifications should be preferred to semaphores for synchronization
Use mutexes for all cases when the goal is controlling access to a shared resource. Note that mutexes in FreeRTOS have priority inheritance mechanisms, thus avoiding the priority inversion problem
Always use osDelay instead of HAL_Delay for code running on FreeRTOS. The former calls vTaskDelay internally and allows the scheduler to run another task whereas the latter will block on the delay and will not allow the processor to execute other tasks. Calls to vTaskDelayUntil are also acceptable for implementing "delays" for time-triggered threads

IO

Non-blocking I/O should be used for any code integrated with FreeRTOS. I/O should use the DMA controller whenever large amounts of data of a known size are to be transmitted or received. For cases where an unknown about of data is to be transmitted or received, interrupt-based I/O is appropriate.

Sometimes when using non-blocking I/O, you don’t want a task to continue execution until the data transfer is done. A task notification and a callback function can be used together to ensure desired behaviour:

First, perform the non-blocking I/O call. This will initiate the transfer of bits, then continue executing the task while the bits are transferred in parallel
Call xTaskNotifyWait. This will block the task, meaning it will stop executing to let other tasks run until the binary semaphore is given
Implement the callback function. This differs slightly depending on which communication module is being used, but it is required that certain names are used for the callback functions (will get to this in a bit). Regardless, each callback function consists of checking the module instance, and then giving the semaphore from ISR. The callback function is called at the end of a non-blocking transfer after all bytes have been received (for DMA) or after all bits have been received (for interrupt-based)

Example:

/**
 * @brief   receives a character of data via the UART2 module using interrupt-
 *          based I/O
 *
 *          This function never returns
 */
void DataReceiptTask(void){
    // Task initialization...

    uint32_t notification;
    for(;;){
        HAL_UART_Receive_IT(&huart2, rxChar, 1);

        status = xTaskNotifyWait(
            0,
            NOTIFIED_FROM_RX_ISR,
            &notification,
            MAX_DELAY_TIME
        );

        if(status != pdTRUE){
	    // xTaskNotifyWait timed out; handle error here
	}

        // Process received data
        // ... Your code here
    }
}

/**
  * @brief  This function is called whenever a reception from a UART
  *         module is completed. For this program, the callback behaviour
  *         consists of unblocking the thread which initiated the I/O and
  *         yielding to a higher priority task from the ISR if there are
  *         any that can run
  * @param  huart pointer to a UART_HandleTypeDef structure that contains
  *         the configuration information for UART module corresponding to
  *         the callback
  */
void HAL_UART_RxCpltCallback(UART_HandleTypeDef* huart){
    BaseType_t xHigherPriorityTaskWoken = pdFALSE;
    if(huart -> instance == USART2){
        xTaskNotifyFromISR(
            DataReceiptTaskHandle,
            NOTIFIED_FROM_RX_ISR,
            eSetBits,
            &xHigherPriorityTaskWoken
        );
    }
    portYIELD_FROM_ISR(xHigherPriorityTaskWoken);
}

A few things to note here:

The xTaskNotifyWait timeout argument is MAX_DELAY_TIME. In this example, this would be a user-defined constant equal to the maximum acceptable wait time. Any longer than this, and program execution resumes in this thread, but with status equal to an error code, which enables us to react to the issue
In the callback function, we use an if statement so that the callback can be implemented differently for each instance of the communication module
We must use xTaskNotifyFromISR in the callback functions

NOTE: The callback function for, say, the USART modules, will not be included in usart.c by default. You have to write this function yourself. However, if you go to (for example) Drivers/stm32h7xx_HAL_Driver/src/stm32h7xx_hal_uart.c, you can find weak definitions for the callback functions by searching for the text "callbacK'. These weak functions specify the exact form that the callback function must have in order for it to be called. The __weak attribute just means that in order to use this function, you need to provide an implementation for it.

Constants, macros, and inline functions

Macros must only be used to conditionally compile code or conditionally include files (i.e. include guard). On that note, each header file must begin with a unique include guard. Exception: see rule 2
(C only) The rare exception for which a macro is permitted for data is when you need to initialize something at compile-time that does not allow initialization with a constant
(C++ only) constexpr must be used in place of macros, and is preferred to const
Inline functions must be used in place of macro functions as they are type-safe

Enums

Prefer to use enums wherever it makes sense in terms of enhancing readability
(C++ only) Prefer to use enum classes over regular enums

/** @brief Colors of the rainbow */
enum class Colors{
    RED,
    ORANGE,
    YELLOW,
    GREEN,
    BLUE,
    INDIGO,
    VIOLET
}

...
// Usage
switch(myColor){
    case Colors::RED:
        doSomething();
        break;
    ...
    default:
        break;
}

Functions

Where it is possible for anything to go wrong in a function, that function should return status codes

Scope qualifiers

Never allocate storage for a variable in a header file; this creates many issues that have to do with variables being redefined, etc. Instead, allocate variables in the .c file of the module the header is related to, and declare the variable in the header using the keyword extern. Note: keep in mind that global variables are generally bad practice, so usage of this technique should be rather limited.