read button state problem

Hi!
I am new here and with Arduino, and I cant figure out how to make the following code to work.
I have 8 leds and 8 button, but the button work if I use only 2 of them not all 8. Sorry for my english!
If someone here would like to help me I really apreciate.
Thank you!

// function reads the push button switch states, debounces and latches the LED states
// toggles the LED states on each push - release cycle
void ButtonDebounce(void)
{
static byte buttonState[8] = {LOW, LOW, LOW, LOW, LOW, LOW, LOW, LOW}; // the current reading from the input pin
static byte lastButtonState[8] = {LOW, LOW, LOW, LOW, LOW, LOW, LOW, LOW}; // the previous reading from the input pin

// the following variables are long's because the time, measured in miliseconds,
// will quickly become a bigger number than can be stored in an int.
static long lastDebounceTime[8] = {0}; // the last time the output pin was toggled
long debounceDelay = 50; // the debounce time; increase if the output flickers

byte reading[8];

reading[0] = digitalRead(41);
reading[1] = digitalRead(42);
reading[2] = digitalRead(43);
reading[3] = digitalRead(44);
reading[4] = digitalRead(45);
reading[5] = digitalRead(46);
reading[6] = digitalRead(47);
reading[7] = digitalRead(48);

for (int i = 0; i < 8; i++) {
if (reading != lastButtonState*) {*
* // reset the debouncing timer*
_ lastDebounceTime = millis();
* }*_

_ if ((millis() - lastDebounceTime*) > debounceDelay) {
// whatever the reading is at, it's been there for longer*

* // than the debounce delay, so take it as the actual current state:*_

* // if the button state has changed:*
if (reading != buttonState*) {*
buttonState = reading*;*

* // only toggle the LED if the new button state is HIGH*
_ if (buttonState == HIGH) {
LED_state = !LED_state*;*
* }
}
}
} // end for() loop*_

Your code is impossible to read due to the italics in it. Read read this before posting a programming question and follow the advice about using code tags.

show us your pinMode() calls.

Change your test to:

If (reading[i] != lastbuttonstate[i])
  { 
    // and so on...

Edit: now I see that square brackets with an "i" within causes italics but doesn't show the intended array index ...

Change your test to:

Well, that would be pretty dumb, now, wouldn't it?

Do you have pullup (or down) resistors (10k) on your inputs?

I'm going to stop using my tablet - seems to screw up my post more often than not.

Should have read like
Change your test to:

if (reading[i] != lastbuttonstate[i]) 
 { ...

The array indexes are needed to determine which elements you're referring to.

Edit: edited my post #3 to fix my missing array index.
@Paul's - yes that did appear stupid. I tried to shortcut and not put my snippet in code tags and it completely changed my post for the worse.

robertaccess:
Edit: edited my post #3 to fix my missing array index.
@Paul's - yes that did appear stupid. I tried to shortcut and not put my snippet in code tags and it completely changed my post for the worse.

And that's exactly what OP did too. Quote the post to see all the missing [i]'s.

// function reads the push button switch states, debounces and latches the LED states
// toggles the LED states on each push - release cycle
void ButtonDebounce(void)
{
    static byte buttonState[8]     = {LOW, LOW, LOW, LOW, LOW, LOW, LOW, LOW};   // the current reading from the input pin
    static byte lastButtonState[8] = {LOW, LOW, LOW, LOW, LOW, LOW, LOW, LOW};   // the previous reading from the input pin
    
    // the following variables are long's because the time, measured in miliseconds,
    // will quickly become a bigger number than can be stored in an int.
    static long lastDebounceTime[8] = {0};  // the last time the output pin was toggled
    long debounceDelay = 50;         // the debounce time; increase if the output flickers
  
    byte reading[8];
    
    reading[0] = digitalRead(41);
    reading[1] = digitalRead(42);
    reading[2] = digitalRead(43);
    reading[3] = digitalRead(44);
    reading[4] = digitalRead(45);
    reading[5] = digitalRead(46);
    reading[6] = digitalRead(47);
    reading[7] = digitalRead(48);
    
    for (int i = 0; i < 8; i++) {
        if (reading[i] != lastButtonState[i]) {
            // reset the debouncing timer
            lastDebounceTime[i] = millis();
        }
      
        if ((millis() - lastDebounceTime[i]) > debounceDelay) {
            // whatever the reading is at, it's been there for longer
            // than the debounce delay, so take it as the actual current state:
        
            // if the button state has changed:
            if (reading[i] != buttonState[i]) {
                buttonState[i] = reading[i];
          
                // only toggle the LED if the new button state is HIGH
                if (buttonState[i] == HIGH) {
                    LED_state[i] = !LED_state[i];
                }
            }
        }
    } // end for() loop

Ahh, I see. Now I wonder how you got the i in square brackets without it turning into italic code and disappearing ...

Dan95:
Ahh, I see. Now I wonder how you got the i in square brackets without it turning into italic code and disappearing ...

Try quoting my post.

This type of code is a candidate for object-oriented programming.

Instead of:

    static byte buttonState[8]     = {LOW, LOW, LOW, LOW, LOW, LOW, LOW, LOW};   // the current reading from the input pin
    static byte lastButtonState[8] = {LOW, LOW, LOW, LOW, LOW, LOW, LOW, LOW};   // the previous reading from the input pin
    static long lastDebounceTime[8] = {0};  // the last time the output pin was toggled

you use

struct Button {
   byte state = LOW;
   byte lastState = LOW;
   long lastDebounceTime = 0;
}
button[8];

The idea is that you can put code into the button class to encapsulate reading the pin and debouncing. Of course, to do this a button needs to know what its pin is:

struct Button {
   const byte pin;
   byte state = LOW;
   byte lastState = LOW;
   long lastDebounceTime = 0;
}
button[8] = {{41}, {42}, {43}, {44}, {45}, {46}, {47}, {48}};

Once you do that, you can put methods in it:

struct Button {
   const byte pin;
   byte state = LOW;
   byte lastState = LOW;
   long lastDebounceTime = 0;

  void update() {
    if(millis() - lastDebounceTime < debounceDelay) return;
    lastState = state;
    state = digitalRead(pin);
    if(lastState != state) lastDebounceTime = millis();
  }

  boolean changed() {
    return lastState != state;
  }

  boolean rising() {
    return lastState == LOW && state == HIGH;
  }

  boolean falling() {
    return lastState == HIGH && state == LOW;
  }
}
button[8] = {{41}, {42}, {43}, {44}, {45}, {46}, {47}, {48}};

from there, the main loop becomes:

void loop()  {
  // first: read all the pins.
  for(int i = 0; i<8; i++) {
    button[i].update();
  }

  // next, do stuff in respose to what happened on the pins
  for(int i = 0; i<8; i++) {
    if(button[i].changed()) {
      // switch the corresponding LED, or whatever
    }
  }
}

This means that your loop() is not full of debouncing and initialization code.

Thank you all for fast answers and help.
Yes I have 10k rezistors.
If I use the code with only 2 leds to control it from push button it's working... :confused:

this is the whole code. Thank you very much guys!

void setup()
{
    // disable Ethernet chip
    ............................................
    
    // initialize SD card
..............................................

    // switches
    pinMode(41, INPUT);
    pinMode(42, INPUT);
    pinMode(43, INPUT);
    pinMode(44, INPUT);
    pinMode(45, INPUT);
    pinMode(46, INPUT);
    pinMode(47, INPUT);
    pinMode(48, INPUT);
    // LEDs
    pinMode(33, OUTPUT);
    pinMode(34, OUTPUT);
    pinMode(35, OUTPUT);
    pinMode(36, OUTPUT);
    pinMode(37, OUTPUT);
    pinMode(38, OUTPUT);
    pinMode(39, OUTPUT);
    pinMode(40, OUTPUT);
    
    Ethernet.begin(mac, ip);  // initialize Ethernet device
    server.begin();           // start to listen for clients
}

void loop()
{
    EthernetClient client = server.available();  // try to get client

    if (client) {  // got client?
        boolean currentLineIsBlank = true;
        while (client.connected()) {
            if (client.available()) {   // client data available to read
                char c = client.read(); // read 1 byte (character) from client
                // limit the size of the stored received HTTP request
                // buffer first part of HTTP request in HTTP_req array (string)
                // leave last element in array as 0 to null terminate string (REQ_BUF_SZ - 1)
                if (req_index < (REQ_BUF_SZ - 1)) {
                    HTTP_req[req_index] = c;          // save HTTP request character
                    req_index++;
                }
                // last line of client request is blank and ends with \n
                // respond to client only after last line received
                if (c == '\n' && currentLineIsBlank) {
                    // send a standard http response header
                    client.println("HTTP/1.1 200 OK");
                    // remainder of header follows below, depending on if
                    // web page or XML page is requested
                    // Ajax request - send XML file
                    if (StrContains(HTTP_req, "ajax_inputs")) {
                        // send rest of HTTP header
                        client.println("Content-Type: text/xml");
                        client.println("Connection: keep-alive");
                        client.println();
                        SetLEDs();
                        // send XML file containing input states
                        XML_response(client);
                    }
                    else {  // web page request
                        // send rest of HTTP header
                        client.println("Content-Type: text/html");
                        client.println("Connection: keep-alive");
                        client.println();
                        // send web page
                        webFile = SD.open("index.htm");        // open web page file
                        if (webFile) {
                            while(webFile.available()) {
                                client.write(webFile.read()); // send web page to client
                            }
                            webFile.close();
                        }
                    }
                    // display received HTTP request on serial port
                    Serial.print(HTTP_req);
                    // reset buffer index and all buffer elements to 0
                    req_index = 0;
                    StrClear(HTTP_req, REQ_BUF_SZ);
                    break;
                }
                // every line of text received from the client ends with \r\n
                if (c == '\n') {
                    // last character on line of received text
                    // starting new line with next character read
                    currentLineIsBlank = true;
                } 
                else if (c != '\r') {
                    // a text character was received from client
                    currentLineIsBlank = false;
                }
            } // end if (client.available())
        } // end while (client.connected())
        delay(1);      // give the web browser time to receive the data
        client.stop(); // close the connection
    } // end if (client)
    
    // read hardware buttons and debounce
    ButtonDebounce();
}

// function reads the push button switch states, debounces and latches the LED states
// toggles the LED states on each push - release cycle
void ButtonDebounce(void)
{
    static byte buttonState[8]     = {LOW, LOW, LOW, LOW, LOW, LOW, LOW, LOW};   // the current reading from the input pin
    static byte lastButtonState[8] = {LOW, LOW, LOW, LOW, LOW, LOW, LOW, LOW};   // the previous reading from the input pin
    
    // the following variables are long's because the time, measured in miliseconds,
    // will quickly become a bigger number than can be stored in an int.
    static long lastDebounceTime[8] = {0};  // the last time the output pin was toggled
    long debounceDelay = 50;         // the debounce time; increase if the output flickers
  
    byte reading[8];
    
    reading[0] = digitalRead(41);
    reading[1] = digitalRead(42);
    reading[2] = digitalRead(43);
    reading[3] = digitalRead(44);
    reading[4] = digitalRead(45);
    reading[5] = digitalRead(46);
    reading[6] = digitalRead(47);
    reading[7] = digitalRead(48);
    
    for (int i = 0; i < 8; i++) {
        if (reading[i] != lastButtonState[i]) {
            // reset the debouncing timer
            lastDebounceTime[i] = millis();
        }
      
        if ((millis() - lastDebounceTime[i]) > debounceDelay) {
            // whatever the reading is at, it's been there for longer
            // than the debounce delay, so take it as the actual current state:
        
            // if the button state has changed:
            if (reading[i] != buttonState[i]) {
                buttonState[i] = reading[i];
          
                // only toggle the LED if the new button state is HIGH
                if (buttonState[i] == HIGH) {
                    LED_state[i] = !LED_state[i];
                }
            }
        }
    } // end for() loop
    
    // set the LEDs
    digitalWrite(33, LED_state[0]);
    digitalWrite(34, LED_state[1]);
    digitalWrite(35, LED_state[2]);
    digitalWrite(36, LED_state[3]);
    digitalWrite(37, LED_state[4]);
    digitalWrite(38, LED_state[5]);
    digitalWrite(39, LED_state[6]);
    digitalWrite(40, LED_state[7]);
      
    // save the reading.  Next time through the loop,
    // it'll be the lastButtonState:
    lastButtonState[0] = reading[0];
    lastButtonState[1] = reading[1];
    lastButtonState[2] = reading[2];
    lastButtonState[3] = reading[3];
    lastButtonState[4] = reading[4];
    lastButtonState[5] = reading[5];
    lastButtonState[6] = reading[6];
    lastButtonState[7] = reading[7];
}

I have to use ............... to get smaller code. I hope you guys understand what missing between

and the rest of the code:

// checks if received HTTP request is switching on/off LEDs
// also saves the state of the LEDs
void SetLEDs(void)
{
    // LED 1 (pin 33)
    if (StrContains(HTTP_req, "LED1=1")) {
        LED_state[0] = 1;  // save LED state
        digitalWrite(33, HIGH);
    }
    else if (StrContains(HTTP_req, "LED1=0")) {
        LED_state[0] = 0;  // save LED state
        digitalWrite(33, LOW);
    }

     ..............................

    // LED 8 (pin 40)
    if (StrContains(HTTP_req, "LED8=1")) {
        LED_state[7] = 1;  // save LED state
        digitalWrite(40, HIGH);
    }
    else if (StrContains(HTTP_req, "LED8=0")) {
        LED_state[7] = 0;  // save LED state
        digitalWrite(40, LOW);
    }
}

// send the XML file with analog values, switch status
//  and LED status
void XML_response(EthernetClient cl)
{
    int sw_arr[] = {41, 42, 43, 44, 45, 46, 47, 48};  // pins interfaced to switches
    
    cl.print("<?xml version = \"1.0\" ?>");
    cl.print("<inputs>");
    // checkbox LED states
    // button LED states

    
    cl.print("<LED>");
    if (LED_state[0]) {
        cl.print("on");
    }
    else {
        cl.print("off");
    }
    cl.println("</LED>");
    
    
    
    .....................................................


    cl.print("<LED>");
    if (LED_state[7]) {
        cl.print("on");
    }
    else {
        cl.print("off");
    }
    cl.println("</LED>");
    
    cl.print("</inputs>");
}

// sets every element of str to 0 (clears array)
void StrClear(char *str, char length)
{
    for (int i = 0; i < length; i++) {
        str[i] = 0;
    }
}

// searches for the string sfind in the string str
// returns 1 if string found
// returns 0 if string not found
char StrContains(const char *str, const char *sfind)
{
    char found = 0;
    char index = 0;
    char len;

    len = strlen(str);
    
    if (strlen(sfind) > len) {
        return 0;
    }
    while (index < len) {
        if (str[index] == sfind[found]) {
            found++;
            if (strlen(sfind) == found) {
                return 1;
            }
        }
        else {
            found = 0;
        }
        index++;
    }

    return 0;
}

[/code]

I have to use ............... to get smaller code.

What you NEED to do is use Reply, not the Quick Reply field, and use the Additional Options link (that doesn't look like a link) below the text entry area to attach your code as ONE file with nothing left out.

    byte reading[8];
    
    reading[0] = digitalRead(41);
    reading[1] = digitalRead(42);
    reading[2] = digitalRead(43);
    reading[3] = digitalRead(44);
    reading[4] = digitalRead(45);
    reading[5] = digitalRead(46);
    reading[6] = digitalRead(47);
    reading[7] = digitalRead(48);
    
    for (int i = 0; i < 8; i++) {
        if (reading[i] != lastButtonState[i]) {
            // reset the debouncing timer
            lastDebounceTime[i] = millis();
        }

There's no need to do that. Put your pin numbers in an array as well. Then you can just read a pin and check for a state change.
```
*   for (int i = 0; i < 8; i++) {

int reading = digitalRead(pin[i]);
       if (lastButtonState[i] != reading) {
         
            // reset the debouncing timer
           lastDebounceTime[i] = millis();
       }*
```