quick multi condition question

I'm an artist, not a programmer. So I'm trying to be as basic in my code as possible.

playing around with some pinball code. I want 3 switches to light up 3 separate LEDs. When all 3 LEDs are lit, I want the lights go out, and are ready to be turned on again the next time you hit them.

right now each of my lights will light up when I hit the switch assigned to them. But when all 3 are lit, they do not turn off again. What am i doing wrong?

// 3 TARGET SCRORE FUNCTION. WHEN A TARGET IS HIT IT LIGHTS UP AND PROVIDES POINTS.
// WHEN ALL 3 TARGETS ARE LIT THEY FLASH AND PROVIDE A JACKPOT
// WHEN JACKPOT IS PROVIDED ALL LIGHTS TURN OFF. 
  


int LED_D1 = 11;
int LED_D2 = 12;
int LED_D3 = 13;
int SWITCH_D1 = 1;
int SWITCH_D2 = 2;
int SWITCH_D3 = 3;


void setup() {
  // put your setup code here, to run once:



pinMode(LED_D1,OUTPUT);
pinMode(LED_D2,OUTPUT);
pinMode(LED_D3,OUTPUT);
pinMode(SWITCH_D1,INPUT);
pinMode(SWITCH_D2,INPUT);
pinMode(SWITCH_D3,INPUT);


}

void loop() {
  // put your main code here, to run repeatedly:

//drop target switch 1  

     if (digitalRead(SWITCH_D1) == LOW)
        {
         digitalWrite(LED_D1,HIGH);
        }
  
//drop target switch 2 

     if (digitalRead(SWITCH_D2) == LOW)
        {
         digitalWrite(LED_D2,HIGH);
        }

//drop target switch 3  

     if (digitalRead(SWITCH_D3) == LOW)
        {
         digitalWrite(LED_D3,HIGH);
        }

//IF ALL 3 ARE LIT UP, AWARD BONUS AND RESET LIGHTS TO LOW//


     if (LED_D1 == HIGH && LED_D2 == HIGH && LED_D3 == HIGH)
        {
         (LED_D1 && LED_D2 && LED_D3 == LOW);
        }

}

This

       {
         (LED_D1 && LED_D2 && LED_D3 == LOW);
        }

is cracked

You might mean

       {
         digitalWrite(LED_D1, LOW);
          digitalWrite(LED_D2, LOW);
         digitalWrite(LED_D3, LOW);
        }

not getting any errors but they still don't turn off when they are all activated...

could there be a problem with my 'IF' line?

Yes, it's also cracked. You forgot to do the digitalRead()'s.

Interesting... now when I push the buttons they stay on for as long as I hold the button, but turn off when released.

Trying to figure out how to tell them to only turn off once all 3 are on. am I not using '&&' correctly?

// 3 TARGET SCRORE FUNCTION. WHEN A TARGET IS HIT IT LIGHTS UP AND PROVIDES POINTS.
// WHEN ALL 3 TARGETS ARE LIT THEY FLASH AND PROVIDE A JACKPOT
// WHEN JACKPOT IS PROVIDED ALL LIGHTS TURN OFF. 
  


int LED_D1 = 11;
int LED_D2 = 12;
int LED_D3 = 13;
int SWITCH_D1 = 1;
int SWITCH_D2 = 2;
int SWITCH_D3 = 3;


void setup() {
  // put your setup code here, to run once:



pinMode(LED_D1,OUTPUT);
pinMode(LED_D2,OUTPUT);
pinMode(LED_D3,OUTPUT);
pinMode(SWITCH_D1,INPUT);
pinMode(SWITCH_D2,INPUT);
pinMode(SWITCH_D3,INPUT);


}

