While loop not behaving as expected!

I am new to Arduino. Trying to help my son control a Knex roller coaster with multiple DC motors and magnetically actuated reed switches to determine the position of two roller coaster cars on same track.

Objective:
To execute a chunk of code continuously until two digital inputs (ie. Switch 0 and Switch 3) go high. This occurs when one of the roller coasters is in the station and the other is on the brake run portion of the track. I am trying to use a while loop to achieve this, but it is not behaving as I expected.

Here is my code ( I only included the pertinent part):

void loop(){
  preStageCoasters();   // call preStageCoasters function
 } 
 
 void preStageCoasters (){
  // read the state of the switches S0 and S3
  int Switch0 = digitalRead(S0);
  int Switch3 = digitalRead(S3);
  
  //continuously execute following code until both switches S0 and S3 go high     
  while ((Switch0 == LOW)  || (Switch3 == LOW)); { 
  // read the state of the switches S0 and S3
    int Switch0 = digitalRead(S0);
    int Switch3 = digitalRead(S3);
  //print the state of both switches to the terminal
    Serial.print("Station Sw0=");
    Serial.print(Switch0);
    Serial.print ("  ");
    Serial.print("Brake Run Sw3=");
    Serial.println(Switch3);
    delay (1000);
  }
 }

Observations:
When I reset the Arduino with both switches low, I get no printing to the terminal. However, if I reset the Arduino with both switches high, I get the following print out on my terminal once every second...

Station Sw0=1 Brake Run Sw3=1
Station Sw0=1 Brake Run Sw3=1
Station Sw0=1 Brake Run Sw3=1

As soon as either one of the switches goes low, the "while" loop is exited and printing stops. When I replaced the "||" with a "&&" in the above while loop, I get the same behavior except now, both switches must go low to stop the printing. Neither makes any sense to me. I thought the code within the while loop was to continuously execute provided that the conditions were met? With both switches high, it should completely bypass the serial printing altogether. Very puzzling!

Questions:

  1. Can anyone explain what is happening here?
  2. How do I achieve what I want to do - namely continuously execute a chunk of code until both switches are high?

Thanks!

Don

