Please use this thread to share with the community what you believe are good (or bad) programming habits, especially in the constrained world of Embedded Systems Programming.  

Quality is not an act, it is a habit. 
Aristotle

Once we are done with this discussion thread, the hope is for readers (Embedded Programmers) to possibly learn about good programming habits they should consider acquiring and maybe become aware of bad programming habits they may have so they can slowly but surely get rid of them.  

Thanks a lot for your time!

#FAQ

A major problem with code is readability.  Here's an example using Booleans, but this could apply to many other areas.

  Even the best coders with well commented code have a tendency to use the stock values (true, false) for Booleans.  It's better to be descriptive so that anyone reading the code can see exactly what was meant.  Myself, I never use true or false.  Instead, I created a header file that contains a couple dozen definitions to replace them.  Here's a few of them:

#define UNSELECT     false
#define SELECT       true

#define INPUT        false
#define OUTPUT       true

#define FORWARD      false
#define REVERSE      true

#define READ         false
#define WRITE        true

I use these when I want to assign or check a simple Boolean value.

But what if you have a Boolean that is going to be passed to a function, and it's not apparent what the parameters are?  Take the following example:

GUI_UpdateControl (ButtonControl, true);

The "true" in this example tells the function to force an update even if the control is already drawn in the proper state, and false tells the function to not draw the control if it's already in the proper state.  But there's no way of knowing that.

One way is with defines similar to the ones above that will redefine true and false to something more descriptive, such as FORCE_UPDATE and NO_FORCE_UPDATE.  But this may introduce errors because some compilers let you enter values other than 0 or 1 without error.

The best method is to create a typedef containing an enum with the two values above.  That way, no other value may be used, and the value is constrained to the two values above.  Try to use a different value and the compiler will bark at you.  This doesn't just apply to Booleans; many other areas can benefit from stronger typing to improve readability.

The takeaway from this post is to try to improve your code so that you and others can understand the code at a glance.  If the code isn't readable, a lot of time is wasted trying to understand what the writer intended, and a lot of errors could creep in that could cause catastrophic results (Google "Toyota unintended acceleration").

There are several good habits that I believe programmers need to follow:

1) Design their code before they write it

2) Use a revision control system and commit often

3) Focus on learning how to most efficiently debug their software. Many developers use only break-points and there are many more efficient techniques (profiling, tracing, deep insight analysis). As much as 40 - 60% of the design cycle is spent debugging!

4) Use a coding and style standard

5) Develop a security strategy for their device early

6) Read my blogs and attend my webinars (just seeing who's paying attention)

There are several bad habits that developers often engage in:

1) Choose free open source over commercial solutions without fully evaluating the integration, training and support costs for "free"

2) Ignore security

3) Don't purchase the tools that they need to do their jobs properly (i.e. debuggers, analyzers, etc)

4) Don't use static code analysis to identify potential bugs in their software

5) Re-invent the wheel (write software solutions that already exist because it would be 'fun')

These are just a few general thoughts that hopefully take a different angle from the posts that I've seen so far. (Lots of good information so far!)

IMHO, re-inventing the wheel is not always a bad thing.

I will go with Dr Richard Feynman: "What I cannot create, I do not understand."

I started in the era of compiler writing because there weren't enough of them yet. :-) I could read all of the texts I wanted; I could borrow code; but, until I built the structures and did the actual work, I really didn't understand.

In the present era of API for everything, I am beginning to doubt if anyone knows or cares what it really happening down at the bare metal level.

I'll agree that reinventing is important for understanding and education but in most development cycles where time and money is short, reinventing can be the life or death of the product or company. 

I think you are right in that very few know what is happening under the API's anymore and it's only going to get worse! Complexity is making it nearly impossible. 

I appreciate your response and sharing your experience!

Nice to see a different angle which seems to be taboo for many [bad habits 1) 3) 5) for example].

If some embedded software engineers were to become racing drivers they would be the ones that gets to the finishing line on a bicycle because they wouldn't dream of investing in the best equipment (to be really competitive). Their tires would be open-source but patched so that they just about held out. Although everyone else has already gone home they will still be fully content because they saved their boss a few bucks on something unnecessary from one of those evil (firmware or HW) manufactures, and showed it can be done, but forgetting the fact that the overtime and delays could be much more expensive and damaging to the team overall.
It is indeed a strange profession....

