Handle Interrupt within a class

Hi,

For my application I wrote a class (SerialTrigger) to convert a digital trigger to a serial signal and vice versa. Instead of polling at a fixed rate, I'm trying to attach an Interrupt to the incoming digital trigger. I want to deal with this inside the class rather than having to clutter up my main code (this would be the easy way).

I read here and here how to do it. At the moment I'm struggling getting the statics right so my code compiles. The error returned is "cannot call member without object". I tried defining the scope of the void (static void SerialTrigger::handleInterrupt), that did not help.

See the snippet of my code below. How should this be done?

Main

#include SerialTrigger.h
pinIn = 18;
pinOut = 19;

SerialTrigger(Serial3, pinIn, pinOut);

void setup() {
 // empty
}

void loop() {
 SerialTrigger.update();
}

SerialTrigger

class SerialTrigger() {
 SerialTrigger();
 void update();
 static void handleInterrupt();
 void sendSerial();
}

SerialTrigger::SerialTrigger(Serial3, pinIn, pinOut) {
 Serial3.begin();
 
 pinMode(pinIn, INPUT);
 attachInterrup(digitalPinToInterrupt(pinIn), handleInterrupt, RISING);
}

static void handleInterrupt() {
 SerialTrigger::sendSerial();
}

void SerialTrigger::sendSerial() {
 char out[9];

 strcpy(out, triggerString);
 strcat(out, "\r");

 Port.print(out);
}

First thing I notice, you're using stuff like pinMode() etc in the constructor. You can't do that. If you now make a global of your variable that is outside the main() (loop() or setup()) and there is no guarantee functions are working. Move stuff like that to a begin() method which you call inside setup().

But the biggest problem, you're never making an object of your class. You have to make an object of it before you can call functions on it. But you're class is SO specific, you probably only ever want one object of it otherwise you're having trouble. Two options here, don't make it a class! Just write functions. Or, make all methods static.

Thanks for the reply, septillion.

  1. Hmm, never though of that as it works well now. I read a bit on it and will change this.

  2. I'm making the object in my main code. That's why I wrote before, I could have the interrupt handled in my main code and call SerialTrigger.sendSerial() in the interrupt handler. But I want to keep it out of my main code.
    Yes, this class is very specific and at this point I'm only using one object of it. I can foresee another one being added in the future though, so I want to stick with the functionality of a class.

  1. depends on your luck with the compiler. It might place it where it's valid, but it might not. So better don't rely on that.

  2. No you're not. At least, yeah, you make an object:

SerialTrigger(Serial3, pinIn, pinOut);

but you never save a reference to it aka you can never use it again :wink:

Normal way of making an object:

SerialTrigger myObject = SerialTrigger(Serial3, pinIn, pinOut);

after which you can use "myObject".

But as I said, because a second object will just mess with the first, just:
a) just make them functions instead of object methods.
b) make a class with only static methods.

It is NEVER safe to use Serial print/write from with in an interrupt handler!

Mark

Ah, yeah, and that.

And I don't see triggerString declared either...

So, helpful feedback, thanks for that!

I did some more digging and found that handling interrupts within a class can be done, but it is cumbersome. I will switch it around handle the interrupt in my main code.

holmes4:
It is NEVER safe to use Serial print/write from with in an interrupt handler!

I was also skeptical of this. I will change it to a flag that is handled in the loop().

septillion:
And I don't see triggerString declared either...

It's not in the code snippet, but it's there :wink:

septillion:
Normal way of making an object:

SerialTrigger myObject = SerialTrigger(Serial3, pinIn, pinOut);

after which you can use "myObject".

Interesting, I have never seen this convention. Even C++ pages adhere to the convention of ClassName ObjectName;.

irPaul:
I will switch it around handle the interrupt in my main code.

But you're not handling them in you're class :wink: You let attachInterrupt() already handle it OUTSIDE your class :wink: So no need to change it.

irPaul:
It's not in the code snippet, but it's there :wink:

Did you read How to use the forum? NO snippets! Post all the code or MCVE.

irPaul:
[..] adhere to the convention of ClassName ObjectName;.

Uhm, that is what I do but you didn't.... You just did:

SerialTrigger(Serial3, pinIn, pinOut);

Where is the object name there?

septillion:

SerialTrigger(Serial3, pinIn, pinOut);

Where is the object name there?

That was a typo. My normal code uses:

SerialTrigger Trigger(Serial3, pinIn, pinOut);

