Code Formatting Epic: Use accepted best practices in code formatting to produce safe software

[ElectricRCAircraftGuy]

I was just skimming your Arduino source code here, for instance: https://github.com/AmboVent/AmboVent/blob/master/.Software/Arduino%20Code/ventilation_machine/ventilation_machine.ino#L208

and I notice your formatting doesn't follow best practices to maximize for readability, maintainability, and safety. I recommend you change the code now and follow best practices as you write new code to minimize changes in the future. Here's a few examples:

1. Use curly braces after every single if statement or any type of loop.
2. Use proper indentation.
3. Put each statement which ends with a semicolon on its own line.
4. Declare variables each on their own line.
5. Don't use integers for states; use enums

Instead of:

if (millis()-last_TST_not_pressed>3000) { LED_USR(1); while (TST==1 || TST_pressed) { read_IO(); }   // wait for button release
                                          state=2;}

Do:

if (millis() - last_TST_not_pressed > 3000) 
{ 
    LED_USR(1); 
    // wait for button release
    while (TST == 1 || TST_pressed) 
    { 
        read_IO(); 
    }   
    state = 2;
}

But instead of that, even better, replace numbers and do this. Notice there's not a single "magic number" anymore. Everything is now named.

#define DELAY_TIME_MS 3000

// This is just an example to make the point
enum states
{
    /// Everything is OK
    STATE_OK = 0,

    /// Error exists and the system must turn itself off
    STATE_ERROR,

    /// Time to draw new air into the bladder
    STATE_AIR_IN,

    /// Number of elements in this enum (NOT ACTUALLY A STATE)
    STATE_COUNT,
};

if (millis() - last_TST_not_pressed > DELAY_TIME_MS) 
{ 
    LED_USR(HIGH); 
    // wait for button release
    while (TST == true || TST_pressed) 
    { 
        read_IO(); 
    }   
    state = STATE_AIR_IN;
}

Instead of this:

case 4:     // toggle sync to patient
  if (patient_triggered_breath==1) display_text_2_lines("Sync to patient","ON  ");
  if (patient_triggered_breath==0) display_text_2_lines("Sync to patient","OFF  ");
  if (TST_pressed)  {
    patient_triggered_breath=1-patient_triggered_breath; delay(110);
    if (patient_triggered_breath==1) display_text_2_lines("Sync to patient","ON  ");
    if (patient_triggered_breath==0) display_text_2_lines("Sync to patient","OFF  ");
    delay (1500);
    exit_menu();
    }
  break;

Do this:

case TOGGLE_SYNC_TO_PATIENT:
    if (patient_triggered_breath) 
    {
        display_text_2_lines("Sync to patient","ON  ");
    }
    else
    {
        display_text_2_lines("Sync to patient","OFF  ");
    }

    if (TST_pressed)  
    {
        patient_triggered_breath = 1 - patient_triggered_breath; 
        delay(110);

        if (patient_triggered_breath) 
        {
            display_text_2_lines("Sync to patient","ON  ");
        }
        else 
        {
            display_text_2_lines("Sync to patient","OFF  ");
        }
        delay(1500);
        exit_menu();
    }

    break;

Those are just a few of dozens and dozens of examples. This is really important to avoid safety pitfalls in the software, accidental mistakes and bugs. Additionally, it makes the code easier for outsiders to contribute and learn.

Note also that as you code and learn, many many many MANY things will be preferences, and there are multiple good ways to do things. However, many things have established "best practices" which avoid bad patterns and unsafe code. Frequently, multiple best practices exist. The things I point out above are best practices and produce safer code. Some of the formatting regarding putting the opening curly brace on a new line or on the same line, and how many spaces to use when indenting, are personal preferences but should be standardized in a code base. Whatever you do, please follow best practices.

[nimrod46]

Well, believe me, I feel you, but this code was basically written in one day for POC, the one who wrote it just had to add more features as the time went by and he did not keep in mind code readability, flexibility, etc.

The idea of open source is for you guys to not just use this project as is, but to also improving it. So you are more than welcome to suggest PR which we will review and push to this project (just like #14) :).

I will keep this issue open so more ppl will understand that this code is far from perfect and maybe even help improve this code.

[giorakor]

you are correct. maorfr re-arranged the code beautifully and it is now the latest ver after his fix - I am closing this issue thanks for commenting

[ElectricRCAircraftGuy]

Now that I know you're willing to accept feedback and merge PRs from others, I'll see if I can find some time one of these evenings to go through it myself and keep on improving things.

Thanks all.

[ElectricRCAircraftGuy]

@giorakor, please re-open this issue. @maorfr made a great contribution, but I still see a ton to fix. I volunteer. You can assign me to this issue.

I consider this issue an "epic", which is larger than a "story", which is larger than a "task". Many PRs could be attached to it until it is complete.

[ElectricRCAircraftGuy]

PR #46 resolves a tiny portion of this issue