Spotting a semantic issue in a debounce

I'm having a hard time finding out why this will give the correct values for presses, but not releases and holds.
I ether missed an important concept or am having a timing issue.

//getvalueTesting
//4 buttons wired pins 2-5
//pin to tactile button to 330k to ground
//uses internal resistence to VCC
//
//goal is an expample that gathers debounced state information
//and combines every switch state within a time window
//into an int value
//ie, buttonstate[4]={1,2,5,1} --> int=1251

byte buttons[]={
  2,3,4,5};
#define NUMBUTTONS sizeof(buttons)

#define BOUNCE 15// time to debounce
#define REGULAR 225
boolean regularEvent[]={
false,false,false,false};

#define HOLD 450
boolean holdEvent[]={
false,false,false,false};

#define LONG 900
boolean longEvent[]={
false,false,false,false};

#define WINDOW 1000

long past=-1;
long diff=-1;
int windowCount=0;

unsigned long releaseTime[]= {
  -1,-1,-1,-1};
unsigned long pressTime[]= {
  -1,-1,-1,-1};
boolean value[]= {
  LOW,LOW,LOW,LOW};
boolean lastValue[]= {
  LOW,LOW,LOW,LOW};
  
boolean pressed[]= {
  false,false,false,false};
  

void setup()
{
  Serial.begin(9600);
  delay(2000);
  //wait to bring up serial monitor
  //------------input setting
  for (byte set=0;set<NUMBUTTONS;set++)
  {
    //sets the button pins
    pinMode(buttons[set],INPUT);
    digitalWrite(buttons[set],HIGH);
  }
} 

void loop()
{
  while(windowCount<=WINDOW)
  // the time window is for the purpose of compiling a
  // unique value from the button over a specified period of time  
  {
    unsigned long currentTime=millis();
    diff=currentTime-past;
    windowCount+=diff;
    for(int i=0;i<NUMBUTTONS;i++)
    {
      value[i]=digitalRead(buttons[i]);
      if(value[i]==LOW && lastValue[i]==HIGH && (millis()-releaseTime[i]) > BOUNCE)
      {
        pressTime[i] = millis();
        pressed[i]=true;
        regularEvent[i]=false;
        holdEvent[i]=false;
        longEvent[i]=false;
        Serial.print(i);
        Serial.println("-pressed");
      }
      else if (value[i] == HIGH && lastValue[i] == LOW && (millis() - pressTime[i]) > BOUNCE)
      { 
        if(pressed[i])
          {       
            releaseTime[i] = millis();
            Serial.print(value[i]);
            //!! I only get the value 1-released
            //inconceivable!
            //I dont think this means what I think it means
            Serial.println("-Release");
          }
      }
      if (value[i]==LOW && (millis()-pressTime[i]) >= REGULAR)
      {
        if(!regularEvent[i])
        {
          //quick press
          regularEvent[i]=true;
          //!!only getting 0-regular press in output
          Serial.print(value[i]);
          Serial.println("-regular press");
        }
        if((millis()-pressTime[i])>= HOLD)
        {
          if (!holdEvent[i])
          {
            holdEvent[i]=true;
            //!!only getting 0-hold event in output
            Serial.print(value[i]);
            Serial.println("-hold event");
          }
        }
      }
      lastValue[i]=value[i];
    }
    past=currentTime;
  }
  //--outside time window
  Serial.print("window-");
  Serial.println(windowCount);
  windowCount=0;
}

with that code, here is what a sample of the output looks like this

window-1001
3-pressed
0-regular press
0-hold event
window-1001
1-Release
window-1001

fine, except the 0 and 1 button were never pressed

more description of purpose is in the code
I tried the bounce library... and I just found that I probably need get more familiar with the core concepts then try to take a shortcut. (and it was working inconsistently but that would be a topic for another thread)

Any ideas on why holds and releases result in static values?

Do the button inputs work reliably with that high a resistor in there?

johncc:
Do the button inputs work reliably with that high a resistor in there?

they have thus far as far as I can tell and I've done a bit of testing

oh... wait that's a typo they are 330om not 330k

Your code dealing with windowCount looks wierd. Why don't you just calculate the difference between now and when the window started? WHy calculate elapsed time per loop and then add them up?

In the context of a system where nothing changes for long intervals (relative to the window size) when is the 'window time' supposed to start?

Exactly what is the code trying to do? It seems to be dealing with press, release,hold, bounce, regular and with flags and times associated with various of those, but it's not obvious how the final output value is supposed to be related to the physical button presses and releases. Ignoring the code itself, and assuming I know nothing about your project, what is this code trying to do?

PeterH:
Your code dealing with windowCount looks wierd. Why don't you just calculate the difference between now and when the window started? WHy calculate elapsed time per loop and then add them up?

it has more to do with the creating a chord then the debounce, sorry its kinda misleading for this question.

In the context of a system where nothing changes for long intervals (relative to the window size) when is the 'window time' supposed to start?

