Button not working to turn LEDs on

So, i'm busy into my Uni project where I aim to have a arduino circuit that sets different lights on for different levels of sounds/vibrations detected by the Piezo disc.(im using Arduino UNO)

Right now i'm trying to get the button to trigger the LED's to turn on, upon press, or at least for 1ms.
But I don't even get that, I've even uploaded other code with the button trigger, re arranged the pins in code correctly and it works just fine.

I've also changed the "boolean LEDgreenon;" to true manually in code and it turns on the LED as commanded.

I know its probably some small mistake, but I've been at this for ages and could use some help.

Here is my code:

int ledpins[] = { 4, 9, 10, 11 };
int pinCount = 4;
int speakeread = A0;

int pinbutton = 3;
int buttonread;

int LEDgreen;
int LEDyellow;
int LEDred;

boolean LEDgreenon;
boolean LEDyellowon;
boolean LEDredon;
int bob = LOW;

void setup()
{
for (int outLEDpin = 0; outLEDpin < pinCount; outLEDpin++) {
pinMode(ledpins[outLEDpin], OUTPUT);
}
pinMode(speakeread, INPUT);
pinMode(pinbutton, INPUT);
Serial.begin(9600);
}
void loop()
{

buttonread = digitalRead(pinbutton);
if(buttonread == HIGH){
LEDgreenon = true;
LEDyellowon = true;
LEDredon = true;
}

if(LEDgreenon == true){
//denys for function
}else{
ledpins[3] = LEDgreen;
}

if(LEDyellowon == true){
}else{
ledpins[2] = LEDyellow;
}

if(LEDredon == true){
}else{
ledpins[1] = LEDred;
}

for (int outLEDpin = 0; outLEDpin < pinCount; outLEDpin++) {
digitalWrite(ledpins[outLEDpin], HIGH);

}

Serial.println(buttonread);

}

This is the particular part that isn't working
buttonread = digitalRead(pinbutton);
if(buttonread == HIGH){
LEDgreenon = true;
LEDyellowon = true;
LEDredon = true;
}
Button read is reading 1 when i press it down, so im guessing the button is reading high & also when I put Serial.println(buttonread); into the if statement it also prints when pressed, but LED's dont turn on?
:drooling_face:

You need some

digitalWrite(ledpins[x], HIGH);
& 
digitalWrite(ledpins[x], LOW);

to make the output pins actually change.

CrossRoads:
You need some

digitalWrite(ledpins[x], HIGH);

&
digitalWrite(ledpins[x], LOW);



to make the output pins actually change.

I've tried placing them, but the LED's never change when the button is pressed.
in both the
if(buttonread == HIGH){
LEDgreenon = true;
LEDyellowon = true;
LEDredon = true;
}
And
if(LEDyellowon == true){
}else{
ledpins[2] = LEDyellow;
}
statements.
Where would you suggest I place them to make the button work?

Put them where you the LEDs to turn on.

You haven't said how things are wired up, I would use the internal pullup resistor and wire the button to connect the pin to Gnd when pressed.
Then:

buttonread = digitalRead(pinbutton);
if(buttonread == LOW){
digitalWrite(LEDgreenon, HIGH); // assumes pin is connected to LED anode, cathode to resistor to Gnd
digitalWrite(LEDyellowon, HIGH);
digitalWrite(LEDredon, HIGH);
}

I attached a picture of how my circuit looks.

Lose the external resistor. Wire one leg of the switch to ground. Wire the other leg to the pin. Enable the internal pullup resistor.

The, delete all that crap with arrays and simply print the state of the pin. When you KNOW that the switch is working. add the code back to turn the LEDs on AND off.

And, next time, tell us what the code ACTUALLY does.

PaulS:
Lose the external resistor. Wire one leg of the switch to ground. Wire the other leg to the pin. Enable the internal pullup resistor.

The, delete all that crap with arrays and simply print the state of the pin. When you KNOW that the switch is working. add the code back to turn the LEDs on AND off.

And, next time, tell us what the code ACTUALLY does.

Right now(Using the code above) the green LED beside the red one turns on, that's all.

What is this supposed to be doing?

ledpins[3] = LEDgreen;

LEDgreen is never assigned a value, so it retains it's initial value of 0. Storing 0 in the array isn't accomplishing anything useful, that I can see.

