using ISR with a parameter

Hi, I would like to be able to do something like the following

attachInterrupt(digitalPinToInterrupt(encoderPin1), GetEncoderPulse(1), CHANGE);

void GetEncoderPulse(int pin){


//do some stuff with using the value of pin


}

obviously this doesn't work, just wondered why?

because that's not how ISR works nor function pointers in C++

use two different functions as ISR, one per interrupt pin, then calling the same function with the pin # as param

void ISRCall(byte pinNb) {
  // do your business here
}
void pin2ISR() {ISRCall(2);}
void pin3ISR() {ISRCall(3);}
...
// in setup
attachInterrupt(digitalPinToInterrupt(2), pin2ISR, CHANGE);
attachInterrupt(digitalPinToInterrupt(3), pin3ISR, CHANGE);

well that told me :slight_smile: realised I could use 2 ISR's just wondered why it wasn't supported after all an ISR is just a function isn't it?

An ISR can be called at any time interrupts are enabled.
Where would the parameter come from?

Not knowing much about how C works in depth I'm not entirely sure what you mean. I am assuming thatattachInterrupt(digitalPinToInterrupt(encoderPin1), GetEncoderPulse(1), CHANGE)defines what happens when an interrupt is triggered and calls, in this case the ISR GetEncoderPulse(1), where the parameter is set, this would be set once and wouldn't be able to change through out the program. You could use one ISR and know what pin had called it. Just thought if that was possible you'd could use less ISR's, Just been reading Nick Gammons article on ISR's .Don't know if that would save much time or resources. I'm more used to Basic, pascal and database work but I would always try to find a way to slim my code down, as I try to do with C, and try to reuse code by placing in procedures so I think that if I had written that attachInterupt routine I think I would have written in a way of passing the pin number back to the ISR.

Rather than try to pass a parameter to an ISR you can get it to read the value in a global variable.

...R

Robin2:
Rather than try to pass a parameter to an ISR you can get it to read the value in a global variable.

...R

No, because both "instances" would use the same global value

AWOL:
No, because both "instances" would use the same global value

So why not have two different global variables?

I can't figure what the OP really wants to achieve.

...R

One ISR, with a runtime parameter based on the source of the interrupt.

AWOL, hole in one, I thought something like that would be better than using several ISR's, it's academic anyway, since it can't be done.

I suggest you use the several ISRs to call a common function. Then each ISR can pass its own parameter. However the total amount of code that runs in response to the interrupt should be kept very short.

Another approach might be for each ISR just to set a flag variable and then the main code can deal with that.

...R

Robin2:
I suggest you use the several ISRs to call a common function. Then each ISR can pass its own parameter.

That is probably the easiest way. But, if you want to get down into the weeds, you can redefine the interrupt vectors to call a common ISR with a parameter:

#include "Arduino.h"

void commonISR(uint8_t interruptNumber);
volatile bool int0Fired = false;
volatile bool int1Fired = false;

void setup() {
  Serial.begin(115200);
  delay(1000);
  Serial.println("Starting");

  pinMode(2, INPUT_PULLUP);
  EIMSK &= ~(1 << INT0);
  EICRA = (EICRA & ~((1 << ISC00) | (1 << ISC01))) | (FALLING << ISC00);
  EIFR |= (1 << INTF0);
  EIMSK |= (1 << INT0);

  pinMode(3, INPUT_PULLUP);
  EIMSK &= ~(1 << INT1);
  EICRA = (EICRA & ~((1 << ISC10) | (1 << ISC11))) | (FALLING << ISC10);
  EIFR |= (1 << INTF1);
  EIMSK |= (1 << INT1);
}

void loop() {
  if (int0Fired) {
    int0Fired = false;
    Serial.println("Interrupt 0 Fired");
  }

  if (int1Fired) {
    int1Fired = false;
    Serial.println("Interrupt 1 Fired");
  }
}

#undef __vector_1
ISR(INT0_vect) {
  commonISR(0);
}

#undef __vector_2
ISR(INT1_vect) {
  commonISR(1);
}

void commonISR(uint8_t interruptNumber) {
  switch (interruptNumber) {
    case 0:
      int0Fired = true;
      break;

    case 1:
      int1Fired = true;
      break;

    default:
      break;
  }
}

This method provides the lowest overhead possible by saving a lookup in the ISR table and an extra function call. It’s up to the user to decide if that is worth the loss of portability between different processor families (and perhaps even between members of the same family).

EDIT:
Looks like you don't need the #undef directives in the above code. Those are vestiges of of my debugging efforts.

I know that this has been a while but I have just found this piece of code which I think, for me anyway, is going to come in handy. I don't fully understand the "#define attachMyInterrupt" just yet but I will take it apart until I do, pretty much like everything else I do :slight_smile:

