Using serialEvent() with arduino mega2560

Hi,
I am using arduino mega2560 R3 with 3 hardware serial inputs and the serialEvent() function. This seems to work fine when the input serial streams only occur on one serial input at a time. When serial input is occurring on all three hardware serial ports at the same time i get slightly garbled output e.g serial streams will be put on the end of the serial stream from a previous read etc.

I have included the code excerpt below and some example output when two of the three hardware serial ports are receiving at the same time.

Any help is much appreciated.

Cheers.

void serialEvent1() {
  if (Serial1.available() > 0) {
    inString =+ "1,";    
    // get the new byte:
    char inChar = (char)Serial1.read(); 
    while (inChar != 'L') {inChar = (char)Serial1.read();}
    // add it to the inputString:
    while (inChar != '\r') {
      while (!Serial1.available()) delay(1);
      inString += inChar; 
      inChar = (char)Serial1.read();
    } 
    
    Serial1.flush(); // stops multiple reads
    
    if (inString.length() > 20) {
      logTag();
      chkIfNewTag();
      inString = "";
    }
    else {
       inString = "";
    }
  }
}

void serialEvent2() {
  if (Serial2.available() > 0) {
    inString += "2,";
     // get the new byte:
     char inChar = (char)Serial2.read(); 
     while (inChar != 'L') {inChar = (char)Serial2.read();}
     // add it to the inputString:
     while (inChar != '\r') {
       while (!Serial2.available()) delay(1);
       inString += inChar; 
       inChar = (char)Serial2.read();
     } 
     
     Serial2.flush(); // stops multiple reads
     
     if (inString.length() > 20) {
       logTag();
       chkIfNewTag();
       inString = "";
     }
     else {
       inString = "";
     }
   }
}

void serialEvent3() {
  if (Serial3.available() > 0) {
    inString += "3,";
     // get the new byte:
     char inChar = (char)Serial3.read(); 
     while (inChar != 'L') {inChar = (char)Serial3.read();}
     // add it to the inputString:
     while (inChar != '\r') {
       while (!Serial3.available()) delay(1);
       inString += inChar; 
       inChar = (char)Serial3.read();
     } 
     
     Serial3.flush(); // stops multiple reads
     
     if (inString.length() > 20) {
       logTag();
       chkIfNewTag();
       inString = "";
     }
     else {
       inString = "";
     }
   }
}

void logTag() {
  tagCnt++;
  //Serial.println(tagCnt,DEC);    //testing
  DateTime now = RTC.now();
  sprintf( timeString, "%d:%d:%d", now.hour(), now.minute(), now.second());
  // if the file is available, write to it:
  //tagString = "LR 0000 0000001234567890";
  //dataFile = SD.open(dateString,FILE_WRITE);
  //Serial.println(dateString);
  if (dataFile) {
    dataFile.print(timeString);
    dataFile.print(",");
    dataFile.println(inString);
    dataFile.flush();
    // print to the serial port too:
    Serial.println(inString);
    //inString = "";
  } 
  // if the file isn't open, pop up an error:
  else {
    Serial.println(F("data logging error"));
    //for (;;){}
  }
  delay(20);
}

Example output from serial terminal
3,LA 00000 LA 00000 0 900 226000106155
1,LR 0000 0000000174825LR 0000 0000000174825496
3,LA 00000 0 900 226000106LA 00000 0 900 226000106155
1,LR 0000 0000000LR 0000 0000000174825496
3,LA 00000 LA 00000 0 900 226000106155
1,LR 0000 0000000174825LR 0000 0000000174825496
3,LA 00000 0 900 226000106LA 00000 0 900 226000106155
1,LR 0000 0000000LR 0000 0000000174825496
1,LR 0000 000000017482596
3,LA 00000 0 900 226000106155
1,LR 0000 0000000174825496
1,LR 0000 0000000174825496
3,LA 00000 0 900 226000106155
1,LR 0000 0000000174825496
3,LA 00000 0 900 226000106155
3,LA LA 00000 0 900 226000106155
1,LR 0000 000000017482549 0000000174825496
3,LA 00000 0 900 226000106155
1,LR 0000LR 0000 00000LR 0000 0000000174825496
3,LA 00000 0 900 226000106155
1,LR 0000 00000001LR 0000 0000000174825496
3,LA 00000 0 900 226000106155
1,LR 0000 000000017482LR 0000 0000000174825496
1,LR 0000 0000000174825496
3,LA 00000 0 900 22LA 00000 0 900 226000106155

You seem to be under the impression that packets arrive all at once. Disabuse yourself of that notion.

