# Blinking LEDs in reverse order

The code below flashes some LEDs in order: pin 1, pin 2, pin 3, pin 4.

After this sequence has completed I would like to reverse direction and show pin 4, pin 3, pin 2, pin 1.

I wrote the code below but it's not working in reverse order.

What might be wrong?

#include <htc.h>

__CONFIG(1,OSCSDIS & HSPLL);
__CONFIG(2,BORDIS & PWRTDIS &WDTDIS);
__CONFIG(3,CCP2RC1);
__CONFIG(4,LVPDIS & STVREN);
__CONFIG(5,UNPROTECT);
__CONFIG(6,WRTEN);
__CONFIG(7,TRU);

#define _XTAL_FREQ   40000000

void delay_sec(unsigned char seconds)    // This function provides delay in terms of seconds
{
unsigned char i,j;

for(i=0;i<seconds;i++)
for(j=0;j<100;j++)
__delay_ms(10);
}
void led_display(char a)
{
switch(a)
{
case 0: PORTB=0x01;PORTD=0x08;  break;
case 1: PORTB=0x02;PORTD=0x04;  break;
case 2: PORTB=0x04;PORTD=0x02;  break;
case 3: PORTB=0x08;PORTD=0x01;  break;
}
}

void main()
{
TRISB=0x00;
TRISD=0x00;
char a,b;

while(1)
{
led_display(a);

a++;
delay_sec(1);
if(a==4)
{
a--;
}
}
}


You increment a every time and then you decrement it if a == 4. So a equals three in the beginning of you while-loop, then you increment it to four and decrement it, because it equals four. Now it is three again when you jump back to the start of loop.

Try something like:

bool directionForward == true;

while(1) {
led_display(a);
delay_sec(1);
if(directionForward) {
a++;
if(a==4) directionForward = false;
} else {
a--;
if(a==0) directionForward = true;
}
}


And one other point: Try to avoid using this delay-function. Use a timer interrupt instead.

• Your first line uses a comparison operator instead of an assignment operator, which is what I'm sure you meant. Also, I don't think htc has a bool type. I do think it has a bit type, though. Not sure if true and false are built-in. – Dan Laks Oct 31 '14 at 17:07

As your code currently stands, it will increment a until it reaches 4, then it will decrement it to 3 and then increment it back to 4.

That is, led_display will be called with the following arguments

0
1
2
3
3
3
....


If I understand you correctly, you would like it to successively increment from 0 until it reaches 3, and then decrement its way down to 0

The way I would implement that is by using a "direction variable", one way of doing so would look like this

TRISB=0x00;
TRISD=0x00;
char a=0, b, dir=1;

while(1)
{
led_display(a);

a+=dir;
delay_sec(1);

if(a==3)
{
dir=-1;
}

else

if(a==0)
{
dir=1;
}

}


NOTE: this code is effectively the same as GER_Moki's, just the sequence and the operations are purposely verbose and closer to your original code for clarity

• Or, even shorter: if (a==3 || a==0) dir = -dir; – RJR Oct 31 '14 at 8:03
• I don't like writing magic numbers... Add #define UP 1 and #define DOWN -1 to the code and use it (dir=UP; dir=DOWN). – GER_Moki Oct 31 '14 at 10:52
• Both completely correct comments. I consciously made the decision to keep it simple and aim it directly toward the questioner, who I feel may be not be a confident programmer; if my answer can address the question as simply as possible, then these two additions can only help along any beginning programmer toward mastery. – Al Longley Oct 31 '14 at 15:55

In addition to what is presented in these other answers, I don't see where the variable a is ever initialized. Since it is a local (on the stack) it is not be guaranteed to be 0 like it would be if it was a global variable.

char a,b;

while(1)
{
led_display(a);


It should be something like:

char a,b;

a = 0;

while(1)
{
led_display(a);


no matter what other changes are made.