#define attachMyInterrupt(pin, mode) attachInterrupt(digitalPinToInterrupt(pin), +[](){ isr(pin); }, mode)
#define PINB 20 //White is B Phase
#define PINA 21 //Green is A Phase

volatile boolean dir;

void isr(uint8_t pin) 
{
  bool pinA=digitalRead(PINA);
  bool pinB=digitalRead(PINB);
  
  if (pin == 20){if (pinA==pinB) dir=HIGH; else dir=LOW;}

  Serial.println(dir);
}

void setup ()
{

 digitalWrite (PINA, HIGH); 
 digitalWrite (PINB, HIGH); 
 attachMyInterrupt(PINA, CHANGE);
 attachMyInterrupt(PINB, CHANGE);
 Serial.begin (115200);
 
}  

void loop ()
{


}

This function works a treat and once I understand it I will be using it. There is another piece of code in there which I adapted from code written by Mr Gammon and Mr Raskell

if (pinA==pinB) dir=HIGH; else dir=LOW;

//Adapted from

if (digitalRead (PINA))
   up = digitalRead (PINB);
 else
   up = !digitalRead (PINB);

The upshot of this is I can get the full resolution from a rotary encoder by using both channels but also get the simplicity of detecting direction from using just one channel.

1 Like

That code simply attaches 2 ISRs -- one for each pin. Those ISR just happen to be Lambda Expressions. It's really no different than having 2 explicit ISR functions -- again one for each pin.

Each of those ISRs then simply calls a 3rd (common) function with a parameter.

Nothing magic there. Just some snazzy abstraction from the lambdas.

That's as maybe and you know more than I, but what it means to me is, in this instance anyway, is I can use one isr to process 2 interrupt pins and detect which pin called the isr which is exactly what I wanted. Must admit though I didn't see your code example of the 27th March, looks interesting but I'm just a beginner in c, having finally been dragged kicking and screaming to it after using basic and pascal, it's actually not that bad, bit incomprehensible at times and not the best structured language, but not so bad.

And I've just noticed that J-M-L posted a pretty good solution too and Robin which I totally missed , I really need a face palm icon!

TobyOne:

attachInterrupt(digitalPinToInterrupt(encoderPin1), GetEncoderPulse(1), CHANGE);

obviously this doesn't work, just wondered why?

Because the declaration of attachInterrupt() is

void attachInterrupt(uint8_t interruptNum, void (*userFunc)(void), int mode)

The second argument is type "void (*)(void)" which means "a pointer to a function that takes no arguments and returns no value". If you try to pass the address of a function that takes a value, you will get a compile error:

void myISR(int arg) {}

void setup()
{
  attachInterrupt(digitalPinToInterrupt(2), myISR, CHANGE);
}
void loop() {}
sketch_aug13a.ino: In function 'void setup()':
sketch_aug13a:5:58: error: invalid conversion from 'void (*)(int)' to 'void (*)()' [-fpermissive]
   attachInterrupt(digitalPinToInterrupt(2), myISR, CHANGE);
                                                          ^

It gets even worse when you try to specify an argument. Then you are not passing the address of (a.k.a. "a pointer to") a function; you are calling the function and passing the result of that call. Since the function returns 'void' you get an error for trying to use a void where a pointer is needed.

  attachInterrupt(digitalPinToInterrupt(2), myISR(1), CHANGE);
sketch_aug13a.ino: In function 'void setup()':
sketch_aug13a:5:61: error: invalid use of void expression
   attachInterrupt(digitalPinToInterrupt(2), myISR(1), CHANGE);
                                                             ^

Note: You can trick attachInterrupt() into taking the pointer to you function by casting it to the correct type, but then there is no place to specify an argument and your function will get whatever garbage happens to be in the register or stack position used to pass the argument.

  attachInterrupt(digitalPinToInterrupt(2), (void (*)()) myISR, CHANGE);

Hi John, I now understand, it just seems that it would add an extra dimension to the attchinterrupt function if it could return the pin that called it, however there is always more than one way to skin a cat and J-M-L,Robin and gfvalvo have offered excellent suggestions one of which I will be using as it does exactly what I want and I fully understand how it works.

TobyOne:
but what it means to me is, in this instance anyway, is I can use one isr to process 2 interrupt pins and detect which pin called the isr

Not really. But, if you're happy with the illusion, then .....

gfvalvo:
Not really. But, if you're happy with the illusion and additional delay, then .....

gfvalvo: sorry where's the illusion here? for all intents and purposes I have one function that is called by either interrupt pins that can tell me which pin called it. Which is what I wanted.