pretty sure the line
while ((Switch0 == LOW) || (Switch3 == LOW)); {
should be
while ((Switch0 == LOW) || (Switch3 == LOW)) {

Basically removed the apostrophe, pretty sure that'll fix your problem

Also you should create the integers
int Switch0 = digitalRead(S0);
int Switch3 = digitalRead(S3);
outside of the loop, that is basically recreating the same integer every time the loop runs which is unnecessary.
so it should become simply:
Switch0 = digitalRead(S0);
Switch3 = digitalRead(S3);
Just make sure to create the variables int Switch0 and int Switch3 before you try to read them.

Hi Don.

Do a serial print of your switches right before you do the while loop, I think your whole loop is already exited before it even begins

Facinerous:
pretty sure the line
while ((Switch0 == LOW) || (Switch3 == LOW)); {
should be
while ((Switch0 == LOW) || (Switch3 == LOW)) {

Basically removed the apostrophe, pretty sure that'll fix your problem

It's the semicolon that he needs to remove. There's no apostrophe there.

bwa haha. Your so right. Bad grammar on my part.

Not apostrophe, semicolon

Facinerous:
pretty sure the line
while ((Switch0 == LOW) || (Switch3 == LOW)); {
should be
while ((Switch0 == LOW) || (Switch3 == LOW)) {

Basically removed the apostrophe, pretty sure that'll fix your problem

Fascinerous - Wow - thanks for the fast and helpful response. Very much appreciated. I removed the semi-colon as suggested and it helped significantly. Here is what I am now observing:

  1. Reset the Arduino with both switches high. No serial printing. That is exactly what I expect and want!
  2. Reset the Arduino with both switches low. Serial printing is now occurring. Perfect!
  3. After step 2, bring both switches high, serial printing continues. Not what I want! It should stop printing the moment both switches go high.

I think I am getting close. I need carefully read and absorb your other comments now.

Don

Facinerous:
Also you should create the integers
int Switch0 = digitalRead(S0);
int Switch3 = digitalRead(S3);
outside of the loop, that is basically recreating the same integer every time the loop runs which is unnecessary.
so it should become simply:
Switch0 = digitalRead(S0);
Switch3 = digitalRead(S3);
Just make sure to create the variables int Switch0 and int Switch3 before you try to read them.

Changing to...

Switch0 = digitalRead(S0);
Switch3 = digitalRead(S3);

...inside the while loop solved the problem. The printing now stops when both switches are high and the printing restarts immediately when either switch goes low. The code now executes exactly as wanted. I don't fully understand why removing the integer declaration inside the loop made this difference, and would be grateful for an explanation. But for now I am very happy. Thank you for your help.

Don

I'm glad to hear you problem is solved.

Basically a while loop will execute the following code if the condition is true.

while (true) Statement;

while (true) {
statement;
statement;
}

In order to have multiple statements execute for a while statement you use the brackets { and }. The compiler treats everything between the brackets as if they were one statement. The } bracket is sort of like ; telling the compiler that the statement is done.

The compiler expected to see some sort of something following your while condition, but since you ended that line with ; it just took that as "done" and kept on going.

The compiler also has a way of handling variables. So Switch0 and Switch3 are variables. You create a variable by using a variable type, then the name. Eg: int Switch0. This tells the compiler to create a variable named Switch0 which is stored in memory as an integer. You can assign that variable a value :
Switch0 = 1
Along with that you can assign a variable a value at initialization:
int Switch0 = 1.

If you initialize a variable inside a loop, the compiler will do it every time the loop runs. I'm not sure how the GCC compiler takes care of garbage but it kind of works like this.

int Switch0 = 1 // Compiler does what you want, creates the variable Switch0 and assigns it the value 1
(Bunches of code)
Switch0 = 0 // Compiler reassigns the variable Switch0 the value 0
int Switch0 = 1 // Compiler looks at the original variable, realizes there is no reason to have the int Switch0 anymore
so it destroys it. Then creates a new variable of type int named Switch0 and assigns it the value
1.
float Switch0 = 1.12 // Compiler looks at the original variable, realizes there is no reason to have the int Switch0
anymore, so it destroys it. Then creates a new variable of type float named Switch0, and
assigns it the value 1.12

So essentially everytime the loop runs you are destroying your original value and making a new one. Then assigning it a value.
int Switch0 = digitalRead(S0);
int Switch3 = digitalRead(S3);

You are calling the digitalRead function, but its giving you a starting value which may not match what your intention is. Thats the best I can explain without looking into the digitalRead function more.

I actually forgot one last thing when it comes to variables involving your question.

Its a little hard to explain, as its getting late for me so I suggest you look into it on your own accord.

Look into variable "scope".

I think in a C compiler the scope of a variable in a while loop gets destroyed when the while loop closes. So having the variable declared outside that while loop will keep it in tact for when the next time that while loop is called. This also relates to functions, and other things. Its a huge discussion and more than I want to type though.

I suspect you will soon find that your approach to this problem can't cope with all the possibilities.

You should read about State machines and study the timing concept in the Blink Without Delay example sketch. Basically, when a car passes a detector a variable in the program records that it has done so and other parts of the code do, or don't do their thing depending on the value of that function. This approach means that any combination of events is easily catered for.

I wrote a demo sketch here Demonstration code for several things at the same time - Project Guidance - Arduino Forum which expands on the Blink Without Delay sketch and should give a general idea of the concept of managing several things.

...R

Facinerous:
The compiler expected to see some sort of something following your while condition, but since you ended that line with ; it just took that as "done" and kept on going.

That makes a lot of sense. Thanks for the explanation.

Facinerous:
int Switch0 = 1 // Compiler does what you want, creates the variable Switch0 and assigns it the value 1
(Bunches of code)
Switch0 = 0 // Compiler reassigns the variable Switch0 the value 0

So essentially everytime the loop runs you are destroying your original value and making a new one. Then assigning it a value.
int Switch0 = digitalRead(S0);
int Switch3 = digitalRead(S3);

You are calling the digitalRead function, but its giving you a starting value which may not match what your intention is. Thats the best I can explain without looking into the digitalRead function more.

I appreciate the time you invested to explain. This one is a little tougher for me to understand because on the same line of code I did a digitalRead of the input pin.

Switch0 = digitalRead(S0);

So, regardless of the starting value, it is immediately overwritten with the current switch state, which is then used in the while loop decision making process. Regardless, your suggestion solved the problem. Thanks for your help.

Robin2:
I suspect you will soon find that your approach to this problem can't cope with all the possibilities.

You should read about State machines and study the timing concept in the Blink Without Delay example sketch. Basically, when a car passes a detector a variable in the program records that it has done so and other parts of the code do, or don't do their thing depending on the value of that function. This approach means that any combination of events is easily catered for.

I wrote a demo sketch here http://forum.arduino.cc/index.php?topic=223286.0 which expands on the Blink Without Delay sketch and should give a general idea of the concept of managing several things.

...R

Robin2 - thanks for the heads up! As you suggested, I am already struggling a bit to cope with all of the possibilities. I will study your example carefully to see if I can apply in my situation.

The brake run section of track and the station section of track have independent motors and a magnetically actuated reed switch to detect the cars. The brake run is just upstream of the station where the people board the coaster. The plan is to use an interrupt routine when the brake run sensor detects a car to immediately stop the brake run in order to prevent the 2nd car from slamming into the rear of the first car.

The station has operating turnstiles which open and close utilizing a 12VDC motor that I am reversing with two relays in an "H" bridge configuration as suggested on another post in this forum. While we are letting people onto the cars, the Arduino will be tied up, hence the need for an interrupt.

I am in the process of writing a flow chart to describe the process and it is getting more complicated than I initially thought it would be.

It is unlikely that anything is happening fast enough to require the complexity of interrupts.

...R

dz63:
While we are letting people onto the cars, the Arduino will be tied up, hence the need for an interrupt.

Tied up doing what?

I am in the process of writing a flow chart to describe the process and it is getting more complicated than I initially thought it would be.

That's almost always the case! :astonished:

Robin2 - utilizing millis instead of delay is a fantastic idea. I was going to use the delay command in my sketch for all of the time delays, but then the Arduino will be tied up and might miss a switch that changes state. Curious to know what "+=" means and why you used it in your sketch.

Henry_Best - I was planning to open the gates, let the people onto the roller coaster, then use the delay command for a 1-2 second time delay before closing the gates. That is when the Arduino would be tied up. In fact I was going to use the delay command for all of my time delays, but instead of that, I am now going to use the method Robin2 described above. That way, I can continually scan all of the switches and not tie up the Arduino.

| am struggling a little bit with the "while loop".

With Switch0 low and Switch3 high, it continues to execute code in the loop as expected.
With Switch3 low and Switch0 high, it jumps out of the while loop. This is unexpected.

I do not want to exit the while loop until both Switch0 and Switch3 go high.

Anybody have any idea why this occurs?

Thanks in advance!

Don

 void preStageCoasters (){
  // read the state of the switches S0 and S3
  int Switch0 = digitalRead(S0);
  int Switch3 = digitalRead(S3);
  
  //continuously execute following code until both switches S0 and S3 go high     
 while ((Switch0 == LOW)  || (Switch3 == LOW)) { 

  // read the state of the switches S0 and S3
    Switch0 = digitalRead(S0);
    Switch3 = digitalRead(S3);

  //print the state of the both swiches to the terminal
    Serial.print("Station Sw0=");
    Serial.print(Switch0);
    Serial.print ("  ");
    Serial.print("Brake Run Sw3=");
    Serial.println(Switch3);
    delay (1000);

    // if no car at the station or no car at the brake run, turn on all four motors
    if ((Switch0 == LOW) && (Switch3 == LOW)){
      digitalWrite (M0, LOW);
      digitalWrite (M1, LOW);
      digitalWrite (M2, LOW);
      digitalWrite (M3, LOW);
    }
    
// if there is no car at station and a car at the brake run, turn on all four motors
    if ((Switch0 == LOW) && (Switch3 = HIGH)){
      digitalWrite (M0, LOW);
      digitalWrite (M1, LOW);
      digitalWrite (M2, LOW);
      digitalWrite (M3, LOW);
    }
    
// if there is a car at station and no car at the brake run, stop station motor only
    if ((Switch0 == HIGH) && (Switch3 = LOW)){
      digitalWrite (M0, HIGH);
      digitalWrite (M1, LOW);
      digitalWrite (M2, LOW);
      digitalWrite (M3, LOW);
    }
    
 // if there is a car at station and a car at the brake run, stop all four motors
    if ((Switch0 == HIGH) && (Switch3 = HIGH)){
      digitalWrite (M0, HIGH);
      digitalWrite (M1, HIGH);
      digitalWrite (M2, HIGH);
      digitalWrite (M3, HIGH);
    } 
  }
 }

a += b is the exact same as a = a + b (C seems to have been designed by lazy typists)

...R

Robin2:
a += b is the exact same as a = a + b (C seems to have been designed by lazy typists)

I never would have guessed that on my own.

I disagree about the lazy typists theory.

The variable 'a' may actually be a long and complicated name. 'a+=b' reduces the number of mistakes that could occur, because that long and complicated name only needs to be correct once.

Furthermore, if the name changes, it only has to be changed once on the line instead of twice. A good IDE can take care of this for you, but not everybody has a good IDE, and nobody had a good IDE when the C language was designed.

vaj4088:
I disagree about the lazy typists theory.

Lighten up. My comment was slightly tongue-in-cheek. += is also used in other languages.

But there are many facets of C/C++ where it seems to me that brevity was preferred over simplicity.

...R

dz63:
[...]
I do not want to exit the while loop until both Switch0 and Switch3 go high.

Anybody have any idea why this occurs?

[...]

[...]

//continuously execute following code until both switches S0 and S3 go high    
while ((Switch0 == LOW)  || (Switch3 == LOW)) {
[...]
 }
}

Uh... isn't that an OR statement? Shouldn't you be using:

 while ((Switch0 == LOW)  && (Switch3 == LOW)) {

(I think you tried that before but had the errant semicolon causing you problems)

Regards,

Brad
KF7FER