Recommended way to deal with queues and pointers to buffers
-
I am working with embedded programming in standard C and there are 2 microcontrollers communication with each other using a home-made protocol. The scheduler and task queues are also home-made so no realtime operating system is used. My tasks look like this:
void (*task_t)(uint8_t* taskData, uint16_t sizeOfTaskDataInBytes);
My add-task-to-queue-function looks like this:
Bool_t addTaskToQueue(queueSelector_e, task_t, uint8_t* taskData, uint16_t sizeOfTaskDataInBytes);
Please note that the taskData is also inserted into the queue (in an area called task data pool area) so it's not just a pointer to taskData that is being queued. The task data to write data from microcontroller 1 to microcontroller 2 needs to look something like this:
struct writeRegTaskData_s {
uint32_t startAddr;
uint16_t numRegsToWrite;
uint8_t dataToWrite[numRegsToWrite]; // When I want the data to be put inside the queue as well
or
uint8_t* dataToWrite // When I have a big static buffer that I don't want to put inside the queue
}The problem is with the dataToWrite parameter. Sometimes a lot of data will be written, for example, during a firmware upgrade, and then it's overkill to put all the dataToWrite into the queue, it's perfectly fine to have one static buffer. At other times, for example when reacting to different push button events, I want to put my dataToWrite on the stack and then add it into my task queue. However, when I do this, I must make sure dataToWrite points to the proper location inside my task queue data pool and not on the original buffer on the stack. What's the recommended way to solve this issue in a clean way?
-
I am working with embedded programming in standard C and there are 2 microcontrollers communication with each other using a home-made protocol. The scheduler and task queues are also home-made so no realtime operating system is used. My tasks look like this:
void (*task_t)(uint8_t* taskData, uint16_t sizeOfTaskDataInBytes);
My add-task-to-queue-function looks like this:
Bool_t addTaskToQueue(queueSelector_e, task_t, uint8_t* taskData, uint16_t sizeOfTaskDataInBytes);
Please note that the taskData is also inserted into the queue (in an area called task data pool area) so it's not just a pointer to taskData that is being queued. The task data to write data from microcontroller 1 to microcontroller 2 needs to look something like this:
struct writeRegTaskData_s {
uint32_t startAddr;
uint16_t numRegsToWrite;
uint8_t dataToWrite[numRegsToWrite]; // When I want the data to be put inside the queue as well
or
uint8_t* dataToWrite // When I have a big static buffer that I don't want to put inside the queue
}The problem is with the dataToWrite parameter. Sometimes a lot of data will be written, for example, during a firmware upgrade, and then it's overkill to put all the dataToWrite into the queue, it's perfectly fine to have one static buffer. At other times, for example when reacting to different push button events, I want to put my dataToWrite on the stack and then add it into my task queue. However, when I do this, I must make sure dataToWrite points to the proper location inside my task queue data pool and not on the original buffer on the stack. What's the recommended way to solve this issue in a clean way?
arnold_w wrote:
I must make sure dataToWrite points to the proper location inside my task queue data pool and not on the original buffer on the stack.
Then you need to allocate a block of memory from the pool, and copy the data there. I assume that you have a pool control mechanism that operates similar to malloc to manage the pool. In either case you pass the buffer pointer to the function that handles the data. You just need to make sure that pool buffers are released after they have been used.
-
arnold_w wrote:
I must make sure dataToWrite points to the proper location inside my task queue data pool and not on the original buffer on the stack.
Then you need to allocate a block of memory from the pool, and copy the data there. I assume that you have a pool control mechanism that operates similar to malloc to manage the pool. In either case you pass the buffer pointer to the function that handles the data. You just need to make sure that pool buffers are released after they have been used.
Yes, the task data pools (one for each task queue) are simple FIFO:s, but they're kept separate from the queues where I store the task function pointers. The only thing I can think of is to place the copied data immediately after the task data struct:
typedef enum {
USE_STATIC_NON_QUEUED_WRITE_DATA = 0,
DATA_TO_WRITE_APPEARS_AFTER_STRUCT = 1
} writeBufferLocationOption_e;struct writeRegTaskData_s {
uint32_t startAddr;
uint16_t numRegsToWrite;
writeBufferLocationOption_e writeBufferLocationOption;
uint8_t* dataToWrite // Set to NULL if writeBufferLocationOption = DATA_TO_WRITE_APPEARS_AFTER_STRUCT
// If writeBufferLocationOption = DATA_TO_WRITE_APPEARS_AFTER_STRUCT then the data to write will appear here
}Bool_t addTaskToQueue(queueSelector_e, task_t, uint8_t* taskData1, uint16_t sizeOfTaskDataInBytes1, uint8_t* taskData2, uint16_t sizeOfTaskDataInBytes2);
When I do want to queue the data, then I'd pass the data as the taskData2 parameter. Can someone think of a cleaner solution?
-
Yes, the task data pools (one for each task queue) are simple FIFO:s, but they're kept separate from the queues where I store the task function pointers. The only thing I can think of is to place the copied data immediately after the task data struct:
typedef enum {
USE_STATIC_NON_QUEUED_WRITE_DATA = 0,
DATA_TO_WRITE_APPEARS_AFTER_STRUCT = 1
} writeBufferLocationOption_e;struct writeRegTaskData_s {
uint32_t startAddr;
uint16_t numRegsToWrite;
writeBufferLocationOption_e writeBufferLocationOption;
uint8_t* dataToWrite // Set to NULL if writeBufferLocationOption = DATA_TO_WRITE_APPEARS_AFTER_STRUCT
// If writeBufferLocationOption = DATA_TO_WRITE_APPEARS_AFTER_STRUCT then the data to write will appear here
}Bool_t addTaskToQueue(queueSelector_e, task_t, uint8_t* taskData1, uint16_t sizeOfTaskDataInBytes1, uint8_t* taskData2, uint16_t sizeOfTaskDataInBytes2);
When I do want to queue the data, then I'd pass the data as the taskData2 parameter. Can someone think of a cleaner solution?
What about something like:
struct writeRegTaskData_s {
writeBufferLocationOption_e writeBufferLocationOption;
uint16_t numRegsToWrite;
uint8_t dataToWrite[1]; // start of actual data
}You can then create your fixed buffer with the above structure, followed immediately by the data space. For the dynamic buffers you just need to calculate the total space required (struct plus data), allocate it and fill in the struct values, and copy the data. You then just have a single pointer to pass to your data maniulation routines.
-
What about something like:
struct writeRegTaskData_s {
writeBufferLocationOption_e writeBufferLocationOption;
uint16_t numRegsToWrite;
uint8_t dataToWrite[1]; // start of actual data
}You can then create your fixed buffer with the above structure, followed immediately by the data space. For the dynamic buffers you just need to calculate the total space required (struct plus data), allocate it and fill in the struct values, and copy the data. You then just have a single pointer to pass to your data maniulation routines.
I don't think I can guarantee that the static non-queued buffers can be put in the end of the structs because most of the time the structs will be created on the stack. Therefore, I think I need pointers to the buffers. I also have a CAN-bus interface and typically the receive packet task would take a struct that looks like this:
struct packet_s {
uint32_t extID;
uint8_t DLC; // Data Length Count (=size of packet in bytes)
uint8_t* data;
};In my application these packets will always be queued (they never use static buffers) and as soon as they are queued the data pointer becomes worthless. The best thing I can think of is the following (a slight variation of Richard MacCutchan's suggestion):
#define PLEASE_SEE_DLC_FOR_PACKET_SIZE (1)
struct packet_s {
uint32_t extID;
uint8_t DLC; // Data Length Count (=size of packet in bytes)
uint8_t data[PLEASE_SEE_DLC_FOR_PACKET_SIZE];
}; -
I don't think I can guarantee that the static non-queued buffers can be put in the end of the structs because most of the time the structs will be created on the stack. Therefore, I think I need pointers to the buffers. I also have a CAN-bus interface and typically the receive packet task would take a struct that looks like this:
struct packet_s {
uint32_t extID;
uint8_t DLC; // Data Length Count (=size of packet in bytes)
uint8_t* data;
};In my application these packets will always be queued (they never use static buffers) and as soon as they are queued the data pointer becomes worthless. The best thing I can think of is the following (a slight variation of Richard MacCutchan's suggestion):
#define PLEASE_SEE_DLC_FOR_PACKET_SIZE (1)
struct packet_s {
uint32_t extID;
uint8_t DLC; // Data Length Count (=size of packet in bytes)
uint8_t data[PLEASE_SEE_DLC_FOR_PACKET_SIZE];
}; -
When you declare your static buffers you just add the space required for the struct at the beginning. Then you only need one pointer whether it is static or dynamic. Keep things simple as much as possible.
-
Good work..!!