supermileage / dynamometer-firmware

0 stars 0 forks source link

Add assertion error logging to project #71

Closed cosparks closed 1 year ago

cosparks commented 1 year ago

ErrorLogger and dyno_assert

This PR adds a new ErrorLogger class to the System folder. This class defines behaviour for asserts, which will come in handy for catching if/when the firmware enters an invalid or unwanted state. For example, if we always expect the value of an integer which is passed to a function to be greater than 0, then we can add:

// in src/main.cpp

void myFunc(int val) {
    dyno_assert( val > 0 );
...
}

In this case, if the input value is less than 1, then dyno_assert will either log a human-readable error message to Serial monitor, to file on SD card, or both (depending on how we have ErrorLogger configured). After logging, control will carry on as normal

Note: dyno_assert does not throw exceptions or change control at all, it just captures some information about the firmware state when it diverges from what we want/expect.

Here is an example of output on failure:

assert failed: ( val > 0 )
    <src/main.cpp,ln 21> in void myFunc()

Other

LaPommeCosmique commented 1 year ago

I think this looks great. What do you think of having assert() return the value of the expression? This would allow branching code while logging errors if necessary, like below:

if ( assert (val > 0) ) {
   // continue with code
} else {
   // error handling
}
cosparks commented 1 year ago

I think this looks great. What do you think of having assert() return the value of the expression? This would allow branching code while logging errors if necessary, like below:

if ( assert (val > 0) ) {
   // continue with code
} else {
   // error handling
}

That's not a bad idea, but kind of goes against how asserts are typically used. Asserts are intended to be a sort of way to define invalid input without adding a bunch of extra logic to handle invalid inputs. In other words, having an assert means that you don't handle the case where the assert fails. They are also supposed to be something that we can ask the compiler optimize out, if after testing we see that everything is working correctly and they are no longer needed (so we should not branch on the result).