0

Thanks for all your feedback. It helped.

I rewrote this yet another time and this time it works perfectly. I included three variations: a simple example using an array of function pointers, a Lambda version and a complex version like what I started with.

#define TASK_COUNT          3

typedef void ( *taskPointer ) ( );

//  Simple Version:
void taskS1 () { Serial.println ( ( char* )"SIMPLE TASK 1" ); }
void taskS2 () { Serial.println ( ( char* )"SIMPLE TASK 2" ); }
void taskS3 () { Serial.println ( ( char* )"SIMPLE TASK 3" ); }

//  Array of pointers to tasks
taskPointer callSimpleTask [ ] = {
taskS1,
taskS2,
taskS3,
};

//  Run the simple test
void showSimpleVersion () {
    Serial.println ( F ( "\nSimple Version:" ) );
    for (uint8_t i = 0; i < TASK_COUNT; i++) {
        callSimpleTask [ i ] ();
    }

}

//  Lambda Version: No separate task definitoins
void ( *callLambdaTask [ ] ) ( ) = {
    [ ] {Serial.println ( ( char* )"LAMBDA TASK 1" ); },
    [ ] {Serial.println ( ( char* )"LAMBDA TASK 2" ); },
    [ ] {Serial.println ( ( char* )"LAMBDA TASK 3" ); },
};

//  Run the Lambda test
void showLambdaVersion () {
    Serial.println ( F ( "\nLambda Version:" ) );
    for (uint8_t i = 0; i < TASK_COUNT; i++) {
        callLambdaTask [ i ] ();
    }

}

//  Complex Version: Each array element is a struct containing a 
//  pointer to the corresponding task and related properties
struct complexTaskDefinition {
    taskPointer task;
    bool repeat;                //  Repeat flashing after delay
    int32_t iterations;         //  # times to repeat... -1 = forever
    uint32_t delay;             //  MS to wait before execution
};

//  Create a pointer to a task definition
complexTaskDefinition *task;

//  Prove that called function can call other functions:
void taskC1 () { showTaskC1(); }
void taskC2 () { showTaskC1 (); }
void taskC3 () { showTaskC1 (); }

void showTaskC1 () {
    Serial.println ( ( char* )"COMPLEX TASK 1" );
}
void showTaskC2 () {
    Serial.println ( ( char* )"COMPLEX TASK 2" );
}
void showTaskC3 () {
    Serial.println ( ( char* )"COMPLEX TASK 3" );
}

complexTaskDefinition tasks [ ] [4] = {
    {taskC1, true, -1, 1000 },  //  taskC1 repeats forever every second
    {taskC2, true, 10, 2000 },  //  taskC2 runs 10 times pausing for 2 seconds between iterations
    {taskC3, false, 3, 3000 },  //  taskC3 runs 3 times with 3 second delays between each, then dies
};

void showComplexTaskList () {
    Serial.println ( F ( "\nComplex Task List:" ) );
    for (uint8_t i = 0; i < TASK_COUNT; i++) {
        task = tasks [ i ];
        Serial.print ( '[' );
        Serial.print ( i );
        Serial.print ( F ( "] REPEAT:" ));
        Serial.print ( ( task->repeat ) ? "YES" : "NO" );
        Serial.print ( F ( ", DELAY:" ) );
        Serial.print ( task->delay );
        Serial.print ( F ( ", ITERATIONS:" ) );
        Serial.print ( task->iterations );
        Serial.println ();
    }
}

void showComplexVersion () {
    Serial.println ( F ( "\nComplex Version:" ) );
    for (uint8_t i = 0; i < TASK_COUNT; i++) {
        task = tasks [ i ];
        task->task ();
//      (( *tasks ) [ i ]).task ();
    }
}

void setup () {
    Serial.begin ( 115200 );
    while (!Serial.availableForWrite ()) {}

    showSimpleVersion ();

    showLambdaVersion ();

    showComplexVersion ();

    showComplexTaskList ();
}

void loop () {
}

Here is the output: Simple Version:

SIMPLE TASK 1
SIMPLE TASK 2
SIMPLE TASK 3

Lambda Version:
LAMBDA TASK 1
LAMBDA TASK 2
LAMBDA TASK 3

Complex Version:
COMPLEX TASK 1
COMPLEX TASK 1
COMPLEX TASK 1

Complex Task List:
[0] REPEAT:YES, DELAY:1000, ITERATIONS:-1
[1] REPEAT:YES, DELAY:2000, ITERATIONS:10
[2] REPEAT:NO, DELAY:3000, ITERATIONS:3

I want to run one more experiment, this time with the complex version as a class.

  • 1
    loop() and setup()? – Juraj Aug 27 '19 at 5:58
  • 1
    What compiler error do you get? – Edgar Bonet Aug 27 '19 at 6:49
  • Board? Arduino IDE version? It can be compiled wihout any warning/error for AVR based board. Btw: &task1, &task2, &task3 is more correct. – KIIV Aug 27 '19 at 7:18
  • @KIIV As from C99 the & is entirely optional for function pointers. Neither is "more correct" than the other, they are both completely correct. – Majenko Aug 27 '19 at 9:37
  • @majenko in C yes. In C++ there are also cases where it's required (like member function pointers). More correct was meant like "yes, i want function pointer, I didn't forget ()" - showing clear intention – KIIV Aug 27 '19 at 9:49
1

Try replacing

