Using an Interrupt from a method inside a class

I read this topic, specifically reply #8, and thought I had it figured out! Unfortunately even with all that wisdom I'm still having problems. I'm writting a class called FlowRate that reads data from a hall effect sensor that uses an interrupt.

I was trying to use the example shown in the forum that I reference above. Here is the sketch whittled down to the minimum to show what the problem is:

#include <FlowRate.h>

FlowRate flowSens1;  //Create a flow rate sensor.

unsigned long restFlRtFreq = 0;  //The time the Flow rate was last checked.
int checkFlRtFreq = 2000;  //This is how often we want to get the flow rate for an intermediate check

void setup() 
{
  Serial.begin(9600);  //Start Serial
  Serial.println("<Arduino Ready...>");

  flowSens1.begin();  
  restFlRtFreq = flowSens1.startInterMeas();
}

void loop() 
{
  if(millis()-restFlRtFreq > checkFlRtFreq)  //if 2 seconds have gone by 
  {
    Serial.print("Intermediate Flow Rate Measurement is:");Serial.print(flowSens1.getInterFlowRate());  //print out the intermediate flow rate.
    restFlRtFreq = flowSens1.startInterMeas();  //Reset the intermediate measuremnt start time as well as when we come back into this if statement
  }
  
}

Here is the header file, severely whittled:

#ifndef FlowRate_h
#define FlowRate_h

#include "Arduino.h"

class FlowRate
{
	public:
		FlowRate();               //Constructor(flowSensPin is assumed 2, calFactor is assumed 1)
	
		void begin();  // This attaches the interupt to the pin and method
		unsigned long startInterMeas();  //Called to when one would like to start an intermediated flow rate measurment						
		float getInterFlowRate();  //Called to get an intermediated flow rate meesurement you have to called
											//startInterMeas() before calling this again as it is the start
		
		
	private:
		static void marshall();  //The method that calls the right instance of the ISR (flowCountInc())
		void flowCountInc();	//Increments the counter.  This is called by the marshall when the interupt
									// flowSensPin RISING occurs
		int _flowSensPin;  //The pin used to flow rate seen by the hall effect sensor (must be interuptable)
		float _calFactor;  //mulitply this times the number of counts to get the amount of fluid in volume (your choice) per second
		volatile  unsigned long _flowCount;  //This is the counter that hold the number of times the hall effect tranducer has signalled
		static FlowRate* _anchor;  //this allows marshall() to call the right flowCountInc().
		unsigned long _flowCountInter;  //This is used to remember the count when an intermeditat flowRate may be requested
		unsigned long _startIntMeasTime;  //This is the time used to start the intermediate measurment of fluid volume
		
};

#endif

and of course the all important cpp file, again severely whittled:

#include "Arduino.h"
#include "FlowRate.h"

FlowRate::FlowRate()
{
	_anchor = this;
	_flowSensPin = 2;  //The pin used to flow rate seen by the hall effect sensor (must be interuptable
	_calFactor = 1.0;  //mulitply this times the number of counts to get the amount of fluid in volume (your choice) per second
	_flowCount = 0;  //The number of times the hall effect tranducer has sent a pulse
	_flowCountInter = 0; //the count start for an intermediate flow rate request
	
	pinMode(_flowSensPin, INPUT);           //Sets the pin as an input
}

void FlowRate::begin()   //This needs to be called to cause the pin to be linked to the Interupt
{
	char irqNum = digitalPinToInterrupt(_flowSensPin);  //If 2 is passed and you are compiling for an UNO you irqNum should be 0.
	if (irqNum != NOT_AN_INTERRUPT) 
	{  
		EIFR = bit(irqNum);         // cancel evt. pending interrupt
		attachInterrupt(irqNum, FlowRate::marshall, RISING); //Configures interruptpin 0 (pin 2 on the Arduino Uno) to run the function 
	}
}
static void FlowRate::marshall()
{
	_anchor->flowCountInc();
}
					

unsigned long FlowRate::startInterMeas()  //Called to when one would like to start an intermediated 
{										//flow rate measurment
	_startIntMeasTime = millis();			
	noInterrupts(); 					//Disable the interrupts on the Arduino	
	_flowCountInter = _flowCount;
	interrupts();	//inable the interrupts on the Arduino	
	return _startIntMeasTime;
}

float FlowRate::getInterFlowRate()	//Called to get an intermediated flow rate measurment
{											
	unsigned long now = millis();
	float flowRate;
	noInterrupts(); //Disable the interrupts on the Arduino	
	flowRate = ((float(_flowCount)-float(_flowCountInter)) * _calFactor)/(float(now - _startIntMeasTime)/1000.0);	//Calcualate the flow rate
	interrupts();	//inable the interrupts on the Arduino	
	_startIntMeasTime = now;  //Update the start time for intermediate flowrate inquiziations.
	return flowRate;
}

