unknown bug Ifs and ORs

Ok here goes the long pain of explanation. :slight_smile: If i run the line if (LimitSWWest == HIGH) I will get the Return

WestSafetySwitch
Turning West

Over and Over and ,, - in turn it doesnt stop the motor Completely it just flips back and forth on the screen, and Pause and starts the motor.
If i run the line changed to

else if (LimitSWWest == HIGH)

It will only run

Turning West

im not sure if the error is Starting from

else if ((Right > threshhold) || (Left > threshhold) || (LimitSWWest == LOW) && (Right > Left + difSence))

Here is the complete Code for your enjoyment :slight_smile:
any suggestions or thoughts would be greatly appreciated :slight_smile:

/* 
 Thomas Wood
 PhotoCell Controlled Stepper Motor Left and Right
 Feb 15,2012
 */

#include <Stepper.h>
#include <LiquidCrystal.h>

const int stepsPerRevolution = 200;  // Turns of the motor 
int inputPhotoLeft = A0; // photresistors pin
int inputPhotoRight = A1; // photresistors pin
int Left = 0; // Store readings from the photoresistors.
int Right = 0; // Store readings from the photoresistors.
int analogInput = A2; //Voltage - analog input
int analogInput1 = A3; //Voltage - analog input
int SafeSW1 = A4;
const int threshhold = 300; // this is here from reading the serial monitors value
//to get a ruff Idea of value to set And how dark you Want your motor to stop moving at 
int difSence = 15; // differenace tolarance between sensors
// float vout = 0.0;
float vin = 0.0;
float vin1 = 0.0;
// variable to store the value 
int value = 0;
int value1 = 0;
int LimitSWWest = 0;
// initialize the stepper library on pins 8 through 11:
Stepper myStepper(stepsPerRevolution, 8,9,10,11);
// initialize the Lcd library on pins 2 through 7:
LiquidCrystal lcd(7, 6, 5, 4, 3, 2);

void setup() {
  myStepper.setSpeed(150);
  pinMode(13, OUTPUT);
  pinMode(analogInput, INPUT);
  pinMode(analogInput1, INPUT);
  pinMode(SafeSW1, INPUT);
  lcd.begin(16, 4);
  Serial.begin(9600);
}
void loop() {
  value = analogRead(analogInput);
  vin = (value * 23.5) /1024;
  value1  = analogRead(analogInput1);
  vin1 = (value1 * 14.0) /1024;
  LimitSWWest = digitalRead(SafeSW1);
  lcd.clear();
  lcd.setCursor(-2, 2);
  lcd.print("Voltage=");
  lcd.print(vin);
  lcd.print("V");
  lcd.setCursor(-2, 3);
  lcd.print("Voltage1=");
  lcd.print(vin1);
  lcd.print("V");
  Left = analogRead(inputPhotoLeft); // sets the photocells position left - right
  Right = analogRead(inputPhotoRight);
  digitalWrite(13,LOW); //led doesnt need to be on to start with 
  //delay(500);
  if  (((Left > threshhold) || (Right > threshhold)) && (Left > Right + difSence))
  {
Serial.println("Turning East    ");
    lcd.setCursor(0, 0);
    lcd.print("Turning East    ");
    lcd.setCursor(0, 1);
    lcd.print("CounterClockWise");
    myStepper.step(-stepsPerRevolution);
     delay(500);
  }

  else if  ((Right > threshhold) || (Left > threshhold) || (LimitSWWest == LOW) && (Right > Left + difSence))
  {
    Serial.println("Turning West    ");
    lcd.setCursor(0, 0);
    lcd.print("Turning West    ");
    lcd.setCursor(0, 1);
    lcd.print("ClockWise       ");
    myStepper.step(stepsPerRevolution);
     delay(500);
  } 



  else if ((Left < threshhold) && (Right < threshhold))
  {  
    lcd.print("To Dark         ");
    lcd.setCursor(0, 0);
    lcd.print("To Dark         ");
    lcd.setCursor(0, 1);
    lcd.print("Motor Stopped   ");
    digitalWrite(8,LOW);
    digitalWrite(9,LOW);
    digitalWrite(10,LOW);
    digitalWrite(11,LOW);
    delay(500);
  }
  /*
  else 
  {
        Serial.println("Nothing Happening");
    lcd.setCursor(0, 0);
    lcd.print("Nothing Happening");
    lcd.setCursor(0, 1);
    lcd.print("Nothing Happening");
    digitalWrite(8,LOW);
    digitalWrite(9,LOW);
    digitalWrite(10,LOW);
    digitalWrite(11,LOW);
    delay(500);

  }*/
   else if (LimitSWWest == HIGH)  
  {
    Serial.println("WestSafetySwitch");
    lcd.setCursor(0, 0);
    lcd.print("WestSafetySwitch");
    lcd.setCursor(0, 1);
    lcd.print("WestSafetySwitch");
    digitalWrite(8,LOW);
    digitalWrite(9,LOW);
    digitalWrite(10,LOW);
    digitalWrite(11,LOW);
    delay(500);

  }
}

