what is the problem with led trainer in arduino?

Question

currently I am trying to develop a light trainer by arduino , as a begging I used 3 led and 3 push button ,the led must work randomly and when the ledX is flash the user press push bottonX and so on of course I must use approximate sensor or something similar to be more reliable

when uploading following code all leds continuous week glow (flash) what the problem? thanks for help.

int ledselect = 0;
int led1 = 11; 
int led2 = 12; 
int led3 = 13; 
int pb1 = 4;
int pb2 = 5;
int pb3 = 6;
void setup() { 
pinMode(led1, OUTPUT);
pinMode(led2, OUTPUT);
pinMode(led3, OUTPUT);
pinMode(pb1, INPUT);
pinMode(pb2, INPUT);
pinMode(pb3, INPUT);
}
void loop() {
int ledselect = random(3); 
switch (ledselect) {
case 0: //if ledcolor equals 0 then the led1 will turn on
digitalWrite(led1, HIGH);
if (pb1== HIGH) ;
digitalWrite(led1,LOW);
break;
case 1: //if ledcolor equals 1 then the led2 will turn on
digitalWrite(led2, HIGH);
if (pb2== HIGH) ;
digitalWrite(led2,LOW);
break;
case 2: //if ledcolor equals 2 then the led3 will turn on
digitalWrite(led3, HIGH);
if (pb3== HIGH) ;
digitalWrite(led3,LOW);
break;
}
} 

Answer 1

In your code you are using comparisons of the form pbX == HIGH, which are always true, since your pbX variables are integers with non-zero values. Probably you want to replace it by digitalRead(pbX)== HIGH, in order to read the switches connected to the pins designated by these variables. And the semicolon after the if clause probably is not doing what you expect. These are just technical problems with your code, but without going in depth into it, it looks to me that the logic implemented is far from your requirements.

Answer 2

thank you it work now

int ledselect = 0;
int led1 = 11; 
int led2 = 12; 
int led3 = 13; 
int pb1 = 4;
int pb2 = 5;
int pb3 = 6;
void setup() { 
pinMode(led1, OUTPUT);
pinMode(led2, OUTPUT);
pinMode(led3, OUTPUT);
pinMode(pb1, INPUT);
pinMode(pb2, INPUT);
pinMode(pb3, INPUT);
}
void loop() {
int ledselect = random(3); 
switch (ledselect) {
case 0:
digitalWrite(led1, HIGH);
delay(10);
while(digitalRead(pb1) != HIGH){delay(10);}
digitalWrite(led1,LOW);
break;
case 1:
digitalWrite(led2, HIGH);
delay(10);
while(analogReadRead(pb2) != HIGH){delay(10);}
digitalWrite(led2,LOW);
break;
case 2:
digitalWrite(led3, HIGH);
delay(10);
while(analogRead(pb3) != HIGH){delay(10);}
digitalWrite(led3,LOW);
break;
}
}

• Passerby: Yes, the delays are also important, otherwise the code runs too quick to see.