void loop() {
  // put your main code here, to run repeatedly:

//drop target switch 1  

     if (digitalRead(SWITCH_D1) == LOW)
        {
         digitalWrite(LED_D1,HIGH);
        }
  
//drop target switch 2 

     if (digitalRead(SWITCH_D2) == LOW)
        {
         digitalWrite(LED_D2,HIGH);
        }

//drop target switch 3  

     if (digitalRead(SWITCH_D3) == LOW)
        {
         digitalWrite(LED_D3,HIGH);
        }

//IF ALL 3 ARE LIT UP, AWARD BONUS AND RESET LIGHTS TO LOW//


     if (digitalRead (LED_D1 == HIGH) && digitalRead (LED_D2 == HIGH) && digitalRead (LED_D3 == HIGH))
        {
         digitalWrite(LED_D1, LOW);
          digitalWrite(LED_D2, LOW);
         digitalWrite(LED_D3, LOW);
        }

}

scarybeard:

     if (digitalRead (LED_D1 == HIGH) && digitalRead (LED_D2 == HIGH) && digitalRead (LED_D3 == HIGH))

{
        digitalWrite(LED_D1, LOW);
         digitalWrite(LED_D2, LOW);
        digitalWrite(LED_D3, LOW);
       }

}

Maybe you really mean:-

if (digitalRead(LED_D1) == HIGH && digitalRead(LED_D2) == HIGH && digitalRead(LED_D3) == HIGH)
{
    digitalWrite(LED_D1, LOW);
    digitalWrite(LED_D2, LOW);
    digitalWrite(LED_D3, LOW);
}

NICE!

THATS THE CLOSEST YET! only problem now is that I turn on light #1, and light #2, then when Light #3 is pressed, 1 and 2 go off but #3 stays on. I have to assume its because there needs to be a delay between me pressing the #3 button, and releasing it.

as its pinball, using the 'delay' function is not an option...

I'm assuming I need to take a look at one of the 'blink without delay' tutorials?

It actually does not stay on. It turns off but because you are not as quick as your arduino, the loop loops, the button3 is still pressed so it lights up again. It's so fast you don't see it

For example You could track the release of the button to turn on the LEDs, not the pressing (might feel weird). When you see the button goes down set a flag and if that flag is true and the button goes back up, then light that pin (and reset the flag). This way you will have button3 back up when you loop and it won't light again the led3

Alternatively do light up on the pressing but only turn off if the 3 lights are ON and the 3 buttons are OFF

scarybeard:
NICE!

THATS THE CLOSEST YET! only problem now is that I turn on light #1, and light #2, then when Light #3 is pressed, 1 and 2 go off but #3 stays on. I have to assume its because there needs to be a delay between me pressing the #3 button, and releasing it.

as its pinball, using the 'delay' function is not an option...

I'm assuming I need to take a look at one of the 'blink without delay' tutorials?

#3 won't actually be "staying" on, since all three are turned off together. It'll be turning back on again since the switch is still pressed. (As you correctly deduced, and as J-M-L wrote as I was typing.)

