[SOLVED] Lights not responding to serial inputs

I am a beginner with the arduino language and general electronics and to try and learn I thought of doing a binary light display. The problem I am facing now is that when I input a number into the arduino serial monitor the lights don't respond until I input the same number a few times.

//Attaches names to the pins I am using
int pin1 = 2;
int pin2 = 3;
int pin3 = 4;
int pin4 = 5;
void setup()
{
  //Sets the pins as output pins
  pinMode(pin1, OUTPUT);
  pinMode(pin2, OUTPUT);
  pinMode(pin3, OUTPUT);
  pinMode(pin4, OUTPUT);
  Serial.begin(9600);
  Serial.println("Enter a positive integer between 0-15");
}
// This creates a function to turn pins on and off
int binaryOutput(char HL1,char HL2,char HL3,char HL4){
  digitalWrite(pin1, HL1);//binary 8
  digitalWrite(pin2, HL2);//binary 4
  digitalWrite(pin3, HL3);// binary 2
  digitalWrite(pin4, HL4);// binary 1
}
// Gets and prints the serial input
int value()
    {
        while (Serial.available() > 0)
        {
        int a = Serial.parseInt();
        Serial.print("You typed in ");
        Serial.print(a, DEC);
        Serial.print("\n");
        return a;
      }
    }
    
void loop(){
  Serial.print(value());
 { 
  while(value() == 0){
// set all lights as low
    binaryOutput(LOW, LOW, LOW, LOW);
  }
  while(value() == 1){
    binaryOutput(LOW, LOW, LOW, HIGH);
   }
//etc...
  }
}

Edit: I am using an arduino uno

I think it would be best if you called value() just once per loop.

You are going to get stuck in a lot of loops waiting for serial to become available, and if it does not match the value, go on to the next loop to get stuck.

How about this:

loop()
{
    int v = value();

    switch (v)
    {
      case 0:
      binaryOutput(LOW, LOW, LOW, LOW);
      break;
      //and so on
    }
}

The logic looks substantially broken. The way you’re reading serial port input doesn’t handle the case where there’s nothing available, and returns an undefined value in that case. In loop() you are reading (trying to read) a new value each time you call value(), although the logic only makes sense if you imagine you’re using the same value throughout.

I suggest you use a conventional approach to reading and buffering the serial input and process it once you have received a complete line. The processing would consist of converting the string to an integer (e.g. by calling atoi() or scanf()) and then turn on the LEDs according to the bit states in that number. The bitRead() function would be the cleanest way to extract the state of each bit, and you could use a FOR loop to read each bit and set the state of the corresponding LED, with the LED pin numbers held in an array.

Here’s the first part - buffering the input messages:

// incomplete, untested
void handleSerial()
{
    const int BUFF_SIZE = 32; // make it big enough to hold your longest command
    static char buffer[BUFF_SIZE+1]; // +1 allows space for the null terminator
    static int length = 0; // number of characters currently in the buffer

    if(Serial.available())
    {
        char c = Serial.read();
        if((c == '\r') || (c == '\n'))
        {
            // end-of-line received
            if(length > 0)
            {
                handleReceivedMessage(buffer);
            }
            length = 0;
        }
        else
        {
            if(length < BUFF_SIZE)
            {
                buffer[length++] = c; // append the received character to the array
                buffer[length] = 0; // append the null terminator
            }
            else
            {
                // buffer full - discard the received character
            }
        }
    }
}

void handleReceivedMessage(char *msg)
{
    // your code here
}

Very simple example or turning the board LED on and off via the serial monitor.

// zoomkat 8-6-10 serial I/O string test
// type a string in serial monitor. then send or enter
// for IDE 0019 and later

int ledPin = 13;
String readString;

void setup() {
  Serial.begin(9600);
  pinMode(ledPin, OUTPUT); 
  Serial.println("serial on/off test 0021"); // so I can keep track
}

void loop() {

  while (Serial.available()) {
    delay(3);  
    char c = Serial.read();
    readString += c; 
  }

  if (readString.length() >0) {
    Serial.println(readString);

    if (readString == "on")     
    {
      digitalWrite(ledPin, HIGH);
    }
    if (readString == "off")
    {
      digitalWrite(ledPin, LOW);
    }

    readString="";
  } 
}

Solved this ages ago, so I just used a ParseInt() function to convert the ascii to an integer and used an if statement to check if there is a new byte in serial e.g.

a = serial.read();
if(!serial.read() =  a){
    a = serial.read();
    val = ParseInt(a);}
else{
    break;}
// This code is untested, this is just something similar to what I did for the display