septillion:
But you're not handling them in you're class :wink: You let attachInterrupt() already handle it OUTSIDE your class :wink: So no need to change it.

I modified it now that the attachInterrupt() refers to a static method that is in my main code. Within this method, I simply use Trigger.setFlag() to set a flag for the class to handle when it updates.

irPaul:
That was a typo. My normal code uses:

THE classic example why to post all the code or MCVE :wink:

irPaul:
I modified it now that the attachInterrupt() refers to a static method that is in my main code. Within this method, I simply use Trigger.setFlag() to set a flag for the class to handle when it updates.

Why? Absolute no need :wink:

irPaul:
I want to deal with this inside the class rather than having to clutter up my main code (this would be the easy way).

There are different ways of preventing cluttered main code. I would organise my code over different files. You can place the ISR and the update function in a file called SerialTrigger.cpp and add a file Serialtrigger.h with necessary supporting stuff; include the .h file in your main sketch.

You already call one function in loop() so you can keep it like that; that function will check the flag (set by the ISR) and, if needed, send data and clear the flag.

Interrupts are only needed if you need immediate action or if it really needs to be autonomous (e.g. flashing a led based on a timer without calling any function from your main code).

Below a simple C example.

SerialTrigger.cpp

#include <Arduino.h>

// function prototypes
void handleInterrupt();

// variables, only known here
static bool trigger = false;

void setupIntPin(byte pinNo)
{
  pinMode(pinNo, INPUT_PULLUP);
  attachInterrupt(digitalPinToInterrupt(pinNo), handleInterrupt, FALLING);
}

void handleInterrupt()
{
  trigger = true;
}

void sendSerial()
{
  if(trigger == true)
  {
    Serial.println("Hello world");
    trigger = false;
  }
}

SerialTrigger.h

void sendSerial();
void setupIntPin(byte pinNo);

sketch

#include "SerialTrigger.h"

void setup()
{
  Serial.begin(57600);
  setupIntPin(2);
}

void loop()
{
  sendSerial();
  // simple debounce
  delay(100);
}

If you have only one instance of your class - you are just using a class for encapsulation - an alternative is C++ namespaces.

septillion:
Normal way of making an object:

SerialTrigger myObject = SerialTrigger(Serial3, pinIn, pinOut);

after which you can use "myObject".

While that works, I personally dislike it from a efficiency and stylistic point of view. I believe that it unnecessarily calls the constructor multiple times: first to create an anonymous object, then to copy it into the named object. Then, the destructor is called on the anonymous object.

Anyway, I’m sure I’ll be quickly corrected if I’m wrong on this. Always happy to learn something new.

My preference for a static object is:

myClass myObject(constructor arguments);

for a dynamic object:

myClass *myObjectPtr = new myClass(constructor arguments);

gfvalvo:
While that works, I personally dislike it from a efficiency and stylistic point of view. I believe that it unnecessarily calls the constructor multiple times: first to create an anonymous object, then to copy it into the named object. Then, the destructor is called on the anonymous object.

I just tested the following form in test code:

SerialTrigger myObject = SerialTrigger(Serial3, pinIn, pinOut);

The constructor was called only once. I'm not a C++ language lawyer, but I suspect that's because it's used in an initializer context rather than in an assignment statement.

If you separated the declaration and assignment like this:

SerialTrigger myObject;
myObject = SerialTrigger(Serial3, pinIn, pinOut);

It's no longer an initializer, and the constructor is called twice.

christop:
I'm not a C++ language lawyer...

Yea, me too neither.

I thought I read it somewhere. Maybe it's only in certain circumstances, like if the class also defines a copy constructor?

Doesn't really matter. I'll drop my first objection and stick with the second -- I just don't like it stylistically. Extra verbiage for no extra value.

Yeah, I'm also not a big fan of:

SerialTrigger myObject = SerialTrigger(Serial3, pinIn, pinOut);

But it's the most clear way of defining, declaring and initialization. And luckily the compiler notices it's an all in one statement and just does it once :slight_smile:

septillion:
Yeah, I'm also not a big fan of:

SerialTrigger myObject = SerialTrigger(Serial3, pinIn, pinOut);

But it's the most clear way of defining, declaring and initialization. And luckily the compiler notices it's an all in one statement and just does it once :slight_smile:

I don't find the needless repetition of the classname clearer, nor do I like to rely on the compiler to
mutate the code I have written to the more logical and efficient construction in place.

I will stick with the

SerialTrigger myObject(Serial3, pinIn, pinOut);