Class instance, ISR and callback = function generator hell? Or am I missing some knowledge?

Hello,
I would love to ask if there is any option to get out from ISR callback hell. I have no idea if there is a way to simplify this problem. I watched this post because it is similar in a way of handling ISR when used with class instance, but does not solve callback in ISR.

My circuit is easy buttons are connected as pull-down resistor like this [VIN = 3.3V, R1 = 10k Ohm]:

Here is the code [solution 1]:

class ButtonInterupt {

protected:
    byte pin;
    volatile unsigned long last_click_millis = 0; // last click millis()
    volatile unsigned long last_click_difference = 0; // difference in [ms] between last two clicks
    int debounce_interval = 50; // debounce effect [ms]

public:
    ButtonInterupt(byte);

    void _button_interupt_isr (void (*)(void) = NULL);

    void setButtonISR(void (*)(void));

    unsigned long getLastClickDifference();
};

ButtonInterupt::ButtonInterupt(byte btn_pin){
    pin = btn_pin;
    pinMode(pin, INPUT); // button is connected in mode pull-down resistor (external) 
}

// I have to create a function which is specific for an instance. See below under // ISRs
void ButtonInterupt::_button_interupt_isr (void (*callback)(void)) {
    if(digitalRead(pin) && millis() - last_click_millis > debounce_interval) { // when btn pressed
        last_click_difference = millis() - last_click_millis;
        last_click_millis = millis();
       
        // do some custom action specific for the instance 
        if(callback) { 
            callback();
        }
    }
}

// set ISR - call in setup()
void ButtonInterupt::setButtonISR(void (*isr)(void)){
    attachInterrupt(digitalPinToInterrupt(pin), isr, RISING);
}

unsigned long ButtonInterupt::getLastClickDifference(){
  return last_click_difference;
}
// ----- end of class method implementations -----

// Create Instances
ButtonInterupt print_btn(PRINT_PIN);
ButtonInterupt count_clicks_btn(COUNT_CLICKS_PIN);

// Callbacks
void print_callback(){ // only some reaction on click
  Serial.println("Print");
}

void count_clicks_callback(){ // use stats from button class
  sum_click_intervals += count_clicks_btn.getLastClickDifference(); // sum all click time differences
  clicks++;
}

// ISRs
void print_isr(){
  print_btn._button_interupt_isr(print_callback);
}

void count_clicks_isr(){
  count_clicks_btn._button_interupt_isr(count_clicks_callback);
}

void setup() {
  Serial.begin(112500);
  
  print_btn.setButtonISR(print_isr);
  count_clicks_btn.setButtonISR(count_clicks_isr);
}

// Some counting as example as task for 
volatile int clicks = 0;
volatile unsigned long sum_click_intervals = 0;

unsigned long last_stat_print = 0;
unsigned long print_interval = 15000;

void loop() {
  if(millis() - last_stat_print >= print_interval){
    last_stat_print += print_interval;
    
    float average = 0;
    if(sum_click_intervals > 0){
      average = clicks/(sum_click_intervals/1000.0);
    }
    
    Serial.print("Seconds: ");
    Serial.println(sum_click_intervals/1000.0);

    Serial.print("Clicks: ");
    Serial.println(clicks);

    clicks = 0;
    sum_click_intervals = 0;

    Serial.print("Clicks/second: ");
    Serial.println(average);
  }
}

My problem is that I need to specify two another functions (Callback + ISR) for every instance to get instance working. I think I don't see advantages of solution 1.

My question is if this is good way or there is some better way how to call custom action in ISR function specific for an instance. Or this code is total trash and the better way to solve this problem is just use this code below.

[solution 2]:

// global variables
    volatile unsigned long print_last_click_millis = 0; // last click millis()
    volatile unsigned long print_last_click_difference = 0; // difference in [ms] between last two clicks
    int print_debounce_interval = 50; // debounce effect [ms] 


    void print_isr () {
    if(digitalRead(PRINT_PIN) && millis() - print_last_click_millis > print_debounce_interval) { // when btn pressed
        print_last_click_difference = millis() - print_last_click_millis;
        print_last_click_millis = millis();
        
         //print or some counting can be pasted here
        Serial.println("Print: No more callbacks."); 
    }
  } 

  // same way for another button ISR