I love the analogy! It definitely puts the whole thing into perspective.

I will mention programming practices and not optimal coding guidelines, these are two different things, although the idea is to have both of them.

0.) Write readable code!

- Indent your code

- Use paranthesys to group operations, don't just rely on the operation order

1.) Comment properly your code! If you don't understand your own code in 3 months, for sure that wasn't not commented properly.

2.) Keep your comments in sync with the code! It is nothing worse then a misleading comment in some tricky flag manipulation.

3.) Use proper naming convention!

- just some: use p_ for pointers 

- use typedef whenever possible

- meaningfull variable names

Note: Please don't use the int type, at least use the short or long .

4.) In C language group your data into structs, C is a data oriented language! 

Its a bad habit having something like:

uint8_t seconds;

uint8_t minutes;

uint8_t hours;  --> group them into one struct time_t (with typedef)

5.) Keep your function and source file line numbers under control! Please don't write a huge 1000 line function.

6.) Try to write simple code, don't try to write "clever code", don't try to outsmart the compiler!

    - Avoid never ending if/else if/else if conditions

    - Avoid complicated logical conditions, ex: if ( !((a&&!b) || (!a&&b)) ) this will be actually always 0, the compiler might even trough a warning or optimize it away completely.

7.) Understand the volatile keyword!

8.) Use #defines instead of magic numbers!

9.) Try to use macros for bit/flag manipulations! Especially in case of complicated shifting/masking operations.

10.) Do use the const keyword for function pointer parameters in case the body of the function should not change the memory pointed to!

11.) Be very careful or avoid shared globals accessed from different threads (main loop and interrupts, or from different tasks)! This will limit the race conditions.

12.) Understand the static keyword

..

The list is quite long, but at some point you should also take care of the memory map, consider the far/near calls and far/near pointers.

And remember, if you think you found a compiler bug(for novice developers), 99.99% its your bug! :)

I hope it helps.

1. Always cater for the "else" condition. For instance you should always have a "default" case in a Switch construction.

2. When using a device with relatively long action times, like AD converters, serial EEPROMs or the once universal character LCDs, don't hang up the processor waiting for completion of the action. Rather poll to see if it is done.

a) this allows effectively more system throughput

b) the system doesn't hang up when the device freezes

c) when used with a timer it is possible to detect when the device has frozen when coupled with a timer. With a little forethought in the design (often it may need hardware) it is possible for the processor to reset the peripheral and re-establish operation.

3. Beware of write only devices, (the aforementioned LCDs can be set up that way) since you cannot detect if they are operational. This includes digital outputs- there are methods you can use to read back outputs. (See Using MCUs: Intelligent Digital Power Outputs and Defensive outputs)

I had a reply ready to post but somehow the browser reloaded the page and I lost it.

However, the following are the bad and good practices came to my mind.

Bad practices.

1) Do not study the data-sheet first. This is probably the worst thing can be done before starting to. You have to know what you are going to program and how. With the therm study I am not saying to study 300, 500 or 1000+ pages, but the most important parts.

2) Using heavyweight function: an example of it is the malloc function. Embedded systems are limited in resources so the use of heavyweight functions is not recommended. I am also referring to MPU with Linux OS systems. I remember I saw a malloc in a thread function in an MPU Linux based embedded system..

3) Using floating points numbers as much as possible with many calculations on devices without FP units (as some 8 bit MCUs). In general, LUT are preferable to use.

Good practices.

1) Read the data-sheet first. :)

2) Keep ISR as short as possible. Interrupts must be as short as possible. I always prefer to just set a flag into the ISR and evaluate it in the 'while(1)' loop. A code example would be:

volatile uint8_t flag;

void gpioISR(void)
{
   /* after clearing pending flags*/
   flag = 0x01;    
}

void timerISR(void)
{
   /* after clearing pending flags*/
   flag = 0x02;    
}

void main(void)
{
   flag = 0;
   /* after peripherals initialization */
   while(1)
   {
       switch(flag)
       {
            case 0x01:
                /* do something */
                break;
            case 0x20:
                /* do something else */
                break;
            default: ;
                break;
        }
    }
}

3) Use data-types according to what you are going to store: if you need to store an 8 bit value, don't use a uint32_t.

