My code reads switch positions wrong. More than one if statment true?

Using Arduino Programming Questions
#1

The code is supposed to controll a camera slider with a stepper motor. I have the stepper motor all wired up and it works great. The speed of the stepper is set by chosing different combinations of the 4 switsches. There are 16 combinations in total. I tested all of this with just the first 3 combinations. (Mode s2, Mode s3 and Mode s4). This worked perfectly, but after adding the other 14 combinations the Serial monitor shows that it does not track the mode correctly anymore. It switches randomly the modes it is in. So it seems as if it is missreading the button combinations. This also happens on the 123Circuits, so its not hardware I think.

I guess there is something wrong with my if statements so that more than one “mode” is true at the same time. Is this the case? With modes I mean “Mode s2”, “Mode s3” and so on.

int Pin2 = 2; //Pin 1 cannot be used for this. Start at 2.
int Pin3 = 3;
int Pin4 = 4;
int Pin5 = 5;
int Pin6 = 6;
int Pin7 = 7;
int Pin8 = 8;
int Pin9 = 9;

int SliderLength = 197; //Travel length in mm
double PulleyDiameter = 6.6; //Pulley Diameter in mm
int TravelTime; //Used for calculations in the loop section.
double Steps; //Steps required to move along the entire Rail.
double StepDelay;
int StepsTaken = 0;

bool s2;   ///s2 stands for switch2_status. It starts at 2 so we can match the pin.
bool s3;
bool s4;
bool s5;
bool s6;
bool s7;

const int stepPin = 8; //defines the step pin
const int dirPin = 9; //defines the direction pin


void setup()
{
  Serial.begin(9600); //Required for Serial Monitor
  pinMode(Pin2, INPUT);
  pinMode(Pin3, INPUT);
  pinMode(Pin4, INPUT);
  pinMode(Pin5, INPUT);
  pinMode(Pin6, INPUT);
  pinMode(Pin7, INPUT);
  pinMode(Pin8, OUTPUT);
  pinMode(Pin9, OUTPUT);
  Steps = 200 * (SliderLength / (3.14 * PulleyDiameter)); ///Calculates the Steps required to move along the entire Rail.
  Serial.println("");
  Serial.print("Steps = ");
  Serial.println(Steps);
  digitalWrite(dirPin, HIGH);  //Defines the direction of travel. LOW reverses it.
}

void MoveStepper()
{
  digitalWrite(stepPin, HIGH); //This moves the stepper by 1 step.
  delayMicroseconds(1000); //Depends on Motor 500-1000 is good
  digitalWrite(stepPin, LOW);
  delayMicroseconds(1000);
  StepsTaken++;
}

