loop is running continuously

i want the lights to on only when message comes but the last for loop runs continuously even i send the message the led state does not change and the for loop continues .if i give a if condition to the for loop it executes only once this is the code can anyone help me with this .thanks in advance

void setup()
{
Serial.begin(9600);
Serial1.begin(9600);
delay(100);
Serial1.println(“AT+CNMI=2,2,0,0,0”);
pinMode(2,OUTPUT);
pinMode(3,OUTPUT);
pinMode(4,OUTPUT);
pinMode(5,OUTPUT);
}
void loop()
{
if(Serial1.available()>0)
{
int p=Serial1.read();

if(p==‘n’)
{
digitalWrite(2,HIGH);
digitalWrite(3,LOW);
digitalWrite(4,LOW);
digitalWrite(5,LOW);
}
if(p==‘e’)
{
digitalWrite(2,LOW);
digitalWrite(3,HIGH);
digitalWrite(4,LOW);
digitalWrite(5,LOW);
}
if(p==‘w’)
{
digitalWrite(2,LOW);
digitalWrite(3,LOW);
digitalWrite(4,HIGH);
digitalWrite(5,LOW);
}
if(p==‘s’)
{
digitalWrite(2,LOW);
digitalWrite(3,LOW);
digitalWrite(4,LOW);
digitalWrite(5,HIGH);
}
if(p==‘r’)
{
for(int i=2;i<6;i++)
{
digitalWrite(i,HIGH);
delay(1000);
digitalWrite(i,LOW);
delay(1000);
}
}

}

}

Why not add a Serial.print() statement or two to loop() and find out what is really happening?

Some mention of what is sending data to the Arduino would be useful.

please read the "how to post to this forum" message and go back and edit your first post and put the code inside code tags so it looks like this:

... code here
    if(p=='r')
    {
      for(int i=2;i<6;i++)
      {
        digitalWrite(i,HIGH);
        delay(1000);
        digitalWrite(i,LOW);
        delay(1000);
      }
    }

That for loop takes 8 seconds to run. During that time you are only running the code in this for loop and not any other code. None of the code in this for loop reads from your serial line, so it won’t respond to any serial commands.

You’re going to have to think of a way to blink your led 4 times without using the delay function or a single for loop to run all the blinks at once. You need a way to do this in a non-blocking fashion. See the Blink Without Delay example for some inspiration.

Also, your first comment gives two cases. 1) it runs continuously. 2) with an "if", it runs only once.

With the code that you have posted, which of the two situations above happens? Or does something else happen?

The way your code is written the code in each of the IF statements will repeat until a new character is received. It is only in the case of the last one that that is obvious.

It would be better to write the code without using delay(). See how to use millis() to manage timing in Several Things at a Time. That way it could respond immediately to a new character.

...R

Robin2:
The way your code is written the code in each of the IF statements will repeat until a new character is received.

I don't think that's true Robin. Next loop, he checks for a new character in and then reads it, or bypasses all the "ifs".

The way your code is written the code in each of the IF statements will repeat until a new character is received.

I don’t see that. Everything is inside the if serial available block. There is no repeating going on, except in the last case.

arduinodlb:
I don't think that's true Robin. Next loop, he checks for a new character in and then reads it, or bypasses all the "ifs".

Yes, I think you are correct. That's the problem trying to read code on the screen rather than in a text editor.

...R

i want the lights to on only when message comes

Below is a very simple example of capturing of input on the arduino serial port and evaluating what was received. For this type of comparison, no other bytes like cr or lf are assumed to be sent.

// 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);
      Serial.println("LED ON");
    }
    if (readString == "off")
    {
      digitalWrite(ledPin, LOW);
      Serial.println("LED OFF");
    }
    readString="";
  } 
}