void FlowRate::flowCountInc()	//This is used to increment the _flowCount variable
{   
	_flowCount++;
}

I really have no idea what "classIrq* classIrq::anchor = NULL;" does in the example and I have no clue as to where it should go?

Also, something that struck me as strange was the line "attachInterrupt(irqNum, classIrq::marshall, CHANGE);" because I've never seen :: used in that way.

Here is the output from the compiler:

C:\Users\mhessel\AppData\Local\Temp\ccCvpFwP.ltrans0.ltrans.o: In function `FlowRate::marshall()':

M:\Arduino\Sketches\libraries\FlowRate/FlowRate.cpp:29: undefined reference to `FlowRate::_anchor'

M:\Arduino\Sketches\libraries\FlowRate/FlowRate.cpp:29: undefined reference to `FlowRate::_anchor'

C:\Users\mhessel\AppData\Local\Temp\ccCvpFwP.ltrans0.ltrans.o: In function `__base_ctor ':

M:\Arduino\Sketches\libraries\FlowRate/FlowRate.cpp:6: undefined reference to `FlowRate::_anchor'

M:\Arduino\Sketches\libraries\FlowRate/FlowRate.cpp:6: undefined reference to `FlowRate::_anchor'

collect2.exe: error: ld returned 1 exit status

Using library FlowRate in folder: M:\Arduino\Sketches\libraries\FlowRate (legacy)
exit status 1
Error compiling for board Arduino/Genuino Uno.

I hope I not giving too much or too little data. Thanks!!!!

mhessel:
I really have no idea what "classIrq* classIrq::anchor = NULL;" does in the example and I have no clue as to where it should go?

Also, something that struck me as strange was the line "attachInterrupt(irqNum, classIrq::marshall, CHANGE);" because I've never seen :: used in that way.

In your case the line should read

FlowRate* FlowRate::anchor = NULL;

can go to the cpp file (or the ino) and it is the definition of the static class member.

You should use

attachInterrupt(irqNum, FlowRate::marshall, CHANGE);

How would you specify a static member function?

About the :: notation you should consult the C++ documentation.

Whandall:
In your case the line should read

FlowRate* FlowRate::anchor = NULL;

Or type safe

FlowRate* FlowRate::anchor = nullptr;

Are there Arduino environments in which that matters?

Both variants compile (for a Mega and ESP32) without warnings.

The first notation will be at least legal for some years to come IMHO.

Hi. Hope this isn’t too much of a hijacking, but I’m just transitioning from C to C++ so the details are interesting to me.

Whandall:

FlowRate* FlowRate::anchor = NULL;

can go to the cpp file (or the ino) and it is the definition of the static class member.

So that I fully understand, this is the definition -- where the RAM storage space for the variable is actually allocated?

That means that:

class FlowRate
{
  private:
    static FlowRate* anchor;
};

is a declaration -- just telling everyone that anchor is a class variable and what it’s type is?

So, in plain old C, it would be similar to:

extern FlowRate* anchor;

The storage space still needs to be allocated somewhere.

Contrast this with:

class FlowRate
{
  private:
    FlowRate* anchor;
};

This seems like a definition -- it’s actually allocating storage space for a variable that belongs to an instance of the class.

So, how close is the above to reality?

Thanks.

At least it's part of my reality.

Great. Then continuing with my stream of consciousness -- There is only one ‘marshall()’ and one ‘anchor’ (they are class (not instance) members). So, if you instantiate and ‘begin()’ multiple objects of this class type, ‘anchor’ will point to the ‘isr()’ of the last object instantiated and ‘marshall()’ will call that particular ‘isr()’ for ANY of the pins attached by the ‘begin()’ functions.

Bottom line, there does not seem to be a good way to have instance-specific ISRs inside a class when you want multiple instances of that class.

So, why jump through the hoops of doing it inside a class just for one instance?

I was very excited to get up this morning and see the help.

I added the line:

FlowRate* FlowRate::anchor = NULL;

I got some errors but then realized I had been using the member name _anchor. So I changed the name a volia, it compiles, and runs!!! Yea!!!!

I had read that it was suggested to use the underscore for all private members. Is this convention not used much?

I'm going to go research and try to deepen my understanding of what this is doing. But I'd like to revisit some of gfvalvo's questions.

If

class FlowRate
{
  private:
    static FlowRate* _anchor;
};

declares the static member _anchor, why do I need

FlowRate* FlowRate::anchor = NULL;

in the cpp file why couldn't I have just put

_anchor=NULL;

in my constructor? Is it because there is only one of these _anchor's no matter how many instances of class FlowRate I have? Yes, I'm still down in the weeds, but thanks for all your guys help!!!

