Simple LED circuit with pushbuttons, code stopped working

Hello,

I have 4 push buttons (from left to right in the attached image: AutoRaise, Limit Switch, Stop, ManualRaise.) and 3 LED’s. The general premise of the system is:

  1. AutoRaise: When pressed and released, the orange and green LED’s remain on, i.e. latched, unless the limit switch or stop buttons are pressed.

  2. ManualRaise: When pressed and held, the orange led remains on. if the button is released, the orange LED turns off.

  3. If the limit switch is pressed whilst the manual raise button is held, the orange LED turns off.

The following code worked last night, when i’ve tried it again today it no longer works, i.e. pressing the manual raise button does nothing and i can’t figure out why since nothing has changed. All other functions work as normal, therefore it’s not the arduino. I’ve replaced all wires, resistors, tried different pins, nothing seems to work.

const int AutoRaise = 12;
const int Stop = 8;  
const int ManualRaise = 7;
const int led = 4;
const int LimitSwitch = 11;
const int ledGreen = 3;
const int ledRed = 2;
int buttonTrigger = 0;
int ledState = 0;

void setup(){
pinMode(led, OUTPUT);
pinMode(ledGreen, OUTPUT);
pinMode(ledRed, OUTPUT);
pinMode(AutoRaise, INPUT);
pinMode(Stop, INPUT);
pinMode(LimitSwitch, INPUT);
pinMode(ManualRaise, INPUT);
}

void loop(){

///////////////AUTO-RAISE///////////////////

if(digitalRead(AutoRaise) == HIGH){        // if the auto raise button is pressed
  digitalWrite(led, HIGH);                         // turn on the LED
  ledState = 1;                                        // set the led state variable to 1
  buttonTrigger = 1;                                // set the button trigger variable to 1
}

/////////////////STOP//////////////////////

if(digitalRead(Stop) == HIGH){           // if the stop button is pressed
    digitalWrite(led, LOW);                     // turn off the led
    ledState = 2;                                   // set the led state variable to 2
    buttonTrigger = 2;                           // and set the button trigger variable to 2
}
	
///////////////LIMIT SWITCH////////////////	
	
if(digitalRead(LimitSwitch) == HIGH){	        // if the limit switch is pressed
    digitalWrite(led, LOW);				// turn off the led
    ledState = 2;						// set the led state variable to 2
    buttonTrigger = 4;					// and set the button trigger variable to 2
}

////////////////MANUAL-RAISE////////////////

// if the manual raise button is pressed

if(digitalRead(ManualRaise) == HIGH){    

// then check led state. if LED on and limit switch isn't made then do nothing   
                                                                     
    if(digitalRead(led) == HIGH && digitalRead(LimitSwitch == LOW)){                           
    }

// however if the manual raise is held, and the limit switch is made

    else if(digitalRead(ManualRaise) == HIGH && digitalRead(LimitSwitch == HIGH)){

// turn off the led      

        digitalWrite(led, LOW);                                                                                          
    }

// otherwise if led is not on and the limit switch is not made

    else{

// set the buttonTrigger variable to "3" 
                                                                                                                     
        buttonTrigger = 3;  
      
// and turn on the LED
                                                                                   
        digitalWrite(led, HIGH);                                                                                   
    }  

// if the manual raise button is now not pressed

}else{                         

// but the LED was turned on by the manual raise button (ensures that other buttons can turn on the led)
// (otherwise the led would remain off as long as the manual raise wasnt pressed)
                                                                                                   
    if(buttonTrigger == 3){    

// then turn off the LED.
                                                                                       
    digitalWrite(led, LOW);                                                                                    
    }
}

//////GREEN LED TO INDICATE TEST START//////

if(ledState == 1){                                                     // led state can only equal one if the auto raise button was pressed
    digitalWrite(ledGreen, HIGH);                                   // therefore turn on the green led to indicate the test is in progress
    }else{                                                                     // as soon as the test is stopped, i.e. led state is 2, or "not" 1,
        digitalWrite(ledGreen,LOW);                                   // turn the green led off.
    }
  
/////RED LED TO INDICATE STOP BUTTON PRESSED//// 

if(ledState == 2){                          // on pressing the stop button, led state variable is set to 2
    digitalWrite(ledRed, HIGH);           // therefore check if led state is equal to 2, and if so turn on the red led
    delay(3000);                               // for 3 seconds (note: 3000 milliseconds = 3 seconds)
    digitalWrite(ledRed, LOW);           // then turn off the red led
    ledState = 0;                              // and put the led state back to 0 (possibly not required to do this?)
  }
}

The line of code that is causing issues is:

else if(digitalRead(ManualRaise) == HIGH && digitalRead(LimitSwitch == HIGH)){
    digitalWrite(led, LOW);                                                                                          
}

Removing this part of the code allows the manual raise button to work as required, however pressing the limit switch whilst manual raise is held no longer turns off the orange LED of course.

Can anyone spot why this might not be working?

Thanks in advance,
Carl

I've replaced all wires, resistors, tried different pins, nothing seems to work.

The Arduino has built in pullup resistors. So much easier to wire switches when using them. Why aren't you?

}else{

Ifyoucan'tbebotheredputtingthecurlybracesonlinesbythemselves,atleastpoundthespacebarnowandthen.

If the code worked yesterday, but not today, and you made no changes to the code, then the code is NOT the problem.

if(digitalRead(ManualRaise) == HIGH && digitalRead(LimitSwitch == HIGH)){

Should that be

if(digitalRead(ManualRaise) == HIGH && digitalRead(LimitSwitch) == HIGH){

?

evanmars:
Should that be

if(digitalRead(ManualRaise) == HIGH && digitalRead(LimitSwitch) == HIGH){

?

You’re probably right but why would that have worked yesterday if it is the solution?

evanmars: if(digitalRead(ManualRaise) == HIGH && digitalRead(LimitSwitch == HIGH)){

Should that be

if(digitalRead(ManualRaise) == HIGH && digitalRead(LimitSwitch) == HIGH){

?

Yes. This is it. I have no idea how/when thats changed, maybe i'm a sleep-arduino-er. Thanks for taking the time to look through!

PaulS:
The Arduino has built in pullup resistors. So much easier to wire switches when using them. Why aren’t you?

}else{

Ifyoucan’tbebotheredputtingthecurlybracesonlinesbythemselves,atleastpoundthespacebarnowandthen.

If the code worked yesterday, but not today, and you made no changes to the code, then the code is NOT the problem.

Is it literally a case of INPUT_PULLUP instead of INPUT at the void setup stage?

hahapointtaken re. }else{

Is it literally a case of INPUT_PULLUP instead of INPUT at the void setup stage?

It's not as simple as that. You need to connect one leg of the switch to ground, and one leg to the digital pin, and to expect that HIGH means not pressed and that LOW means pressed.

While that last part seems confusing, if you think about pushing down on a switch to complete the circuit, the top of the switch is high(er) when not pressed, and low(er) when pressed, so it is easy to keep track of what value to use in if statements, to get the desired results.