4) Use of 32 bits values on 8 bits MCUs. This can give problems, depending on how the compiler generates the ASM code. However you can make your own tricks.

5) Use timers for delays instead loop functions: functions like

void delay(uint32_t value)
{
   for(uint32_t i = 0; i < value; i++) ;
}

are not accurate. Hardware timers can do it without wasting CPU calculations and are more accurate as well.

6) If your device's manufacturer offers you device libraries, use it. The manufacturer knows the device better than you, and libraries can save you a lot of time rather than using direct register assignments with a lot of possible errors.

7) Debugging.

Some general good programming habits I think are:

1) Divide and conquer. 

2) Keep your code as much as portable.

3) Keep your main function as short as possible.

4) Use version control tools (like git) or take control of versions somehow, and do as much versions as you can. Even the most simple change can be important.

Of course more can be said. Perhaps some more experienced users will give more helpful contributions as well.

Regards,

Simon

The holy war has started... Please treat below as IMHO about C/C++ in embedded... There a lot of things to add... 

1. IMHO: macros & preprocessor, in general, is evil. 

If you have to use them, at least use the UPPER_SNAKE_CASE_FOR_MACROS to aware others this is a macro.

2. Good: Set and try to follow common coding style of your company (or see linux coding style, say). Read Google or Apple C/C++ coding style too.

3. Bad: under-commenting.

Althought your code shall speak for itself, a good practice is to add at least a header. A brief description of what function does is usually enough.

/* @fn
* @parm 
* @brief 
* @output 
*/

4. Bad: over-commenting; useless commenting. Don't do that, please.

int i = 0; //initializing i to 0 

5. IMHO (this is arguable), that also a bad habbit is to stick on the "old good" standrds, say C99 instead of moving to C11, etc.

6. Good practice (this is from MISRA actually) always have "else" in "if", even it's empty.

7. Good practice - always use brackets in "if". Less mistakes when your code will be read by others in distributed team. 

In the following code it is very easy to treat "do_that" as an "if" condition, don't write the code like this:

if(something) do_this;
   do_that;

8. Bad: put all files of your project into one folder, including headers.

9. In embedded C programming is not a good idea to pass a huge number of parameters or structures in arguments. This consumes stack and copying also takes time. Pass pointers for huge data structures instead!!

10. Bad: Using meaningless names and any names for temp vars...

instead of writing

int this_is_temporary_variable_for_only_one_run;

use:

int tmp;

11. Bad practice is also when engineer is not using mutexes, not using or handling error codes.

12. Good practice - make your project as portable as you can, well structured, do not expect every MCU is low-endian; assume, that once you or someone else will need to port this code to another platform...

13. Know your storage container and what is "int" width best for your MCU.

14. Good practice: measure the quality of your project by counting a number of "WTF" when someone else is revieing it.

15. Good practice: Review/refactor your code after a year. If you can understand it - your code is not very bad.

P.S. All above is IMHO.

Just last week I had to school a new programmer...he was animating some graphics, and had set up four booleans to represent of four each animation states...he didn't even realize he was implementing a state machine, much less doing it badly.  Lots of logic that really only needed to be an increment to four then reset to one.

It is less likely to happen with high level languages, but with PLCs I have seen this botched several times at different companies and had to clean it up.  It has become among the first things I look for when brought in to salvage a project.

State Machines:

In the embedded world, and especially PLC based controls, there is a common need to sequence a series of operations,  or to have the system switch between some number of "states" (Standy, accepting input, running process, waiting for heater to come up to temp, etc. etc.)  This is so common that it has a name "state machine".

The main thing that all state machines have in common is that the system is only in one of the states at any given time.  It moves between states (the order can change, states can be skipped, etc) depending on various conditions like timers, inputs, error conditions, etc.

Example: A washing machine has several states:

-Stopped

-Filling

-Draining

-Hi-speed agitation

-Low-speed agitation

-Spin

A Wash cycle involves Filling->Agitation->Draining->Spin->Filling(rinse)->Agitation...etc.

The machine moves from Stopped to filling based on user input, then moves from filling to agitation based on water level, and between other states based on time...and may jump from spin to stop if an unbalanced load is detected.  Several cycles may be programmed each with it's own sequence of states.