taskDefinition _tasks [ ] [4] = {

by

taskDefinition _tasks [ ] = {

In short, you have defined _tasks as a 2D array of objects (an array of arrays of objects) while you intend to use it as a 1D array.


Note that the error message is quite explicit if you take the time to parse it:

request for member 'task' in '_tasks[((int)i)]',

You are requesting _tasks[i].task,

which is of non-class type

but _tasks[i] is not an object.

type 'taskDefinition [4]'

It is instead an array of four taskDefinition objects.


Edit: Fixing the answer to match the latest version of the question.

Your code works, but not quite as you expect it to work. You wrote:

complexTaskDefinition tasks [ ] [4] = ...;

Here you are defining a 2D array of objects, while your intention was to define a one-dimensional array (why do I feel like I am repeating myself?).

complexTaskDefinition tasks [ ] [4] = {
    {taskC1, true, -1, 1000 },
    {taskC2, true, 10, 2000 },
    {taskC3, false, 3, 3000 },
};

This definition, although syntactically correct, is awfully wacky. You are defining tasks as an array of 3 items. Each item is itself an array of 4 objects, where the first object of the array is explicitly initialized, and the three others are implicitly initialized to all-bits-zero. So basically your definition is equivalent to this explicit version:

complexTaskDefinition tasks [3] [4] = {
    {{taskC1, true, -1, 1000 }, {nullptr, false, 0, 0},
     {nullptr, false, 0, 0},    {nullptr, false, 0, 0}},
    {{taskC2, true, 10, 2000 }, {nullptr, false, 0, 0},
     {nullptr, false, 0, 0},    {nullptr, false, 0, 0}},
    {{taskC3, false, 3, 3000 }, {nullptr, false, 0, 0},
     {nullptr, false, 0, 0},    {nullptr, false, 0, 0}}
};

Then you wrote:

task = tasks [ i ];

Here, tasks[i] is a row of the 2D array, i.e. a 1D array of 4 objects, whereas task is a pointer to object. The assignment is legit because here, like in most contexts, the array “decays” to a pointer to its first element. The above line is equivalent to the explicit version:

task = &tasks[i][0];

Quite fortunately, this happens to point to one of the elements of the array that has been explicitly initialized!

As an illustration of the above, here is a modified version of showComplexTaskList() that loops over all the tasks in the array:

void showComplexTaskList () {
    Serial.println ( F ( "\nComplex Task List:" ) );
    Serial.print(F("number of tasks: "));
    Serial.println(sizeof tasks / sizeof(complexTaskDefinition));
    for (uint8_t i = 0; i < TASK_COUNT; i++) {
        for (uint8_t j = 0; j < 4; j++) {
            task = &tasks[i][j];
            Serial.print ( '[' );
            Serial.print ( i );
            Serial.print ( F ("]["));
            Serial.print ( j );
            Serial.print ( F ( "] REPEAT:" ));
            Serial.print ( ( task->repeat ) ? "YES" : "NO" );
            Serial.print ( F ( ", DELAY:" ) );
            Serial.print ( task->delay );
            Serial.print ( F ( ", ITERATIONS:" ) );
            Serial.print ( task->iterations );
            Serial.println ();
        }
    }
}

Its output is:

Complex Task List:
number of tasks: 12
[0][0] REPEAT:YES, DELAY:1000, ITERATIONS:-1
[0][1] REPEAT:NO, DELAY:0, ITERATIONS:0
[0][2] REPEAT:NO, DELAY:0, ITERATIONS:0
[0][3] REPEAT:NO, DELAY:0, ITERATIONS:0
[1][0] REPEAT:YES, DELAY:2000, ITERATIONS:10
[1][1] REPEAT:NO, DELAY:0, ITERATIONS:0
[1][2] REPEAT:NO, DELAY:0, ITERATIONS:0
[1][3] REPEAT:NO, DELAY:0, ITERATIONS:0
[2][0] REPEAT:NO, DELAY:3000, ITERATIONS:3
[2][1] REPEAT:NO, DELAY:0, ITERATIONS:0
[2][2] REPEAT:NO, DELAY:0, ITERATIONS:0
[2][3] REPEAT:NO, DELAY:0, ITERATIONS:0

So your program works almost by chance: an error in the way you define the array cancels an error in the way you use it. This kind of cancelled error method is very bad programming practice. You are wasting memory in unused complexTaskDefinition objects. More importantly, your code doesn't express your intent, which makes the program way harder to understand and to debug.

The fix is to declare a 1D array, as this has always been the intention:

complexTaskDefinition tasks [ ] = {
    {taskC1, true, -1, 1000 },
    {taskC2, true, 10, 2000 },
    {taskC3, false, 3, 3000 },
};

Now, you can check that this array does only have three tasks, as reported by sizeof tasks / sizeof(complexTaskDefinition). To use the array, you can use a pointer if you want:

task = &tasks[i];
// ...
Serial.print ( ( task->repeat ) ? "YES" : "NO" );

Or you can directly request a member of tasks[i], as in

void showComplexVersion () {
    Serial.println ( F ( "\nComplex Version:" ) );
    for (uint8_t i = 0; i < TASK_COUNT; i++) {
        tasks[i].task();
    }
}

Now I have a question for you. Since I already explained your 1D vs. 2D array mistake in the first iteration of my answer, why did you persist with this in your edited question? If you are going to blatantly ignore the answers you get, at least say so in the question. This could save time to those who might consider helping you.

  • I appreciate the hard work your did to explain this to me and I am learning a log. I did intend the array to be a 2D array, but my problem was with defining the task to be called within that array. I found a good post that showed me how to do that wit a callback and that worked... when the parent is the .INO file. See this post: arduino.stackexchange.com/questions/68299/… – Bob Jones Aug 30 '19 at 22:33

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.