libretiny-eu / libretiny

PlatformIO development platform for IoT modules
http://docs.libretiny.eu/
MIT License
383 stars 55 forks source link

Arduino Ring Buffer is not suitable for serial interface #154

Open hn opened 11 months ago

hn commented 11 months ago

Libretiny uses the Arduino Ring Buffer for serial port i/o. However, the implementation is not interrupt-safe and there are no plans to change this.

This causes hard to debug problems with serial i/o. One pain point is that the Arduino Ring buffer is very error prone when used in such a situation (it uses a redundant way to store its state). And if it breaks, it will never be good again (no self-healing). You have to (in most cases) reboot the device or restart the component in question.

As far as I can (quickly) see the ESP8266 and ESP32 serial port components do not use the Arduino Ring Buffer, but implement some own 'magic' for the buffer.

I suggest replacing the Arduino Ring Buffer within the serial component with a better option (at least it should not break permanently).

hn commented 11 months ago

Please consider the work of @KurtE for a possible solution: https://github.com/hn/ginlong-solis/issues/4#issuecomment-1673194618 and https://github.com/hn/ginlong-solis/issues/4#issuecomment-1674801822

I'm using https://github.com/hn/ginlong-solis/commit/76d81ce4d0418884d14362b2f8980c9652d40ebd as a temporary workaround.

kuba2k2 commented 11 months ago

It seems that using the SafeRingBuffer would be a viable option, since it's only a small wrapper around the RingBuffer from Arduino API. On that note, it's pretty disappointing that the API is so much different from most actual official Arduino cores (even things like Serial.printf(), which makes Arduino devs very angry when mentioned).

If the wrapper adds too much overhead by using the synchronized blocks, it could be rewritten to use FreeRTOS' mutexes or semaphores instead.

hn commented 11 months ago

If I understand correctly, ESP32 uses this:

#define UART_MUTEX_LOCK()    do {} while (xSemaphoreTake(uart->lock, portMAX_DELAY) != pdPASS)
#define UART_MUTEX_UNLOCK()  xSemaphoreGive(uart->lock)