Do a search on button debouncing. Perhaps the search term: "Arduino button debounce". The state change is what's important, again as J-M-L says.
There are plenty of examples on the web and these forums. (Can't think of a particular one at the moment.)

If it comes to buttons, I like the GitHub - thomasfredericks/Bounce2: Debouncing library for Arduino and Wiring library.

Debouncing and state changes included.

I moved the switch on pin 1 to pin 4, and I use INPUT_PULLUP switches closing to GND.

The bonusflash will be interrupted by a new hit.

#include <Bounce2.h>

// 3 TARGET SCRORE FUNCTION. WHEN A TARGET IS HIT IT LIGHTS UP AND PROVIDES POINTS.
// WHEN ALL 3 TARGETS ARE LIT THEY FLASH AND PROVIDE A JACKPOT
// WHEN JACKPOT IS PROVIDED ALL LIGHTS TURN OFF.

const byte LED[] = { 11, 12, 13 };
const byte SWITCH[] = { 2, 3, 4};

Bounce target[3];

byte litLeds;
byte bonusFlash;

void setup() {
  for (byte idx = 0; idx < 3; idx++) {
    pinMode(LED[idx], OUTPUT);
    target[idx].attach(SWITCH[idx], INPUT_PULLUP);
  }
}

void setAll(bool state) {
  for (byte idx = 0; idx < 3; idx++) {
    digitalWrite(LED[idx], state);
  }
}

void loop() {
  static unsigned long lastFlash;
  unsigned long topLoop = millis();

  for (byte idx = 0; idx < 3; idx++) {
    if (target[idx].update()) {
      if (target[idx].fell()) {
        if (bonusFlash) {
          setAll(false);
          bonusFlash = 0;
          litLeds = 0;
        }
        digitalWrite(LED[idx], HIGH);
        litLeds |= (1 << idx);
        if (litLeds == 7) {
          bonusFlash = 10;
          litLeds = 0;
        }
      }
    }
  }

  if (bonusFlash) {
    if (topLoop - lastFlash > 100) {
      lastFlash = topLoop;
      --bonusFlash;
      setAll(bonusFlash & 1);
    }
  }
}

Thanks for the feedback Whandall, but I got a bunch of errors when I tried that code and I don't understand arrays and such well enough to know why, or to try and fix it. :confused:

That's why I have been trying to do things the long and inefficient way. I don't have enough experience to even know what I'm looking at when I see stuff like

for (byte idx = 0; idx < 3; idx++)

I have no idea what that line is doing. But I can understand 'if's and 'else's.... barely...

So I re-wrote some stuff, and it is almost working. Debounce seems handled. but I am still having the same problem. At the very end, when I tell the code "if all 3 lights are lit, turn off all 3 lights" they all just remain on. They no longer turn off all 3 then turn on the 3rd one before I realized it was off. They just all 3 remain on.

I'm currently looking into adding an 'else' statement to see if that's what I'm missing. and I'm going to try and debug the result by asking it to turn on a 4th light when the other 3 are lit just to make sure it is even understanding my 'if'.

Heres the current code:

// 3 TARGET SCORE FUNCTION. 
// WHEN A TARGET IS HIT IT LIGHTS UP AND PROVIDES POINTS.
// IF A TARGET IS HIT AGAIN WHILE IT IS ALREADY LIT, IT STILL SCORES POINTS AND STAYS LIT.
// WHEN ALL 3 TARGETS ARE LIT THEY FLASH AND PROVIDE A JACKPOT
// WHEN JACKPOT IS PROVIDED ALL LIGHTS TURN OFF. READY TO BE ACTIVATED AGAIN. 
 
int dropButton1 = 2;         // the number of the input pin
int dropButton2 = 3;        
int dropButton3 = 4;  
     
int dropLight1 = 11;       // the number of the output pin
int dropLight2 = 12;     
int dropLight3 = 13;      

int state1 = LOW;      // the current state of the output pin
int state2 = LOW;      
int state3 = LOW;     

int reading1;   // the current reading from the input pin
int reading2;
int reading3; 

int previous1 = LOW;    // the previous reading from the input pin
int previous2 = LOW;   
int previous3 = LOW;    


long time = 0;         // the last time the output pin was toggled
long debounce = 200;   // the debounce time, increase if the output flickers


void setup()
{
  pinMode(dropButton1, INPUT);
  pinMode(dropButton2, INPUT);
  pinMode(dropButton3, INPUT);
  
  pinMode(dropLight1, OUTPUT);
  pinMode(dropLight2, OUTPUT);
  pinMode(dropLight3, OUTPUT);
}

void loop()
{

  //droptarget_01//
  
   reading1 = digitalRead(dropButton1);
      
        if (reading1 == HIGH && previous1 == LOW && millis() - time > debounce) {

            state1 = HIGH;
      
          time = millis();    
        }
      
        digitalWrite(dropLight1, state1);
      
        previous1 = reading1;

  //droptarget_02//
  
   reading2 = digitalRead(dropButton2);
      
        if (reading2 == HIGH && previous2 == LOW && millis() - time > debounce) {

            state2 = HIGH;
      
          time = millis();    
        }
      
        digitalWrite(dropLight2, state2);
      
        previous2 = reading2;

   //droptarget_03//
  
   reading3 = digitalRead(dropButton3);
      
        if (reading3 == HIGH && previous3 == LOW && millis() - time > debounce) {

            state3 = HIGH;
            
          time = millis();    
        }
      
        digitalWrite(dropLight3, state3);
      
        previous3 = reading3;

  
  //RESET TARGETS - BONUS SCORE

if (digitalRead(state1) == HIGH && digitalRead(state2) == HIGH && digitalRead(state3) == HIGH)
{

  digitalWrite(state1, LOW);
  digitalWrite(state2, LOW);
  digitalWrite(state3, LOW);
} 
}
int state1 = LOW;      // the current state of the output pin
int state2 = LOW;     
int state3 = LOW;     

if (digitalRead(state1) == HIGH && digitalRead(state2) == HIGH && digitalRead(state3) == HIGH)

Ups.

I don't know what that means?

What do you expect the value of digital pin 0 (or 1) to be?
Why should you read it three times?

What do you want to read? Maybe dropLight1, droplight2 and droplight3?

Same for this

  digitalWrite(state1, LOW);
  digitalWrite(state2, LOW);
  digitalWrite(state3, LOW);

i'm not using digital pin 0 or 1. my input pins are (2,3,4) my output LED pins are (11,12,13) and (10 for debugging the code)

what do you mean by reading it 3 times?

right now each of those 'state' lines is referring to different pins. not the same one 3 times.

if I cut that down to just a single 'state'

int state = LOW;

then all 3 lights come on when any 1 button is pushed:

void loop()
{

  //droptarget_01//
  
   reading1 = digitalRead(dropButton1);
      
        if (reading1 == HIGH && previous1 == LOW && millis() - time > debounce) {

            state = HIGH;
      
          time = millis();    
        }
      
        digitalWrite(dropLight1, state);
      
        previous1 = reading1;

  //droptarget_02//
  
   reading2 = digitalRead(dropButton2);
      
        if (reading2 == HIGH && previous2 == LOW && millis() - time > debounce) {

            state = HIGH;
      
          time = millis();    
        }
      
        digitalWrite(dropLight2, state);
      
        previous2 = reading2;

   //droptarget_03//
  
   reading3 = digitalRead(dropButton3);
      
        if (reading3 == HIGH && previous3 == LOW && millis() - time > debounce) {

            state = HIGH;
            
          time = millis();    
        }
      
        digitalWrite(dropLight3, state);
      
        previous3 = reading3;

You are using pins 0 and 1 because LOW == 0 and HIGH == 1.

Which pin willdigitalRead(state1)read?

int state1 = LOW;      // the current state of the output pin
int state2 = LOW;     
int state3 = LOW;

if (digitalRead(state1) == HIGH && digitalRead(state2) == HIGH && digitalRead(state3) == HIGH)
{

  digitalWrite(state1, LOW);
  digitalWrite(state2, LOW);
  digitalWrite(state3, LOW);
}

ok, I didnt think the naming of things was that critical. But I replaced all my numbers with A,B,C,D instead and it runs exactly the same.

I did replace the (state) checks :

  //RESET TARGETS - BONUS SCORE

if (digitalRead(state1) == HIGH && digitalRead(state2) == HIGH && digitalRead(state3) == HIGH)
{

  digitalWrite(state1, LOW);
  digitalWrite(state2, LOW);
  digitalWrite(state3, LOW);
}
}

with (dropLight) checks:

  //RESET TARGETS - BONUS SCORE
if (digitalRead(dropLightA)==HIGH && digitalRead(dropLightB)==HIGH && digitalRead(dropLightC)==HIGH)
{
  digitalWrite(dropLightA, LOW);
  digitalWrite(dropLightB, LOW);
  digitalWrite(dropLightC, LOW);
  digitalWrite(dropLightD, HIGH);
}
}

