Serial.print different behavior before/after function pointers assignments

Projects Programming
1

Hi,

I'm trying to run my multithreading code. During my initial run testing I encountered program problems.

Also I have to check that the addresses of the function pointers aren't affected of assignment in the .ino sketch or in the library.

But for now, the problem I want to know is that the content of the function arguments pointer doesn't hold up after the assignment of function pointers, but before the assignment it prints ok.

This is my sketch:

#include "glcd_spi.h"

////////////////////////////////////////////////////////////////////////////////////////
// enumerations
typedef enum: uint8_t {NOT_FINISHED,FINISHED,DONE}STATE;

// variables
typedef void (*f_ptr)(void *args);

// structs
typedef struct __attribute__((__packed__)){
  uint8_t tsk_cnts, tsk_cntr;
  uint8_t *tsk_args;
  STATE thrd_st;
  STATE *tsk_st;
  f_ptr *fptr;
}THREAD_t;

// img example
const unsigned char heart [] PROGMEM = {
  0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
  0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
};

// threads
THREAD_t LCD_THRD;

void setup() {
  Serial.begin(9600);

  // thread tasks
  LCD_THRD.tsk_cnts = 4;

  // thread tasks function pointers
  LCD_THRD.fptr[LCD_THRD.tsk_cnts];

  // thread functions arguments
  uint8_t lcd_args[LCD_THRD.tsk_cnts] = {70,10,12,heart};
  LCD_THRD.tsk_args = lcd_args;

  // thread functions state flags
  extern STATE lcd_st_flag;
  LCD_THRD.tsk_st = &lcd_st_flag; 

  // here is no problem, index 0 works
  Serial.println(LCD_THRD.tsk_args[0]);
  Serial.println(LCD_THRD.tsk_args[1]);

  // thread functions pointers
  LCD_THRD.fptr[0] = glcd_init;
  LCD_THRD.fptr[1] = glcd_graphics_mode;
  LCD_THRD.fptr[2] = glcd_clr;
  LCD_THRD.fptr[3] = glcd_img;

  // here is the problem, index 0 doesn't work
  Serial.println(LCD_THRD.tsk_args[0]);
  Serial.println(LCD_THRD.tsk_args[1]);
  

}  

void loop() {

}
2

Did you think you were declaring an array here?

  LCD_THRD.fptr[LCD_THRD.tsk_cnts];

That is not declaring an array.

Also, this:

  uint8_t lcd_args[LCD_THRD.tsk_cnts] = {70,10,12,heart};

Includes 'heart' which is a pointer to an array.

3

ToddL1962:
Did you think you were declaring an array here?

  LCD_THRD.fptr[LCD_THRD.tsk_cnts];

That is not declaring an array.

I thought I'm initializing the array. OK, got your point ! It's already initialized, I just have to use it directly.

Also, this:

  uint8_t lcd_args[LCD_THRD.tsk_cnts] = {70,10,12,heart};

Includes 'heart' which is a pointer to an array.

OK, what should I do at this point ?
I want to put all the args in one array to use it in one go.
How about declaring the array as void * ? which takes anything.

4

wolfrose:
I thought I'm initializing the array. OK, got your point ! It's already initialized, I just have to use it directly.OK, what should I do at this point ?
I want to put all the args in one array to use it in one go.
How about declaring the array as void * ? which takes anything.

fptr is not an array. It is a single pointer to a function which takes a single argument of type 'void *'
If your tsk_cnts is 4 and you are expecting to then have 4 pointers to functions, then you need one more level of indirection. You would then malloc() a chunk of memory large enough to hold 4 fptr types and then you can initialize the array.

5

blh64:
fptr is not an array. It is a single pointer to a function which takes a single argument of type 'void *'
If your tsk_cnts is 4 and you are expecting to then have 4 pointers to functions, then you need one more level of indirection. You would then malloc() a chunk of memory large enough to hold 4 fptr types and then you can initialize the array.

How about declaring the function pointer like this way:

typedef void (**f_ptr)(void *args);

Or as you suggested using malloc, but how about calloc ? I learned that it's same as malloc but calloc initialize space to zero.

6

OK, got it:

I used this and it worked :slight_smile:

LCD_THRD.fptr = calloc(4,sizeof(f_ptr));

Thanks,