To low experience programmers (especially with PLCs) it makes perfect sense to assign a boolean variable (tag in PLC speak) to represent each state. As each state completes, it clears it's own flag, and sets the flag to activate the next state.

DO NOT _EVER_ USE BOOLEANS TO INDICATE EACH STATE!   It may work fine when you first program it, but if code has much service life at all, eventually you or a successor will make changes that will result in:

A) Two or more states being active at the same time...because only booleans indicate the state, there is nothing stopping any number of them from being true.  Remember, if it is a state machine, only ONE state can be valid at a time.

B) All of the booleans get cleared, and now you are in NO state, with no way to know where you were, and no way to get to what should have been the next state.

Either of the above problems leaves no evidence of how it got into this mess, and if it only happens occasionally it is very difficult to troubleshoot.

The Fix: 

1) Use ONE integer variable to select the state. (I'd call it iCycleState for the washing machine example above)  This is fairly natural in high level languages that have CASE or SWITCH statements, because these make the whole state machine one very tidy block of code.  If the language is lacking CASE/SWITCH, then just use multiple IFs (or Compare= contacts in ladder logic)...just group them all together though!

3) When you first write the code, don't number the the states 1,2,3 etc.   Number them 10,20,30...  or even 100, 200, 300...  Because sure as anything, you will need to add some states later, and this will give you a place to insert them into the natural sequencing of the system instead of tacking them on at the end where they don't really belong.   In some cases there are natural "sub states"  In that case I'll use the 100, 200,300 scheme for the main states and the tens position for the sub states.

You can move between states in two ways:

-Assign a new value to iCycleState

-Increment iCycleState to reach next state.   This tends to break when you add in new states, so don't do this...it only really works for state machines that do a fixed sequence of steps, but the general case is skipping between various states, backing up, etc.

Regardless, this guarantees that only only one state can be active at a time.  It automatically turns off the current state when a new state is selected.   It gives you one variable to monitor the status of the state machine, either via output or online debugging.

In high level languages, use a Default/Otherwise/Else on your Switch/Case statement. In other languages make sure you at least detect too high and too low invalid states...  At some point you or someone else may delete a state and something will try to enter it.

This is mainly an issue with PLCs and machines running a RTOS.   Standard computers tend to execute things sequentially, so performing a sequence of steps is their forte.

Use symbolic enums in C rather than integer types with #define for the state machine state constants.

Understand when and how to use volatile.

Use <stdint.h> (uint8_t, int8_t, etc.) for types with specific bit widths (NOT int and long and short) and <stdbool.h> (bool, true, false).

In order to perform parallel timing operations, standard practice is to use multiple counters running inside ISR routines. To make it easy for understanding, I use simple structs.

struct MyStruct{

volatile bool Flag;

volatile _UWORD Counter;

}KeyScanDelay,ToneDelay,KeyTone,AlarmTone;

//.....

//.....

void IsrCompareMatchTimer1(void)

{

if(KeyScanDealy.Flag)

{

if(++KeyScanDelay.Counter>99)

{

//KeyScanTask

KeyScanDelay.Counter=0;

KeyScanDealy.Flag=0;

}

}

if(KeyTone.Flag)

{

if(++KeyTone.Counter>299)

{

KeyBeep();

KeyTone.Counter=0;

KeyTone.Flag=0;

}

}

}

The code is more understandable and , most importantly, it is enough if you understand the functionality as 'Flag' and 'Counter' are common for every MyStruct type.

1) Do not EVER start coding, or modifying code until you...

Write down EXACTLY how you are going to test this, AND set up the test.

2) Write comments explaining the PURPOSE of the code.

The worst (which I have literally seen in several projects):

i = i+1; // Add 1 to i

Better:

chIdx++; // move to next channel to process

Comments should also explain the reason for the approach taken if not blindingly obvious to someone other than yourself.

One thing i know i sometimes loose myself to is pack more and more features into the code - like 'hey i have a idea what this can also be used for..' or 'this is a nice addition..' - so the good habit i try to develop for my self is something like:

write or have a clear list of functionality that your project has to full-fill.
And write your code so you can efficiently extend it later if needed but focus on this base/required functionality - and only add new ideas if all required things work fine.

i think this is mostly a subject if you have a private/hobby project or a project with much time/ broad specification and big freedoms in the process..

for me its good to get rememberd myself to this occasionally ;-)