Different Behavior Depending on Order of the Inputs?

I am using two switches (S1 & S2), connected to an Arduino Uno R3 to control the behavior of four motors (M0, M1, M2, M3) within the setup function of my sketch. The switches are magnetically actuated reed switches which drive the Arduino input pin LOW when the magnet approaches the switch. I am using the Arduino internal pull-up resistors on the inputs.

What I want to happen is as follows:

If S1 and S2 are both LOW, turn off M0, M1, M2, M3. If S1 and S2 are both HIGH, turn on M0, M1, M2, M3. If S1 is LOW, and S2 is HIGH, turn off M0, and turn on M1, M2, M3. If S1 is HIGH and S2 is HIGH, turn on M0, M1, M2, M3.

Here is what actually happens. Both switches start out HIGH and all four motors immediately turn on.

If S1 goes LOW first, M0 turns off as desired. If S2 goes LOW next, all four motors turn off as desired. Perfect! This is exactly what I want.

The problem is when I reverse the order of the switches. This is where it gets really weird!

If S2 goes LOW first, all four motors stay on as desired. If S1 goes LOW next, only M1, M2, M3 turn off but motor M0 stays on!

Why does it matter what order I turn on the switches? And why does M0 remain on when both switches are LOW? The code clearly specifies all motors off when both switches are LOW. I am sure there is very logical explanation, but I can't figure out what it is?

The Arduino is being used to control a model roller coaster for my son. I am a mechanical engineer with very limited programming experience, so please take it easy on me.

Thanks in advance for any helpful suggestions.

Don

Here is an abbreviated version of my code. Please ask if you would like me to include more details in the sketch...

// map the input pin numbers to integer variables
const int S0 = A0;   
const int S1 = A1;

// map the output pin numbers to integer variables
const int M0 = 3;     
const int M1 = 4;
const int M2 = 5;
const int M3 = 6;
  
// the following code only runs only once during setup:
void setup() {                

// define these pins as inputs with an internal 20k ohm pull-up resistor
pinMode(S1, INPUT_PULLUP);
pinMode(S2, INPUT_PULLUP);

  
//initialize all inputs to high during setup (ie. switch is off)
digitalWrite(S1, HIGH); 
digitalWrite(S2, HIGH); 
   

// define these pins as outputs
pinMode(M0, OUTPUT);
pinMode(M1, OUTPUT);
pinMode(M2, OUTPUT);
pinMode(M3, OUTPUT);
  
  
//turn all motors off to start with by outputting a HIGH to the relay modules.
digitalWrite(M0, HIGH);
digitalWrite(M1, HIGH);
digitalWrite(M2, HIGH);
digitalWrite(M3, HIGH);

// Call returnCarsToStart function to move cars in the "start" position 

returnCarsToStart();   
}


void loop() {
/many functions here that are working fine.....
}

void returnCarsToStart (){

     while ((digitalRead(S1) == HIGH)  || (digitalRead(S2) == HIGH)) { 
  
      
      if ((digitalRead(S1) == LOW) && (digitalRead(S2) == LOW)){
      digitalWrite (M0, HIGH);
      digitalWrite (M1, HIGH);
      digitalWrite (M2, HIGH);
      digitalWrite (M3, HIGH);
    }
   
   
      if ((digitalRead(S1) == LOW) && (digitalRead(S2) == HIGH)){
      digitalWrite (M0, HIGH);
      digitalWrite (M1, LOW);
      digitalWrite (M2, LOW);
      digitalWrite (M3, LOW);
    }
    
      if ((digitalRead(S1) == HIGH) && (digitalRead(S2) == HIGH)){
      digitalWrite (M0, LOW);
      digitalWrite (M1, LOW);
      digitalWrite (M2, LOW);
      digitalWrite (M3, LOW);
    } 
       
      if ((digitalRead(S1) == HIGH) && (digitalRead(S2) == LOW)){
      digitalWrite (M0, LOW);
      digitalWrite (M1, LOW);
      digitalWrite (M2, LOW);
      digitalWrite (M3, LOW);
    
    
    } 
  }
 }
// define these pins as inputs with an internal 20k ohm pull-up resistor
pinMode(S1, INPUT_PULLUP);
pinMode(S2, INPUT_PULLUP);

  
//initialize all inputs to high during setup (ie. switch is off)
digitalWrite(S1, HIGH); 
digitalWrite(S2, HIGH);

