Skip to content

Latest commit

 

History

History
507 lines (390 loc) · 17.6 KB

sws.md

File metadata and controls

507 lines (390 loc) · 17.6 KB
Raw

SWS: Software Writing Standards

⚠️ This document is under development

Task Ref: UT_2.0.1

Changelog

  • 2019-12-10 - Richard Abrams GDP201920

    • Creation

  • 2020-03-11 - Richard Abrams GDP201920

    • Struct/header standard
    • Removed “usage” section prototype, this is now in the Software Manual document

  • 2020-10-02 - Duncan Hamill GDP202021

    • Significant update and expansion of SWS

Preface

The following points apply in general to the entire software repository, including this document.

  • Terms such as “must”, “should” and respective negatives are to be interpreted as outlined in RFC2119.
  • Semantic version numbering is to be used for files, with version number included in a comment at the top of the file. See semver for instruction.
  • The OBC firmware is stored on GitHub under the UoS3 Project.

Principles

This document provides a series of standards for writing code in the uos3-obc-fw repository. The standard follows 3 main principles:

  1. Safety is critical to the mission

    Mission safety cannot be comprised in any way, so operations that may result in errors must be guarded against and justified. Therefore a standardised result system is used, which is detailed in the error handling section.

  2. Readable code is better than concise code

    "Clever" code and one-liners may seem like a good idea but they just end up causing confusion and wasting time when someone else comes around to review/fix your code. For this reason things like

    float angular_rate_rads = fabs(sample_time_s) < FLT_EPSILON
    ? p_status_report_inout.f_division_by_zero = true, 0.0
    : angular_change_rad / sample_time_s;

    should be expanded to something like

    float angular_rate_rads;

if (fabs(sample_time_s) < FLT_EPSILON) {
    p_status_report_inout.f_division_by_zero = true;
    angular_rate_rads = 0.0;
}
else {
    angular_rate_rads = angular_change_rad / sample_time_s;
}

  3. Modularity is preferred

    Modular code (i.e. code which is clearly separated out into separate modules based on use/purpose) provides greater organisation and compartmentalisation than monolithic code. The directory structure encourages separation of modules, and the build system (CMake) allows static linking to different libraries based on different executables needs.

    This is covered in the sections on the build system, directory structure, and module structure.

These principles should be followed at all times. If you encounter a situation not covered by this standard you should:

  1. Think of these principles in how you solve the situation
  2. Consult with others to check its a reasonable solution
  3. Add it to the standard

With that said, lets dive in.

Language

The language used for the OBC firmware is C99, and must be compiled with GCC using the ARM embedded tool chain provided here.

Version Control

Git is being used as the version control tool for the software repository. Any source other than that in the main GitHub repository shall not be considered a more complete/up-to-date version of the source code.

You must ensure that any changes are committed and pushed into the remote repository. Shared devices (such as the Raspberry Pi) must not be used to store development branches.

You should not make code changes on any shared devices. Instead you should make changes on a private branch, and then transfer the code to the shared device. This avoids any untracked changes being run on the shared device.

Build System

The build system used for the OBC firmware is CMake. A minimum version of 3.16.3 is required, as this is the version that Ubuntu 20.04 ships with.

In the past a combination of Makefiles, bash scripts, and python scripts were used to build and flash the software. This has been replaced by CMake and a few small bash scripts.

Module Structure

The code base and functionality of the software is divided into modules. One definition of a module, and the one that is used here, is given by:

A module

  1. encapsulates code and data to implement a particular functionality,
  2. has an interface that lets clients access its functionality in a uniform manner,
  3. is easily pluggable with another module that expects its interface,
  4. is usually packaged in a single unit so that it can be easily deployed,

In the case of this software these points can be explained to mean:

  1. A module deals with a single functionality of the vehicle, such as the GNSS module.
  2. Has public functions available for use by other modules,
  3. Creates a level of abstraction over its internal functioning so that the user does not have to think of these problems.
  4. Is compiled as a static library and linked into other modules through the build system.

A module should be contained within a single folder which is named after the module itself. The module folder should contain at least:

  • CMakeLists.txt - CMake listing for the module which should create a new library using the module name.
  • <ModuleName>_public.h - declarations of functions and defines available for use by other modules.
  • <ModuleName>_public.c - implementations of public functions and static definitions.

In addition, the module may contain:

  • <ModuleName>_private.h - declarations of private functions and defines for use within the module itself.
  • <ModuleName>_private.c - implementations of private functions and static definitions.

By convention an external module MUST NOT import or use a module's private items.

Any long functions (>100 lines) should be kept in their own file following the <ModuleName>_<function_name>.c convention.

Naming Conventions

