debouncing query

Here's the code for a 2 button foot-controller that will send midi messages to a looping pedal. Using simpler code, I've checked the midi transmit function and the button connections, but when I put them into this debounced SWITCH statement, nothing is sent out.

The general premise seems to have worked on previous pedals, I just can't see the error and I'm at the outer edge of my programming skills ;(

Any general coding tips, especially regarding redundant code is also appreciated!

// midi controller for line 6 HX effects to control looper
// july 19

#include <MIDI.h>
#include <midi_Defs.h>
#include <midi_Message.h>
#include <midi_Namespace.h>
#include <midi_Settings.h>
MIDI_CREATE_DEFAULT_INSTANCE();
// Constants
#define SWITCH1 2 //rec/play
#define SWITCH2 3 // stop
#define SWITCHES 2    // how many switches?

int switches[SWITCHES] = { SWITCH1, SWITCH2};
int switchState[SWITCHES] = { LOW, LOW };   // Initial state of switch
int currentSwitch = 0;
int recording = 0; // state or recording
unsigned long SwitchMillis = 0; // when button was released
unsigned long DebounceMillis = 20; //
unsigned long CurrentMillis = 0;

void setup()
{
  MIDI.begin(1);
  for ( currentSwitch = 0; currentSwitch < SWITCHES; currentSwitch++ )
  {
    pinMode( switches[currentSwitch], INPUT );          // Set pin for switch
    digitalWrite( switches[currentSwitch], HIGH );      // Turn on internal pullup
  }
}

void loop()
{
  CurrentMillis = millis();  // get the time at the start of this loop()
  if (CurrentMillis - SwitchMillis >= DebounceMillis)  //is it time to check the switches?
  {
    SwitchMillis = millis(); //re-initilize Timer
    for ( currentSwitch = 0; currentSwitch < SWITCHES; currentSwitch++ )
    {
      if ( (digitalRead(switches[currentSwitch]) != switchState[currentSwitch] ) && (switchState[currentSwitch] == HIGH) )
      {
        switch ( currentSwitch )
        {
          case 0: // rec or play
            if (recording == 0)
            {
              MIDI.sendControlChange(60, 0, 1); // rec
              recording = 1;
            }
            else
            {
              MIDI.sendControlChange(60, 127, 1); // play
              recording = 0;
            }
            break;
          case 1:  // stop
            {
              MIDI.sendControlChange(61, 0, 1); // stop
              break;
            }
        } // end switch
      } // end digiread
    } // end go through switches
    switchState[currentSwitch] = digitalRead( switches[currentSwitch] );
  } // go time to check switches
} // end loop

You're never putting the value of the switches into your array, so they're always LOW, so your switch statement is never executed.

It may look like you are, but the issue is that this statement:

    switchState[currentSwitch] = digitalRead( switches[currentSwitch] );

is after your for loop. currentSwitch is probably 2 at this point which also means that you're writing off the end of your array. You can prove it to yourself by printing currentSwitch.

Try moving that statement up into the for loop i.e. above the brace that's just before it.

Thanks for this, I changed the code to

        } // end switch
      } // end digiread
    switchState[currentSwitch] = digitalRead( switches[currentSwitch] );
    } // end go through switches
  } // go time to check switches

Is that what you meant? Sadly no result ;(

The way you have this written you have to read the switch twice and it may not have the same value each time. Also, you can't print the state as it is before the IF test

 if ( (digitalRead(switches[currentSwitch]) != switchState[currentSwitch] ) && (switchState[currentSwitch] == HIGH) )

Much better to write a short function that reads all the switches and saves their states. For example

void readSwitches() {
    for ( currentSwitch = 0; currentSwitch < SWITCHES; currentSwitch++ )
    {
      prevSwitchState[currentSwitch] = switchState[currentSwitch]; // save the old version
      switchState[currentSwitch] = digitalRead(switches[currentSwitch])
    }
}

Then you can iterate over the array of switchState[]s to see if any of them requires action - for example

void applySwitches() {
   for ( currentSwitch = 0; currentSwitch < SWITCHES; currentSwitch++ )
   {
      if (switchState[currentSwitch] == HIGH and prevSwitchState[currentSwitch] == LOW) {
          // do something
      }
   }
}

...R

Robin - after some head-scratching, I think I have implemented your idea and it seems to work!

The code is now

// midi controller for line 6 HX effects to control looper
// july 19

#include <MIDI.h>
#include <midi_Defs.h>
#include <midi_Message.h>
#include <midi_Namespace.h>
#include <midi_Settings.h>
MIDI_CREATE_DEFAULT_INSTANCE();
// Constants
#define SWITCH1 2 //rec/play
#define SWITCH2 3 // stop
#define SWITCHES 2    // how many switches?

int switches[SWITCHES] = { SWITCH1, SWITCH2};
int switchState[SWITCHES] = { LOW, LOW };   // Initial state of switch
int prevSwitchState[SWITCHES] = { HIGH, HIGH };
int currentSwitch = 0;
int recording = 0; // state or recording
unsigned long SwitchMillis = 0; // when button was released
unsigned long DebounceMillis = 20; //
unsigned long CurrentMillis = 0;

void setup()
{
  MIDI.begin(1);
  for ( currentSwitch = 0; currentSwitch < SWITCHES; currentSwitch++ )
  {
    pinMode( switches[currentSwitch], INPUT );          // Set pin for switch
    digitalWrite( switches[currentSwitch], HIGH );      // Turn on internal pullup
  }
}

void loop()
{
  CurrentMillis = millis();  // get the time at the start of this loop()
  if (CurrentMillis - SwitchMillis >= DebounceMillis)  //is it time to check the switches?
  {
    SwitchMillis = millis(); //re-initilize Timer
    readSwitches();
    for ( currentSwitch = 0; currentSwitch < SWITCHES; currentSwitch++ )
    {
      if (switchState[currentSwitch] == HIGH and prevSwitchState[currentSwitch] == LOW)
      {
        switch ( currentSwitch )
        {
          case 0: // rec or play
            if (recording == 0)
            {
              MIDI.sendControlChange(60, 127, 1); //rec
              recording = 1;
            }
            else
            {
              MIDI.sendControlChange(610, 127, 1); // play
              recording = 0;
            }
            break;
          case 1:  // stop
            {
              MIDI.sendControlChange(61, 0, 1); // stop
              break;
            }
        } // end cases
      } // end is state changed
    } // end go through switches
  } // end of check time loop
} // end loop

void readSwitches() {
  for ( currentSwitch = 0; currentSwitch < SWITCHES; currentSwitch++ )
  {
    prevSwitchState[currentSwitch] = switchState[currentSwitch]; // save the old version
    switchState[currentSwitch] = digitalRead(switches[currentSwitch]);
  }
}

Many thanks for this....

You might consider refactoring loop: it's long and has a lot of control constructs in it, which led to the problem I mentioned above being hard to see.

While I was trying to figure out what was wrong, I took your switch statement and put it in a function of its own which shrank loop down enough that I could see it all at once.