CMSIS-Driver icon indicating copy to clipboard operation
CMSIS-Driver copied to clipboard

Improvements to buffer handling (ESP32)

Open xennex22 opened this issue 3 years ago • 1 comments

I've noticed that the way data is transferred from the serial buffer to the final user buffer is fairly fragile. It is hard to balance the wifi thread (reading and parsing serial data) with the user thread (making use of the final data). A slight hiccup in the RTOS scheduling results in an out of memory error, which always faults the application for me.

There are the steps in the data flow for me:

  • DMA -> serial
  • serial -> parser
  • parser -> socket
  • socket -> user

What I have noticed is that if one step can't copy all the input data into the output buffer it fails. But it does not matter if it can't copy all the input data. Ultimately it only is when the first step serial DMA overflows that you can't recover.

ReceiveData in ESP32.c is a little convoluted but copies as much data at it can until it runs out of input data or fills up the output buffer. But when it fills up the output buffer it fails, whereas if it returns without an error everything is fine. The rest of the input data will be copied when ReceiveData is next called and the parser buffer has been emptied. https://github.com/ARM-software/CMSIS-Driver/blob/b2a6e81d3123b7532653c61edbbbda7c3e0d22ac/WiFi/ESP32/ESP32.c#L622-L625 If the err = 1U; is replaced by break; then the overall robustness of the whole system is improved.

Same with the AT_NOTIFY_CONNECTION_RX_INIT handler in AT_Notify. If there is not enough room in the socket memory for all the input data, then nothing is copied at all. Again if it copies whatever it can copy then the socket buffer will be (hopefully) emptied by the user thread and everything works ok. https://github.com/ARM-software/CMSIS-Driver/blob/b2a6e81d3123b7532653c61edbbbda7c3e0d22ac/WiFi/ESP32/WiFi_ESP32.c#L147 Replaced with if (rx_num >= (2U)) { Works ok. I think I'm relying on BufWrite to limit the size, but it works.

Also I think the WiFi_Thread loop waits for the WIFI_THREAD_POOLING_TIMEOUT delay between loops when there is data to parse and it should set the thread flags to continue without delay. But the above buffer changes made a huge difference to the reliability of reading data from the ESP32.

xennex22 avatar Dec 25 '21 17:12 xennex22

Thank you once again for your valuable input and analysis. We will review this and change the code where necessary!

VladimirUmek avatar Jan 03 '22 08:01 VladimirUmek