gfvalvo:
There is only one ‘marshall()’ and one ‘anchor’ (they are class (not instance) members). So, if you instantiate and ‘begin()’ multiple objects of this class type, ‘anchor’ will point to the ‘isr()’ of the last object instantiated and ‘marshall()’ will call that particular ‘isr()’ for ANY of the pins attached by the ‘begin()’ functions.

Bottom line, there does not seem to be a good way to have instance-specific ISRs inside a class when you want multiple instances of that class.

So, why jump through the hoops of doing it inside a class just for one instance?

It's good way to have the interrupt handled by an object and a non-static member function.

If you have more than on object, and one of them will be selected somehow to handle a common interrupt,
anchor could point to a linked list of objects and the marshall function could select the object.

mhessel:
... why do I need

FlowRate* FlowRate::anchor = NULL;

in the cpp file why couldn't I have just put

_anchor=NULL;

in my constructor? Is it because there is only one of these _anchor's no matter how many instances of class FlowRate I have? Yes, I'm still down in the weeds, but thanks for all your guys help!!!

To reserve the memory of the variable and initialize it.

That pointer is valid before any object exists that it points to, so initializinig it in a constructor is not an option.
Besides that, it would be initialized each time an object is created... and here be overwritten by this.

Whandall:
If you have more than on object, and one of them will be selected somehow to handle a common interrupt,
anchor could point to a linked list of objects and the marshall function could select the object.

Clever.

Here's a somewhat less-clever (more simple-minded) solution that I present for your consideration. It's not as encapsulated, but should get the job done.

class dog
{
  public:
    void dogIsr();
    void processDogIsrResults();

  private:
    volatile uint32_t interruptCount = 0;
    static const uint32_t doSomethingThreshold = 1000;
};

void dog::dogIsr() {
  interruptCount++;
}

void dog::processDogIsrResults() {
  uint32_t localCount;
  boolean doSomething = false;

  noInterrupts();
  localCount = interruptCount;
  if (localCount >= doSomethingThreshold) {
    interruptCount = 0;
    doSomething = true;
  }
  interrupts();
  if (doSomething) {
    // Do something profound with localCount
  }
}

dog dog1, dog2, dog3;

void setup() {
  pinMode(1, INPUT_PULLUP);
  pinMode(2, INPUT_PULLUP);
  pinMode(3, INPUT_PULLUP);
  attachInterrupt(digitalPinToInterrupt(1), isr1, RISING);
  attachInterrupt(digitalPinToInterrupt(2), isr2, RISING);
  attachInterrupt(digitalPinToInterrupt(3), isr3, RISING);
}

void loop() {
  dog1.processDogIsrResults();
  dog2.processDogIsrResults();
  dog3.processDogIsrResults();
}

void isr1() {
  dog1.dogIsr();
}

void isr2() {
  dog2.dogIsr();
}

void isr3() {
  dog3.dogIsr();
}

gfvalvo:
Bottom line, there does not seem to be a good way to have instance-specific ISRs inside a class when you want multiple instances of that class.

well... there is a (good) way to do this

// CLASS DEFINITION

class FanSpeed {
  
  public:
    void 
      setup(uint8_t irq_pin, void (*ISR_callback)(void), int value),
      handleInterrupt(void);
    double
      getSpeed(),
      getHertz();

  private:
    double
      _timeConstant = 60000000.0;
    uint32_t
      _lastMicros = 0UL,
      _interval = 60000000UL;
    void(*ISR_callback)();
};

void FanSpeed::setup(uint8_t irq_pin, void (*ISR_callback)(void), int value)
{
  attachInterrupt(digitalPinToInterrupt(irq_pin), ISR_callback, value);
}

inline void FanSpeed::handleInterrupt(void)
{
  uint32_t nowMicros = micros();
  _interval = nowMicros - _lastMicros;
  _lastMicros = nowMicros;
}

double FanSpeed::getSpeed()
{
  if (micros() - _lastMicros < 1000000UL) // has rotated in the last second
  {
    return _timeConstant / _interval;
  }
  else
  {
    return 0;
  }   
}

double FanSpeed::getHertz()
{
  if (micros() - _lastMicros < 1000000UL) // has rotated in the last second
  {
    return getSpeed() * 60.0;
  }
  else
  {
    return 0;
  }   
}

// PROGRAM START

FanSpeed* fan1;
uint8_t fan1pin = 2;

FanSpeed* fan2;
uint8_t fan2pin = 3;

void setup()
{
  Serial.begin(9600);
  pinMode(fan1pin, INPUT_PULLUP);
  fan1 = new FanSpeed();
  fan1->setup(fan1pin, []{fan1->handleInterrupt();}, FALLING);
  fan2 = new FanSpeed();
  fan2->setup(fan2pin, []{fan2->handleInterrupt();}, FALLING);
}

