This really is picky, picky, picky, and I recognize that.

However, source code is written for human beings, not computers, so your source code should be crystal clear, and as readable as possible.

  Keep this in mind as I get picky, picky, picky.

1. There's nothing harmful in your header, led.h, but you didn't wrap it so that it's included only once. You can see this practice in all sorts of header files (stdio.h for instance) and it's done so that you don't compile errors when both .h and .c files include the same header file.

    #include <avr/io.h>
   
    #ifndef _LED_H_
    #define _LED_H_
   
    #define LEDPORT PORTE_OUT
    #define LEDPORT_DIR PORTE_DIR
   
    void init(void);
    void toggleLights(int ledPosition);
   
    #endif
   

2. Here's the picky part...

Boolean tests in C language are oddly handled.

  We tend to think of one as true, and zero as false, but that's not exactly right.

  Zero is false, and not zero is true.

so, your while loop:

while(1)
 

will run fine, and you may think it's amazingly clear. I do understand that. But so will

while( -666 )

or

while(8675309), and that is signfificantly less clear.

Really, what you're looking to express is TRUE or FALSE.

You can find this sort of definition all over the place in 'C' language headers:

#define FALSE 0
#define TRUE !FALSE

or sometimes

#define TRUE 1

which I like even more, because it removes the ambiguity.

And, generally, I agree with much of what has been said above.

Enjoy learning C! I love the language, and believe me when I tell you that you can write beautiful, easily maintained code in C, that's not a horrible mess when debugging, and doesn't eat all the memory in the machine.

Lazy programmers do that... even in languages that manage memory for you.

-john

This really is picky, picky, picky, and I recognize that.

However, source code is written for human beings, not computers, so your source code should be crystal clear, and as readable as possible. Keep this in mind.

1. There's nothing harmful in your header, led.h, but you didn't wrap it so that it's included only once. You can see this practice in all sorts of header files (stdio.h for instance) and it's done so that you don't compile errors when both .h and .c files include the same header file.

    #include <avr/io.h>
   
    #ifndef _LED_H_
    #define _LED_H_
   
    #define LEDPORT PORTE_OUT
    #define LEDPORT_DIR PORTE_DIR
   
    void init(void);
    void toggleLights(int ledPosition);
   
    #endif
   

2. Here's the picky part:

Boolean tests in C language are oddly handled. We tend to think of one as true, and zero as false, but that's not exactly right. Zero is false, and not zero is true.

So, your while loop:

    while(1)

will run fine, and you may think it's amazingly clear. I do understand that. But so will

    while( -666 )

or

    while(8675309)

and that is significantly less clear.

Really, what you're looking to express is TRUE or FALSE.

You can find this sort of definition all over the place in C language headers:

    #define FALSE 0
    #define TRUE !FALSE

or sometimes:

    #define TRUE 1

which I like even more, because it removes the ambiguity.

And, generally, I agree with much of what has been said above.

This really is picky, picky, picky, and I recognize that.

However, source code is written for human beings, not computers, so your source code should be crystal clear, and as readable as possible.

Keep this in mind as I get picky, picky, picky.

1 - There's nothing harmful in your header, led.h, but you didn't wrap it so that it's included only once. You can see this practice in all sorts of header files (stdio.h for instance) and it's done so that you don't compile errors when both .h and .c files include the same header file.

#include <avr/io.h>

#ifndef _LED_H_
#define _LED_H_

#define LEDPORT PORTE_OUT
#define LEDPORT_DIR PORTE_DIR
void init(void);
void toggleLights(int ledPosition);

#endif

2. Here's the picky part...

Boolean tests in C language are oddly handled.

We tend to think of one as true, and zero as false, but that's not exactly right.

Zero is false, and not zero is true.

so, your while loop:

while(1)

will run fine, and you may think it's amazingly clear. I do understand that. But so will  while( -666 )
or while(8675309), and that is signfificantly less clear.

Really, what you're looking to express is TRUE or FALSE.

You can find this sort of definition all over the place in 'C' language headers:

#define FALSE 0
#define TRUE !FALSE

or sometimes

#define TRUE 1

which I like even more, because it removes the ambiguity.

And, generally, I agree with much of what has been said above.

Enjoy learning C! I love the language, and believe me when I tell you that you can write beautiful, easily maintained code in C, that's not a horrible mess when debugging, and doesn't eat all the memory in the machine.

Lazy programmers do that... even in languages that manage memory for you.

-john

This really is picky, picky, picky, and I recognize that.

However, source code is written for human beings, not computers, so your source code should be crystal clear, and as readable as possible.

Keep this in mind as I get picky, picky, picky.

1. There's nothing harmful in your header, led.h, but you didn't wrap it so that it's included only once. You can see this practice in all sorts of header files (stdio.h for instance) and it's done so that you don't compile errors when both .h and .c files include the same header file.

    #include <avr/io.h>
   
    #ifndef _LED_H_
    #define _LED_H_
   
    #define LEDPORT PORTE_OUT
    #define LEDPORT_DIR PORTE_DIR
   
    void init(void);
    void toggleLights(int ledPosition);
   
    #endif
   

2. Here's the picky part...

Boolean tests in C language are oddly handled.

We tend to think of one as true, and zero as false, but that's not exactly right.

Zero is false, and not zero is true.

so, your while loop:

while(1) 

will run fine, and you may think it's amazingly clear. I do understand that. But so will 

while( -666 )

or

while(8675309), and that is signfificantly less clear.

Really, what you're looking to express is TRUE or FALSE.

You can find this sort of definition all over the place in 'C' language headers:

#define FALSE 0
#define TRUE !FALSE

or sometimes

#define TRUE 1

which I like even more, because it removes the ambiguity.

And, generally, I agree with much of what has been said above.

Enjoy learning C! I love the language, and believe me when I tell you that you can write beautiful, easily maintained code in C, that's not a horrible mess when debugging, and doesn't eat all the memory in the machine.

Lazy programmers do that... even in languages that manage memory for you.

-john