I see what you are getting at there, I didn't think about it because in my last implementation it worked its self out somehow. The window is supposed to start when values start being detected. For right now this works well enough for me test things out. Just shows whats falling within the window.

Exactly what is the code trying to do? It seems to be dealing with press, release,hold, bounce, regular and with flags and times associated with various of those, but it's not obvious how the final output value is supposed to be related to the physical button presses and releases. Ignoring the code itself, and assuming I know nothing about your project, what is this code trying to do?

the goal is to debounce over multiple button instances first and foremost then also flag different press types and seed* the press type based on other button states
The later two objectives I'm working on currently and not in the op

*- if the other buttons are active, the monitored button's reported value is augmented by this to report a unique value. That unique value becomes a key in a chord

the code is basically trying to prove that I can debounce multiple buttons and construct an int "chordValue" from it

Let me try to rephrase that, and see whether I've understood you correctly.

You have a set of inputs - four, in this example. The states of the inputs change over time. The state of all the inputs at a given time constitutes a chord, and the state of the inputs over time represents a sequence of chords.

Because the inputs don't necessarily all change state at the same instant, when the inputs are changing you wait until they have settled into their new state before you determine that a new chord has occurred.

In a sense what you're doing is not debouncing the inputs individually - you are debouncing their combined state.

In that case I'd implement a function to return the instantaneous states of the inputs as a number (each input corresponding to one bit of the number) and use an asynchronous debouncing algorithm along the following lines:

int getDebouncedState()
{
    static int oldState = 0;
    static int debouncedState = 0;
    static unsigned long lastChangeTime = millis();

    int newState = readRawState();
    if(newState != oldState)
    {
        // one of the inputs has changed state
        lastChangeTime = millis();
        oldState = newState;
    }
    else
    {
        // the inputs have not changed
        if((oldState != debouncedState) && (millis() - lastChangeTime > SETTLING_TIME))
        {
            // the inputs are stable and in a new state
            debouncedState = newState;
        }
    }
    return debouncedState;
}

You'd call that periodically like this:

int newChord = getDebouncedState()

if(newChord != oldChord)
{
    handleChordChange(oldChord, newChord);
    oldChord = newChord;
}

PeterH:
You have a set of inputs - four, in this example. The states of the inputs change over time. The state of all the inputs at a given time constitutes a chord, and the state of the inputs over time represents a sequence of chords.

I might word it differently but that is basically the jist of it

In a sense what you're doing is not debouncing the inputs individually - you are debouncing their combined state.

I fumbled over the wording of this at first thinking it implied that was done, which is most certainty false. This is Peter's idea to the solution, Its a good approach, Less complex.

Here is what what was ran to test it, using what Peter has provided

//getvalueTesting2
//4 buttons wired pins 2-5
//pin to tactile button to 330om to ground
//uses internal resistence to VCC
//
//goal is an expample that gathers debounced state information
//and combines every switch state within a time window
//into an int value
//ie, buttonstate[4]={1,2,5,1} --> int=1251

byte buttons[]={
  2,3,4,5};
#define NUMBUTTONS sizeof(buttons)

#define BOUNCE 15// time to debouncege
int oldChord=0;

void setup()
{
  Serial.begin(9600);
  //wait to bring up serial monitor
  //------------input setting
  for (byte set=0;set<NUMBUTTONS;set++)
  {
    //sets the button pins
    pinMode(buttons[set],INPUT);
    digitalWrite(buttons[set],HIGH);
  }
} 

void loop()
{
  int newChord = getDebouncedState();
  if(newChord>1111)
  {
    Serial.println(newChord);
  }
  /* if(newChord != oldChord)
   {
   handleChordChange(oldChord, newChord);
   oldChord = newChord;
   }*/
}

int getDebouncedState()
{
  static int oldState = 0;
  static int debouncedState = 0;
  static unsigned long lastChangeTime = millis();

  int newState = rawInput();
  if(newState != oldState)
  {
    // one of the inputs has changed state
    lastChangeTime = millis();
    oldState = newState;
  }
  else
  {
    // the inputs have not changed
    if((oldState != debouncedState) && (millis() - lastChangeTime > BOUNCE))
    {
      // the inputs are stable and in a new state
      debouncedState = newState;
    }
  }
  return debouncedState;
}

void handleChordChange(int oldChord,int newChord)
{

}

int rawInput()
{
  int incrementor=1;
  int value=1111;//2 will represent high as zero may prove problematic for low
  for(int i=0;i<NUMBUTTONS;i++)
  {
    if(digitalRead(buttons[i])==LOW)
    {
      value+=incrementor;
      incrementor*=10;
    } 
    else
    {
      incrementor*=10;
    }
  }
  return value;
}

Its amusing to do the wave on the serial monitor and see the 2 jump side to side
now its just a matter of having to figure out how to handleChordChange, I'll work on it a bit and report back whether I run into more complications or not. I've been dug out of a rut twice now and I'm greatful. :smiley:

out of curiosity I'm still interested in the fail point of the code in the op, for anyone interested in educational pursuits.

Sorry - what I mean to say is that this is what you want to achieve.

