UART Buffer Overruns in CPU Intensive Conditions

Hey guys,

I have a tricky problem on my hands and have run out of ideas. Here’s the situation:

I am connecting the Electron to another device via the Serial1 port, running at 57600bps. The serial port is free-running, and is constantly outputting messages of variable payload size (17 bytes up to 360 bytes) as defined in its protocol ICD. I have about 20 different message types all running at different frequencies, ranging from 1Hz up to 50Hz.

When I am triggering the function to collect the received packets, stuff them in my UDP buffer, and send them out, I start to lose serial data.

I have tried two approaches so far:

  1. In my application loop, have a loop that reads the serial data into a circular buffer, and when enough messages have been read to generate a UDP packet of under 1024 bytes, shoot it out. This did not work because whenever the UDP packing function was being called, a bunch of serial messages were being dropped. I diagnosed this by comparing a message counter in my read loop with the UDP function being called, and one without. The counter without the UDP function call was incrementing at much faster rate (approximately 4-fold, meaning 3 out of 4 packets were being dropped).

  2. In my application loop, run only the function that packs the UDP packets to send them out, and reading the serial only via the serialEvent1() function. Less packets were being dropped this way, but still enough to break the protocol handshake/heartbeat.

This is what serialEvent1() looks like:

void serialEvent1(void){

uint8_t temp;
mavlink_message_t message;
mavlink_status_t status;
bool new_message = false;

while(Serial1.available() > 0){

  temp = Serial1.read();
  // -- Check for a complete message
  if(mavlink_parse_char(MAVLINK_COMM_0, temp, &message, &status)){
                 // -- Got full message, copy it for processing later!
          memcpy((uint8_t *) &message_buffer[message_buffer_next], (uint8_t *) &message, sizeof(mavlink_message_t));
  		if(message_buffer_next == BUFFER_SIZE - 1){
  			message_buffer_next = 0;
  		} else {
  			message_buffer_next++;
  		}
  }

}

}

This is what the process_udp function looks like:

uint8_t udp_buffer[1024];
uint16_t buffer_offset;

void process_udp(){

if(message_buffer_next != message_buffer_index){

  // -- if message will blow buffer, send it.
  if(message_buffer[message_buffer_index].length + buffer_offset > 1024){
  
  	IPAddress ip(x, x, x, x);
  	udp.sendPacket((uint8_t *) &udp_buffer, buffer_offset, ip, 5000);
  	buffer_offset = 0;
  }

  // -- copy new message onto buffer
  memcpy((uint8_t *) &udp_buffer[buffer_offset], (uint8_t *) &message_buffer[message_buffer_index], message_buffer[message_buffer_index].length);
  buffer_offset += message_buffer[message_buffer_index].length;

  if(message_buffer_index == BUFFER_SIZE - 1){
  	message_buffer_index = 0;
  } else {
  	message_buffer_index++;
  }

}

}

And in my mainloop, I am simply calling process_udp(). From my understanding, calling it like this will execute the function only when there is spare CPU time, and interrupt on the serial port when new data available. This should not result into any dropped data, unless there is something I’m missing? Isn’t the serialEvent1() function the result of a hardware interrupt?

I have noticed also, that the small I pack my UDP packets, the more data I’m dropping, which hints at some blocking process in the sendto function.

Any ideas?

No, it’s not - although I’ve proposed such a change a long time ago and repeat that proposal from time to time again.

serialEvent1() will be executed as often as loop() and if loop() blocks, serialEvent1() will not be executed at all.
It is true that the serial data is received via a hardware interrupt, but this ISR only places the new data in the 64byte ring buffer, which needs to be read frequently enough to never overflow.

I’m also not sure where this is coming from

The system keeps running an “eternal” loop inside its main() function and one of the tasks inside that loop is to execute loop() (and serialEvent1()) on each iteration irrespective of “CPU load”.

1 Like

@screaming-cockatoo, are you running with SYSTEM_THREAD(ENABLED)?

As @ScruffR said, it would be great to get a callback from the UART ISR but such is not the case. One approach is to call a Serial1 “servicing” function throughout your code. This function would read whatever data is available in the Serial buffer and store it in a larger buffer that you control.

Another approach is to use SparkIntervalTimer to create a hardware timer interrupt at fixed intervals, say 5ms. This value is chosen since at 57,600 baud, 64 bytes take about 11.1ms to transfer. In the timer ISR, Serial1.read() could be called since the function reads from the Serial1 buffer and not the hardware. Serial1 functions could not be called from anywhere in the loop() code to avoid conflict.

The trick, I believe, is to use a double buffer approach where incoming data (loaded via the ISR) is put into one buffer while another buffer can be used for the UDP prep data. Once UDP is sent, the buffers can be swapped. However, this only works if the process_udp() completes in less than the time it takes to fill the collection buffer. At 1024 bytes, that’s about 178ms.

4 Likes

What I meant by this is that, if I set up my firmware architecture to operate mostly only on ISRs, then whatever code is in the main() loop only gets executed when the microcontroller is not busy servicing said ISRs, i.e. idle time. Essentially, I think we are saying the same thing.

Does this mean that I will not be able to receive messages with payloads that exceed 64 bytes?

I am not, and will give this a try. Reading up on this, am I understanding correctly that threading will be enabled for system tasks?

I thought this is what I’m doing through

memcpy((uint8_t *) &message_buffer[message_buffer_next], (uint8_t *) &message, sizeof(mavlink_message_t));

In serialEvent1(). I’m basically copying that message in another buffer and picking it up later in the main() loop, for packing and dispatching. I do see now what you’re saying with packing time vs reading time. Theoretically, based on this concept, If I make the udp output buffer 32bytes, I should have enough time to pack and send before filling up the 64 bytes.

That being said, I’ll try SYSTEM_THREAD(ENABLED), and will give SparkIntervalTimer a spin to see if I can meet my timing requirements.

Nope, it does not - but only if you read the 64byte buffer frequently enough to keep it from filling up - but that’s not guaranteed via serialEvent1() since this is not interrupt driven and hence will be slowed down by whatever happens in loop() (and other functions sharing the same code path).

1 Like