void setup() {
  Serial.begin(112500);
  pinMode(PRINT_PIN, INPUT);
  attachInterrupt(digitalPinToInterrupt(PRINT_PIN), print_isr, RISING);
}

I'll be happy for any response. Thank you in advance. :slight_smile:

It's not clear to me what problem you are having. Are you not getting the results you expect? What did you expect to happen and what happened instead?

I tried to compile your sketch and had to work around a few errors, like variables being used before they were defined.

class ButtonInterupt
{
  protected:
    byte pin;
    volatile unsigned long last_click_millis = 0; // last click millis()
    volatile unsigned long last_click_difference = 0; // difference in [ms] between last two clicks
    unsigned debounce_interval = 50; // debounce effect [ms]

  public:
    ButtonInterupt(byte);

    void _button_interupt_isr (void (*)(void) = NULL);

    void setButtonISR(void (*)(void));

    unsigned long getLastClickDifference();
};

ButtonInterupt::ButtonInterupt(byte btn_pin)
{
  pin = btn_pin;
  pinMode(pin, INPUT); // button is connected in mode pull-down resistor (external)
}

// I have to create a function which is specific for an instance. See below under // ISRs
void ButtonInterupt::_button_interupt_isr (void (*callback)(void))
{
  if (digitalRead(pin) && millis() - last_click_millis > debounce_interval)  // when btn pressed
  {
    last_click_difference = millis() - last_click_millis;
    last_click_millis = millis();

    // do some custom action specific for the instance
    if (callback)
    {
      callback();
    }
  }
}

// set ISR - call in setup()
void ButtonInterupt::setButtonISR(void (*isr)(void))
{
  attachInterrupt(digitalPinToInterrupt(pin), isr, RISING);
}

unsigned long ButtonInterupt::getLastClickDifference()
{
  return last_click_difference;
}
// ----- end of class method implementations -----

const byte PRINT_PIN = 2;
const byte COUNT_CLICKS_PIN = 3;

// Create Instances
ButtonInterupt print_btn(PRINT_PIN);
ButtonInterupt count_clicks_btn(COUNT_CLICKS_PIN);

// Some counting as example as task for
volatile int clicks = 0;
volatile unsigned long sum_click_intervals = 0;

unsigned long last_stat_print = 0;
unsigned long print_interval = 15000;

// Callbacks
void print_callback()  // only some reaction on click
{
  Serial.println("Print");
}

void count_clicks_callback()  // use stats from button class
{
  sum_click_intervals += count_clicks_btn.getLastClickDifference(); // sum all click time differences
  clicks++;
}

// ISRs
void print_isr()
{
  print_btn._button_interupt_isr(print_callback);
}

void count_clicks_isr()
{
  count_clicks_btn._button_interupt_isr(count_clicks_callback);
}

void setup()
{
  Serial.begin(112500);

  print_btn.setButtonISR(print_isr);
  count_clicks_btn.setButtonISR(count_clicks_isr);
}



void loop()
{
  if (millis() - last_stat_print >= print_interval)
  {
    last_stat_print += print_interval;

    float average = 0;
    if (sum_click_intervals > 0)
    {
      average = clicks / (sum_click_intervals / 1000.0);
    }

    Serial.print("Seconds: ");
    Serial.println(sum_click_intervals / 1000.0);

    Serial.print("Clicks: ");
    Serial.println(clicks);

    clicks = 0;
    sum_click_intervals = 0;

    Serial.print("Clicks/second: ");
    Serial.println(average);
  }
}

So, I think you have multiple/many buttons and are concerned about the code complexity and duplication of routines on a per-button basis?

Look into how keypads are decoded; much simplier approach, IMO

I also think you may be using the wrong approach to handle the switches.

However, if your problem is about handling ISR and class instances, I did some research some time ago and wrote it up in a blog post. Maybe that can help.
https://arduinoplusplus.wordpress.com/2021/02/05/interrupts-and-c-class-instances/