void loop()
{
  static uint32_t lastMillis = 0;
  if (millis() - lastMillis > 1000UL)
  {
    char message[64] = "";
    sprintf(message, "Fan1 Speed:%4d RPM\t%4d Hz", uint32_t(floor(fan1->getSpeed() + 0.5)), uint32_t(floor(fan1->getHertz() + 0.5)));
    Serial.println(message);
    sprintf(message, "Fan2 Speed:%4d RPM\t%4d Hz", uint32_t(floor(fan2->getSpeed() + 0.5)), uint32_t(floor(fan2->getHertz() + 0.5)));
    Serial.println(message);
    lastMillis = millis();
  }
}

@BulldogLowell,

If you would be so kind, please parse / explain the "[]{fan2->handleInterrupt();}" syntax.

Also, the follow change to your code will at least compile:

  //fan2->setup(fan2pin, []{fan2->handleInterrupt();}, FALLING);
  attachInterrupt(digitalPinToInterrupt(fan2pin),  (void (*)())(&fan2->handleInterrupt), FALLING);

But, will it work?

Thanks.

gfvalvo:
@BulldogLowell,

If you would be so kind, please parse / explain the "[]{fan2->handleInterrupt();}" syntax.

Also, the follow change to your code will at least compile:

  //fan2->setup(fan2pin, []{fan2->handleInterrupt();}, FALLING);

attachInterrupt(digitalPinToInterrupt(fan2pin),  (void (*)())(&fan2->handleInterrupt), FALLING);



But, will it work?

Thanks.
[]{fan2->handleInterrupt();}

an anonymous function or lambda which works in the attachInterrupt() function (macro).

likewise you could put a function (pointer) as long as it had the same function signature (i.e. takes no parameters and returns void)

For me the code compile without that bad change.

There are some warnings...

SomeWhere\CallMemberLambda\CallMemberLambda.ino: In function 'void loop()':

SomeWhere\CallMemberLambda\CallMemberLambda.ino:83:132: warning: format '%d' expects argument of type 'int', but argument 3 has type 'uint32_t {aka long unsigned int}' [-Wformat=]

     sprintf(message, "Fan1 Speed:%4d RPM\t%4d Hz", uint32_t(floor(fan1->getSpeed() + 0.5)), uint32_t(floor(fan1->getHertz() + 0.5)));

                                                                                                                                    ^
SomeWhere\CallMemberLambda\CallMemberLambda.ino:83:132: warning: format '%d' expects argument of type 'int', but argument 4 has type 'uint32_t {aka long unsigned int}' [-Wformat=]
SomeWhere\CallMemberLambda\CallMemberLambda.ino:85:132: warning: format '%d' expects argument of type 'int', but argument 3 has type 'uint32_t {aka long unsigned int}' [-Wformat=]

     sprintf(message, "Fan2 Speed:%4d RPM\t%4d Hz", uint32_t(floor(fan2->getSpeed() + 0.5)), uint32_t(floor(fan2->getHertz() + 0.5)));

                                                                                                                                    ^
SomeWhere\CallMemberLambda\CallMemberLambda.ino:85:132: warning: format '%d' expects argument of type 'int', but argument 4 has type 'uint32_t {aka long unsigned int}' [-Wformat=]

Whandall:
For me the code compile without that bad change.

Bulldog’s original code compiled for me too. I was just wondering if my use of casting:
(void (*)())(&fan2->handleInterrupt)
would function properly, not just compile without error.

There are some warnings...

The warnings depend on what processor I try to compile for. No warnings for Uno. Yes warning for Teensy. IDE 1.82.

Anyway, the warnings are just sprint() complaining. They’re unrelated to Bulldog’s technique for the interrupt attachment.

gfvalvo:
Bulldog’s original code compiled for me too. I was just wondering if my use of casting:
(void (*)())(&fan2->handleInterrupt)
would function properly, not just compile without error.
The warnings depend on what processor I try to compile for. No warnings for Uno. Yes warning for Teensy. IDE 1.82.

No, your method can not work.

I got the warnings for a Mega.

Whandall:
I got the warnings for a Mega.

sprintf() warnings, as pointed out

Still errors (on 8-bit Arduinos). The second %d should print the high part of the first.

Whandall:
Are there Arduino environments in which that matters?

Both variants compile (for a Mega and ESP32) without warnings.

The first notation will be at least legal for some years to come IMHO.

Might be, depends on the function you try to call.
have a look at Better types in C++11 - nullptr, enum classes (strongly typed enumerations) and cstdint - Cprogramming.com

And its part of C++11 so it will be valid for a long long time as well :slight_smile: