Why are LED 0 and 1 on as well?

Question

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);
   }
}

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. — Kees Bakker, Jan 12 '15 at 09:19

score 12 · Accepted Answer · answered Jan 12 '15 at 09:35

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);

answered Jan 12 '15 at 09:35

Peter Bloomfield

10,932
9
47
87

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! – Kees Bakker Jan 12 '15 at 09: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! – Kees Bakker Jan 12 '15 at 09:50
Alternatively use byte instead of int. So byte ledPins[] = {3, 5, 6, 9, 10}; – Gerben Jan 12 '15 at 13:24
Sweet! Thanks for the help, I will remember this! – Kees Bakker Jan 12 '15 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 '15 at 17:12

score 4 · Answer 2 · answered Jan 12 '15 at 09:18

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'.

answered Jan 12 '15 at 09:18

Mark Williams

624
4
11

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. – Kees Bakker Jan 12 '15 at 09:20

Why are LED 0 and 1 on as well?

2 Answers2