Trying to point a completion function to a class function

I have a base class operating SPI with interrupt enabled - "A". I have two or more classes ("B", "C", etc.) derived from "A" to operate different peripherals. In the interrupt routine I want to call a completion function. The compiler does not object if the completion function is not a class function but does if it is. The error message seems to indicate that this is a casting problem: "error: argument of type 'void (LCDnokia12::)(uint8_t)' does not match 'void (*)(uint8_t)'"

The bares bones of the relevant code is:

class A
{
    ...
    public
    ...
    void (*completionFunction)(uint8_t purpose);    // Function pointer
    ...
    void func1(A *client);  // Function in class A
    ...
}

volatile A *Aptr;   // Pointer to the current client.

void A::func1(A *client)    // Called by client
{
    Aptr = client;
}

ISR(SPI_vect)
{
    Aptr->completionFunction(SPI_TRANSFER_DONE);    // Use completion function
}


class B : A 
{
    ...
    bCompletionFunction(uint8_t status);    // Forward declaration of actual completion function
    ...
}

B:B()   // B constructor
{
    completionFunction = bComletionFunction; // This is the problem!!!
}

void    B::someFunction()
{
    func1(this);    // Establish this as client.

void    B::bCompletionFunction(uint8_t status)  // Completion function
{

}

Any suggestions as to how to fix this problem?

The compiler does not object if the completion function is not a class function but does if it is.

For which instance of the class do you want to call the method? When you can define this, you can figure out how to proceed.

I don't see how you can call a class function from an ISR without some sort of "glue" function. After all, class functions need to be part of a class (that is, we need to know the "this" pointer).

Sorry Paul. I don't follow. The problem line is in the class B constructor which is called when I instantiate B. So the instance is "this". In fact there will be only one instance of B.

Nick, the global "Aptr" points to the particular instance in operation when the interrupt occurs. In the ISR, the completion function is prefaced by "Aptr->" to specify the current instance.

However, if I cannot solve the problem I am asking about, I will point the completion function variable to a non-class routine that can access a pointer to the instance of B which can then call the real completion function. I just cannot figure why that should be necessary.

In the B constructor, it now knows where the completion function is and should be able to place the pointer for access by the ISR. I just cannot figure out how to coerce the compiler to accept that this is legitimate.

Can you post test code that compiles (apart from the specific error)? I am juggling things around here (like deleting "...") to try to get to your error.

Right now I have:

class B : A 
{
    bCompletionFunction foo (uint8_t status);   // Forward declaration of actual completion function
};

giving:

sketch_jan27b:23: error: 'bCompletionFunction' does not name a type
sketch_jan27b:26: error: expected unqualified-id before ':' token
sketch_jan27b:31: error: no 'void B::someFunction()' member function declared in class 'B'

My apologies, Nick. That should have been:

class B : A 
{
    ...
    void   bCompletionFunction(uint8_t status);    // Forward declaration of actual completion function
    ...
}

Sorry Paul. I don't follow. The problem line is in the class B constructor which is called when I instantiate B. So the instance is "this". In fact there will be only one instance of B.

But, the compiler has no way of knowing that you only plan to create one instance of the class. If you want to specify a class member as a callback method, that member must be static.

Of course, the drawback to this is that the static member does not have access to any specific instance's non-static data (even if there is only one instance).

In fact there will be only one instance of B.

Then make all class members (methods and variables) static.

If you are only planning to have one of them you could make a namespace and not a class, eg. as I did recently (got the idea from someone else):

namespace ManchesterInterrupts {
  
  // constants
  
  // this it the bit rate (data rate is half that as it takes two transitions per data bit)
  const unsigned long PULSE_WIDTH = 500;  // uS
  const int PERCENTAGE_VARIANCE = 10;     // how much we tolerate the pulse width being out (max 50)
  
 // ...
  
  // variables
  
  // state machine
  extern byte manchesterState;
  
  // true if we got the first of the two bits per data bit
  extern boolean gotFirst;
  
 // ...
  
  // functions
  
  void isr ();
  int available ();
  int read ();

} // end of namespace ManchesterInterrupts

You still have the nice documentation of being able to refer to things like this:

if (ManchesterInterrupts::available ()) ...

The reason for preferring a class is that there are several SPI peripherals sharing a base class which handles the actual SPI. Also, I am quite familiar with classes.

What I have done that does compile is to add a static function to which the compiler is happy to make a pointer. The static function then references a private global which points to the instance to call the actual completion class function. Now I just have to see if I can get the whole thing to work!

Thanks for your suggestions and comments; they have been helpful.

Base class... child classes with different methods... virtual functions (i.e. polymorphism) ?

Something along these lines:

class A {
    public:
    A();
    
    virtual void completionFunction(uint8_t purpose);
};


class B : A {
    public:
    B();
    
    virtual void completionFunction(uint8_t purpose);
};


class C : A {
    public:
    C();
    
    virtual void completionFunction(uint8_t purpose);
};



A a;
B b;
C c;

A *aPtr;


// somehwere in setup() or loop()
// we'll have things like:
// aPtr = &a
// aPtr = &b
// aPtr = &c
// i.e. the current client will be selected by making the
// base class pointer point to either the base or child classes

ISR(SPI_vect) {
    // correct function is automatically selected
    // by C++ virtual methods mechanism
    aPtr->completionFunction(SPI_TRANSFER_DONE);
}

mromani,
The idea is along that line except the change in who is the current client happens at a much lower level.

Class B (ie., peripheral B) is transmitting. An event (eg., external interrupt) requests that class C begin transmission soon. Class C requests service but is denied because SPI is busy. However, C has a completion routine and is therefore entered into the queue. A short time later (measured in tens of microseconds) B’s transmission ends. B’s completion function is called with the argument “SPI_TRANSFER_DONE”. If B has been assembling more data to send and is ready, it will then request service but will be denied because there is at least one other in the queue. However, it will be added to the queue.

Then C’s completion code is called with the argument “SPI_NOW_AVAILABLE”. The pointer now selects class C, C begins transmission and control returns to whatever was interrupted when B’s last byte finished transmitting.

The whole point is to minimize delays between the time peripheral X requests service and when that service is actually available. The other consideration is to minimize the time spent servicing each interrupt so everything has to be ready to roll before a call for service is made.

I’ll post again when I have something actually working with multiple peripherals. I have been trying to get a handle on how much time some simple code actually takes and have been getting interesting results. For instance, shifting an integer 15 (i << 15) takes 107 cycles and (i<<j, where j is 15) takes 112; that’s 7 microseconds with a 16 MHz clock!

Pete

I understand the problem you’re trying to solve is quite complex and requires not only careful coding but probably som triky bits too.

However I was concentrating on the (IMHO) improper use of function pointers in an OO context. It seems to me you’re adopting a pure-C approach that ultimately mimics polymorphism which is done properly by C++ via the virtual methods mechanism (which in a way involves function pointers).

class A
{
    ...
    void (*completionFunction)(uint8_t purpose);	// Function pointer
    ...
}

class B : A 
{
    ...
    bCompletionFunction(uint8_t status);
    ....
}

B:B()	// B constructor
{
    completionFunction = bComletionFunction; // This is the problem!!!
}

The proper way to write this type of code is (IMHO):

class A
{
    ...
    virtual void completionFunction(uint8_t purpose) = 0;    // (1)
    ...
}

class B : A 
{
    ...
    virtual void completionFunction(uint8_t status);
    ....
}

B:B()	// B constructor
{
    // no need for this step, it's automatically done by C++
    //completionFunction = bComletionFunction; // This is the problem!!!
}

B::completionFunction(uint8_t status) {
    // B specific version of this function
}

(1) “= 0” means the function is just declared but not implemented in A. This turns A into an “abstract” class, that cannot be instantiated.
Every child class of A is expected to implement completionFunction.
If A can have its own version of completionFunction instead, you should omit the “= 0” part and just implement it.

Now let’s suppose we have B and C, which both have a their own particular implementation of completionFunction.
Let’s also suppose we have a pointer to base class A:

A *aPtr;

If we call completionFunction on aPtr

aPtr->completionFunction();

the virtual methdos mechanics will figure out at runtime which exact version of completionFunction has to be called, based on the actual type of the pointed object (which can be B, C or A (if it’s not an abstract class)).

(edit: all of this is a c++ problem, and has little to do with how you select which object aPtr points to, or how you queue requests)

To make sure all the bits were in place I put together a very simple but compiled and tested example.
A “logger” base class has two child classes that “log” messages on LCD and via Serial. The main loop “logs” millis() value on both classes via base class pointers.

I don’t want to decrease the signal/noise ratio of this thread too much, so the code is attached. :slight_smile:

VirtualMethodsTest.ino (1.94 KB)

I agree that the use of virtual functions overridden by a sub class makes good sense. Originally I had in my mind the possibility of using more than one completion function in a given class but have abandoned that.

Meanwhile I have a lot of software to debug and will post when all is working.

Many thanks.

Pete