Why isn't this code working?

Hello!
So i tried to make a code that will light up some leds when i send 1,2 or 3 on the serial monitor forlater be used to light up the leds useing buttons on my xbox one controller with proceseing!
But it doesn't work :frowning:

void setup() {
  //put your setup code here, to run once:
  Serial.begin(9600);
  pinMode(3, OUTPUT);
  pinMode(4, OUTPUT);
  pinMode(5, OUTPUT);
}

void loop() {
 
  int a = Serial.read();
  switch (a) {
    case 1:
      digitalWrite(3, HIGH);
      delay(2000);
      digitalWrite(3, LOW);
      break;
    case 2:
      digitalWrite(4, HIGH);
      delay(2000);
      digitalWrite(4, LOW);
      break;
    case 3:
      digitalWrite(5, HIGH);
      delay(2000);
      digitalWrite(5, LOW);
      break;
    default:
      //if nothing else matches, do the default
      //    default is optional
      break;
  }


  \
}

Thanks guys

case 1:
Try
case '1':

etc.

The serial input basics tutorial shows how to reliably receive data from the serial port.

I suspect your getting dirt characters in your serial stream and its turning of the lights before they get a chance to light up. Try limiting the received chars to the the decimal range for numbers in the ASCII table I.E. 48 to 57 asciitable

void setup() {
  //put your setup code here, to run once:
  Serial.begin(9600);
  pinMode(3, OUTPUT);
  pinMode(4, OUTPUT);
  pinMode(5, OUTPUT);
}

void loop() {
  int option = 0;
  char a = Serial.read();
  if (a >= 48 && a <=57) {
      option = a.toInt();
      Serial.println(option);
  }
  switch (option) {
    case 1:
      digitalWrite(3, HIGH);
      delay(2000);
      digitalWrite(3, LOW);
      break;
    case 2:
      digitalWrite(4, HIGH);
      delay(2000);
      digitalWrite(4, LOW);
      break;
    case 3:
      digitalWrite(5, HIGH);
      delay(2000);
      digitalWrite(5, LOW);
      break;
    default:
      //if nothing else matches, do the default
      //    default is optional
      break;
  }
}

Your case labels are looking for integers. They need to look for characters.
Try

   case '1':

etc.

It isn’t a good idea to just read the Serial port when you don’t know if a character has been read - and the vast majority of the time there won’t be one. However, in your case it won’t matter.
But to fix it, you should first check whether Serial.available() tells you that there is at least one character to read.
If you add this statement as the first statement in the loop() function, it will fix it. If there’s nothing to read it will just return from the loop() function and nothing will happen until at least one character arrives.

 if(Serial.available() < 1)return;

Pete

groundFungus:
The serial input basics tutorial shows how to reliably receive data from the serial port.

i don't understad that post

Andreiva:
i don't understand that post

Did you follow the link and try to understand that thread?

el_supremo:
Your case labels are looking for integers. They need to look for characters.
Try

   case '1':

etc.

It isn’t a good idea to just read the Serial port when you don’t know if a character has been read - and the vast majority of the time there won’t be one. However, in your case it won’t matter.
But to fix it, you should first check whether Serial.available() tells you that there is at least one character to read.
If you add this statement as the first statement in the loop() function, it will fix it. If there’s nothing to read it will just return from the loop() function and nothing will happen until at least one character arrives.

 if(Serial.available() < 1)return;

Pete

I tried this but it doesn’t work :frowning:

adwsystems:
Did you follow the link and try to understand that thread?

Yes and the guy uses more “voids” and i don’t understand that , i just want something simple.

I tried this but it doesn't work

Post the code that you tried.

Pete

KawasakiZx10r:
I suspect your getting dirt characters in your serial stream and its turning of the lights before they get a chance to light up. Try limiting the received chars to the the decimal range for numbers in the ASCII table I.E. 48 to 57 asciitable

void setup() {

//put your setup code here, to run once:
  Serial.begin(9600);
  pinMode(3, OUTPUT);
  pinMode(4, OUTPUT);
  pinMode(5, OUTPUT);
}

