Guidance on handling a SPDT switch with Pin Change Interrupts

As a novice, I’m looking for a little bit of feedback on how I’m approaching a problem.

What I’m trying to do is detect when a SPDT switch is toggled forward or back, and if a toggle was momentary (but not a bounce) or if it’s an extended hold. My intent is to increment an actuator forward or back through a set schedule of positions with brief clicks, and to move it smoothly forward and back with a press and hold until the switch is released.

I believe there are several things I will need to do for this to work reliably, but I’m fairly novice, so I welcome any feedback.

I believe that what I need to do is:

(1) Set up a pin change interrupt on Port D so that either pin 2 or 4 will trigger an interrupt. Sample code below shows how I’ve tried to implement this step.

(2) Second, debounce the interrupt.

(3) Third, determine which pin triggered the interrupt and then call the appropriate function for forward or backward travel.

(4) The actuating function will evaluate whether the switch was momentarily toggled or if it’s still being pushed and held.

My questions are:

(A) Is the code below a reasonable approach to doing a pin change interrupt with a debounce?

(B) I’m not really sure of a great way to detect whether the switch was briefly toggled or is being pushed and held. Any suggestions on how to handle this are very welcome.

Thanks in advance,
Chris

const byte fwdPIN = 2;   // Uses this pin number to check if momentary switch is activated for forward direction
const byte revPIN = 4;   // Uses this pin number to check if momentary switch is activated for reverse direction


// Variables that support switching using a button on an interrupt
volatile byte portDHistory = 0b00000101; // Default is high on PD0 and PD2 because of the pullups
volatile int fwdSwitchState = HIGH;      // Initial forward switch state
volatile int revSwitchState = HIGH;      // Initial reverse switch state

volatile unsigned long lastInterruptTime = 0; // The last time the interrupt was triggered
const unsigned long debounceTime = 10;  // Debounce time; decrease if quick button presses are ignored

/* Interrupt Service Routines (ISR) */
ISR (PCINT2_vect)                         
{
    // Figure our which pin is triggering the interrupt
    byte changedbits = PIND ^ portDHistory;    // XOR the PortD register with the portDHistory register gives 0's on the bits that are the same, and 1's in the bits that are different
    portDHistory = PIND;
    if(changedbits & (1 << PIND2))             // AND the operation with a bitmask using the bit shift register in order to isolate specific bit.
    {
        /* PCINT16 changed */
        unsigned long currentTime = millis();        
        if ((currentTime - lastInterruptTime) >= debounceTime) // Debounce the interrupt
        {
          // Do something
           
        }
        lastInterruptTime = currentTime;
    }
    
    if(changedbits & (1 << PIND4))
    {
        /* PCINT18 changed */
        unsigned long currentTime = millis();
        if ((currentTime - lastInterruptTime) >= debounceTime) // Debounce the interrupt
        {
          // Do something
        }
        lastInterruptTime = currentTime;
    }
}

void setup()
{
  
  pinMode(fwdPIN, INPUT);
  pinMode(revPIN, INPUT);
  digitalWrite(fwdPIN, INPUT_PULLUP);
  digitalWrite(revPIN, INPUT_PULLUP);
  
  // Pin change interrupts
  noInterrupts();                      // Switch interrupts off while messing with their settings
  PCMSK2 |= 0b00000101;                // Turn on PD0 (PCINT16, physical pin 2) and and PD2 (PCINT18, physical pin 4)
  PCICR |= 0b00000100;                 // Enables port D Pin Change Interrupts for PCINT16 (pin 2) and PCINT18 (pin 4)
  interrupts();                        // Turn interrupts back on
}

void loop()
{
  
  
}

Welcome to the Forum!

I’m looking for a little bit of feedback on how I’m approaching a problem.

It is very rare to need to use a interrupt for a button/switch . As long as there is no blocking code (delay() etc.) in the time it take you to press the switch the arduino will check it thousands of times.

I see you have tried to use millis() to debounce the interrupt, that doesn’t work because millis() is disabled during interrupts. But it will work for debouncing a button . Also ISR’s should always be kept short you want to get in and get out.

But today I found ( @NewbieHack) a new (to me) way to debounce that is really simple, non-Blocking and easy to manipulate.
I already wrote some test code for it and with a little tweaking it might be just what you need.

const byte buttonPinA =6;   // Switch  A
int pressedCounterA = 0;
int releasedCounterA = 0;

const byte buttonPinB = 7;   // Switch  B
int pressedCounterB = 0;
int releasedCounterB = 0;

byte pressedState = 0;

void setup() {
  pinMode (buttonPinA, INPUT_PULLUP);
  pinMode (buttonPinB, INPUT_PULLUP);
}

void loop() {
  byte buttonState = buttonCheck();   // get results from buttonCheck()

  switch (buttonState)
  {
    // case 1:                 // Button A Pressed but we don't know if it is a Quick or Long Press
    // could do something here or eliminate any case that's not needed
    //   break;
    case 2:                 // Button A Long Press
      // start doing stuuff you want during Button A Long Press
      break;
    case 3:                 // Button A Quick Press
      // do stuff you want when Button A Quick Press
      break;
    case 4:                 // Button A Long Press Released
      // stop doing Button A Long Press stuff
      break;
    // case 5:                 // Button B Pressed but we don't know if it is a Quick or Long Press
    // could do something here or eliminate any case that's not needed
    //   break;
    case 6:                 // Button B Long Press
      // start doing stuuff you want during Button B Long Press
      break;
    case 7:                 // Button B Quick Press
      // do stuff you want when Button B Quick Press
      break;
    case 8:                 // Button B Long Press Released
      // stop doing Button B Long Press stuff
      break;
  }
}