The following naming conventions apply for the software:

  • Modules shall follow the UpperCamelCase naming scheme.

    • Abbreviations should have their first letter capitalised and all subsequent letters are lower case, such as Gnss or Imu.
    • Module names should not be shortened, for instance Buffer is preferred over Buf.

  • Functions shall follow the <ModuleName>_<function_name> naming scheme, that is the module name in UpperCamelCase, followed by the function name in lower_snake_case. For example Gnss_get_data or Imu_init.

    • Initialisation functions shall be called init.
    • Functions names should start with a verb, such as get, calc, or do. Verbs should be shortened to less than 4 characters when possible.

  • Variables and function arguments must follow the lower_snake_case naming scheme.

    • Pointers must:
      • be prefixed with p_. Double pointers must be prefixed with pp_. Higher orders of pointers should not be used.
      • have the declaring asterisk in contact with the variable name, not the type: int *p_number, NOT int* p_number.
    • Unions must be prefixed with u_.
    • Function arguments must be suffixed depending on their use:
      • _in for inputs
      • _out for outputs
      • _inout for both inputs and outputs
    • If a quantity has an associated unit it must be suffixed with that unit:
      • Units shall form a single 'word' in lower_snake_case, for instance use ms for meters/second not m_s.
      • Units with powers shall omit the power: mss for meters/second^2.
      • The definition comment of the variable/argument shall specify in words the units, for example: 
        /**
 * @brief The velocity
 *
 * @units meters/second
 */
float velocity_ms = 0.0;
    • For example:
      • p_output_data_out for a pointer to some output data which will be written to by the function.
      • elapsed_time_us for an elapsed time in microseconds.

  • Constants and preprocessor definitions must follow the UPPER_SNAKE_CASE naming scheme.

    • Constants and defines must be prefixed with the module name, such as GNSS_NMEA_LENGTH_BYTES.
    • Constants and defines shall follow the same rules as variables and function arguments with regards to units, however they shall be in UPPER_SNAKE_CASE.
    • Defines should be used over constants. This is because defines do not add to the symbol count and memory usage of the software.
    • Define values must be wrapped in brackets to avoid evaluation issues, for instance #define MY_DEF (12) over #define MY_DEF 12.

Commenting

General Comments

Comments should explain code, not describe it. For example:

/* Calculate the angular rate so we can determine when the s/c has despun */
float angular_rate_rads = angular_change_rad / sample_time_s;

should be preferred over

/* get the angular rate */
float angular_rate_rads = angular_change_rad / sample_time_s;

Long comments are not to be discouraged. If you need a paragraph to explain why you're doing something it's likely a very complicated procedure and should be explained in depth so that:

  1. You can check you're doing the right thing
  2. Other people can verify that the code is doing what it's supposed to as a part of a review
  3. Future maintainers can understand the purpose of the code.

Block Title Comments

Two levels of block title comments are used:

/* -------------------------------------------------------------------------
 * SECTION TITLE
 * ------------------------------------------------------------------------- */

/* ---- SUBSECTION TITLE ---- */

Section titles should be reserved for use outside of a function body, i.e. for separating the includes and defines sections of a source file. If you find yourself needing more than two levels of section titles it is likely you need to refactor into functions or rethink the design of the function itself.

Documentation Comments

The doxygen javadoc comment style shall be used for all documentation comments. If using the Doxygen Documentation Generator VSCode extension these comments are automatically inserted when writing /** and pressing enter. This is the recommended way of creating documentation comments. If in doubt follow the doxygen guidelines.

Files shall start with a doxygen file header comment, for example:

/**
 * @ingroup demo
 * 
 * @file demo_launchpad_blinky.c
 * @author Duncan Hamill (dh2g16@soton.ac.uk/duncanrhamill@googlemail.com)
 * 
 * @brief TM4C Launchpad Blinky Demo
 * 
 * This demo file is used to verify the build system functionality with the
 * TM4C launchpad. It will toggle the onboard LED if SW1 is pressed, and keep
 * the LED on if SW2 is pressed. If buttons are released the LED is turned off.
 * 
 * Originally written by Yusef Karim, adapted for UoS3.
 * 
 * @version 0.1
 * @date 2020-10-02
 * @copyright Copyright (c) UoS3 2020
 */

In files which define a module (for instance the module _public.h header) a doxygen group shall be defined for that module, for instance:

/* (At end of header comment:)
 * @defgroup demo_launchpad_blinky Demo Launchpad Blinky
 * @{
 */

The group must also be closed at the end of the file, with a comment including the group name:

/** @} */ /* End of demo_launchpad_blinky */

All other files in that module shall include the @ingroup directive in their header comment, including the opening/closing markers as described in doxygen/Grouping.

Numerical Protection Comments

When numerical protection is needed (see Numerical Protection) a comment explaining the need for the protection and the solution should be included, for example:

float angular_rate_rads;

