if loop

I am controlling a light with an Arduino UNO and a 4 channel 5v relay board. The inserted sketch works well to turn on and off the light when I press the letter “a” in the serial monitor. It also turns off when I press any other key. What I am trying to do is when I type the letter “b” I want it to cycle on and off with a delay I specify. It seems when I use another “if” statement it doesn’t work properly. I have also tried “else if”. I’m just not sure how to code it properly. I would also like to be able to add programming to control other keys to make it blink at different rates based on the key typed into the serial monitor. Any help would be greatly appreciated.
Thanks.

int light = 13;
#define a = HIGH;

void setup(){
  Serial.begin(9600);

  //Set all the pins we need to output pins

  pinMode(13, OUTPUT);
}
  
  void loop (){
  if (Serial.available()) {

    //read serial as a character
    char ser = Serial.read();
  
  if(ser == 'a') {
    
    digitalWrite (light, HIGH);
    delay (1000);
    
  }
  else{
    
    digitalWrite (light, LOW);
    delay (1000);
  }
  }

  }

Post the code you say doesn't work.

I thought I posted it. I used the insert code link. Do you want me to post it in the body of the msg directly?

AWOL:
Post the code you say doesn't work.

It seems that this suggestion is very clever. If you don't supply us with the code that doesn't work how do you want that someone find the error in that code?

welchsc:
I thought I posted it. I used the insert code link. Do you want me to post it in the body of the msg directly?

Maybe the problem is saying

welchsc:
(...) The inserted sketch works well (...)

sorry here it is.

int light = 13;
#define a = HIGH;

void setup(){
Serial.begin(9600);

//Set all the pins we need to output pins

pinMode(13, OUTPUT);
}

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

//read serial as a character
char ser = Serial.read();

if(ser == 'a') {

digitalWrite (light, HIGH);
delay (1000);

}
else{

digitalWrite (light, LOW);
delay (1000);
}
}

}

What is this:

#define a = HIGH;

?

sorry my brain isn't working. Here is the code that isn't working.

int light = 13;
#define a = HIGH;

void setup(){
  Serial.begin(9600);

  //Set all the pins we need to output pins

  pinMode(13, OUTPUT);
}
  
  void loop (){
  if (Serial.available()) {

    //read serial as a character
    char ser = Serial.read();
  
  if(ser == 'a') {
    
    digitalWrite (light, HIGH);
    delay (1000);
  }
    
  else if(ser == 's') {
  
    digitalWrite (light, HIGH);
    delay (1000);
    digitalWrite (light, LOW);
    delay (1000);  
  }
  
  else{
    
    digitalWrite (light, LOW);
    delay (1000);
  }
  }
  }
#define a = HIGH;

What's that supposed to mean?

Delta_G:

#define a = HIGH;

What's that supposed to mean?

or even

 if(ser == 'HIGH')

:slight_smile:

KenF:
or even

 if(ser == 'HIGH')

:slight_smile:

Nop! It means:

 if(ser == '= HIGH')

Don't?

It also turns off when I press any other key.

Which lines of code will be executed if the character received is not 'a' or 's' ?

i removed #define a HIGH; as I'm noticing it's not needed. I am stuck trying to make the letter "b" loop turning the light on and off until I type another key.

Sorry I meant the letter "s" not "b"

welchsc:
i removed #define a HIGH; as I'm noticing it's not needed. I am stuck trying to make the letter "b" loop turning the light on and off until I type another key.

Is that what you want to do?
Try this:

int light = 13;

boolean flick = false;

void setup(){
  Serial.begin(9600);

  //Set all the pins we need to output pins

  pinMode(13, OUTPUT);
}

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

    //read serial as a character
    char ser = Serial.read();

    if(ser == 'a') {

      digitalWrite (light, HIGH);
      delay (1000);
      flick = false;
    }

    else if(ser == 's') {
      flick = true;

    }

    else{
      flick = false;
      digitalWrite (light, LOW);
      delay (1000);
    }
  }
  
  if (flick == true) {
    digitalWrite (light, HIGH);
    delay (1000);
    digitalWrite (light, LOW);
    delay (1000);  
  }    
}

I think a better approach is set the state of the pin with the serial input and have the main loop react to the status. Otherwise it is hard to get the strobing to work properly.

Eg:

state = 0 // 0=Low, 1 = Strobe, 2=High

loop() {

//Set state from input
if (Serial.available()) {
char ser = Serial.read();

if(ser == 'a') state=2;
else if (ser == 'b') state=1;
else state=0;
}

// Do the output according to the state
switch (state) {
case 0:
// Set low
case 1:
//Set opposite to previous
case 2:
//Set high
}
delay(1000);
}

removed #define a HIGH; as I'm noticing it's not needed

and it was written wrongly anyway.

I'm just not sure how to code it properly. I would also like to be able to add programming to control other keys to make it blink at different rates based on the key typed into the serial monitor.

In your current logic, you are always winding up at

else{
    digitalWrite (light, LOW);
    delay (1000);

and turning the light off when there is no serial input. Is that what you want?

I would recommend changing to switch / case statements http://arduino.cc/en/Reference/SwitchCase
rather than layered ifs and elseifs. Based on the serial input, you will switch to a different blink pattern.

You should take a look at the "blink without delay" in the digital examples section of the IDE, and learn how to use this timer approach in your coding.

Use this project as an opportunity to get away from using delay and to learn switch/case as a way to handle the logic.

AndersM and luisilva it works perfectly thanks!! If I wanted to add another key stroke which syntax should I use (another else if statement)? Also I have decoded signals from my remote control. I would like to use them to turn on, off, and blink the light. Do I need to use the string variable to read the codes in? Again not sure of how the syntax would be written.

and turning the light off when there is no serial input.

Actually the code to turn off the LED is inside the check to see whether serial data is available, so any character, or LF/CR will turn off the LED and confuse things even further