7 
  // thread functions arguments
  uint8_t lcd_args[LCD_THRD.tsk_cnts] = {70,10,12,heart};
  LCD_THRD.tsk_args = lcd_args;

You are creating a local variable, and then pointing a pointer to it. Once setup() exits, that local variable no longer exists and the pointer is no longer valid. After you create the local, allocate enough memory to contain it and copy the data to the allocated memory. Something like:

  // thread functions arguments
  uint8_t lcd_args[LCD_THRD.tsk_cnts] = {70,10,12,heart};
  LCD_THRD.tsk_args = malloc(sizeof lcd_args);
  memcpy(*LCD_THRD.tsk_args, lcd_args, sizeof lcd_args);
8

johnwasser:

  // thread functions arguments

uint8_t lcd_args[LCD_THRD.tsk_cnts] = {70,10,12,heart};
 LCD_THRD.tsk_args = lcd_args;



You are creating a local variable, and then pointing a pointer to it. Once setup() exits, that local variable no longer exists and the pointer is no longer valid. After you create the local, allocate enough memory to contain it and copy the data to the allocated memory. Something like:

// thread functions arguments
 uint8_t lcd_args[LCD_THRD.tsk_cnts] = {70,10,12,heart};
 LCD_THRD.tsk_args = malloc(sizeof lcd_args);
 memcpy(*LCD_THRD.tsk_args, lcd_args, sizeof lcd_args);

OK, thanks for the help. I've 3 questions:

  1. What's better, put the array in global region or use the method you suggested ? or both are the same ?

  2. I have problems including "heart" inside the array since it's array of integers. Is there a way to put them in one container ? maybe a struct or union.

  3. Why not to use calloc ?

9

I did this:

  struct args {uint8_t arg1, arg2, arg3; const unsigned char*pxl;};
  struct args lcd_args = {0,1,0,PIC3};
  LCD_THRD.tsk_args = malloc(sizeof lcd_args);
  memcpy(*LCD_THRD.tsk_args, lcd_args, sizeof lcd_args);

And got this error:

weather_station:36:55: error: cannot convert 'setup()::args' to 'const void*' for argument '2' to 'void* memcpy(void*,     const void*, size_t)'

memcpy(*LCD_THRD.tsk_args, lcd_args, sizeof lcd_args);

cannot convert 'setup()::args' to 'const void*' for argument '2' to 'void* memcpy(void*, const void*, size_t)'
10

wolfrose:

  1. What's better, put the array in global region or use the method you suggested ? or both are the same ?

The disadvantage of making the array global is that the size must be a compile-time constant. The advantage is that the compiler can tell you if you don't have enough memory for the array.

wolfrose:
2. I have problems including "heart" inside the array since it's array of integers. Is there a way to put them in one container ? maybe a struct or union.

Your array of arguments is an array of single bytes. Since neither the "heart" array not a pointer to it will fit in a byte, you can't use it as an argument. One work-around would be to make your argument array 16-bit integers. Then you could store the address of "heart".

wolfrose:
3. Why not to use calloc?

If the only advantage is that the allocated memory is initialized to zero then the fact that the array is immediately filled with data makes that no advantage.

11

johnwasser:
The disadvantage of making the array global is that the size must be a compile-time constant. The advantage is that the compiler can tell you if you don't have enough memory for the array.

I really like your answers, to me this clears things that I was having partially understanding of a global and local variables.
This is really brings to me the core decision of whether to choose global declaration or local; either way, there has to be memory usage.
The global is of course constant in memory, but I think with local I have to use free( ) and I can use dynamic sizing.

Your array of arguments is an array of single bytes. Since neither the "heart" array not a pointer to it will fit in a byte, you can't use it as an argument. One work-around would be to make your argument array 16-bit integers. Then you could store the address of "heart".

That's absolutely a clever solution. Thanks for this tip.
I'm working on a dynamic struct.
I declared this in header file:

typedef struct __attribute__((__packed__)) fptr_args{
    uint8_t arg_n;       // number of arguments
    fptr_args *arg_list; // pointer to list of argument
}fptr_args;

Then used it in .ino file:

  // arguments
  fptr_args lcd_args;
  lcd_args.arg_n = 4;
  lcd_args.arg_list = malloc(3*sizeof(uint8_t)+1*sizeof(uint8_t*));

  lcd_args.arg_list = {0,1,0,PIC3};       // this issued an error

But when I tried to define the pointer to struct, I got the error.

