Displaying binary to LEDs

Question

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

}

Answer 1

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/

Answer 2

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

Cheers!