Amazingly this finally does something close to what is intended

//getvalueTesting2
//4 buttons wired pins 2-5
//pin to tactile button to 330om to ground
//uses internal resistence to VCC
//
//goal is an expample that gathers debounced state information
//and combines every switch state within a time window
//into an int value
//ie, buttonstate[4]={1,2,5,1} --> int=1251

byte buttons[]={
  2,3,4,5};
#define NUMBUTTONS sizeof(buttons)

#define BOUNCE 15// time to debounce

#define WINDOW 125



void setup()
{
  Serial.begin(9600);
  //wait to bring up serial monitor
  //------------input setting
  for (byte set=0;set<NUMBUTTONS;set++)
  {
    //sets the button pins
    pinMode(buttons[set],INPUT);
    digitalWrite(buttons[set],HIGH);
  }
} 

void loop()
{

  Serial.println(getValue());

}
//-----------------
int getValue()
{
  boolean basePress=true;
  boolean regularPress=true;
  boolean longPress=true;
  int builtChord=0;
  int oldChord=0;
  int baseChord=1111;//assumed inactivity when called
  unsigned long past=millis()-1;//first millis diff is engineered here to make sure things get on the ball
  int windowCount=0;
  while(windowCount<=WINDOW || getDebouncedState()>1111)//gather chordValue with in this time frame
  {
    unsigned long currentTime=millis();
    if(baseChord!=1111 || windowCount>BOUNCE)// activity detection, start timer
    {
      unsigned long diff=currentTime-past;
      windowCount+=diff;
    }
    //--gather chord 
    baseChord=getDebouncedState();
    if(baseChord > 1111)
    {
      if(basePress && windowCount > 20)
      {
        if(baseChord!=oldChord)
        {
          baseChord-=oldChord;
          //simply change baseChord to the differance of the value allready build// not really functional
          builtChord+=baseChord;
          basePress=false;
        }
        oldChord=baseChord;
      }
      if(regularPress && windowCount > 100)
      {
        if(baseChord!=oldChord)
        {
          baseChord-=oldChord;
          builtChord+=baseChord;
          regularPress=false;
        }
        oldChord=baseChord;
      }
      if(longPress && windowCount > 200)
      {
        if(baseChord!=oldChord)
        {
          baseChord-=oldChord;
          builtChord+=baseChord;
          longPress=false;
        }
        oldChord=baseChord;        
      }
    }
    //builtChord+=baseChord;
    //--set the past time
    past=currentTime;
  }
  //Serial.println(windowCount);
  return builtChord;
}

int getDebouncedState()
{
  static int oldState = 0;
  static int debouncedState = 0;
  static unsigned long lastChangeTime = millis();

  int newState = rawInput();
  if(newState != oldState)
  {
    // one of the inputs has changed state
    lastChangeTime = millis();
    oldState = newState;
  }
  else
  {
    // the inputs have not changed
    if((oldState != debouncedState) && (millis() - lastChangeTime > BOUNCE))
    {
      // the inputs are stable and in a new state
      debouncedState = newState;
    }
  }
  return debouncedState;
}

int rawInput()
{
  int incrementor=1;
  int value=1111;//2 will represent high as zero may prove problematic for low
  for(int i=0;i<NUMBUTTONS;i++)
  {
    if(digitalRead(buttons[i])==LOW)
    {
      value+=incrementor;
      incrementor*=10;
    } 
    else
    {
      incrementor*=10;
    }
  }
  return value;
}

I'm still having a hard time producing a reliable/consistent values when pressing more then two keys at the same time, but this is kinda primitive and needs some more tweaking.

My idea of creating compiling a unique value, seems a little weak but it works as long as only a few samples are taken before an overflow of "builtChord" happens.

if(regularPress && windowCount > 100)
      {
        if(baseChord!=oldChord)
        {
          baseChord-=oldChord;
          builtChord+=baseChord;
          regularPress=false;
        }
        oldChord=baseChord;
      }

main issue now is that, what seems like solid combinational press to me is a really sloppy one to the arduino over the course of 120ms or so. This is like the debounce issue except more dubious... or maybe I'm over-thinking it.

unless maybe I can put debounce on top of the debounce to "settle" the combos? I'll try it.. in the mean time, I would appreciate an pointers though

Does this suggest your settling time is too short?

I just change the times the samples are taken, and got much better results

I have to be cautious with the window size, if its too large, buttons feel unresponsive. Even at 125ms I can sense a bit of response lag. Its responsive enough for testing though

Seems odd putting two of the samples outside the time window but its allowed with the control on the time window

while(windowCount<=WINDOW || getDebouncedState()>1111)

right now I have the second sample at 200ms and third at 500

Accounting for a quick press and a regular press would have to been done within the window I think, but that would retain the "asynchronous combo issue" as ill call it.

I also just noticed that
if(baseChord!=oldChord)
filters holds states out of being detected

as a result of that line though "rocker gestures" are detected almost perfectly, which feels as though it is putting more then 26 unique combinations within the current capability