S1 and S2 are INPUT. Writing to them does NOT make them HIGH or LOW. Writing to them turns on or off the pullup resistor.

pinMode(M3, OUTPUT);

Where is M3 defined?

void loop() {
/many functions here that are working fine.....
}

I refuse to take your word for that.

PaulS: S1 and S2 are INPUT. Writing to them does NOT make them HIGH or LOW. Writing to them turns on or off the pullup resistor.

My misunderstanding. Thank you for the correction.

PaulS: Where is M3 defined?

const int M3 = 6;

See above. I abbreviated the sketch in order to post it. Sorry, I missed this line. Will add to my original sketch above.

PaulS: void loop() { /many functions here that are working fine..... }

I refuse to take your word for that.

I realize the above discepancies don't inspire a lot of confidence in my sketch. I can only give you my word that the functions work. I have stepped through the program a about a dozen times and everything seems to be working except for this little quirk in the returnCarsToStart function. I wish I could post the whole sketch, but it exceeds 9000 characters, the limit for this forum. Let me know if you need more details and I will add it above.

If you have any ideas what might cause M0 to remain on when S1 and S2 are LOW, I would be very grateful to hear from you Paul.

Thanks, Don

If you're talking about the returnCarsToStart function, it can never get to that part where both switches are low because it is wrapped in a while clause that is looking for at least one of them to be HIGH. So they can't possibly both be low.

Delta_G: If you're talking about the returnCarsToStart function, it can never get to that part where both switches are low because it is wrapped in a while clause that is looking for at least one of them to be HIGH. So they can't possibly both be low.

Delta_G - Oh yes, that's it! As soon as the second switch goes low, it jumps out of the while loop before it gets a chance to shut off M0! Thank you for your explanation. I don't know how I missed that.

What I was trying to accomplish is to stay in the loop until both switches S1 and S2 go LOW, but then shut of all four motors M0, M1, M2, M3 before exiting the loop.

I guess I could add four digitalWrite statements to shut off all four motors following the While loop. Not a very eloquent solution. If anyone knows a better way to do this, I would be very grateful to hear about it.

May I suggest that you don't read switches in IF or WHILE statements. Read the switches and save their values and use the saved values for the decisions. It makes the logic easier to follow and it affords the opportunity to use Serial.println() to display the values so you can see what the decisions are being based on.

I also suggest that you read all the switches at the same time - perhaps in a readSwitches() function to declutter the logic part of the code.

...R

Delta_G: If you're talking about the returnCarsToStart function, it can never get to that part where both switches are low because it is wrapped in a while clause that is looking for at least one of them to be HIGH. So they can't possibly both be low.

I stopped the motors after the While loop rather than inside the while loop. This has solved the problem. Thanks Delta_G for your input.

Robin2: May I suggest that you don't read switches in IF or WHILE statements. Read the switches and save their values and use the saved values for the decisions. It makes the logic easier to follow and it affords the opportunity to use Serial.println() to display the values so you can see what the decisions are being based on.

I also suggest that you read all the switches at the same time - perhaps in a readSwitches() function to declutter the logic part of the code.

...R

Robin2 - Thank you for your suggestions. Much appreciated! I created a readSwitches() function as suggested, and called this function within the returnCarsToStart() function, but am now getting "scope" errors. How do I transfer the variables like Switch1, Switch2 from the readSwitches() function to the returnCarsToStart() function? I think I have to declare them as global variables - yes?

Snippets of the offending code for reference are below. I get a "Switch1 was not declared in this scope" error on this line...

while (Switch1 == HIGH) || (Switch2 == HIGH))