and now when all 3 (ABC) lights are lit the 4th (D) light comes on, but ABC still do not turn off. So at least the code is recognizing that they are all lit. I now dont know how to make them turn off...

Here is the whole code again without numbered naming conventions anymore:

// 3 TARGET SCRORE FUNCTION. 
// WHEN A TARGET IS HIT IT LIGHTS UP AND PROVIDES POINTS.
// IF A TARGET IS HIT AGAIN WHILE IT IS ALREADY LIT, IT STILL SCORES POINTS AND STAYS LIT.
// WHEN ALL 3 TARGETS ARE LIT THEY FLASH AND PROVIDE A JACKPOT
// WHEN JACKPOT IS PROVIDED ALL LIGHTS TURN OFF. READY TO BE ACTIVATED AGAIN. 
 
int dropButtonA = 2;         // the number of the input pin
int dropButtonB = 3;        
int dropButtonC = 4;  
     
int dropLightA = 11;       // the number of the output pin
int dropLightB = 12;     
int dropLightC = 13; 
int dropLightD = 10;     

int stateA = LOW;      // the current state of the output pin
int stateB = LOW;      
int stateC = LOW;   
int stateD = LOW;  


int readingA;   // the current reading from the input pin
int readingB;
int readingC; 

int previousA = LOW;    // the previous reading from the input pin
int previousB = LOW;   
int previousC = LOW;    


