5

I was just playing around a bit and ran into a strange problem. LED 3, 5, 6, 9 and 10 are on, as intended. For some reason LED 0 and 1 are on as well. Can someone explain why this is happening?

int ledPins[] = {3, 5, 6, 9, 10};
int pinCount = sizeof(ledPins);

void setup(){
  for (int i = 0; i < pinCount; i++){
    pinMode(ledPins[i], OUTPUT);
  }
}

void loop(){
   for (int i = 0; i < pinCount; i++){
     digitalWrite(ledPins[i], 1);
   }
}
4
  • What kind of Arduino are you using? Uno?
    – Omer
    Jan 12, 2015 at 9:13
  • Yes, it is an Arduino Uno. Jan 12, 2015 at 9:13
  • Can you show the wiring?
    – jfpoilpret
    Jan 12, 2015 at 9:14
  • Sorry, that would be quite difficult at this very moment. Though, I tested with some different code, like very specific numbers, such as digitalWrite(5, 1); for example, then it only turns LED 5 on, and none of the others. It must be code related, I guess. Jan 12, 2015 at 9:19

2 Answers 2

12

The behaviour of your program is undefined because you're reading past the end of your array.

In C/C++, the sizeof operator always gives you the size of something in bytes. It doesn't directly tell you the number of items in an array. Since int is usually 2 bytes for Arduino programming, the pinCount variable is actually being initialised to 10 (5 items x 2 bytes each).

The result is that each for loop is actually doing 10 iterations. The first 5 iterations are OK, reading correctly from the ledPins array. However, the rest of the iterations are reading whatever data happens to be in memory after that. This means it could be trying to initialise and switch on any pin, potentially including 0 and 1.

I can't guarantee that this is the source of the problem, but it's certainly very important to fix.

The solution is quite easy though. Divide the total size of the array by the size of a single element:

int pinCount = sizeof(ledPins) / sizeof(int);
5
  • 1
    Thanks for the help! That's some very important information right there. It doesn't completely solve the problem, but that's okay. I'm getting closer and learning more, that's what counts! Jan 12, 2015 at 9:41
  • 1
    Actually with the 2 answers combined, the problem is pretty much gone. Thanks for the help everyone. I just had to initialize 0 and 1 as well, other than that it's fixed with this answer! Jan 12, 2015 at 9:50
  • Alternatively use byte instead of int. So byte ledPins[] = {3, 5, 6, 9, 10};
    – Gerben
    Jan 12, 2015 at 13:24
  • Sweet! Thanks for the help, I will remember this! Jan 12, 2015 at 14:51
  • Better, define a macro to do the calculation, e.g. #define ARRAY_SIZE(array) (sizeof(array) / sizeof(array[0])) and then use int pinCount = ARRAY_SIZE(ledPins); or just skip the pinPount variable all together and use the macro directly. See also this answer.
    – hlovdal
    Jan 12, 2015 at 17:12
4

You haven't initialised the other pins at all; or included your circuit, but clearly you have LEDs on more pins than those defined (5 defined, 7 on).

The other LEDs are, therefore, in an undefined state - and may well be attached to pins set 'on' by default in your circuit. You probably need to either move 0/1 off the serial interface or explicitly set them 'off'.

1
  • That's very true, I thought I had to initialize them for them to become active in the first place. I do indeed have more than 5 LEDs on the breadboard, but was only going to use 5 of them. Jan 12, 2015 at 9:20

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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