Is there a way to get this to work ?

If the only advantage is that the allocated memory is initialized to zero then the fact that the array is immediately filled with data makes that no advantage.

Yep, I agree with you, because I read that calloc require more compile time than malloc.

12

OK, I found a work around:

  const unsigned char heart [] PROGMEM = {0xff, 0xff, 0xff };

  lcd_args.arg_n = 4;
  lcd_args.arg_list = malloc(3*sizeof(uint8_t)+1*sizeof(uint8_t*));

  *(uint8_t*)(lcd_args.arg_list+0) = 20;
  *(uint8_t*)(lcd_args.arg_list+1) = 90;
  *(uint8_t*)(lcd_args.arg_list+2) = 80;
  *(uint8_t*)(lcd_args.arg_list+3) = heart;  // get error for this one

It works but in the desktop compiler, the compiler + run takes some time, I don't know if this is heavy on Arduino nano.

So I get an error for the last one which is why I've done all this, is to assign an array to index 3 but I get error.

What should I do ?

13

This compiles without error for me:

////////////////////////////////////////////////////////////////////////////////////////
// enumerations
typedef enum : uint8_t {NOT_FINISHED, FINISHED, DONE} STATE;


// variable types
typedef void (*f_ptr)(void *args);


// structs
struct Task
{
  f_ptr fPtr;
  STATE state;
  void *arg;
};


struct Thread
{
  uint8_t taskCount;
  uint8_t taskIndex;
  STATE state;
  Task *tasks;
};


// img example
const unsigned char heart[] PROGMEM =
{
  0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
  0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
};


// threads
Thread LCDThread;


void setup()
{
  Serial.begin(9600);


  // thread tasks
  LCDThread.taskCount = 4;
  LCDThread.tasks = (Task *) calloc(LCDThread.taskCount, sizeof (Task));


  static uint8_t ARG0 = 70;
  LCDThread.tasks[0].fPtr = glcd_init;
  LCDThread.tasks[0].arg = (void *) &ARG0;


  static uint8_t ARG1 = 00;
  LCDThread.tasks[1].fPtr = glcd_graphics_mode;
  LCDThread.tasks[1].arg = (void *) &ARG1;


  static uint8_t ARG2 = 12;
  LCDThread.tasks[2].fPtr = glcd_clr;
  LCDThread.tasks[2].arg = (void *) &ARG2;


  LCDThread.tasks[3].fPtr = glcd_img;
  LCDThread.tasks[3].arg = (void *)heart;
}


void loop() {}


void glcd_init(void *args) {};
void glcd_graphics_mode(void *args) {};
void glcd_clr(void *args) {};
void glcd_img(void *args) {};
14

johnwasser:
This compiles without error for me:

Thank you so much :slight_smile:
I also like your reformulation for what I want to do.

15

Hi,

I want to thank you again, I got your idea of doing 2 structs.

I now have more flexible way of managing the thread and its related tasks.

header.h

//////////////////////////////////////////////////////////////////////////////////
// enumerations
typedef enum {NOT_FINISHED, FINISHED, DONE}STATE;

// variables
typedef void (*f_ptr)(void *arg1,void *arg2,void *arg3,void *arg4);
typedef void args;

// structs
typedef struct __attribute__((__packed__)) TASK{
	STATE   tsk_st;                 // 1 flag b/c only 1 run at a time
    f_ptr   tsk_fptr;
    uint8_t args_cnts;
	args    *tsk_args;
}TASK;

typedef struct __attribute__((__packed__)) THREAD{
	uint8_t tsk_cnts, tsk_cntr;     // task information
	STATE   thrd_st;                // thread flag states
	TASK	*tsk;					// task member
}THREAD;

main.c

THREAD thrd2;

const unsigned char PIC1[] = {3, 8, 7, 1, 2};

uint8_t ret_val;

int main(){

    thrd2.thrd_st = NOT_FINISHED;
    thrd2.tsk_cnts = 5;
    thrd2.tsk_cntr = 0;

    thrd2.tsk = malloc(5*sizeof(TASK));

    thrd2.tsk[0].tsk_fptr = (void(*)())function0; // no args
    thrd2.tsk[0].args_cnts = 1;
    thrd2.tsk[0].tsk_st = 0;

    thrd2.tsk[1].tsk_fptr = (void(*)())function1; // 1 arg
    thrd2.tsk[1].args_cnts = 0;
    thrd2.tsk[1].tsk_st = 0;


    thrd2.tsk[2].tsk_fptr = (void(*)())function2; // 3 args
    thrd2.tsk[2].args_cnts = 1;
    thrd2.tsk[2].tsk_st = 0;

    thrd2.tsk[3].tsk_fptr = (void(*)())function3; // 1 arg 1 return
    thrd2.tsk[3].args_cnts = 1;
    thrd2.tsk[3].tsk_st = 0;

    ((void(*)())thrd2.tsk[0].tsk_fptr)(0);              // 0
    ((void(*)())thrd2.tsk[1].tsk_fptr)(1);              // 1
    ((void(*)())thrd2.tsk[2].tsk_fptr)(9,8,&PIC1);      // 3
    ret_val = ((uint8_t(*)())thrd2.tsk[3].tsk_fptr)(7); // 1 arg 1 ret

    printf("\n................................\nret_val = %d\n",
           ret_val);



    return 0;
}
16

Hi,

I've done this example on desktop compiler, it worked but don't you think it's a lot of work casting everything ?

#include <stdio.h>
#include <stdlib.h>
#include <stdint.h>
#include "task_manager.h"

THREAD thrd2;

const unsigned char PIC1[] = {3, 8, 7, 1, 2};

int main(){

    ///////////////////////////////////////////////////////////////////////////
    // thread config
    thrd2.thrd_st = NOT_FINISHED;
    thrd2.tsk_cnts = 5;
    thrd2.tsk_cntr = 0;

    // tasks memory allocation
    thrd2.tsk = malloc(5*sizeof(TASK));

    ///////////////////////////////////////////////////////////////////////////
    // #0: task0 setting    --    no args
    thrd2.tsk[0].tsk_fptr = (void(*)())function0;
    thrd2.tsk[0].args_cnts = 1;
    thrd2.tsk[0].tsk_st = 0;
    thrd2.tsk[0].tsk_args = NULL;

    ///////////////////////////////////////////////////////////////////////////
    // #1: task1 setting    --    1 arg
    thrd2.tsk[1].tsk_fptr = (void(*)())function1;
    thrd2.tsk[1].args_cnts = 0;
    thrd2.tsk[1].tsk_st = 0;

    thrd2.tsk[1].tsk_args = malloc(1*sizeof(uint8_t));
    *((uint8_t *)thrd2.tsk[1].tsk_args+0) = (uint8_t)9;


    ///////////////////////////////////////////////////////////////////////////
    // #2: task2 setting    --    3 args
    thrd2.tsk[2].tsk_fptr = (void(*)())function2;
    thrd2.tsk[2].args_cnts = 1;
    thrd2.tsk[2].tsk_st = 0;

    thrd2.tsk[2].tsk_args = malloc(2*sizeof(uint8_t)+1*sizeof(unsigned char*));

    *((uint8_t *)thrd2.tsk[2].tsk_args+0)  = (uint8_t)9;
    *((uint8_t *)thrd2.tsk[2].tsk_args+1)  = (uint8_t)3;
    *((uint8_t **)thrd2.tsk[2].tsk_args+2) = (unsigned char*)&PIC1;


    ///////////////////////////////////////////////////////////////////////////
    // #3: task3 setting    --    1 arg 1 return
    thrd2.tsk[3].tsk_fptr = (void(*)())function3;
    thrd2.tsk[3].args_cnts = 1;
    thrd2.tsk[3].tsk_st = 0;

    thrd2.tsk[3].tsk_args = malloc(2*sizeof(uint8_t)+
                                   1*sizeof(unsigned char*));
    *((uint8_t *)thrd2.tsk[3].tsk_args+0) = (uint8_t)15;

    ///////////////////////////////////////////////////////////////////////////
    // calling functions

    // task#0: 0 args
    ((void(*)())thrd2.tsk[0].tsk_fptr)(0);

    // task#1: 1 args
    ((void(*)())thrd2.tsk[1].tsk_fptr)(1);

    // task#2: 3 args
    ((void(*)())thrd2.tsk[2].tsk_fptr)(*((uint8_t *)thrd2.tsk[2].tsk_args+0),
                                       *((uint8_t *)thrd2.tsk[2].tsk_args+1),
                                       *((uint8_t **)thrd2.tsk[2].tsk_args+2));

    // task#3: 1 arg 1 ret
    *((uint8_t *)thrd2.tsk[3].tsk_args+1) =
    ((uint8_t(*)())thrd2.tsk[3].tsk_fptr)(*((uint8_t *)thrd2.tsk[3].tsk_args+0));

    printf("\n................................\nret_val = %d\n",
           *((uint8_t *)thrd2.tsk[3].tsk_args+1));

    return 0;
}
17

wolfrose:
don't you think it's a lot of work casting everything?

I would just wrap each function in a wrapper that takes no arguments. The wrapper function could then pass whatever arguments you want to the function.

void function0w(void) {function0();}. // Not really needed. Could just call function0() directly.
void function1w(void) {function1(9);}
void function2w(void) {function2(9, 3, &PIC1);}  // PIC1 is global
void function3w(void) {RetValByte = function3();} // RetValByte is global
18

Hi,

I have a problem I've been trying to fix it for days now.

The best thread manager structs until now are the ones you suggested and I've been working on that. But now I'm having some problems assigning array address to void pointer then trying to dereference it.

These are my structs in header:

//////////////////////////////////////////////////////////////////////////////////
// enumerations
typedef enum {NOT_FINISHED, FINISHED, DONE}STATE;

// variables
typedef void (*f_ptr)();
typedef void args;

// structs
typedef struct __attribute__((__packed__)) TASK{
	STATE   tsk_st;                 // 1 flag b/c only 1 run at a time
    f_ptr   tsk_fptr;
    uint8_t args_cnts;
	args    *tsk_args;
}TASK;

typedef struct __attribute__((__packed__)) THREAD{
	uint8_t tsk_cnts, tsk_cntr;     // task information
	STATE   thrd_st;                // thread flag states
	TASK	*tsk;					// task member
}THREAD;

In source --> desktop compiler:

int main(){
    const uint8_t array[] = {3, 8, 7, 1, 2};

    // allocate & initialize thread object using a pointer to struct
    THREAD *th0 = malloc(1*sizeof(THREAD));
    
    // ... doing rest of initializing the struct elements
    
    th0->tsk[0].tsk_args = malloc(2*sizeof(uint8_t)+1*sizeof(uint8_t*));   // allocating 2 bytes and 1 pointer

    *((uint8_t*)th0->tsk[0].tsk_args+0) = 9;                     // passing value for 1st member
    *((uint8_t*)th0->tsk[0].tsk_args+1) = 10;                    // passing value for 1st member
    *(uint8_t**)(th0->tsk[0].tsk_args+2) = &array;               // passing array adress to pointer <<-- problem here

    printf("%d\n",*((uint8_t*)th0->tsk[0].tsk_args+0));
    printf("%d\n",*((uint8_t*)th0->tsk[0].tsk_args+1));

    printf("%d\n",*((uint8_t**)th0->tsk[0].tsk_args+2)[0]);      // can't print this one
    printf("%d\n",*((uint8_t**)th0->tsk[0].tsk_args+2)[1]);      // also this one

    return 0;
}
19

OK, got the source of the problem:

I have to dereference like this:

    printf("%d\n",*(*((uint8_t**)th0->tsk[0]->tsk_args+2)+0));
    printf("%d\n",*(*((uint8_t**)th0->tsk[0]->tsk_args+2)+1));

but still one problem which is I have to index tak as a pointer and not as an array.

20

if I allocate 4 tasks and TASK is actually a pointer, how to dereference the middle element:

    // allocate & initialize 4 tasks
    th0->tsk = malloc(4*sizeof(TASK));

    // allocate & initialize task0 args
    th0->tsk+0->args_cnts = 0;
    th0->tsk+0->tsk_st = 0;
    th0->tsk+0->tsk_fptr = (void(*)())function0;

    th0->tsk+0->tsk_args = malloc(2*sizeof(uint8_t)+1*sizeof(uint8_t*));

it used to be:

    // allocate & initialize 4 tasks
    th0->tsk = malloc(4*sizeof(TASK));

    // allocate & initialize task0 args
    th0->tsk[0].args_cnts = 0;
    th0->tsk[0].tsk_st = 0;
    th0->tsk[0].tsk_fptr = (void(*)())function0;

    th0->tsk[0].tsk_args = malloc(2*sizeof(uint8_t)+1*sizeof(uint8_t*));

but indexing it as array caused later problems in #18