Try without the parentheses, I've never used them before and it seems to work, maybe you are confusing the compiler

Are you thinking to remove all the additional parentheses () from all of the

if statement

or just out of,to be like

else if (Right > threshhold || Left > threshhold || LimitSWWest == LOW && Right > Left + difSence)

Like that ?

I don't think that

else if  ((Right > threshhold) || (Left > threshhold) || (LimitSWWest == LOW) && (Right > Left + difSence))

is the same as

else if  (Right > threshhold || Left > threshhold || LimitSWWest == LOW && Right > Left + difSence)

I could be wrong, but I believe that && has higher precedence than || (C++ Operator Precedence - cppreference.com), which would make the second one equivalent to

else if  (Right > threshhold || Left > threshhold || (LimitSWWest == LOW && Right > Left + difSence))

I could be wrong, but I believe that && has higher precedence than ||

And and Or have the same precedence. But that precedence is quite different from that of +. You need parentheses around the addition bit on the end.

else if (Right > threshhold || Left > threshhold || LimitSWWest == LOW && Right > (Left + difSence))

And and Or have the same precedence.

That disagrees with the document I linked, which has And (level 13) above Or (level 14), but it isn't the official language spec, so could be wrong, I guess. According to the same table, addition is much higher (level 6), so would be done before anything else there.

This is why I use parentheses to make sure the order is as I want it to be.

Try without the parentheses, I've never used them before and it seems to work, maybe you are confusing the compiler

Awful advice, IMO. Try adding more parenthesis. I assure you that the compiler can handle pretty much any number of parenthesis just fine.

In particular:if  ((Right > threshhold) || (Left > threshhold) || (LimitSWWest == LOW) && (Right > Left + difSence))is looking pretty ambiguous. I never like to count on left-to-right evaluation of expressions when parenthesis can be added to make things explicit. (It comes from having used languages that defaulted to right-to-left evaluation.)

OK After thinking about what was said , and working on that path , this is what i am up with .

/* 
 Thomas Wood
 PhotoCell Controlled Stepper Motor Left and Right
 Feb 15,2012
 */

#include <Stepper.h>
#include <LiquidCrystal.h>

const int stepsPerRevolution = 200;  // Turns of the motor 
int inputPhotoLeft = A0; // photresistors pin
int inputPhotoRight = A1; // photresistors pin
int Left = 0; // Store readings from the photoresistors.
int Right = 0; // Store readings from the photoresistors.
int analogInput = A2; //Voltage - analog input
int analogInput1 = A3; //Voltage - analog input
int SafeSW1 = A4; //switch pin for limit
const int threshhold = 300; // this is here from reading the serial monitors value
//to get a ruff Idea of value to set And how dark you Want your motor to stop moving at 
int difSence = 15; // differenace tolarance between sensors
// float vout = 0.0;
float vin = 0.0;
float vin1 = 0.0;
// variable to store the value 
int value = 0;
int value1 = 0;
int LimitSWWest = 0; //varable for limit switch
// initialize the stepper library on pins 8 through 11:
Stepper myStepper(stepsPerRevolution, 8,9,10,11);
// initialize the Lcd library on pins 2 through 7:
LiquidCrystal lcd(7, 6, 5, 4, 3, 2);

