Menu

#201 xQueueSelectFromSet not returning all pending queues when xQueueOverwrite is used

v1.0 (example)
closed-fixed
nobody
None
5
2019-12-03
2019-11-28
No

After creating QueueSet with two queues and writing to them using xQueueOverwrite, only the second write has an effect on xQueueSelectFromSet. With xQueueSend everything is as expected - two members are returned.

Discussion

  • Richard Barry

    Richard Barry - 2019-11-28
    • status: open --> closed-invalid
     
  • Richard Barry

    Richard Barry - 2019-11-28

    You are not able to use xQueueOverwrite() with a queue set as the set expects the number of items available in the queue set to equal the sum of the items contains in all the queues in that set. I have made a note to make this explicit in the documentation.

     
  • Mariusz Ryndzionek

    Where can I find the note exactly? The documentation states that "uxEventQueueLength must be set to the sum of the lengths of the queues added to the set". The queues used are of size 1 and the QueueSet to 2. Using a QueueSet like this is very valid and useful configuration.
    And what about this https://sourceforge.net/p/freertos/code/2530/ ?

     

    Last edit: Mariusz Ryndzionek 2019-12-04
  • Richard Barry

    Richard Barry - 2019-11-28
    • status: closed-invalid --> open
     
  • Richard Barry

    Richard Barry - 2019-11-28

    Thanks for the link - I will take a look - maybe my last message was not valid. I've re-opened the ticket until I've looked deeper.

     
  • Richard Damon

    Richard Damon - 2019-11-28

    I would think that xQueueOverwrite should work fine with a QueueSet, it would require that the QueueSet NOT be notified if the Overwrite actually overwrote the last data, as that would get the QueueSet out of sync, but make sense as there is already a notification for the first write in the queue set, which will now point to the new updated entry.

     
  • Richard Barry

    Richard Barry - 2019-11-28

    I have created a test that replicates this behaviour.

     
  • Richard Barry

    Richard Barry - 2019-11-28

    I think I can see why this is happening. Within prvNotifyQueueSetContainer(), which is in FreeRTOS/Source/queue.c, locate the line:

    xReturn = prvCopyDataToQueue( pxQueueSetContainer, &pxQueue, xCopyPosition );

    and update it to:

    xReturn = prvCopyDataToQueue( pxQueueSetContainer, &pxQueue, queueSEND_TO_BACK );

    Please report back if that fixes your problem.

    Note it makes the xCopyPosition parameter passed to prvNotifyQUeueSetContainer() obsolete, I have removed the parameter in the master copy of queue.c, but won't check the change in until we have confidence it fixes the issue without breaking anything else (all the tests are passing now, including the new one I added to replicate your scenario).

    Apologies for closing this issue prematurely - I had forgotten we had updated to enable queue overwrites in sets.

     
  • Mariusz Ryndzionek

    Thanks for quick and concrete response! Now it seems that it works most of the time, but sometimes this assert is being hit:
    configASSERT( pxQueueSetContainer->uxMessagesWaiting < pxQueueSetContainer->uxLength );
    It definitely wasn't hit earlier. In my test I'm using xQueueOverwriteFromISR called from an interrupt.
    I think that a bigger redesign of QueueSet is needed. Instead of an ordinary queue this probably needs to be something more like a set pending queues paired with a counter/counting semaphore.

     

    Last edit: Mariusz Ryndzionek 2019-11-29
  • Richard Barry

    Richard Barry - 2019-12-03
    • status: open --> closed-fixed
     
  • Richard Barry

    Richard Barry - 2019-12-03

    This has now also been fixed in the xQueueOverwriteFromISR() function where previously the queue set was updated even if the queue in which data had been overwritten already contained data. Please see the /FreeRTOS/Source/queue.c file here: https://sourceforge.net/p/freertos/code/HEAD/tree/trunk/FreeRTOS/Source/queue.c

     
  • Mariusz Ryndzionek

    Thank you! What will be the next release containing this fix?

     

Log in to post a comment.

MongoDB Logo MongoDB