Missing data when passing array by-reference using xQueueSend()/xQueueReceive()

system · April 17, 2018, 6:45pm

nicholasd wrote on Tuesday, April 17, 2018:

I’ve been experiencing difficulties in passing arrays via message queues using FreeRTOS. I am using Freertos v9.0, and my target device is the STM32L053 MCU (Cortex-M0+).

I have two threads:

vKeypad() scans a keypad and sends a queue to vRadio that contains the corresponding keypad message. This thread runs periodically every ~100ms.
vRadio() waits to receive queue contents, and then transmits the received message using a packet radio.

My application previously used the queue to pass a pointer to a 4-byte array to vRadio. This worked fine and data could be written by vKeypad() and read by vRadio() correctly without aparent issue. Recently my requirements changed, and I now need to use a 5-byte array instead of a 4-byte array, and the queue now seems to only pass the first four bytes properly, but the 5th byte is always 0 no matter what I do.

In my code below, I write a value of 55 to queueTX[4], the 5th element of the array whose pointer will be passed to the consumer thread vRadio. So I send queueTX= {11, 22, 33, 44, 55} to the array in vKeypad, and when vRadio receives the queue, queueRX contains {11, 22, 33, 44, 0}.

I’ve posted simplified pieces relevant source code below:

I initialize the queue as follows:

xRadioMessageQueue = xQueueCreate(1, sizeof(uint8_t *));

vKeypad thread (queue producer):

void vPortExpanderTask(void const * argument) {

    uint16_t buttonState = 0x0000;

    uint8_t queueTX[5] = {};

    while(1) {
        buttonState = readPortExpander(&hi2c1, PORT_EXP_ADDRESS);

        // If a button has been pressed
        if (buttonState) {
            // Mock payload...
            queueTX[0] = 11;
            queueTX[1] = 22;
            queueTX[2] = 33;
            queueTX[3] = 44;
            queueTX[4] = 55;

            // Push data to xRadioMessageQueue
            if ( xQueueSendToBack(xRadioMessageQueue, &queueTX, 10) != pdPASS ) {
                // Queue full
            } else {
                // Queue write successful
            }
        }

        vTaskDelay(100);
    }
}

vRadio thread (queue consumer):

void vRadio(void const * argument) {  
  
  uint8_t queueRX[5] = {}; // Data received by xRadioMessageQueue queue
  uint8_t destAddr = 1;

  while(1) {

    if ( xQueueReceive(xRadioMessageQueue, &queueRX, portMAX_DELAY) == pdPASS ) {
      // Data has been successfully received by the queue
      // Transmit the message
      radioSendTo(&g_radio, (uint8_t *) queueRX, sizeof(queueRX), dest_addr);
    }

    vTaskDelay(250);
  }
  
}

I would prefer to keep passing the array by reference, but if this implementation is flawed or anyone has suggestions on how to fix this, I would VERY much appreciate it!

Thanks.

rtel · April 18, 2018, 3:01am

rtel wrote on Wednesday, April 18, 2018:

xRadioMessageQueue = xQueueCreate(1, sizeof(uint8_t *));

This is creating a queue that will hold pointers - each pointer is 4
bytes, hence 4 bytes are copied in and 4 bytes are copied out when
sending and receiving respectively.

richard-damon · April 18, 2018, 10:35am

richard_damon wrote on Wednesday, April 18, 2018:

Actually, he says that he want to pass the array by reference (which I take to mean that he want to pass the address of the array), The problem is that when he get the data he is doing it wrong.

The receiver should look like

  int *queueRx;  // NOT queueRX[5]]
  
  if ( xQueueReceive(xRadioMessageQueue, &queueRX, portMAX_DELAY) == pdPASS ) {

If you want to pass an array ‘by reference’ then you are passing the data via a pointer to the data so that is what you what to get out of the queue, and address. Note, if you do it this way, the source array needs to keep the sent values until they have been consumed by the receiver, as they haven’t been stored anywhere else.

This is one way to handle the sending of variable length messages via a queue, as the message as far as the queue is concerned is still always the same size, the size of a pointer.

system · April 18, 2018, 2:58pm

nicholasd wrote on Wednesday, April 18, 2018:

Thank you both for your replies.

Just to confirm, would this be the proper way implement the pass-by-reference via the xQueueSendToBack()/xQueueReceive()?

Producer thread:

void vPortExpanderTask(void const * argument) {

   ...

    uint8_t queueTX[5] = {};

    while(1) {
        buttonState = readPortExpander(&hi2c1, PORT_EXP_ADDRESS);

        if (buttonState) {
            queueTX[0] = 1;
            queueTX[1] = 2;
            queueTX[2] = 3;
            queueTX[3] = 4;
            queueTX[4] = 5;

            // Send queueTX (i.e. starting address of queueTX array) to xRadioMessageQueue
            xQueueSendToBack(xRadioMessageQueue, &queueTX, 10);
        }

        vTaskDelay(100);
    }
}

Consumer thread:

void vRadio(void const * argument) {

    uint8_t * queueRX_ptr;
    uint8_t queueRX[5];

    ...
    
    while(1) {

        if ( xQueueReceive(xRadioMessageQueue, &queueRX_ptr, portMAX_DELAY) == pdPASS )  {
            // Populate queueRX[] with contents stored at address queueRX_ptr
            queueRX[0] = *queueRX_ptr;
            queueRX[1] = *queueRX_ptr++;
            queueRX[2] = *queueRX_ptr++;
            queueRX[3] = *queueRX_ptr++;
            queueRX[4] = *queueRX_ptr++;

            radioSendTo(&g_radio, (uint8_t *) queueRX, sizeof(queueRX), dest_addr);
        }

            ...
            
    }
        
}

I tried what’s shown above and only the first value was correct (value of 11), but the rest were not. I also tried to use xQueueReceive(xRadioMessageQueue, queueRX_ptr, portMAX_DELAY), writing directly to the pointer, but the MCU ran into a hard fault when this was used.

I could alternatively initialize xRadioMessageQueue to have elements five byte elements instead of four, but I’m not sure if this would be considered bad practise when using queues?

rtel · April 18, 2018, 3:06pm

rtel wrote on Wednesday, April 18, 2018:

I think you are still mixing passing by reference (queuing just a
pointer to the data, so you pass the address of the pointer into the
xQueueSendFunction(), and receive the data into a POINTER, not an ARRAY
as you are below), and passing by copy (where you would write all the
bytes into the queue, and receive into a separate ARRAY, not just a
pointer).

However, if you only have one array, and you want to pass by reference,
both the sender and receiver will end up pointing to the same array - as
Richard D pointed out - you will only have one copy of the data. If
that is what you truly want to do, at the risk of the data being
corrupted before the receiver has a chance to read it, then I don’t
think a queue is the right primitive to use any way. You can have the
sender and receiver access the same array without passing a reference to
it on a queue - after all the reference (a pointer to the array) will be
identical in each case as the array never changes its address location.
Instead you could simply use a task notification to unblock the receiver
when the data in the array has changed and have the receiver read it
from the array. If on the other hand you want to actually copy the data
to the receiver, so it can’t be corrupted before the receiver reads it,
consider using a stream buffer instead.

system · April 18, 2018, 4:49pm

nicholasd wrote on Wednesday, April 18, 2018:

Are you suggesting that it could be more efficient to declare the 5-element queue as a global variable, and simply have the producer thread give a semaphore, and have the consumer thread blocking until that semaphore is given? I may try that to simplify things.

Also, thanks for suggesting using the stream buffer - I hadn’t looked into them before until now, but they look like they’d be a great fit for this message-passing.

rtel · April 18, 2018, 5:27pm

rtel wrote on Wednesday, April 18, 2018:

I’m suggesting that, if you really want to have a single array that two
tasks access directly (and potentially simultaneously [!] because all
you are doing is passing a pointer to the array to the reader task so
not duplicating the data) then you gain nothing from using a queue. The
data you send on the queue is always identical, being the address of the
array, and you are just using the queue as a means of unblocking the
receiving task. To that end, you can choose the most light weight
method of unblocking the task, which would be a direct to task
notification (not a semaphore, see
https://www.freertos.org/RTOS-task-notifications.html)

However, both tasks accessing the same data would seem to be dangerous
as there is the risk of data being missed if it is updated twice before
the reading task reads it, or corrupted, if it is updated half way
through the reading task reading it. Therefore using a stream buffer
that can contain three or four blocks of 5 data bytes would prevent that.