void setup() {
  myStepper.setSpeed(150);
  pinMode(13, OUTPUT);
  pinMode(analogInput, INPUT);
  pinMode(analogInput1, INPUT);
  pinMode(SafeSW1, INPUT); // signal for limit switch
  lcd.begin(16, 4);
  Serial.begin(9600);//debug
}
void loop() {
  value = analogRead(analogInput);
  vin = (value * 23.5) /1024;
  value1  = analogRead(analogInput1);
  vin1 = (value1 * 14.0) /1024;
  LimitSWWest = digitalRead(SafeSW1);
  lcd.clear();
  lcd.setCursor(-2, 2);
  lcd.print("Voltage=");
  lcd.print(vin);
  lcd.print("V");
  lcd.setCursor(-2, 3);
  lcd.print("Voltage1=");
  lcd.print(vin1);
  lcd.print("V");
  Left = analogRead(inputPhotoLeft); // sets the photocells position left - right
  Right = analogRead(inputPhotoRight);
  digitalWrite(13,LOW); //led doesnt need to be on to start with 
  //delay(500);
  if  (((Left > threshhold) || (Right > threshhold)) && (Left > Right + difSence))
  {
Serial.println("Turning East    "); //debug
    lcd.setCursor(0, 0);
    lcd.print("Turning East    ");
    lcd.setCursor(0, 1);
    lcd.print("CounterClockWise");
    myStepper.step(-stepsPerRevolution);
     //delay(500);
  }

 else if (((Right > threshhold) || (Left > threshhold)) && ((Right > Left + difSence) && (LimitSWWest == LOW)))
//debug line test
//else if  (Right > threshhold || Left > threshhold || LimitSWWest == LOW && Right > (Left + difSence))
//else if  (Right > threshhold || Left > threshhold || (LimitSWWest == LOW && Right > Left + difSence))
// original statement else if  ((Right > threshhold) || (Left > threshhold) || (LimitSWWest == LOW) && (Right > Left + difSence))
  {
    Serial.println("Turning West    "); //debug
    lcd.setCursor(0, 0);
    lcd.print("Turning West    ");
    lcd.setCursor(0, 1);
    lcd.print("ClockWise       ");
    myStepper.step(stepsPerRevolution);
    // delay(500);
  }
  
else if(LimitSWWest == HIGH)  
  {
    Serial.println("WestSafetySwitch"); //debug
    lcd.setCursor(0, 0);
    lcd.print("WestSafetySwitch");
    lcd.setCursor(0, 1);
    lcd.print("WestSafetySwitch");
    digitalWrite(8,LOW);
    digitalWrite(9,LOW);
    digitalWrite(10,LOW);
    digitalWrite(11,LOW);
    delay(500);

  } 


//debug
  else if ((Left < threshhold) && (Right < threshhold))
  {  
    lcd.print("To Dark         ");
    lcd.setCursor(0, 0);
    lcd.print("To Dark         ");
    lcd.setCursor(0, 1);
    lcd.print("Motor Stopped   ");
    digitalWrite(8,LOW);
    digitalWrite(9,LOW);
    digitalWrite(10,LOW);
    digitalWrite(11,LOW);
    delay(500);
  }
  
  else 
  {
        Serial.println("NothingHappening");//debug
    lcd.setCursor(0, 0);
    lcd.print("NothingHappening");
    lcd.setCursor(0, 1);
    lcd.print("NothingHappening");
    digitalWrite(8,LOW);
    digitalWrite(9,LOW);
    digitalWrite(10,LOW);
    digitalWrite(11,LOW);
    delay(500);

  }//debug
  Serial.print("Left \t");
  Serial.print(Left);
  Serial.print("  Right \t");
  Serial.println(Right);
//debug
}

Here is the modified Bit that is orretly working

else if (((Right > threshhold) || (Left > threshhold)) && ((Right > Left + difSence) && (LimitSWWest == LOW)))

youll notice the Remarked debugging bits :slight_smile:
As always thanks for everyone input/thoughts/possable ideas :slight_smile:
now i an add both direction limit switchs- so nothing cathes on fire :slight_smile:

else if (((Right > threshhold) || (Left > threshhold)) && ((Right > Left + difSence) && (LimitSWWest == LOW)))

Also, don't be afraid to break up your complicated "if" statements:

else if ((Right > threshhold) || (Left > threshhold)) {
  // movement needed !
  if ((Right > Left + difSence) && (LimitSWWest == LOW)) {
    // But only if we're within reasonable limits
    //... Do Stuff ...
  }
}

(I have no idea if those comments are correct. Just guessing.)

awsome thinking ,, but on those statements will need to be true to opperate that function, so maybe breaking those lines may do more harm than good :slight_smile:

but i keep thinking ,, wonder if there is another way to run the sketch ,, such as a array ./switch statement kinda thing .. whats anyone think ?.. maybe to make it more compact/simple ?.. lol less line easyer for coding :slight_smile: