UART line read corrupted unless in main loop

Status
Not open for further replies.

swap

Member
I have a Teensy 3.6 with Serial1 connected to an FPGA, and Serial3 connected to an RS-232 transceiver which is connected to my computer.
I just want to relay the messages from the FPGA to my computer, but they keep getting corrupted if watching the serial port isn't the *only* thing the Teensy is doing.

Each message from the FPGA follows this format:
A000001_B000001_C000002_D000002E

There are no newlines, E is the delimiting character, and the 6 numbers after each letter are what I'm really after.

This code gives me the following:
Code:
#define fpga_serial Serial1
#define serial_to_host Serial3

void setup() {
  fpga_serial.begin(9600);
  serial_to_host.begin(9600);
}

char character = 0;

void loop() {
  if (fpga_serial.available()) {      
    character = fpga_serial.read();
    if(character != 'E'){
      serial_to_host.write(character);
    }else{
      serial_to_host.println();
    }
  }
}

Screen Shot 2021-09-23 at 10.29.39 PM.png

Great. However, if I do anything else, the characters start to get corrupted. For example,


Code:
#define fpga_serial Serial1
#define serial_to_host Serial3


void setup() {
  fpga_serial.begin(9600);
  serial_to_host.begin(9600);
  serial_to_host.println("init");
}


void loop() {
  if (fpga_serial.available()) {
    if(fpga_serial.read() == 'E'){
      read_fpga_line();      
    }
  }
}

void read_fpga_line(){
  char character = 0;
  while(character != 'E'){
    serial_to_host.write(character);  
    character = fpga_serial.read();
  }
  fpga_serial.println();
  fpga_serial.clear();
}
This results in corrupted characters, no newlines, and the the output stops printing anything after a couple seconds:
Screen Shot 2021-09-23 at 10.38.04 PM.jpg

I have tried while loops, for loops and found that any delay or logic during the UART read line operation leads to corrupted characters.

Now, the output of the FPGA may be a little "shaky". What I mean is that a logic analyzer sometimes has a hard time parsing the output of the FPGA. In this screenshot, you'll see part of an FPGA message, but each character, although recognized, has a "framing error".
(disregard the trace on channel 1)Screen Shot 2021-09-23 at 11.55.33 AM.jpg

I can't do much about the FPGA, and I can accept that I won't get every message, but I would very much like to write a method that just "tries" to catch a coherent string from it and send it along.
Any tips on how to write a function or state machine that can "focus" the UART and get a coherent string? DMA comes to mind but that seems overcomplicated. I have tried increasing the RX buffer but that didn't seem to fix the problem. So far, this works the best (although it is still useless, it doesn't crash and it gives me some consistently non-garbage characters):

Code:
#define fpga_serial Serial1
#define serial_to_host Serial3

uint8_t FPGArxBuffer[100]  DMAMEM;

void setup() {
  fpga_serial.begin(9600);
  serial_to_host.begin(9600);
  serial_to_host.println("init");
  fpga_serial.addMemoryForRead(FPGArxBuffer, sizeof(FPGArxBuffer));
}


void loop() {
  if (fpga_serial.available()) {
    if(fpga_serial.read() == 'E'){
      read_fpga_line();      
    }
  }
}

void read_fpga_line(){
  char character = 0;
  for(int i = 0; i < 32; i++){
    serial_to_host.write(fpga_serial.read());  
  }
  serial_to_host.println();
  fpga_serial.clear();
}

Result:Screen Shot 2021-09-23 at 10.50.12 PM.png

Help?
 
Code:
void read_fpga_line(){
  char character = 0;
  for(int i = 0; i < 32; i++){
    [B][COLOR="#FF0000"]while (fpga_serial.available() == 0) {;} //wait for a character[/COLOR][/B]
    serial_to_host.write(fpga_serial.read());  
  }
  serial_to_host.println();
  fpga_serial.clear();
}

Don't know it is what you want, but you get the idea.. you have to make sure that a character is available. If not, read() returns -1 ! - it does not wait for a char.
 