PaulS:
What is this supposed to be doing?

ledpins[3] = LEDgreen;

LEDgreen is never assigned a value, so it retains it's initial value of 0. Storing 0 in the array isn't accomplishing anything useful, that I can see.

Yes, the bottom for statement is setting all the pins to HIGH and the If statement
if(LEDyellowon == true){
}else{
ledpins[2] = LEDyellow;
}
is saying if EG LEDyellowon is not true then set to zero, through the, as you said 0 constants because it wasn't assigned a value, but if it IS true then the for loop will assign it to high, because it doesn't change anything.

Assigning a value to an array doesn't make the pin do anything. Only digitalWrite() can actually affect the state of the pin. Frankly. I can't understand why you are diddling with the values in the array of pin numbers.

PaulS:
Assigning a value to an array doesn't make the pin do anything. Only digitalWrite() can actually affect the state of the pin. Frankly. I can't understand why you are diddling with the values in the array of pin numbers.

Well i'm kinda going in and out with understanding my own code, the reason for arrays was to experiment because I previously didn't know how to use them, as really all I've been using before is if and else statements, but my code was terribly structured and I often lost what was happening where. This time all i can see is the button is not working and I can't do anything to get it to do so.

const byte buttonpin = 3;

void setup() {
  // put your setup code here, to run once:
  pinMode( buttonpin, INPUT );
  Serial.begin(9600);
}

void loop() {
  // put your main code here, to run repeatedly: 
  if( digitalRead(buttonpin) )
    Serial.println("Pressed");
  else
    Serial.println("Not pressed");

  delay( 100 );
}

Run this sketch, open Serial Monitor, and press the button a few times and see if Serial is printing the correct state when you press the button.

You need to know FOR SURE that your button is hooked up right and working. Make no changes to your hardware when trying this.

Jiggy-Ninja:

const byte buttonpin = 3;

void setup() {
  // put your setup code here, to run once:
  pinMode( buttonpin, INPUT );
  Serial.begin(9600);
}

void loop() {
  // put your main code here, to run repeatedly:
  if( digitalRead(buttonpin) )
    Serial.println("Pressed");
  else
    Serial.println("Not pressed");

delay( 100 );
}




Run this sketch, open Serial Monitor, and press the button a few times and see if Serial is printing the correct state when you press the button.

You need to know FOR SURE that your button is hooked up right and working. Make no changes to your hardware when trying this.

Yep, works.

Thanks for your help everyone, I've decided to start from scratch again, now that I have a little more understanding.

Piezor:

PaulS:
Lose the external resistor. Wire one leg of the switch to ground. Wire the other leg to the pin. Enable the internal pullup resistor.

The, delete all that crap with arrays and simply print the state of the pin. When you KNOW that the switch is working. add the code back to turn the LEDs on AND off.

And, next time, tell us what the code ACTUALLY does.

Right now(Using the code above) the green LED beside the red one turns on, that's all.

Would that happen to be the one hooked up to pin 4?

I see your problem. You need to go through the program step-by-step to understand this stuff.

LEDgreen, LEDyellow, and LEDred aren't set to any value. As global variables, they default to 0. Your array starts off holding:

{4, 9, 10, 11}

On the first pass through loop, assuming the button is not held down, elements 1, 2, and 3 are set to 0, putting this in your array:

{4, 0, 0, 0}

On the pass through your loop, only pin 4 is turned on.

Second time through the loop, let's assume the button is pressed. You're array still has the same values left over from the previous iteration (4, 0, 0, 0). This time, when all your if statements evaluate to true, nothing happens. No values in the array change. Again, only pin 4 will be set HIGH, even though the button is pushed.

As a test, after uploading this sketch, hold the button down and press the RESET button on the Arduino. This time, the first pass through the loop will be when the button is HIGH, resulting in no changes to the array values. In the loop, pins 4, 9, 10, & 11 will get set HIGH. Assuming that's what your LEDs are hooked up to, they'll turn on.

As an exercise, predict what will happen when you release the button after the LEDs are on. Will they turn off, or remain on? Go through your program just like I did, and you'll understand the answer to that.

This sketch is just so WRONG though that there is no saving it. You really do just need to scrap it an start over.