/* ---- NUMERICAL PROTECTION ----
 *
 * To prevent a division by zero here sample time is compared to the floating 
 * point epsilon value. If the sample time is close to zero the angular rate is
 * patched to zero in order to avoid large spikes in the angular rate signal.
 *
 * In addition the division by zero flag is raised in the status report.
 */
if (fabs(sample_time_s) < FLT_EPSILON) {
    p_status_report_inout.f_division_by_zero = true;
    angular_rate_rads = 0.0;
}
else {
    angular_rate_rads = angular_change_rad / sample_time_s;
}

File Format

File Structure

C source code and header files should use the following format:

/**
 *  File header comment 
 */

/* -------------------------------------------------------------------------
 * INCLUDES
 * ------------------------------------------------------------------------- */

/* Standard library includes */
#include <stdio.h>

/* TivaWare includes */
#include "inc/tm4c123gh6pm.h"

/* Internal includes */
#include "driver/spi/spi_public.h"

/* -------------------------------------------------------------------------
 * DEFINES
 * ------------------------------------------------------------------------- */

/* -------------------------------------------------------------------------
 * GLOBALS
 * ------------------------------------------------------------------------- */

/* -------------------------------------------------------------------------
 * ENUMS
 * ------------------------------------------------------------------------- */

/* -------------------------------------------------------------------------
 * STRUCTS
 * ------------------------------------------------------------------------- */

/* -------------------------------------------------------------------------
 * FUNCTIONS
 * ------------------------------------------------------------------------- */

Header File Include Guards

Header files shall contain include guards in the following format:

/**
 *  File header comment 
 */

#ifndef H_FILENAME_H
#define H_FILENAME_H

/* ... */

#endif /* H_FILENAME_H */

where FILENAME is a direct upper-case conversion of the file name, for instance MyModule_a_long_file_name.h becomes MYMODULE_A_LONG_FILE_NAME.

Line Ending

Files shall follow the LF (line feed) end of line sequence, for compatibility with Linux. Windows editors should be set to use the LF line ending sequence.

Error Handling

TODO

Numerical Protection

In keeping with the safety focus of the software numerical protection shall be used when necessary. This is to prevent:

  • Division by zero
  • Out of bounds access of an array
  • Underflow/overflow of data types
  • Null pointer dereferencing (segfault)
  • Union undefined behaviour
  • Floating point resolution issues (0.1 + 0.2 != 0.3)

In order to catch the following operations shall have a numerical protection comment which explains the possible numerical issue, the protection mechanism used, and the impact of that mechanism.

  • All division operations - to prevent division by zero.
  • All array access/pointer indexing operations - to prevent out of bounds access.
  • All mutating operations (i.e. addition, subtraction, multiplication, bit shifting, etc.) - to prevent underflow/overflow.
  • All pointer dereferencing - to prevent segmentation faults
  • All union access - to prevent undefined behaviour
  • All floating point comparisons - to prevent resolution issues

Some examples of numerical protection mechanisms and comments:

/* ---- NUMERICAL PROTECTION ----
 *
 * To prevent a division by zero here sample time is compared to the floating 
 * point epsilon value. If the sample time is close to zero the angular rate is
 * patched to zero in order to avoid large spikes in the angular rate signal.
 *
 * In addition the division by zero flag is raised in the status report.
 */
if (fabs(sample_time_s) < FLT_EPSILON) {
    p_status_report_inout.f_division_by_zero = true;
    angular_rate_rads = 0.0;
}
else {
    angular_rate_rads = angular_change_rad / sample_time_s;
}

/* ---- NUMERICAL PROTECTION ----
 * 
 * To prevent out of bounds access on the array the indexing variable is 
 * limited to the defined length of the array.
 */
for (i = 0; i < ARRAY_LENGTH; i++) {
    /* ---- NUMERICAL PROTECTION ----
     * 
     * No protection against overflow is required here as the type of the 
     * array element (int32_t) is capable of storing the maximum value that 
     * could be written to the array (ARRAY_LENGTH + 42, where ARRAY_LENGTH = 
     * 6, and 6 + 42 < INT32_MAX(2,147,483,647))
     */
    array[i] = i + 42;
}

While this system may seem verbose and unnecessary, it is in fact critical to ensuring that the software is written in a safe and robust way. These issues are unlikely to be detected at compile time and could easily be missed in the test campaigns. Therefore they are only likely to be found at writing or code review.

It is possible for almost any statement in the software to result in numerical issues that could affect the spacecraft's safety. It is therefore critical that the developer properly consider the implications of each statement they write.

By enforcing the use of numerical protection comments and protection mechanisms the developer and reviewer can be sure that they have considered every possibility. The detail in the comments also ensures that future maintainers are able to understand the possible problems and chosen solutions.