There may be several different things going on here:
For example if your code is busy doing other things and you receive lots of data on Serial1, while you are not processing it,
I believe Serial1 has software buffer of 64 bytes. So if you are doing something long enough that you don't process anything and 64 bytes were received, additional bytes will be tossed.

Or it could be something like by default the RX buffer of Serial1 is 64 bytes and the TX buffer for Serial3 is 40 bytes. So if you try to dump a lot of data to Serial3 that was stored up in Serial1, then your code may sit there while enough stuff is output on Serial3 to allow the rest of it to be stored in the TX buffer...

Some different approaches you could try include:
We added methods to Serial to allow you to add additional buffers to the Serial objects.
virtual void addMemoryForRead(void *buffer, size_t length) {serial_add_memory_for_read(buffer, length);}
virtual void addMemoryForWrite(void *buffer, size_t length){serial_add_memory_for_write(buffer, length);}
That you could add additional buffer to Serial1 read and/or Serial3 write ...

You could use the function like: serialEvent1 to forward the data. Maybe something like:

Code:
void serialEvent1() {
    uint8_t buffer[32]
    int count = Serial1.available(); 
    if (count > sizeof(buffer)) count = sizeof(buffer);
    int count_avail_write = Serial3.availableForWrite();
    if (count > count_avail_write) count = count_avail_write;
    int n = Serial1.readBytes((char *)buffer, count);
    Serial3.write(buffer, n);
}
Warning typed on fly so probably issues. But the idea is that every time yield is called, directly or indirectly If there is any pending input Serial data on Serial1, this
function is called, which will try to transfer as many bytes as possible from Serial1 to Serial3 without blocking.

Note: yield is called, when you exit loop each time, plus every time the code calls something like delay...
 
Update -

I have been able to get the consistent message-forwarding behavior I want, mostly thanks to the readBytes function which I was previously unaware of.

However, more was required. I would appreciate any insights into why some of the following measures were necessary:

The issue that readBytes() resolved was the corruption of pretty much every message by using a while or for loop with read(). I bet this has something to do with yield() being called with each loop and readBytes being "closer to the metal".

Next I ran into issues with 'recognizable characters, but in the wrong places/orders'. I was getting output like this:
Code:
A767747_B000000_C817324_D579434E
...other tasks...
A767743EA767743_B000000_C817324_
...other tasks...
A767751_B000000_C817324_D5794340
...other tasks...
A767736_B000000_C817324_D579434E
So: missing delimiters, characters out of order... how???

Anyway, the fix was ultimately to use the code below. Reading in 2 messages worth, saving the first half and discarding the other half produced totally stable output. Really don't understand how that works and would appreciate any insights. Is it due to the variable types I've chosen??

Also, resetting the serial port anytime a suspicious delay in received characters appeared helped reduce glitches. Now that is likely entirely due to this squirrely FPGA but it is weird that my logic analyzer no longer has issues parsing the UART but the Teensy (3.6) needs a repetitive smacking to get consistent behavior.

Ultimately this is resolved. Thank you all for the help. However, any tips on how to "do this right" given the final code below would be appreciated. This is just a snippet of a larger state machine (simulated by the 100ms delay and "...other tasks..." line).



The final code:
Code:
#define fpga_serial Serial1
#define serial_to_host Serial3

#define expected_msg_len 32
#define num_msgs 10

#define FPGA_RX_TIMEOUT 82 // time in ms to wait for an 'E' character before moving on

void setup() {
  fpga_serial.begin(9600);
  serial_to_host.begin(115200);
  serial_to_host.println("init");
}

void loop() {
  delay(100); // Do other stuff and/or sleep
  serial_to_host.println("...other tasks...");
  
  int avail = fpga_serial.available();
  if(avail > expected_msg_len){
    read_fpga_line();
  }else{
    //try restarting the port?
    reset_fpga_serial();
  }
}

// call this when the fpga messages look weird (broken formatting, missing delimiters...) 
void reset_fpga_serial(){
  fpga_serial.end();
  delay(5);
  fpga_serial.begin(9600);
}