// the loop routine runs over and over again forever:
void loop()
{
  s2 = digitalRead(Pin2);  ///This checks the status of the switches.
  s3 = digitalRead(Pin3);
  s4 = digitalRead(Pin4);
  s5 = digitalRead(Pin5);
  s6 = digitalRead(Pin6);
  s7 = digitalRead(Pin7);

  if (s2 == LOW && (s3 & s4 & s5) == HIGH) ///LOW means switch is pressed/closed/conductive.
  {
    if (StepsTaken < Steps)
    { Serial.println("Mode = s2");
      TravelTime = 10; //The slider will move the distance in x seconds.
      StepDelay = ((TravelTime / Steps) - 0.002) * 1000; //This calculates the delay required in ms between steps to make the journey in the desired amount of time. 0.001 Seconds are deducted because the MoveStepper Method includes 2x 500microseconds delays.
      MoveStepper();
      Serial.print("Stepdelay = ");
      Serial.println(StepDelay);
      delay (StepDelay);
      // THis is in miliseconds (ms) Not MICRO!

    }
  }


  if (s3 == LOW && (s2 & s4 & s5) == HIGH) ///LOW means switch is pressed/closed/conductive.
  {
    if (StepsTaken < Steps)
    { Serial.println("Mode = s3");
      TravelTime = 30; //The slider will move the distance in 30s
      StepDelay = ((TravelTime / Steps) - 0.002) * 1000; //This calculates the delay required in ms between steps to make the journey in the desired amount of time. 0.001 Seconds are deducted because the MoveStepper Method includes 2x 500microseconds delays.
      MoveStepper();
      Serial.print("Stepdelay = ");
      Serial.println(StepDelay);
      delay (StepDelay); // THis is in miliseconds (ms) Not MICRO!

    }
  }
  if (s4 == LOW && (s2 & s3 & s5) == HIGH) ///LOW means switch is pressed/closed/conductive.
  {
    if (StepsTaken < Steps)
    { Serial.println("Mode = s4");
      TravelTime = 60; //The slider will move the distance in 30s
      StepDelay = ((TravelTime / Steps) - 0.002) * 1000; //This calculates the delay required in ms between steps to make the journey in the desired amount of time. 0.001 Seconds are deducted because the MoveStepper Method includes 2x 500microseconds delays.
      MoveStepper();
      Serial.print("Stepdelay = ");
      Serial.println(StepDelay);
      delay (StepDelay); // THis is in miliseconds (ms) Not MICRO!
    }
  }
  if (s5 == LOW && (s2 & s3 & s4) == HIGH) ///LOW means switch is pressed/closed/conductive.
  {
    if (StepsTaken < Steps)
    { Serial.println("Mode = s5");
      TravelTime = 120; //The slider will move the distance in 30s
      StepDelay = ((TravelTime / Steps) - 0.002) * 1000; //This calculates the delay required in ms between steps to make the journey in the desired amount of time. 0.001 Seconds are deducted because the MoveStepper Method includes 2x 500microseconds delays.
      MoveStepper();
      Serial.print("Stepdelay = ");
      Serial.println(StepDelay);
      delay (StepDelay); // THis is in miliseconds (ms) Not MICRO!
    }
  }
  if ((s2 & s3) == LOW && (s4 & s5) == HIGH) ///LOW means switch is pressed/closed/conductive.
  {
    if (StepsTaken < Steps)
    { Serial.println("Mode = s2,3");
      TravelTime = 300; //The slider will move the distance in 30s
      StepDelay = ((TravelTime / Steps) - 0.002) * 1000; //This calculates the delay required in ms between steps to make the journey in the desired amount of time. 0.001 Seconds are deducted because the MoveStepper Method includes 2x 500microseconds delays.
      MoveStepper();
      Serial.print("Stepdelay = ");
      Serial.println(StepDelay);
      delay (StepDelay); // THis is in miliseconds (ms) Not MICRO!
    }
  }
#2

Do you understand what (s2 & s4 & s5) == HIGH is doing?

#3

The test when the AND equals HIGH works - if it is meant to test that all three are HIGH.
But this:

(s2 & s3) == LOW

is true if either or both are LOW. It does not mean "if both are LOW". For that you would need to use:

!(s2 & s3) == HIGH

Pete

#4

It is, of course, better to spell out exactly what you are comparing.

if(s2 == LOW && s3 == HIGH && s4 == HIGH && s5 == HIGH)

Of course, that is terribly complicated. Setting 4 bits in a byte, using bitSet(), and then using a switch statement with the 16 cases is far easier to follow than a bunch of complicated if statements.

#5

PaulS:
It is, of course, better to spell out exactly what you are comparing.

if(s2 == LOW && s3 == HIGH && s4 == HIGH && s5 == HIGH)

Of course, that is terribly complicated. Setting 4 bits in a byte, using bitSet(), and then using a switch statement with the 16 cases is far easier to follow than a bunch of complicated if statements.

How would yo do that?

#6

How would yo do that?

byte pinInfo =0;
for(byte b=0; b<switchCount; b++)
{
   bitWrite(b, digitalRead(pin[b]);
}

Of course, this requires that you put the pin numbers in an array (which you should have when you started numbering the variables) and that you define how many switches there are (using the name switchCount, or changing the for statement to use your value).

Then, you can use

   switch(pinInfo)
   {
      case 0:
         // No switches pressed
         break;
      case 1:
         // Only switch one pressed
         break;
      case 2:
         // Only switch two pressed
         break;
      case 3:
         // Switches one and two pressed
         break;
      // Only 12 (or more) cases to go...
   }
#7

An alternative is the use of structs and unions.

struct SPEEDSWITCHES
{
  byte b2: 1;
  byte b3: 1;
  byte b4: 1;
  byte b5: 1;
};

union SPEED
{
  SPEEDSWITCHES switches;
  byte speedsetting;
};
SPEED speed;

The struct contains the status of the 4 buttons. The union allows you to either access the bits (based on the switches) or the value (0..15).

And in loop

  // read switches
  speed.switches.b2 = !digitalRead(Pin2);
  speed.switches.b3 = !digitalRead(Pin3);
  speed.switches.b4 = !digitalRead(Pin4);
  speed.switches.b5 = !digitalRead(Pin5);

  // print result (only 4 switches so limit to 15 max.
  Serial.print("Speed: "); Serial.println(speed.speedsetting & 0x0F);

The first 4 lines read the switches, invert the read value (because LOW indicates pressed) and stores them it in the associated bits. With no switches pressed, the print statement will give 0, with all switch pressed it will give 15.

#8

Thank you for the explanation, I will look into the switch thing for my next project. I need to finish this tonight and don´t have time to change it all.

I went ahead and changed the if statements to say exactly what they are supposed to do. It still does not work. Any if statement that contains more than 1 pressed switch does not work. Only single switches work, so I have 4 modes at the moment.

Here is the new code(minus the end because of line count):

int Pin2 = 2; //Pin 1 cannot be used for this. Start at 2.
int Pin3 = 3;
int Pin4 = 4;
int Pin5 = 5;

int Pin8 = 8;
int Pin9 = 9;

int SliderLength = 197; //Travel length in mm
double PulleyDiameter = 6.6; //Pulley Diameter in mm
int TravelTime; //Used for calculations in the loop section.
double Steps; //Steps required to move along the entire Rail.
double StepDelay;
int StepsTaken = 0;

bool s2;   ///s2 stands for switch2_status. It starts at 2 so we can match the pin.
bool s3;
bool s4;
bool s5;


const int stepPin = 8; //defines the step pin
const int dirPin = 9; //defines the direction pin


void setup()
{
  Serial.begin(9600); //Required for Serial Monitor
  pinMode(Pin2, INPUT);
  pinMode(Pin3, INPUT);
  pinMode(Pin4, INPUT);
  pinMode(Pin5, INPUT);
  
  pinMode(Pin8, OUTPUT);
  pinMode(Pin9, OUTPUT);
  Steps = 200 * (SliderLength / (3.14 * PulleyDiameter)); ///Calculates the Steps required to move along the entire Rail.
  Serial.println("");
  Serial.print("Steps = ");
  Serial.println(Steps);
  digitalWrite(dirPin, HIGH);  //Defines the direction of travel. LOW reverses it.
}

void MoveStepper()
{
  digitalWrite(stepPin, HIGH); //This moves the stepper by 1 step.
  delayMicroseconds(1000); //Depends on Motor 500-1000 is good
  digitalWrite(stepPin, LOW);
  delayMicroseconds(1000);
  StepsTaken++;
}

// the loop routine runs over and over again forever:
void loop()
{
  s2 = digitalRead(Pin2);  ///This checks the status of the switches.
  s3 = digitalRead(Pin3);
  s4 = digitalRead(Pin4);
  s5 = digitalRead(Pin5);


  if (s2 == LOW && s3 == HIGH && s4 == HIGH && s5 == HIGH) ///LOW means switch is pressed/closed/conductive.
  {
    if (StepsTaken < Steps)
    { Serial.println("Mode = s2");
      TravelTime = 10; //The slider will move the distance in x seconds.
      StepDelay = ((TravelTime / Steps) - 0.002) * 1000; //This calculates the delay required in ms between steps to make the journey in the desired amount of time. 0.001 Seconds are deducted because the MoveStepper Method includes 2x 500microseconds delays.
      MoveStepper();
      Serial.print("Stepdelay = ");
      Serial.println(StepDelay);
      delay (StepDelay);
      // THis is in miliseconds (ms) Not MICRO!

    }
  }


  if (s3 == LOW && s2 == HIGH && s4 == HIGH && s5 == HIGH) ///LOW means switch is pressed/closed/conductive.
  {
    if (StepsTaken < Steps)
    { Serial.println("Mode = s3");
      TravelTime = 30; //The slider will move the distance in 30s
      StepDelay = ((TravelTime / Steps) - 0.002) * 1000; //This calculates the delay required in ms between steps to make the journey in the desired amount of time. 0.001 Seconds are deducted because the MoveStepper Method includes 2x 500microseconds delays.
      MoveStepper();
      Serial.print("Stepdelay = ");
      Serial.println(StepDelay);
      delay (StepDelay); // THis is in miliseconds (ms) Not MICRO!

    }
  }
  if (s4 == LOW && s2 == HIGH && s3 == HIGH && s5 == HIGH) ///LOW means switch is pressed/closed/conductive.
  {
    if (StepsTaken < Steps)
    { Serial.println("Mode = s4");
      TravelTime = 60; //The slider will move the distance in 30s
      StepDelay = ((TravelTime / Steps) - 0.002) * 1000; //This calculates the delay required in ms between steps to make the journey in the desired amount of time. 0.001 Seconds are deducted because the MoveStepper Method includes 2x 500microseconds delays.
      MoveStepper();
      Serial.print("Stepdelay = ");
      Serial.println(StepDelay);
      delay (StepDelay); // THis is in miliseconds (ms) Not MICRO!
    }
  }
  if (s5 == LOW && s2 == HIGH && s3 == HIGH && s4 == HIGH) ///LOW means switch is pressed/closed/conductive.
  {
    if (StepsTaken < Steps)
    { Serial.println("Mode = s5");
      TravelTime = 120; //The slider will move the distance in 30s
      StepDelay = ((TravelTime / Steps) - 0.002) * 1000; //This calculates the delay required in ms between steps to make the journey in the desired amount of time. 0.001 Seconds are deducted because the MoveStepper Method includes 2x 500microseconds delays.
      MoveStepper();
      Serial.print("Stepdelay = ");
      Serial.println(StepDelay);
      delay (StepDelay); // THis is in miliseconds (ms) Not MICRO!
    }
  }
  if (s2 = LOW && s3 == LOW && s4 == HIGH && s5 == HIGH) ///LOW means switch is pressed/closed/conductive.
  {
    if (StepsTaken < Steps)
    { Serial.println("Mode = s2,3");
      TravelTime = 300; //The slider will move the distance in 30s
      StepDelay = ((TravelTime / Steps) - 0.002) * 1000; //This calculates the delay required in ms between steps to make the journey in the desired amount of time. 0.001 Seconds are deducted because the MoveStepper Method includes 2x 500microseconds delays.
      MoveStepper();
      Serial.print("Stepdelay = ");
      Serial.println(StepDelay);
      delay (StepDelay); // THis is in miliseconds (ms) Not MICRO!
    }
  }
  if (s2 = LOW && s4 == LOW && s3 == HIGH & s5 == HIGH) ///LOW means switch is pressed/closed/conductive.
  {
    if (StepsTaken < Steps)
    { Serial.println("Mode = s2,4");
      TravelTime = 600; //The slider will move the distance in 30s
      StepDelay = ((TravelTime / Steps) - 0.002) * 1000; //This calculates the delay required in ms between steps to make the journey in the desired amount of time. 0.001 Seconds are deducted because the MoveStepper Method includes 2x 500microseconds delays.
      MoveStepper();
      Serial.print("Stepdelay = ");
      Serial.println(StepDelay);
      delay (StepDelay); // THis is in miliseconds (ms) Not MICRO!
    }

  }
  if (s2 = LOW && s5 == LOW && s3 == LOW && s4 == HIGH) ///LOW means switch is pressed/closed/conductive.
  {
    if (StepsTaken < Steps)
    { Serial.println("Mode = s2,5");
      TravelTime = 1200; //The slider will move the distance in 30s
      StepDelay = ((TravelTime / Steps) - 0.002) * 1000; //This calculates the delay required in ms between steps to make the journey in the desired amount of time. 0.001 Seconds are deducted because the MoveStepper Method includes 2x 500microseconds delays.
      MoveStepper();
      Serial.print("Stepdelay = ");
      Serial.println(StepDelay);
      delay (StepDelay); // THis is in miliseconds (ms) Not MICRO!
    }

  }
  if (s3 == LOW && s4 == LOW && s2 == HIGH && s5 == HIGH) ///LOW means switch is pressed/closed/conductive.
  {
    if (StepsTaken < Steps)
    { Serial.println("Mode = s3,4");
      TravelTime = 1800; //The slider will move the distance in 30s
      StepDelay = ((TravelTime / Steps) - 0.002) * 1000; //This calculates the delay required in ms between steps to make the journey in the desired amount of time. 0.001 Seconds are deducted because the MoveStepper Method includes 2x 500microseconds delays.
      MoveStepper();
      Serial.print("Stepdelay = ");
      Serial.println(StepDelay);
      delay (StepDelay); // THis is in miliseconds (ms) Not MICRO!
    }

  }
  if (s3 == LOW && s5 == LOW && s2 == HIGH && s4 == HIGH) ///LOW means switch is pressed/closed/conductive.
  {
    if (StepsTaken < Steps)
    { Serial.println("Mode = s3,5");
      TravelTime = 2400; //The slider will move the distance in 30s
      StepDelay = ((TravelTime / Steps) - 0.002) * 1000; //This calculates the delay required in ms between steps to make the journey in the desired amount of time. 0.001 Seconds are deducted because the MoveStepper Method includes 2x 500microseconds delays.
      MoveStepper();
      Serial.print("Stepdelay = ");
      Serial.println(StepDelay);
      delay (StepDelay); // THis is in miliseconds (ms) Not MICRO!
    }

  }

}
#9

When two or more switches are pressed, they don't all turn on at exactly the same time. If s2, s3 and s4 are pressed, your loop might see s3 on first, then s4 also comes on and then s2.
You need to add debouncing to your code so that you see the final value and ignore the intermediate ones.

Pete

#10