long time = 0;         // the last time the output pin was toggled
long debounce = 200;   // the debounce time, increase if the output flickers


void setup()
{
  pinMode(dropButtonA, INPUT);
  pinMode(dropButtonB, INPUT);
  pinMode(dropButtonC, INPUT);
  
  pinMode(dropLightA, OUTPUT);
  pinMode(dropLightB, OUTPUT);
  pinMode(dropLightC, OUTPUT);
  pinMode(dropLightD, OUTPUT);
}

void loop()
{

  //droptarget_01//
  
   readingA = digitalRead(dropButtonA);
      
        if (readingA == HIGH && previousA == LOW && millis() - time > debounce) {

            stateA = HIGH;
      
          time = millis();    
        }
      
        digitalWrite(dropLightA, stateA);
      
        previousA = readingA;

  //droptarget_02//
  
   readingB = digitalRead(dropButtonB);
      
        if (readingB == HIGH && previousB == LOW && millis() - time > debounce) {

            stateB = HIGH;
      
          time = millis();    
        }
      
        digitalWrite(dropLightB, stateB);
      
        previousB = readingB;

   //droptarget_03//
  
   readingC = digitalRead(dropButtonC);
      
        if (readingC == HIGH && previousC == LOW && millis() - time > debounce) {

            stateC = HIGH;
            
          time = millis();    
        }
      
        digitalWrite(dropLightC, stateC);
      
        previousC = readingC;

  
  //RESET TARGETS - BONUS SCORE
if (digitalRead(dropLightA)==HIGH && digitalRead(dropLightB)==HIGH && digitalRead(dropLightC)==HIGH)
{
  digitalWrite(dropLightA, LOW);
  digitalWrite(dropLightB, LOW);
  digitalWrite(dropLightC, LOW);
  digitalWrite(dropLightD, HIGH);
}
}

scarybeard:
ok, I didnt think the naming of things was that critical.

Naming does not matter at all, taking the wrong variables does, or using pin values as pin numbers.

If you begin numbering (or lettering) variables you should use arrays (most of the times).
If you don't know arrays, learn about them.

This code turns the target lights off.

  digitalWrite(dropLightA, LOW);
  digitalWrite(dropLightB, LOW);
  digitalWrite(dropLightC, LOW);

If they are still on, your code switches them back on.

so are you are saying this cannot be done without using arrays? ...Can you explain why?

"If they are still on, your code switches them back on." <-- any advice on where they are being switched back on? or how to fix that?

I get that arrays are a more elegant way of writing what I'm trying to do. It's even the RIGHT way to write what I'm trying to do. But you seem to be avoiding telling me where the problem is in my code, and just referring that I 'go learn to code'...

I cant help but feel like my experience on this forum has been very much like asking "Hey, how do I bake a great chocolate cake?" and the forum responding "Go to chef school."