byte buttonCheck() {                 // read,debounce and determine buttonState
  if (digitalRead(buttonPinA == 1))
  {
    pressedCounterA++;            // add one to pressedCounter
    releasedCounterA = 0;         // reset releasedCounter
    if (pressedCounterA > 500)    // check if button is debounced,//  Adjust this <<<<<<<<<<<<<
    {
      pressedState = 1;          // Button has been Pressed, it may qualify as a Quick Press
    }
    if (pressedCounterA > 1000)   //  Adjust this <<<<<<<<<<<<<<
    {
      pressedState = 2;      // Long Press Detected
      pressedCounterA = 0;
    }
    else                         // buttonPin reads 0
    {
      releasedCounterA++;         // add one to releasedCounter
      if (releasedCounterA < 1000) // check if button is debounced, Adjust this <<<<<<<<<<<<<<
      {
        pressedState = 3;       // Quick Press Detected
      }
      else
      {
        pressedState = 4;        // Long Press Released
      }
      pressedCounterA = 0;        // reset pressedCounterA
    }
  }
  if (digitalRead(buttonPinB == 1))
  {
    pressedCounterB++;            // add one to pressedCounterB
    releasedCounterB = 0;         // reset releasedCounterB
    if (pressedCounterB > 500)    // check if button is debounced, Adjust this <<<<<<<<<<<<<<

    {
      pressedState = 5;          // Button has been Pressed, it may qualify as a Quick Press
    }
    if (pressedCounterB > 1000)    //  Adjust this <<<<<<<<<<<<<<
    {
      pressedState = 6;      // Long Press Detected
      pressedCounterB = 0;
    }
    else                         // buttonPin reads 0
    {
      releasedCounterB++;         // add one to releasedCounter
      if (releasedCounterB < 1000) // check if button is debounced
      {
        pressedState = 7;       // Quick Press Detected
      }
      else
      {
        pressedState = 8;        // Long Press Released
      }
      pressedCounterB = 0;        // reset pressedCounterB
    }
  }
  return pressedState;
}

you need to adjust the counters to work best for you and your switches But it’s just a little testing to get it right.

The demo Several Things at a Time reads a switch without interrupts and with a very simple debounce system ( a brief interval between reads).

...R

Common misunderstanding of what an “interrupt” does.

Hutkikz, thanks for the welcome and the helpful comment. I didn't realize that millis() couldn't be called during a custom ISR, so thank you for pointing that out.

I supposed I should explain why I was pursuing interrupts in the first place. My original plan was to use interrupt driven functionality to measure things relative to time. So rather than waiting for things to happen in the main loop, the interrupt handlers will quickly set timing derived state when an input changes (encoders and switches) and then resume the main loop. My thinking was that this would make the timing of things much more robust and frees up dependency of timing and debounce from how long the loop takes to execute. My goal was to put only high level decision making in the loop.

Clearly, I need to give a bit more thought to how I'm approaching this problem.

CCYoung: So rather than waiting for things to happen in the main loop, the interrupt handlers will quickly set timing derived state when an input changes (encoders and switches) and then resume the main loop.

No matter what approach you take the Arduino will be carrying out 16 million instructions per second all the time. You do not change that just by using an interrupt. If you take polling code out of loop() and don't replace it with something else it will just mean that loop() repeats more frequenctly. If loop() is repeating and doing nothing else it might just as well be polling an I/O pin.

You should also realize that, although an interrupt can respond quickly to an event, it actually uses more CPU cycles compared to doing the same job with polling. Because the interrupt has to save and restore the registers every time it is triggered and subsequently returns to the main code.

...R

CCYoung: My thinking was that this would make the timing of things much more robust and frees up dependency of timing and debounce from how long the loop takes to execute. My goal was to put only high level decision making in the loop.

And that is the basic misunderstanding.

The loop is the "worker". And de-bouncing and timing is the high level decision making. As Robin2 points out, 16 million instructions per second is one million times faster than your finger on the switch. Nor is your switch timing as a "HID", in any respect critical; the shortest "hold" timing for your PC keyboard is a quarter of a second (which in fact, is the only such timing you want to use unless you suffer from Parkinson's Disease).

So your loop code can - and should perform the de-bouncing and "hold" timing and a lot more simultaneously. Performing debounce in interrupt routines is a horrible kludge and while you can do it, it is much less robust than having your loop() poll in milliseconds.

It should be obvious that if the de-bouncing period is many milliseconds (10 is not a bad figure) then polling for each millisecond (since the loop() should almost always cycle many times per millisecond) will be entirely adequate. However I personally prefer an algorithm (which I can cite) which polls the switch at priority before checking millis() so that de-bouncing is verified when the switch has remained stable for all of the debounce interval, rather than being checked only at that interval after the first change is detected.

I didn't realize that millis() couldn't be called during a custom ISR, so thank you for pointing that out.

But, it can be. What millis() won't do is report different times if called more than once in an ISR, since the interrupts that cause millis() to change output are disabled.

If the intent is simply to record when an interrupt happened, calling millis() is perfectly reasonable.

If the intent is to waste time, using an interrupt was not the proper approach.