Go Down

Topic: Using an Interrupt from a method inside a class (Read 6461 times) previous topic - next topic

mhessel

Oct 19, 2017, 08:00 am Last Edit: Oct 19, 2017, 08:28 am by mhessel
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:

Code: [Select]
#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:

Code: [Select]
#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:

Code: [Select]
#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!!!!
They should just call this Fortran 2000!

Whandall

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
Code: [Select]
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
Code: [Select]
attachInterrupt(irqNum, FlowRate::marshall, CHANGE);
How would you specify a static member function?

About the :: notation you should consult the C++ documentation.
Ah, this is obviously some strange usage of the word 'safe' that I wasn't previously aware of. (D.Adams)

septillion

In your case the line should read
Code: [Select]
FlowRate* FlowRate::anchor = NULL;
Or type safe
Code: [Select]
FlowRate* FlowRate::anchor = nullptr;
Use fricking code tags!!!!
I want x => I would like x, I need help => I would like help, Need fast => Go and pay someone to do the job...

NEW Library to make fading leds a piece of cake
https://github.com/septillion-git/FadeLed

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.
Ah, this is obviously some strange usage of the word 'safe' that I wasn't previously aware of. (D.Adams)

gfvalvo

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.

Code: [Select]
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:
Code: [Select]
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:
Code: [Select]
extern FlowRate* anchor;
The storage space still needs to be allocated somewhere.

Contrast this with:
Code: [Select]
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.
No technical questions via PM. They will be ignored. Post your questions in the forum so that all may learn.

Whandall

#5
Oct 19, 2017, 03:10 pm Last Edit: Oct 19, 2017, 03:12 pm by Whandall
At least it's part of my reality.
Ah, this is obviously some strange usage of the word 'safe' that I wasn't previously aware of. (D.Adams)

gfvalvo

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?
No technical questions via PM. They will be ignored. Post your questions in the forum so that all may learn.

mhessel

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

I added the line:

Code: [Select]
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

Code: [Select]
class FlowRate
{
  private:
    static FlowRate* _anchor;
};


declares the static member _anchor, why do I need

Code: [Select]
FlowRate* FlowRate::anchor = NULL;

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

Code: [Select]
_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!!!
They should just call this Fortran 2000!

Whandall

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.
Ah, this is obviously some strange usage of the word 'safe' that I wasn't previously aware of. (D.Adams)

Whandall

... why do I need

Code: [Select]
FlowRate* FlowRate::anchor = NULL;

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

Code: [Select]
_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.
Ah, this is obviously some strange usage of the word 'safe' that I wasn't previously aware of. (D.Adams)

gfvalvo

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.
Code: [Select]
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();
}
No technical questions via PM. They will be ignored. Post your questions in the forum so that all may learn.

BulldogLowell

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

Code: [Select]

// 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();
  }
}



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:
Code: [Select]

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

But, will it work?

Thanks.
No technical questions via PM. They will be ignored. Post your questions in the forum so that all may learn.

BulldogLowell

@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:
Code: [Select]

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

But, will it work?

Thanks.
Code: [Select]
[]{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)






Whandall

For me the code compile without that bad change.

There are some warnings...

Code: [Select]
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=]
Ah, this is obviously some strange usage of the word 'safe' that I wasn't previously aware of. (D.Adams)

Go Up