Device-os: SerialEvent not called often enough, leading to lost Serial data

[monkbroc]

Bug Report

When using the serialEvent(), serialEvent1, etc handlers, if the loop() function takes a long time to run because of delay() calls, there is a chance that the serial buffer will overflow before the next call of serialEvent().

Expected Behavior

serialEvent() should be called often enough to avoid losing characters.

One implementation proposal would be to call serialEvent() during Particle.process() (or whatever background processing function is called during calls to delay).

Another is to make serialEvent a full system firmware event (see community discussion in references section).

Observed Behavior

serialEvent() is only called between executions of loop().

Steps to Reproduce

Connect a GPS, like the Particle Asset Tracker, to a Photon and run the test app.

With the delay, some GPS sentences will be truncated. Without the delay, all sentences are received.

Test App

// test with and without threading. Problem occurs in both cases currently
SYSTEM_THREAD(ENABLED);

void setup() {
  Serial.begin();
  Serial1.begin(9600);
  // Particle asset tracker GPS enable
  pinMode(D6, OUTPUT);
  digitalWrite(D6, LOW);
}

void loop() {
  delay(1000); // remove to remove corruption of GPS phrases
}

void serialEvent1() {
  while(Serial1.available()) {
    Serial.write(Serial1.read());
  }
}

References

See community discussion with proposal to make serialEvent a full fledged system firmware event: https://community.particle.io/t/proposal-bucket/22073

[aeris-ming]

I think you can try to use software timer and check serial data each 1ms, For 9600 I think it is ok

[dmiddlecamp]

I'd love the option to use a true interrupt on serial so we can react and pull data immediately, instead of a periodic poll or check, is that something we could support? Also a configurable serial buffer size (or a BYO buffer) would be really helpful for devices that do a lot of serial traffic.

[monkbroc]

An OS Thread is also an option but right now Thread is an undocumented API.

[aeris-ming]

@dmiddlecamp Linux is periodic poll too,and Photon Serial1 has 64byte buffer.

[m-mcgowan]

callbacks and interrupts aren't the easiest to deal with, and might not work well with different hardware platforms. I would prefer to add a blocking read option to the serial stream so that callbacks are not needed and the programming model is more familiar - active rather than reactive. This also allows the solution to use the least power, since there is no continual polling - the thread will not be scheduled while it is waiting for the serial I/O.

[dmiddlecamp]

for example, some user products are reading tens of KB over serial at a time, and 64 byte buffer with periodic poll means that if some system thing blocks for even a few milliseconds (64 ms to fill the buffer at 9600 baud) there is a good chance of dropping bytes.

[wvdv2002]

It would be great to be able to assign your own buffer to the Serial ports, just like the USB serial ports. That would solve a lot of my problems as well. Talking to 3rd party hardware for which you cannot change the serial protocol, with the Electron having other things to do, I have a really hard time receiving 130 byte packets at 115200 baud error free....

[kubark42]

That lack of a configurable serial port buffer, or a serial port interrupt callback, is a serious hindrance to complex applications. Fast serial ports can easily overfill such a teeny buffer. Operations which can be blocking for long periods of time, such as an HTTP POST, mean that the traditional solution of simply slowing down the incoming baud rate won't work. Nor should we have to do that, it's a bit of a hack.

The STM32 has some incredibly good and fast DMA buffers, allowing for a larger secondary buffer should be straightfoward and barely impact performance.

[elcojacobs]

I agree with kubark42. Using DMA to put the incoming serial data in a buffer would be reliable and would not even involve the main processor. Interacting with that buffer could be polling or event based and it would be easy to let the user configure the size of that buffer.

[technobly]

Lots of great ideas in this thread! As @aeris-ming suggested, I've used the software timer technique with good results in the past. Here's an example (read comments):

// NOTE: This is not a proper buffer!
// FIXME: Create a circular buffer!!

#include "Particle.h"

SYSTEM_THREAD(ENABLED);

#define SERIAL_UPDATE_MS (1)
#define MY_BUFFER_MAX    (2048)
uint8_t  my_buffer[MY_BUFFER_MAX] = {0};
uint16_t my_buffer_idx = 0;

void readSerial() {
    while (Serial.available()) {
        uint8_t c = Serial.read();
        if (my_buffer_idx < MY_BUFFER_MAX) {
            my_buffer[my_buffer_idx++] = c;
        }
    }
}

Timer serialTimer(SERIAL_UPDATE_MS, readSerial);

void setup()
{
   Serial.begin();
   pinMode(D7, OUTPUT);
   serialTimer.start();
}

void loop()
{
    if (my_buffer_idx) {
        // we have serial data!

        // consume it! (echo just to show we got it)
        Serial.println("ECHOING BACK ...");
        uint16_t idx = 0;
        while (idx < my_buffer_idx) {
            Serial.print((char)my_buffer[idx++]); // FIFO
        }
        my_buffer_idx = 0;
        Serial.println();
    }

    // Block loop for a while (try to send a big block of data when D7 is lit)
    digitalWrite(D7, HIGH);
    delay(5000);
    digitalWrite(D7, LOW);
    delay(100);
}

[kubark42]

@technobly, @aeris-ming, looks good! The name "Software Timer" hides the fact that those are indeed asynchronous timers, and so run independently of other threads.

I've noticed on the Particle Electron that the background delay between loop() calls is 10ms. The upshot is that 57600 baud = 6400bytes/s = 64 bytes in 10ms is already too fast to be able to, in 100% of cases, guarantee that the loop() code services the serial queue in time.

Because the proposed code runs in its own FreeRTOS thread, this looks to be a reasonable workaround. I'm not a big fan of having to solve the problem this way, as it leads to less readable and more brittle code as well as still having an artificially low upper limit of around 500,000 baud, but in a pinch this approach is definitely workable.

[ScruffR]

THe issue with Software Timers is that they all run on one thread, so one misbehaving timer callback will impact all others.
And since the FreeRTOS timeslice is 1ms calling the serialTimer on each and every possible timeslice will most likely cause interference.

Having a BYO buffer option which - if possible - can be hooked up via DMA seems a far better option.
Possible related points to discuss would be: circular/ring vs. double buffer, head/tail inidcation for ring buffer, block or overwrite on buffer overrun, all these user-selectable, ...

[kubark42]

This is a better workaround, written by a Particle engineer: https://github.com/rickkas7/SerialBufferRK