Long/Short press Buttons using one shared Interrupt Pin [Solved]

Hello,

I'm spending quiet a while on this Problem, and just can't find the fault.
I use 4 Buttons, which are connected to 4 digital inputs, and additional to one interrupt input (via a diode).
The schematic is attached. When the Buttons close they are connected to +5V.

the problem is that it doesn't work properly, especially the long press mode. The buttons are really high quality, so i didn't use debouncing here (for the short press mode).
I guess the fault should be in the interrupt function, but I just can't find it.
Maybe one of you will find it very quick. (To miss the forest for the trees could be my problem ...)

Thank you!

const int button[] = {
  10,9,8,7};
//storing the button state for short press mode
volatile int state[] = {
  LOW, LOW, LOW, LOW};
//storing the button state for long press mode
volatile int state_long[] = {
  LOW, LOW, LOW, LOW};
//stores the time each button went high or low
volatile unsigned long current_high[4];
volatile unsigned long current_low[4];


void setup()
{
  for(int i=0; i<4; i++)
  {
    pinMode(button[i], INPUT);
  }
  attachInterrupt(0, read_button, CHANGE);
}


void loop()
{
  for(int i=0; i<4; i++)
  {
    if(state[0] == HIGH)
    {
      //do something when button 0 was pressed short ...
      state[0] = LOW;
    }    
        if(state_long[3] == HIGH)
    {
      //do something when button 3 was pressed long ...
      state[0] = LOW;
    } 
  }
}


//is called when the Interrupt Pin is pressed or released (CHANGE Mode)
void read_button()
{
  //cycles through the buttons to find out which one was pressed or released
  for(int i=0; i<4; i++)
  {
    //if this is true the button was just pressed down
    if(digitalRead(button[i]) == HIGH)
    {
      //note the time the button was pressed
      current_high[i] = millis();
    }
    //if no button is high one had to be released. The millis function will increase while a button is hold down the loop function will be cycled (no change, so no interrupt is active) 
    else
    {
      current_low[i] = millis();
      //determines the time passed since the button pressed down
      if((current_low[i] - current_high[i]) < 500)
      {
        state[i] = !state[i];
      }
      //if the time between press and release is higher than 500 and lower than 1000 ms it is a long press
      else if((current_low[i] - current_high[i]) >= 500 && (current_low[i] - current_high[i]) < 1000)
      {
        state_long[i] = !state_long[i];
      }
    }
  }
}

What exactly happens? and what should happen?

...R

himijendrix:
the problem is that it doesn't work properly, especially the long press mode.

You need to provide a lot more detail about what the problem is if you want us to help solving it.

himijendrix:
The buttons are really high quality, so i didn't use debouncing here

You can get contact bounce on good quality switches too.

You are so right PeterH, all the trouble was caused by bouncing!
I changed the code a little bit and now it works very well. The interrupt function doesn't even disturb the serial connection, which is used simultaneously.

Here is the Code again, in case somebody wants to use it:

const int button[] = {
  10,9,8,7};
//stores if the switch was high before at all
volatile int state[] = {
  LOW, LOW, LOW, LOW};
storing the button state for short press mode
volatile int state_short[] = {
  LOW, LOW, LOW, LOW};
//storing the button state for long press mode
volatile int state_long[] = {
  LOW, LOW, LOW, LOW};
//stores the time each button went high or low
volatile unsigned long current_high[4];
volatile unsigned long current_low[4];


void setup()
{
  for(int i=0; i<4; i++)
  {
    pinMode(button[i], INPUT);
  }
  attachInterrupt(0, read_button, CHANGE);
}


void loop()
{
  for(int i=0; i<4; i++)
  {
    if(state[0] == HIGH)
    {
      //do something when button 0 was pressed short ...
      state[0] = LOW;
    }    
        if(state_long[3] == HIGH)
    {
      //do something when button 3 was pressed long ...
      state[0] = LOW;
    } 
  }
}


//is called when the Interrupt Pin is pressed or released (CHANGE Mode)
void read_button()
{
  //cycles through the buttons to find out which one was pressed or released
  for(int i=0; i<4; i++)
  {
    //if this is true the button was just pressed down
    if(digitalRead(button[i]) == HIGH)
    {
      //note the time the button was pressed
      current_high[i] = millis();
      state[i] = HIGH;
    }
    //if no button is high one had to be released. The millis function will increase while a button is hold down the loop function will be cycled (no change, so no interrupt is active) 
     if(digitalRead(button[i] == LOW) && state[i] == HIGH)
    {
      current_low[i] = millis();
      if((current_low[i] - current_high[i]) > 50 && (current_low[i] - current_high[i]) < 800)
      {
        state_short[i] = !state_short[i];
        state[i] = LOW;
      }
      else if((current_low[i] - current_high[i]) >= 800 && (current_low[i] - current_high[i]) < 4000)
      {
        state_long[i] = !state_long[i];
        state[i] = LOW;
      }
    }
  }
}