Hi,
And thanks for the reply although i dont quite understand what you mean. As i understand it the serialEvent() functions run in-between each run of the main loop. If there is serial data in one or more of the hardware serial buffers serialEvent1(), serialEvent2() and serialEvent(3) will be triggered one after the other, updating the value of inString as they do so and in this case logging each time.

Have i got this wrong?

Cheers.

You got it right.

Here is the main loop with "loop()" and the "serialEventRun()":
https://github.com/arduino/Arduino/blob/master/hardware/arduino/cores/arduino/main.cpp
Here is the "serialEventRun()" function itself:
https://github.com/arduino/Arduino/blob/master/hardware/arduino/cores/arduino/HardwareSerial.cpp

But not many might have used it with 4 serial ports, so you could have run into a bug.

I'm sorry, but I have been staring at your code for a while and all I can see is a dense fog of characters.
Can you make a well commented single function for the four serial ports ?
Did you notice the "=+" ? It should be "+=".

With the wrong serial input, that code could get stuck in a loop. Or the inChar might overflow the ram.
I'm also not sure about the 'flush' function.

Hi Caltoa,
And thanks for the reply.

Year i did spot the =+ and have changed it. I will check out the links. I thought it may have a been a timing issue with respect to to much data coming in at once, so i tried putting in some delays in the serialEvent() functions but this had no effect - in fact it seemed to make it worse.

The flush function is supposed to prevent multiple reads but i am not sure if i am making the correct use of it.

Cheers.

A lot of incoming data ? In that case the delays could be a problem indeed.
You have 1ms during reading and 20ms when the string has finished. That is a lot, because it happens for each serial port. When the delay is busy for one, there could be data incoming for the other three, and the buffers are not very big.
Can you rewrite it, whithout any delay at all ?

I thought it may have a been a timing issue with respect to to much data coming in at once, so i tried putting in some delays in the serialEvent() functions but this had no effect - in fact it seemed to make it worse.

You need to determine if the incoming data has some type of delimiter to separate data in the packet and to signal the end/start of the next data stream.

Hi ,
zoomkat: The data streams are delimited by line feeds and carriage returns so i collect data until a line feed is reached and don't start data collection until i receive an "L" (start of the Stream). This data is put into the String called "inString".

Caltoa: I have removed all delays apart from the one in the serialEvent() that makes sure there is something to receive and i have reduced this to 500uS. Incoming stream is at 9600 baud. This did help a little bit but still getting strings written on the end of other strings. Sometimes i get three complete strings stored in "instring".

One thing i haven't tried is to flush the hardware buffer before reading instead of after reading it which makes more sense.

Cheers.

I don't understand why you need the flush at all. I also can't see through your code how it would behave with 4 incoming data streams, since you process parts of the incoming data for just one stream, while the others are filling the buffers in the Serial library. I have a bad feeling about that code, and that feeling is most of the times right :wink:
I wrote "without delay", I ment without any delay and without any waiting. So also without the 'while' loops.

You have code that could block the sketch, it is a so called "blocking code", and I would like to change it into "non-blocking code".

You could make 4 buffers (or strings) and keep storing data without any delay into the buffers. Once you receive the end of the string, you process the buffer. The 'SerialEvent' could fill the buffers (with overflow test) and the processing could be in the loop() function.

This data is put into the String called "inString".

Isn't that the problem?
There's only one "inString"

I don't think its a great way to use serialEvent - you are taking over from the
main program and reading in entire lines in your serialEvent handler.

Instead you should just handle as many characters as are immediately available(),
maintaining whatever state necessary to carry on when the next serialEvent call
happens.

That way you don't hold up the main program or any other serialEvents, and you
aren't spraying away valuable CPU time in calls to delay() busy-waiting.

You will need 4 buffers to accumulate separately from each Serial device, but that's
a good thing if you think about it (consider if one serial device fails to send a '\r' at
all?)

Hi,
And thanks for all the replies.

I have now done the following;
Created a separate string for each serialEvent() to read data into and moved all the post processing to the main loop so that all the serialEvents()'s only read available data into a string.

This approach did make an improvement but was still creating strings with data written on top of each other but the frequency with which this is happening is now less - about every 20 times through the loop errors would occur. I then cheated slightly and told the post processing to ignore strings over a certain length and this has created the output i want although it is not strictly ideal as some data gets ignored but it is still getting enough to be acceptable.

I will continue to work on this to make it more accurate.

Again, thanks for all your help.

Cheers.

There is no real reason to use serialEvent(). Nothing that it does can't happen in the body of if(Serial.available() > 0).

You modified your code, and it still doesn't work quite the way you want. So, where is the modified code?