void returnCarsToStart (){
// continuously execute following code until a car is located at 
// both the station (S1 = LOW) and the brake run (S2 = LOW)    
    
    readSwitches();   //read state of all switches
    
    while (Switch1 == HIGH)  || (Switch2 == HIGH)) { 
    
   // if there is a car at the station (S1 = LOW) and no car at the brake run (S2 = HIGH)
   // then turn off motor M0 and turn on motors M1, M2, M3.
      if ((Switch1 == LOW) && (Switch2 == HIGH)){
      digitalWrite (M0, HIGH);
      digitalWrite (M1, LOW);
      digitalWrite (M2, LOW);
      digitalWrite (M3, LOW);
    }
    
    // if there is no car at the station (S1 = HIGH) and no car at the brake
   //  run (S2 = HIGH)then turn on all four motors M0, M1, M2, M3.
      if ((Switch1 == HIGH) && (Switch2 == HIGH)){
      digitalWrite (M0, LOW);
      digitalWrite (M1, LOW);
      digitalWrite (M2, LOW);
      digitalWrite (M3, LOW);
    } 
       
   // if there is no car at the station (S1 = HIGH) and a car at the brake
   //  run (S2 = LOW)then turn on all four motors M0, M1, M2, M3.
      if ((Switch1 == HIGH) && (Switch2 == LOW)){
      digitalWrite (M0, LOW);
      digitalWrite (M1, LOW);
      digitalWrite (M2, LOW);
      digitalWrite (M3, LOW);
    
    } 
  }
 }
 void readSwitches(){
  // read the state of all switches and store in local variable
 int Switch0 = digitalRead(S0);
 int Switch1 = digitalRead(S1);
 int Switch2 = digitalRead(S2);
 int Switch3 = digitalRead(S3);
 int Switch4 = digitalRead(S4);
 int Switch5 = digitalRead(S5);
 int Switch6 = digitalRead(S6);

//print the state of all switches, timers, flags, and counters to the terminal
    Serial.print("S0=");
    Serial.print(Switch0);
    Serial.print ("  ");
    Serial.print("S1=");
    Serial.print(Switch1);
    Serial.print ("  ");
    Serial.print("S2=");
    Serial.print(Switch2);
    Serial.print ("  ");
    Serial.print("S3=");
    Serial.print(Switch3);
    Serial.print("  ");
    Serial.print("S4=");
    Serial.print(Switch4);
    Serial.print("  ");
    Serial.print("S5=");
    Serial.print(Switch5);
    Serial.print("  ");
    Serial.print("S6=");
    Serial.print(Switch6);
    Serial.print("  ");
    Serial.print("CC=");
    Serial.print(cycleCounter);
    Serial.print("  ");
    Serial.print("T1=");
    Serial.print(gateOpenDelayTime);
    Serial.print("  ");
    Serial.print("T2=");
    Serial.print(gateCloseDelayTime);
    Serial.print("  ");
    Serial.print("T3=");
    Serial.print(stationStartDelayTime);
    Serial.print("  ");
    Serial.print("T4=");
    Serial.print(brakeRunStartDelayTime);
    Serial.print("  ");
    Serial.print("F1=");
    Serial.print(carLoadedFlag);
    Serial.println("  ");
    delay (500);

  }

Here is the error codes...

latest_program_Jan_1_test_2015:227: error: expected primary-expression before '||' token
latest_program_Jan_1_test_2015:227: error: 'Switch2' was not declared in this scope
latest_program_Jan_1_test_2015:227: error: expected `;' before ')' token
latest_program_Jan_1_test_2015:517: error: expected `}' at end of input

Thanks for your help!

Don

dz63: How do I transfer the variables like Switch1, Switch2 from the readSwitches() function to the returnCarsToStart() function? I think I have to declare them as global variables - yes?

I would just use global variables - in the small space in an Arduino it is easy to keep things straight whereas in a PC with 2GB of RAM it might be harder.

Also, I would call readSwitches() from loop() rather than from a subsidiary function - just so I can easily see the concept of the code by a quick read of loop().

...R

Use mouse click AFTER ( or ) to identify the "code block" and you will see the syntax error on this line

while (Switch1 == HIGH) || (Switch2 == HIGH)) {

Robin2: I would just use global variables - in the small space in an Arduino it is easy to keep things straight whereas in a PC with 2GB of RAM it might be harder.

You do realize that global variables are sacrilege on this forum! ;)

Robin2: Also, I would call readSwitches() from loop() rather than from a subsidiary function - just so I can easily see the concept of the code by a quick read of loop().

...R

Good advice. Just implemented it and old Arduino is misbehaving now. Trying to debug it right now. Thanks for your helpful suggestions!

Vaclav: Use mouse click AFTER ( or ) to identify the "code block" and you will see the syntax error on this line

while (Switch1 == HIGH) || (Switch2 == HIGH)) {

Good catch! All fixed up now. Thanks!

dz63: You do realize that global variables are sacrilege on this forum! ;)

Anarchy rules :)

...R