void read_fpga_line(){
  uint8_t buff[expected_msg_len * 2];
  elapsedMillis rxTimer;

  //read characters until you find an 'E' or until timeout
  while(fpga_serial.read() != 'E' && rxTimer < FPGA_RX_TIMEOUT){}

  //The next 32 characters should be one message, write those into a buffer
  uint8_t n = fpga_serial.readBytes((char *) buff, expected_msg_len*2);
  
  serial_to_host.print(">"); //delimiting character for FPGA message line
  serial_to_host.write(buff,expected_msg_len);
  serial_to_host.println();
}


And the final output:Screen Shot 2021-09-24 at 5.28.43 PM.png
 
Seeing this is not a good thing:
Code:
void loop() {
  delay(100); // Do other stuff and/or sleep

9600 baud isn't fast - but 960 incoming bits in that 1/10th of a second is 96 bytes of time - if the fpga bytes are continuous it assures data will be lost with default buffer sizes not extended per post #4.

Is that a placeholder for expected 'busy work' in the sketch?

It limits loop() to no more 10 cycles per second to monitor the input.

A T_3.6 will typically cycle loop() and some 100,000's of times per second when not overly 'busy' that would offer more chances for keeping the incoming bytes form overflowing the buffers.
 
As MCU32 said, the reason your first program worked was that each read() was preceded by a check of available(), so read() always returned a valid character. In your second program, where you started having problems, you check available() and then for character 'E', and if you find it you call fpga_read_line(), which then tries to read an entire line with no further calls to available(). In other words, you're reading a line that hasn't arrived yet. The problem was not related to the Teensy doing "other things".
 
As MCU32 said, the reason your first program worked was that each read() was preceded by a check of available(), so read() always returned a valid character. In your second program, where you started having problems, you check available() and then for character 'E', and if you find it you call fpga_read_line(), which then tries to read an entire line with no further calls to available(). In other words, you're reading a line that hasn't arrived yet. The problem was not related to the Teensy doing "other things".

Looks like that was 'sort of' addressed in p#5 before calling:
Code:
  int avail = fpga_serial.available();
  if(avail > expected_msg_len){
    read_fpga_line();

Though if chars are lost the messages will be corrupted - maybe that is why the double read works - the first fits and the second is truncated ... after seeing 'recognizable characters, but in the wrong places/orders'
 
Looks like that was 'sort of' addressed in p#5 before calling:

I did not read through all of the subsequent updates, but wanted to point out why the problem first appeared, i.e. calling read() when the the buffer is empty. That eventually got fixed by replacing read() with readBytes(N), which blocks until N bytes are read or the serial timeout occurs.
 
I did not read through all of the subsequent updates, but wanted to point out why the problem first appeared, i.e. calling read() when the the buffer is empty. That eventually got fixed by replacing read() with readBytes(N), which blocks until N bytes are read or the serial timeout occurs.

Likewise - started reading here on the updated code and seeing the likely loss of data with the delay(100) allowing buffers to overfill.

The code as written won't work effectively
 
Is that a placeholder for expected 'busy work' in the sketch?

Yep, this is all in a standalone file that is separate from the main code that it will be integrated with. Other devices (I2C, SD card, other serial ports) are behaving just fine.

I briefly tested a version of the code that waited for serial.available to return more than the expected message length, but it also produced unpredictable behavior and corrupted characters.
The only repeatable and functioning code I have seen so far is what I posted above, and this is the final version (with a different serial port reset functionality):



Code:
void loop() {
  if(read_fpga_line()){
    fpga_comms_fail_counter = 0;
  }else{
    fpga_comms_fail_counter++;
    if(fpga_comms_fail_counter > RX_FAIL_LIMIT){
      reset_fpga_serial();
    }
    
  }
  delay(50); //tested with both this delay and the rest of the state machine that effectively takes 50+ ms
}



bool read_fpga_line(){

  uint8_t fpga_avail = fpga_serial.available();
  if (fpga_avail < expected_msg_len) 
  {
    return false; //don't do anything if you haven't received a message
  }
  
  uint8_t buff[expected_msg_len * 2];
  elapsedMillis rxTimer;

  //read characters until you find an 'E' or until timeout
  while(fpga_serial.read() != 'E' && rxTimer < FPGA_RX_TIMEOUT){}

  //The next 32 characters should be one message, write those into a buffer
  uint8_t n = fpga_serial.readBytes((char *) buff, expected_msg_len*2);
  
  serial_to_host.print(">"); //delimiting character for FPGA message line
  serial_to_host.write(buff,expected_msg_len);
  serial_to_host.println();
  return true;
}


Note: delays under 30 ms in loop() result in a long delay (a few seconds) before the UART seems to "lock on" and receive messages consistently, and then the characters are typically in the wrong order. But increasing the delay over 40 produces totally consistent behavior. Probably has something to do with the fact that one full message (32 characters) lasts about 41 ms according to my logic analyzer.

Again, there isn't an actual delay in the full code, but there will be an interval timer that runs read_fpga_line() if it hasn't been run in over 41 ms.

Also, after integrating this into the code where "other stuff" is going on, everything works fine.


And please, if there is a design pattern for this that I should be following, I'd love to learn it.
 
if there is a design pattern for this that I should be following, I'd love to learn it.

In your function shown below, you first check to see how many bytes are available. If you have at least "expected_msg_len", you enter a while() loop that calls read() until you get 'E' or a timeout, but there you should also be checking to make sure you haven't read more than "fpga_avail" bytes. I understand that you expect to find an 'E', but what if you don't?

When you call readBytes(), you don't know how many bytes are available, but you do correctly assign the return value to "n". After that, though, you write() with the value expected_msg_len. You should use "n", because it's possible that n < expected_msg_len.

I'm not sure I would say there is a design pattern you should follow. You just need to keep experimenting and make sure that you understand the serial API and add checks to make sure that your code does the right thing even when the buffer does not contain what you expect.

Code:
bool read_fpga_line(){

  uint8_t fpga_avail = fpga_serial.available();
  if (fpga_avail < expected_msg_len) 
  {
    return false; //don't do anything if you haven't received a message
  }
  
  uint8_t buff[expected_msg_len * 2];
  elapsedMillis rxTimer;

  //read characters until you find an 'E' or until timeout
  while(fpga_serial.read() != 'E' && rxTimer < FPGA_RX_TIMEOUT){}

  //The next 32 characters should be one message, write those into a buffer
  uint8_t n = fpga_serial.readBytes((char *) buff, expected_msg_len*2);
  
  serial_to_host.print(">"); //delimiting character for FPGA message line
  serial_to_host.write(buff,expected_msg_len);
  serial_to_host.println();
  return true;
}
 
Did you try some of the stuff I mentioned in previous post...
That is if you are overrunning buffers, you can simply tell the system add more memory to your buffer...

But I don't necessarily think that is the only thing you need to do here...

That is for example you are calling: fpga_serial.readBytes((char *) buff, expected_msg_len*2);
Now if your messages are expected_msg_len, this code is going to read as many characters as possible characters into your buffer until either expected_msg_len*2 bytes are received
or until you get a timeout between characters. And a timeout by default is 1 second.

So during this entire time your code is going to hung up here. If that is fine with your program that is fine. Also if during that second you do receive additional characters, you could start reading in the next message, of which you are not looking for the starting character and the like and as such you will probably be throwing away messages... As your next time you will toss everything again that you already read.

You can help this out, by looking up on the readBytes help page and see there is a call to set the timeout. So you could set this reasonably short, like lets say the time to receive a couple of characters...

Or your code could be pretty simply and if all you are doing is waiting for an E character and then keep a count of how many bytes you have received, and exported back to host and when you get the correct count for the message, then switch state back to looking for message start...

This could be done in the loop code and/or could be done in the SerialEvent1 code base... Or could setup to use an IntervalTimer to every time period that you choose you read in as many bytes as possible such that you neither delay waiting for input bytes nor wait for room in output queue...
 
Status
Not open for further replies.
Back
Top