Programming Style Guide

This document provides certain guidelines to improve code consistency across files worked on by different members

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 filled out in order for Doxygen to parse the file

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

1. Each file must contain a Doxygen-style comment at the top with the @file, @brief, and @author statements filled
2. Each file should correspond to some module, and thus should define a Doxygen group (@defgroup) encompassing the file contents
3. Functions must contain a Doxygen-style comment above them with the @brief statement filled in at a minimum. @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. For functions with void return type, "@return None" should be used, unless the function never returns (i.e. is a thread). Also, functions should contain non-Doxygen-style comments in their body for anything non-obvious
4. Code examples in Doxygen comments must either use ticks "`", or @code @endcode to denote code. Example:

/**
 * @brief   Reads the motor supply voltage
 * @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
 * @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){
	/* 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;
}

5. 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;
}

6. All structs and enums must have their own documentation, and same for their members. The documentation of a struct or enum should precede the definition of the struct or enum. 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:

/** Enumerates the names of the week */
typedef enum days_e{
    MONDAY,         /**< The first day of the week     */
    TUESDAY,        /**< The second day of the week    */
    WEDNESDAY,      /**< The third day of the week     */
    THURSDAY,       /**< The fouth 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_t;

Naming

1. 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
2. Never begin the name of anything with two underscores
3. A name shall only be all caps when it is for a constant or macro

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. Example:

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

Code generation

1. The configuration of all peripherals must be done from within Cube
2. FreeRTOS should be enabled in Cube, but beyond this, Cube should not (for the Robot program: must not) be used for anything FreeRTOS-related
3. Pin names should be labelled in Cube once their functionality has been decided

Interrupts and concurrency

1. All things that may be written to inside an ISR must be declared volatile
2. Task notifications shall be preferred to binary semaphores for synchronization
3. Use mutexes for all cases when the goal is controlling access to a shared resource. This is because mutexes in FreeRTOS have priority inheritance mechanisms, thus avoiding the priority inversion problem
4. 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:

1. 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
2. Call xTaskNotifyWait. This will block the task, meaning it will stop executing to let other tasks run until the binary semaphore is given
3. 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
  * @return None
  */
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:

1. The callback function HAL_UART_RxCpltCallback shown above should be located in usart.c, while the DataReceiptTask should be in freertos.c
2. 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
3. In the callback function, we use an if statement so that the callback can be implemented differently for each instance of the communication module
4. 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

1. Constants shall be preferred over macros as constants are type-safe
2. Macros shall not be used to define functions (unless there is sufficient justification). Instead, inline functions shall be used as they are type-safe
3. Macros shall not be used without good justification which is documented

Functions

Where it is possible for anything to go wrong in a function, make that function return status codes. That is, make it return 1 if successful, otherwise a negative error code

Scope qualifiers

1. 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 as extern in the header. Example: module.c

...
uint8_t importantNumber;
const SOME_CONST = 42;
...

module.h

...
extern uint8_t importantNumber;
extern const SOME_CONST;
...

2. If a variable or function can be made static, it should be static. When static variables and functions are defined in the scope of a file, it makes them "private" in the sense that they can only be seen from within the file (or any file which includes it). Having private data and functions is often useful. Example: by making sensitive data static, you protect it from being tampered with from outside the library which is using it, thus reducing the probability that the code will be misused

Dead code

1. Delete unused variables (i.e. if the compiler issues a warning that a variable is unused, you better have a good reason for keeping it around). This does not hold for 3rd party code we are using, it only holds for code we write
2. If there is code that is now outdated and will never be used, it should be deleted