If statements inside while loop.

Hi. I’m trying to do a line follower vehicle with my arduino. I need it to follow the line but stop otherwhise. (According to a value given from the terminal port). I implemented the following code (here i’m only posting the main segment), but it does not work when i try to stop the car.

if(serial =='S'){
  shieldbot.drive(0,0);
  Serial.println("Stop");
}

do
{
if(error == 0) {
  shieldbot.forward(); 
  Serial.println("Forward");
}
if(error >0) {
  shieldbot.drive(0, 127);
  Serial.println("Right");
}

if(error <0){
  shieldbot.drive(127, 0);
  Serial.println("Left");}
} while(serial == 'G' && Serial.available() == 0);

We need to see more of your code. Where does serial get updated?

Aaargh. What a mess. You need to get your curly braces sorted, that's your problem right there.

MorganS:
We need to see more of your code. Where does serial get updated?

Here is the rest of the coda

#include<Shieldbot.h> //Incluyo la libreria
Shieldbot shieldbot = Shieldbot(); //Declaro un objeto de tipo Shieldbot
int S1, S2, S3, S4, S5; //Establezco variables correspondientes a los sensores
int posA, PosO; //Establezco variables correspondientes a la posición
int error; //Declaro la variable error
int valor = 0;
int pinAnalogo = 4;
int serial = 0;

void setup() {
  Serial.begin(9600); //Abre el puerto serie configurando la velocidad en 9600 bps
  shieldbot.setMaxSpeed(50, 49); //Establece la velocidad máxima de los carritos
  }
void loop(){
 //Leo el sensor de proximidad
 valor = analogRead(pinAnalogo);

int serial = Serial.read();

 //Leo los sensores
 S1 = shieldbot.readS1();
 S2 = shieldbot.readS2();
 S3 = shieldbot.readS3();
 S4 = shieldbot.readS4();
 S5 = shieldbot.readS5();
 //Establezco la posicion objetivo
PosO = 0;
//Determino la posicion del carrito de acuerdo a cada uno de los sensores
if(S1 == LOW){posA = 2;}
if(S2 == LOW){posA = 1;}
if(S3 == LOW){posA = 0;}
if(S4 == LOW){posA = -1;}
if(S5 == LOW){posA = -2;}

error = PosO - posA;
//Determino el criterio de parada

if(Serial.available()>0 && serial =='S'){
  shieldbot.drive(0,0);
  Serial.println("Stop");
}

do
{
if(error == 0 && valor>200){
  shieldbot.forward(); 
  Serial.println("Forward");
}
if(error >0 && valor>200) {
  shieldbot.drive(0, 127);
  Serial.println("Right");
}

if(error <0 || valor>200){
  shieldbot.drive(127, 0);
  Serial.println("Left");}
} while(Serial.available() == 0 && serial == 'G');


}
int serial = Serial.read();

You already have one variable names serial defined at global scope. It's a really really bad idea to have two variables with the same name. Lose the "int" so you're using your global variable.

Also, you should probably test for Serial.available before you do that read. If there's nothing available it's going to give you a funny result.

You don't need to test for available to use that value though.

Say you send an S. The line I mention in the last post reads it. Now there is nothing left available, and serial is S.

Later you have this:

if(Serial.available()>0 && serial =='S'){
  shieldbot.drive(0,0);
  Serial.println("Stop");
}

So the bot will only stop if you just read the S and there just happens to be another character available to read. You really don't need to check available here.

int serial = Serial.read();

Serial.read() returns a char. (A character: letter or number or symbol.) While you can store a char in an int with no loss of precision, it's not a good idea. Keep it as a char.

if(Serial.available()>0 && serial =='S')

So this only takes effect when we received an 'S' AND there's another character in the serial buffer after the S?

do{
    ....
} while(Serial.available() == 0 && serial == 'G');

This will always run at least once - it doesn't check the while condition until after it has done the action inside the loop. Then you never update serial inside the loop so checking if it's 'G' does nothing - it can never change inside the loop.

I did some modifications to the code… But the problem now is that the car does not quite follow the line. And without all de Serial port stuff it used to work very well.

#include<Shieldbot.h> //Incluyo la libreria
Shieldbot shieldbot = Shieldbot(); //Declaro un objeto de tipo Shieldbot
int S1, S2, S3, S4, S5; //Establezco variables correspondientes a los sensores
int posA, PosO; //Establezco variables correspondientes a la posición
int error; //Declaro la variable error
int valor = 0;
int pinAnalogo = 4;
int serial = 0;

void setup() {
  Serial.begin(9600); //Abre el puerto serie configurando la velocidad en 9600 bps
  shieldbot.setMaxSpeed(50, 49); //Establece la velocidad máxima de los carritos
  }
