Rx Queue #2
Rx Queue #2
Conversation
| struct CanardRxQueueItem* next; | ||
| CanardCANFrame frame; | ||
| }; | ||
|
|
There was a problem hiding this comment.
What is the size of the queue? If there is no boundary for the size of the queue, then what if the rx processing speed is slower than the rx incoming speed? I guess the system would run out of the memory and hard fault.
I think it will be better to keep track of the length of the queue somehow.
| struct CanardRxQueueItem* rxQueueHead = NULL; | ||
| struct CanardRxQueueItem* rxQueueTail = NULL; | ||
|
|
||
| void enqueueRxFrame(CanardCANFrame* rx_frame) { |
There was a problem hiding this comment.
Currently, this enqueue function returns nothing. It would be so much better if this function could return some sort of error code. For instance,
enum rxQueueErrorCode{ SUCCESS, EMPTYITEM, OVERFLOW }
| extern struct CanardRxQueueItem* rxQueueHead; | ||
| extern struct CanardRxQueueItem* rxQueueTail; | ||
|
|
||
| void enqueueRxFrame(CanardCANFrame* rx_frame); |
There was a problem hiding this comment.
more comments to describe what the function does, its parameters, and return value are greatly appreciated.
|
|
||
| if (!rxQueueHead) { | ||
| rxQueueTail = NULL; // after dequeuing, the queue is empty | ||
| } |
There was a problem hiding this comment.
Since you dynamically allocated the queue item, the allocated memory should be handled carefully when we are done with it. You should really explicitly write a comment somewhere to let people know they need to free the memory block themselves after they pop it from the queue. Or, more preferably, you handle it for the user.
This PR changes to Rx ISR logic to shorten the interrupt. The ISR callback function HAL_CAN_RxFifo0MsgPendingCallback now adds the received CANFrame into a queue (CanardRxQueue) instead of processing the frame with canardHandleRxFrame immediately. The queue is continuously dequeued in the superloop.
Flow chart can be found here: https://uwarg-docs.atlassian.net/wiki/spaces/ZP/pages/2859827205/Rx+Queue+to+Shorten+ISR
Why is this Change Needed: The interrupt is currently very long because the canardHandleRxFrame is long. Additionally, interrupts are processed in LIFO order, so old interrupts can remain unprocessed for a long time. The queue creates FIFO order instead, so the interrupt itself is much shorter because we call canardHandleRxFrame in the superloop and not in the ISR.
How users can use the code: Functionally, the code works the same as before this change.
What tests has been done to prove the code is working: Code was tested on a server controller with through the pixhawk. The servo received messages and responded as expected.