# Displaying binary to LEDs

I've written a program for Arduino UNO and 3 LED lights. The program is supposed to turn an inputted number into binary and then display it on the LEDs, on being 1 and off being 0 I have looked it over quite a few times and don't understand why it is not working as it should. I'm a beginner at working with Arduino so I probably just need a more experienced eye to show me where I have messed up. I have a link to a picture of the Arduino setup below. Thanks! https://drive.google.com/file/d/0B8k_WNZ3eFOkZHkwNUllTEp4WGM/view?usp=sharing

``````//setting up lights
const int lightOne = 13;
const int lightTwo = 12;
const int lightThree = 11;

//array of the lights for when printing the binary
int lights[] = {lightOne, lightTwo, lightThree};

void setup() {
//setting pin modes and starting up the serial monitor
pinMode(lightOne, OUTPUT);
pinMode(lightTwo, OUTPUT);
pinMode(lightThree, OUTPUT);
Serial.begin(9600);
}

//converts a given number into binary
String binary(int number) {
String r;
while(number!=0) {
r = (number % 2 == 0 ? "0" : "1")+r;
number /= 2;
}
return r;
}

//outputs the binary string to the lights (not expecting numbers larger than     9)
void printBinary(String binaryNumber) {
for (int i = 0; i < sizeof(binaryNumber); i++) {
digitalWrite(lights[i+11], (binaryNumber[i] == '0' ? LOW : HIGH));
delay(2000);
}
}

void loop() {
int input = Serial.read();                //get the number
String binaryNumber = binary(input);      //convert number to binary
Serial.println(binaryNumber);             //print binary to serial monitor
printBinary(binaryNumber);                //display number in binary on     LEDs

}
``````

# Welcome to SE Arduino!

## Long Version

You have a couple of things to look at:

1. Don't use `String` class it has too much overhead for Arduino.
2. Your conversion function is not really necessary because you can convert it on your `Serial.println()` by specifying `binary` as output. eg: `Serial.println(number, BIN)`;
3. The `Serial.read` returns an integer, but that `int` only contains a `byte` (unsigned) value anyway (unless there's nothing to be read, in which case it returns `-1`). Because you're taking a number (not really relevant) and you want it to be a number on the return, just use `int` or `byte` (unsigned: `0 <-> 255`) or `char` (signed: `-127 <-> +128`).
4. You could/should/doesn't matter if you use `Serial.available()` or not, it is a matter for yourself, I usually do, but it's non blocking anyway and if there's nothing there to be read, it'll just continue on anyway. Although if you're not going to use it, you should test for that `-1` as what you're doing at the moment is taking that and converting it to binary (which is `0xFF` which is everything on). This is one of your major problems in the code (the other is in the Short Version section.
5. You're using `"0"` and `"1"` which are `ASCII` values, and you haven't indicated how you're getting the number. So it may be `ASCII` (`byte`) or actually a `byte` value. ie, `"0"` is `48 (dec)` or `30 (hex)`. Whereas a `0` is just `zero`. Funnily enough, because you're using modulo (`%`) it doesn't really matter because you're only interested in the lowest nibble, which for the sake of your code, doesn't matter what's encoded at the higher end. ie: `1` (`0b00000001`) is the same as `"1" = 49 (dec) = 0x31 = 0b00110001` when used with `%2`.
6. Ditch your `delay(2000)`. Delays are evil* :) You could write the values to the pins all day, you're not going to cause any problems, if you want to output less to your serial port (which you should do), you could do timed outputs back to Serial. ie, do your `Serial.println()` once a second, or record the values into backup/old variables and compare them, and only print the output if the value changes
7. The big one is in the next section

## Short Version

This is your main problem, on this line:

``````digitalWrite(lights[i+11], (binaryNumber[i] == '0' ? LOW : HIGH));
``````

I think you need:

``````digitalWrite(lights[i], (binaryNumber[i] == '0' ? LOW : HIGH));
``````

You already have the pin value encoded in the array, you don't want to add 11 to the index, that's going to throw you above your three light values :)

* Delays are evil: It's a matter of opinion, and that's just how I feel, I actually googled it and this is just a random response. But it contains the crux of what I'd say anyway: http://www.makeuseof.com/tag/arduino-delay-function-shouldnt-use/

• I was going to rewrite your code, but you've given it a pretty good stab anyway, so take my info above on board and give it another shot. If you have any problems post your new code in an update to the question and we can help you work on it further. If you're under the pump and just want an answer, let us know and I or someone else can give you how we'd do it, but there's nothing better to learning than doing it yourself. It will stick in your head a lot better after giving it another go yourself. You won't make mistakes like that `index` again (or not in that way, anyway) Dec 13, 2015 at 10:51
• Thanks! I actually originally didn't have the index mistake, but when I was looking back over it last night my tired mind though I had made a mistake where there wasn't one and changed it. Dec 13, 2015 at 14:00
• It is amazing what the tired mind does sometimes. I find I am often trying to finish "one thing" before calling it a night, and it can have me stumped for hours! At some point, I give up and call it quits. The next morning, refreshed, you take one look at it and see something that you simply can't believe you missed the night before, and in 30 seconds you've fixed the previous 4+ hours of angst. You think to yourself, the next time this happens, I'll just go to bed. But the next time it happens, you spend another night on one problem, that takes another 30 secs to fix the next day, again! lol Dec 13, 2015 at 21:21
• I'll definitely do that next time and save myself the trouble lol Dec 13, 2015 at 21:50

You need to check that there are characters available before reading. Please see documentation of Serial.read() (and Serial.available()).

Cheers!