void loop(){
//Leo el sensor de proximidad
valor = analogRead(pinAnalogo);

//Leo los sensores
 S1 = shieldbot.readS1();
 S2 = shieldbot.readS2();
 S3 = shieldbot.readS3();
 S4 = shieldbot.readS4();
 S5 = shieldbot.readS5();
 //Establezco la posicion objetivo
PosO = 0;
//Determino la posicion del carrito de acuerdo a cada uno de los sensores
if(S1 == LOW){posA = 2;}
if(S2 == LOW){posA = 1;}
if(S3 == LOW){posA = 0;}
if(S4 == LOW){posA = -1;}
if(S5 == LOW){posA = -2;}

error = PosO - posA;
//Determino el criterio de parada

//Leo el puerto serial
serial = Serial.read();

if(Serial.available()>0 && serial =='S'){
  shieldbot.drive(0,0);
  Serial.println("Stop");
}

do
{
if(error == 0 && valor>200){
  shieldbot.forward(); 
  Serial.println("Forward");
}
if(error >0 && valor>200) {
  shieldbot.drive(0, 127);
  Serial.println("Right");
}

if(error <0 || valor>200){
  shieldbot.drive(127, 0);
  Serial.println("Left");}
} while(Serial.available() == 0 && serial == 'G');
}

MorganS:

int serial = Serial.read();

Serial.read() returns a char. (A character: letter or number or symbol.) While you can store a char in an int with no loss of precision, it's not a good idea. Keep it as a char.

No, Serial.read() returns an int, not a char. It returns -1 if no character is ready.

Regards,
Ray L.

The thing is that I cannot make the car STOP when I write an S on the serial port

Yes, we understand. All the comments above are to help you fix that. You are currently not reading the Serial port in a way that processes each character correctly, which is why hitting 'S' does not do anything.

Change your code so it looks like this:

void loop()
{
    ...
    if (Serial.available() > 0)
    {
        int serial = Serial.read();

        if (serial == 'S')
        {
            // stop code here
            ...
        }
        else if (serial == 'G')
        {
            // go code here
            ...
        }        
    }
    ...      
}

arduinodlb:
Yes, we understand. All the comments above are to help you fix that. You are currently not reading the Serial port in a way that processes each character correctly, which is why hitting 'S' does not do anything.

Change your code so it looks like this:

void loop()

{
    ...
    if (Serial.available() > 0)
    {
        int serial = Serial.read();

if (serial == 'S')
        {
            // stop code here
            ...
        }
        else if (serial == 'G')
        {
            // go code here
            ...
        }       
    }
    ...     
}

arduinodlb:
Yes, we understand. All the comments above are to help you fix that. You are currently not reading the Serial port in a way that processes each character correctly, which is why hitting 'S' does not do anything.

Change your code so it looks like this:

void loop()

{
    ...
    if (Serial.available() > 0)
    {
        int serial = Serial.read();

if (serial == 'S')
        {
            // stop code here
            ...
        }
        else if (serial == 'G')
        {
            // go code here
            ...
        }       
    }
    ...     
}



The thing is that I already did those modifications to the code and it does not do whats inside of the (serial == 'G') statement properly. I used a while and then change it to an else if. But it still does not work. 
And that used to work pretty good when i was not reading the serial port.

Please define "Does not work" and "worked pretty good". What does it do or not do?

Delta_G:
Please define “Does not work” and “worked pretty good”. What does it do or not do?

It used to follow a black line of tape in the floor. And now it moves and changes its direction but does not follow the line. But it stops…

You've added this valor stuff since then. What does valor represent? What is that sensor and what is it reading?

Look at the three if statements in the code in reply #7. For either forward or right, valor must be greater than 200.

But the last if, the one for left, uses OR. So if the value of valor is greater than 200, the bot will turn left regardless of what error is.

So in order for forward or right to happen, the condition is satisfied to make left happen. I don't see how it would ever go forward or right then.

Delta_G:
Look at the three if statements in the code in reply #7. For either forward or right, valor must be greater than 200.

But the last if, the one for left, uses OR. So if the value of valor is greater than 200, the bot will turn left regardless of what error is.

So in order for forward or right to happen, the condition is satisfied to make left happen. I don't see how it would ever go forward or right then.

Value is a proximity sensor. But i'm actually not using it right know so I modify the code and remove the value part. Even before reply #10. Apparently the Serial part of the code does not allow the other part to work

if(Serial.available()>0 && serial =='S')

You still have this issue. Why don't you correct it?

And why are you using a do - while instead of a plain old while loop? Or better yet, let the loop function do the looping and put that code in if / else?

A do - while will always run the code at least once unconditionally. So even if you're not in go mode, it's still going to go through the forward / left / right code. That hardly seems like what you want here.