Take the 2-minute tour ×
Arduino Stack Exchange is a question and answer site for developers of open-source hardware and software that is compatible with Arduino. It's 100% free, no registration required.

I have made this small program to lighten up in sequence 8 LEDs, it starts fine, but it stops working after a minute or so and the first 2 LEDs stay permanently ON, its like the chip stoped, here is the code :

// Initialize an array with all 8 leds
// and give them the corresponding
// digital number.
const byte led[8] = {0,1,2,3,4,5,6,7};


// the setup routine runs once when you press reset:
void setup() {

  // initialize all 8 digital I/O pins as outputs.
  for (byte i = 0; i<9; i++) {
    pinMode(led[i], OUTPUT);
  }
  pinMode(13, OUTPUT);
}

void left (void);
void right (void);

// the loop routine runs over and over again forever:
void loop() {

  right();
  left();

}


void left (void) {
  for (byte i = 0; i<9; i++) {
    digitalWrite(led[i], HIGH);
    delay(50);
    digitalWrite(led[i], LOW);
  }
}

void right (void) {
  for (byte i = 8; i>0; i--) {
    digitalWrite(led[i], HIGH);
    delay(50);
    digitalWrite(led[i], LOW);
  }
}

I am using a very old arduino duemilanove.

share|improve this question

3 Answers 3

up vote 2 down vote accepted

You had several problems with your loop conditions. Arrays are 0 indexed. If we declare const byte led[8], led[0] is the first element and led[7] is the 8th and last element.

Also, in void right(), you declared i as a byte which is an 8-bit, unsigned number from 0 to 255. While you wrote the conditional to be i>0, it needs to be i>-1 or i>=0. So, if we change the coditional and keep i as a byte the loop would decrement i like 8,7,6,5,4,3,2,1,0,255,254... forever. So instead, i should be declared as a char which is an 8-bit, signed number from -128 to 127.

// Initialize an array with all 8 leds
// and give them the corresponding
// digital number.
const byte led[8] = {0,1,2,3,4,5,6,7};

// the setup routine runs once when you press reset:
void setup() {
  // initialize all 8 digital I/O pins as outputs.
  for (byte i = 0; i<8; i++) {
    pinMode(led[i], OUTPUT);
  }
  pinMode(13, OUTPUT);
}

void left (void);
void right (void);

// the loop routine runs over and over again forever:
void loop() {
  right();
  left();
}

void left (void) {
  for (byte i = 0; i<8; i++) {
    digitalWrite(led[i], HIGH);
    delay(50);
    digitalWrite(led[i], LOW);
  }
}

void right (void) {
  for (char i = 7; i>=0; i--) {
    digitalWrite(led[i], HIGH);
    delay(50);
    digitalWrite(led[i], LOW);
  }
}
share|improve this answer
    
+1 even though Peter said it above, until I saw 8,7,6,5,4,3,2,1,0,255,254... the penny hadn't dropped. –  Madivad Mar 28 at 1:18

Your loop conditions aren't quite right, which could be resulting in undefined behaviour. I highly doubt it would cause it to stop working spontaneously, but it's possible in theory.

In each loop that counts upwards, you basically need to stop before the counter reaches 8. For example:

for (byte i = 0; i<8; i++) ...

Note that it's i<8 instead of i<9.

The reason is that the array is only 8 elements long, so the indices run from 0 to 7 (inclusive). Currently, your loops are going all the way up to and including 8, which means it's trying to read past the end of your array.

You'll need to fix that in setup() and left().

The loop that goes downwards in right() is a little more difficult. At the moment, it runs through indices 8 to 1, meaning it has the same problem of reading past the array. However, it also never actually reaches index 0.

It's harder to fix because byte is an unsigned type, so you can't use i>=0 as your condition (as it will never terminate). Personally, I would rewrite the loop to count upwards, but then reverse the index order inside the loop, so that 0 becomes 7, and so on. For example:

for (byte i = 0; i<8; i++) {
    digitalWrite(led[7-i], HIGH);
    delay(50);
    digitalWrite(led[7-i], LOW);
}

Like I said, I doubt this will actually solve your problem. It's worth trying though.

share|improve this answer

Thank you guys, its fixed now, i just dont understand why i cant use byte if i do, i>=0, this way i never goes to 255 anyway and if does, it doesn matter, unless im missing something. Here goes the final working code :

/*
  Blink
  Turns on all 8 leds in sequence back and forth.

 */


// Initialize an array with all 8 leds
// and give them the corresponding
// digital number.
const char led[8] = {0,1,2,3,4,5,6,7};
#define speed 40

// the setup routine runs once when you press reset:
void setup() {

  // initialize all 8 digital I/O pins as outputs.
  for (int i = 0; i<8; i++) {
    pinMode(led[i], OUTPUT);
  }
  pinMode(13, OUTPUT);
}

void left (void);
void right (void);

// the loop routine runs over and over again forever:
void loop() {

  // Turn off the buzzer on pin 13
  digitalWrite(13, LOW);

  while(1) {
  right();
  left();
  }

}


void left (void) {
  for (int i = 0; i<8; i++) {
    digitalWrite(led[i], HIGH);
    delay(speed);
    digitalWrite(led[i], LOW);
  }
}

void right (void) {
  for (int i = 7; i>=0; i--) {
    digitalWrite(led[i], HIGH);
    delay(speed);
    digitalWrite(led[i], LOW);
  }
}
share|improve this answer
    
yeah it will, in a for loop where the a byte is decrementing, if you use: i>=0 then this will always evaluate to true. When i==1 it will decrement to 0. When i==0 it will decrement (and rollover) to 255. This wouldn't be a problem if you used i>0 (or used a signed type), but then you wouldn't be using the 0th byte because you'd be leaving the loop before 0 was iterated over, ie led[0]. –  Madivad Mar 28 at 12:21
    
But then i == 255 will be false ????? –  Electropepper Mar 28 at 12:23
    
but i>0 would be true :) –  Madivad Mar 28 at 12:28
    
also, it would matter because if it did go to led[255], that is pointing to another area of ram not associated with led and would be unpredictable –  Madivad Mar 28 at 12:29
    
Ok @Madivad i got it now :), thank you. –  Electropepper Mar 28 at 12:34

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.