void loop() {
  int option = 0;
  char a = Serial.read();
  if (a >= 48 && a <=57) {
      option = a.toInt();
      Serial.println(option);
  }
  switch (option) {
    case 1:
      digitalWrite(3, HIGH);
      delay(2000);
      digitalWrite(3, LOW);
      break;
    case 2:
      digitalWrite(4, HIGH);
      delay(2000);
      digitalWrite(4, LOW);
      break;
    case 3:
      digitalWrite(5, HIGH);
      delay(2000);
      digitalWrite(5, LOW);
      break;
    default:
      //if nothing else matches, do the default
      //    default is optional
      break;
  }
}

Grown-ups write it   if (a >= '0' && a <= '9')

Try this, it may not be the correct way but it works.

But using delay for 2 seconds you can do nothing else

int a = 0;
void setup() {
  //put your setup code here, to run once:
  Serial.begin(9600);
  pinMode(3, OUTPUT);
  pinMode(4, OUTPUT);
  pinMode(5, OUTPUT);
  digitalWrite(3, LOW);
  digitalWrite(4, LOW);
  digitalWrite(5, LOW);
}

void loop() {
  if (Serial.available()) {

    a = Serial.parseInt();

    switch (a) {
      case 1:
        digitalWrite(3, HIGH);
        delay(2000);
        digitalWrite(3, LOW);
        break;
      case 2:
        digitalWrite(4, HIGH);
        delay(2000);
        digitalWrite(4, LOW);
        break;
      case 3:
        digitalWrite(5, HIGH);
        delay(2000);
        digitalWrite(5, LOW);
        break;
      default:
        //if nothing else matches, do the default
        //    default is optional
        break;
        while (Serial.available() > 0) Serial.read();
    }

  }

}

el_supremo:
Post the code that you tried.

int a = 0;
void setup() {
  //put your setup code here, to run once:
  Serial.begin(9600);
  pinMode(3, OUTPUT);
  pinMode(4, OUTPUT);
  pinMode(5, OUTPUT);
}

void loop() {
  if(Serial.available() < 1)return;
  }
  switch (a) {
    case '1':
      digitalWrite(3, HIGH);

      break;
    case '2':
      digitalWrite(4, HIGH);

      break;
    case '3':
      digitalWrite(5, HIGH);

      break;
    default:
      //if nothing else matches, do the default
      //    default is optional
      break;
  }


  
}
  if(Serial.available() < 1)return;
  }
  switch (a) {

Have you forgotten something?
(Hint: what is the value of ‘a’?)

Thanks guys!
The code from Steveiboy worked like a charm!
Now i need to move on Processing :frowning:

Andreiva:
Thanks guys!
The code from Steveiboy worked like a charm!
Now i need to move on Processing :frowning:

But do you understand why?

It is not enough to say it’s working, then go on to the next problem for someone else to solve.

A slight correction in your codes, and they work!

void setup() 
{
  //put your setup code here, to run once:
  Serial.begin(9600);
  pinMode(3, OUTPUT);
  pinMode(4, OUTPUT);
  pinMode(5, OUTPUT);
}

void loop() 
{
   //wait for interrupt to be made by Receiver Section of UART when character comes from Serial Monitor
}

void serialEvent()   //when a character comes from Serial Monitor, the MCU is interrupted, and it comes here
{
  byte a = Serial.read();
  switch (a) 
  {
    case '1':             //from Serial Monitor, you receive 0x31 for 1; so, must check for that ('1' or 0x31)
      digitalWrite(3, HIGH);
      delay(2000);
      digitalWrite(3, LOW);
      break;
    case '2':
      digitalWrite(4, HIGH);
      delay(2000);
      digitalWrite(4, LOW);
      break;
    case '3':
      digitalWrite(5, HIGH);
      delay(2000);
      digitalWrite(5, LOW);
      break;
    default:
      //if nothing else matches, do the default
      //    default is optional
      break;
  }
}

GolamMostafa:
A slight correction in your codes, and they work!

Or you could ditch the utterly pointless serialEvent and save typing.

larryd:
But do you understand why?

It is not enough to say it’s working, then go on to the next problem for someone else to solve.

If you say it like this ,idk because it looks similar to my last code
hmm

Andreiva:
If you say it like this ,idk because it looks similar to my last code
hmm

I found the issue with my last code:
i was smart enoght to forget to type in

a = Serial.read;

I am so smart

a = Serial.read;But the compiler then reminded you that you aren't quite that smart.