Reading from multiple serials

Hey guys,

First of all, thanking Robin for his tutorial on serial basics here.
http://forum.arduino.cc/index.php?topic=396450

Robin's code showed me how to read the Serial more efficiently. I had to go a little more further to use his code for reading multiple serials at the same time (with a common function with start and end markers).

Problem is that, after a couple of serial reads, the output seems to be mixing up. Means the new out put comes with some mixed up some parts of previous buffer. The variables used is that of local scope, so shouldn't be problem right? To my knowledge, the variables declared inside the function will be cleared after the execution is complete.

I'm using the CRC32 library from here, GitHub - bakercp/CRC32: An Arduino library for calculating a CRC32 checksum.
to ensure what I send and receive are same.

So the serial data will have
a start character '<'
and an end character '>'
and crc code seprated with '|'

Data is json. So it will look like the below (actual valid serial data)

<6260500|{"H":{"O":0,"M":3,"K":"","N":0}}>
and another one
<2198828917|{"H":{"O":0,"M":3,"K":"BkJ7jZv0b","N":2133462144}}>

Guys, i'm still a noob with all these, and I'm not at all sure if what I'm doing is all good or correct. Kindly take a look at my code below and let me know your comments.

#include <CRC32.h>

#define MAX_PACKET_SIZE     600
#define CHECKSUM_CHAR_SIZE  15
#define START_MARKER '<'
#define END_MARKER '>'

struct SystemData
{
    boolean enableDebugMode   = false;
    boolean enableMegaTestMode  = false;
    boolean iAmTheServer    = false;
    char payload[MAX_PACKET_SIZE];
    String apiKey;
    uint32_t deviceKey;
    uint32_t serverId;
};

struct Serial0Data
{
    boolean newData = false;   
    unsigned long lastPing = 0;
};

struct Serial1Data
{
    boolean newData = false;
    unsigned long lastPing = 0;
};

Serial0Data serial0Data;
Serial1Data serial1Data;
SystemData systemData;

void setup() 
{
    Serial.begin(115200);
    Serial1.begin(115200);
    Serial2.begin(115200);
    Serial.println("<Arduino is ready>");
}

void loop() 
{
    checkAndParseSerials(Serial2, systemData.payload, serial0Data.newData, serial0Data.lastPing);
    
    if (serial0Data.newData)
    {
        serial0Data.newData = false;
        Serial.print("From Serial0: ");
        Serial.println(systemData.payload);
    }
      
    checkAndParseSerials(Serial1, systemData.payload, serial1Data.newData, serial1Data.lastPing);

    if (serial1Data.newData)
    {
        serial1Data.newData = false;
        Serial.print("From Serial1: ");
        Serial.println(systemData.payload);
    }
}

void checkAndParseSerials(Stream &port, char * payload, boolean &newData, unsigned long &lastPing) 
{
    char receivedChars[MAX_PACKET_SIZE + CHECKSUM_CHAR_SIZE];
    static boolean recvInProgress = false;
    static int ndx = 0;
    char rc;
    
    while (port.available() > 0 && newData == false) 
    {
        rc = port.read();

        if (recvInProgress == true) 
        {
            if (rc != END_MARKER) 
            {
                receivedChars[ndx] = rc;
                ndx++;
                
                if (ndx >= MAX_PACKET_SIZE + CHECKSUM_CHAR_SIZE) 
                {
                    ndx = (MAX_PACKET_SIZE + CHECKSUM_CHAR_SIZE) - 1;
                }
            }
            else 
            {
                receivedChars[ndx]  = '\0';
                recvInProgress      = false;
                ndx                 = 0;
                lastPing            = millis();
                
                if (verifiedSerialData(receivedChars, payload))
                {
                    newData = true;
                }
                
            }
        }
        else if (rc == START_MARKER)
        {
            ndx = 0;
            //memset(receivedChars, 0, MAX_PACKET_SIZE);
            recvInProgress = true;
        }
    }
}

bool verifiedSerialData(char * receivedChars, char * payload)
{
    char checksum[CHECKSUM_CHAR_SIZE];
    char * strtokIndx;
    
    //memset(checksum, 0, CHECKSUM_CHAR_SIZE);
    memset(payload, 0, MAX_PACKET_SIZE - CHECKSUM_CHAR_SIZE);

    if (strlen(receivedChars))
    {
        if (strstr(receivedChars, "|"))
        {                                        
            strtokIndx = strtok(receivedChars,"|");
       
            strcpy(checksum, strtokIndx);
            strtokIndx = strtok(NULL, "|");
            strcpy(payload, strtokIndx);    
    
            size_t numBytes = String(payload).length() - 1;
               
            CRC32 crc;
    
            crc.update(payload, numBytes);
               
            if ((String(crc.finalize()) == String(checksum)) && (checksum))
            {                                                            
                return true;
            }
            else
            {
                Serial.println("CRC code test FAILED!");
                return false;  
            }
    
            crc.reset();
        }
    }
        
    return false;
}

I should be doing something very silly here. Maybe clear the input buffer of serials after read? I dont know. Tried to troubleshoot for long, but I dont know the way out of this problem.

@Robin if you can also take a look on this, would be very helpful since its mainly an edited version of your code.

Thanks in advance.

mithunrockey:
To my knowledge, the variables declared inside the function will be cleared after the execution is complete.

No:

void myFunction() {
  int localVariable1 = 5;
  int localVariable2;
}

On every entry to myFunction(), localVariable1 will be assigned a value of 5. localVariable2 will have an indeterminate value.

If that was my problem I think I would just duplicate the function recvWithStartEndMarkers() and create a separate set of variables for it. It may not be great programming style but you would probably have it working now. It's not as if you are using 23 different serial ports :slight_smile:

Am I correct in thinking you have only one receivedChars[] array into which the data is put. The system in my examples would need a separate array for each Serial port.

It also looks like you have receivedChars[] as a local variable that is not static - that won't work unless the whole message happens to be in the Serial input buffer when you call the function. My code is written on the assumption that it will need several calls to the function to get a complete message.

...R

Robin2:
If that was my problem I think I would just duplicate the function recvWithStartEndMarkers() and create a separate set of variables for it.

Another way is to build a class that handles this. I did this for a project a while ago, looking for messages to come from either a hardware serial device or a software serial instance.

The class is a bit complex, but you can see @Robin's skeleton in the checkForNewMessage() function:

#include <SoftwareSerial.h>

#define MESSAGE_LENGTH 64

class SerialMessenger {
  public:
    SerialMessenger( HardwareSerial& device) {hwStream = &device;}
    SerialMessenger( SoftwareSerial& device) {swStream = &device;}
    void begin(uint32_t baudRate);
    char* checkForNewMessage(const char endMarker, bool errors);
    
  private:
    HardwareSerial* hwStream;
    SoftwareSerial* swStream;
    Stream* stream;
    char incomingMessage[MESSAGE_LENGTH];
    size_t idx = 0;
};

char* SerialMessenger::checkForNewMessage(const char endMarker, bool errors = false)
{
  stream = !hwStream? (Stream*)swStream : hwStream;
  if (stream->available())
  {
    if (stream->peek() == '\r')
    {
      (void)stream->read();
      return nullptr;
    }
    incomingMessage[idx] = stream->read();
    if (incomingMessage[idx] == endMarker)
    {
      incomingMessage[idx] = '\0';
      idx = 0;
      return incomingMessage;
    }
    else
    {
      idx++;
      if (idx > MESSAGE_LENGTH - 1)
      {
        if (errors)
        {
          stream->print(F("{\"error\":\"message too long\"}\n"));
        }
        while (stream->read() != '\n')
        {
          delay(50);
        }
        idx = 0;
        incomingMessage[idx] = '\0';
      }
    }
  }
  return nullptr;
}

void SerialMessenger::begin(uint32_t baudRate)
{
  if (hwStream)
  {
    hwStream->begin(baudRate);
  }
  else
  {
    swStream->begin(baudRate);
  }
}

/**************** Class Definition Complete ****************/

SoftwareSerial softSerial(2, 3);

SerialMessenger hw(Serial);
SerialMessenger sw(softSerial);

void setup()
{
  hw.begin(115200);
  sw.begin(57600);
}


void loop()
{
  if(const char* hwMessage = hw.checkForNewMessage('\n'))
  {
    Serial.println(hwMessage);
  }

  
  if(const char* swMessage = sw.checkForNewMessage('\n'))
  {
    Serial.println(swMessage);
  }
}

EDIT: this looks just for an end marker... in this case a new line.

BulldogLowell:
Another way is to build a class that handles this.

I can't get away from the style police :slight_smile: :slight_smile: :slight_smile:

I only have hammers in my toolbox :slight_smile:

...R

Thank you for all your suggestions.

@Robin, I was trying to optimise the code so that it only uses less memory. Once a serial is parsed successfully, it is then moved to payload variable which are separate for each serials. The problems still existed even when each serial had its own separate receivedChars variable.

Will make separate functions and check this.

@Bulldog, thanks for your code and input, I'll check that also tomorrow.

@gfvalvo, I was talking about the scope of variable inside the functions. The variable should not be having the value it had during its previous execution of the same function.

mithunrockey:
@Robin, I was trying to optimise the code so that it only uses less memory. Once a serial is parsed successfully, it is then moved to payload variable which are separate for each serials. The problems still existed even when each serial had its own separate receivedChars variable.

You have not said if you can be certain that data will only arrive on one Serial port at any one time. If you cannot be certain of that then you will need two separate arrays to hold the incoming data.

...R

You seem to be sharing the receivedChars buffer between the serial ports. That will only work if it's guaranteed that you will never receive at the same time from the different ports.

I think the better approach is to have a buffer in the struct.

You don't have to define 2 structs.

struct SerialData
{
    boolean newData = false;
    unsigned long lastPing = 0;
    // you can add a receivedChars buffer here
    ...
};

SerialData serial0Data;
SerialData serial1Data;

I was under the impression that the serial1 data will only be parsed after parsing serial0 fully. And that cycle continues. So tried to save some memory with a common array with local scope.

Data will not be in one at a time order.

@sterretje, it was exactly the same as you pointed. As mentioned above, I merged both later to save some memory. Will split it again.

mithunrockey:
Data will not be in one at a time order.

Then you definitely need separate locations in which to save the two sets of data.

Think about it. 2 bytes come in on Serial0, then a byte comes in on Serial1. Then another byte on Serial0 followed by 2 bytes on Serial1 etc etc. The Arduino is very much faster than serial data - even at 500,000 baud.

...R

Thanks guys.

As suggested, I have created separate buffers for serials which now solved the problem for me. Data received from both serials looks good, however the crc check of data shows a mismatch occasionally (GitHub - bakercp/CRC32: An Arduino library for calculating a CRC32 checksum.), but I think it is fine and I can live with it.

Till this time, this is the case when I try to read (only read) from multiple Serials.

The serial1, serial2 reads are happening in the loop continuously flawlessly, UNLESS I send a large string to Serial1 or Serial2

Not sure why arduino stops reading further. I'm using a Mega. The serial 1 and serial2 output looks like received nothing from that point. Is this a buffer overflow problem? I have verified that there is still incoming data to Serial1 and Serial2 the time when arduino output freezes.

This is one of the strings I tried to send to the Serial2

String temp = "";

Serial1.print(temp);

please see my code below.

#define MAX_PACKET_SIZE 600 #define CHECKSUM_CHAR_SIZE 15 #define START_MARKER '<' #define END_MARKER '>' #define PARTITION_MARKER '|'

struct Serial0Data
{
boolean newData = false;
unsigned long lastPing = 0;
boolean recvInProgress = false;
char payload[MAX_PACKET_SIZE];
char checksum[CHECKSUM_CHAR_SIZE];
};

struct Serial1Data
{
boolean newData = false;
unsigned long lastPing = 0;
boolean recvInProgress = false;
char payload[MAX_PACKET_SIZE];
char checksum[CHECKSUM_CHAR_SIZE];
};

Serial0Data serial0Data;
Serial1Data serial1Data;

void setup()
{
Serial.begin(115200);
Serial1.begin(115200);
Serial2.begin(115200);
Serial.println("");
}

void loop()
{
checkAndParseSerial(Serial, serial0Data.payload, serial0Data.checksum, serial0Data.recvInProgress, serial0Data.newData, serial0Data.lastPing);

if (serial0Data.newData)
{
serial0Data.newData = false;
Serial.print("From Serial0: ");
Serial.println(serial0Data.payload);
}

checkAndParseSerial1(Serial1, serial1Data.payload, serial1Data.checksum, serial1Data.recvInProgress, serial1Data.newData, serial1Data.lastPing);

if (serial1Data.newData)
{
serial1Data.newData = false;
Serial.print("From Serial1: ");
Serial.println(serial1Data.payload);
}
}

void checkAndParseSerial(Stream &port, char * payload, char * checksum, boolean &recvInProgress, boolean &newData, unsigned long &lastPing)
{
char rc;
static int ndx = 0;
static boolean payloadData = false;

while (port.available() > 0 && newData == false)
{
rc = port.read();

if (recvInProgress == true)
{
if (rc != END_MARKER)
{
if (rc == PARTITION_MARKER) // For crc match only, not used in this example, please ignore.
{
checksum[ndx] = '\0';
payloadData = true;
ndx = 0;
}
else
{
if (payloadData)
{
payload[ndx] = rc;

if (ndx >= MAX_PACKET_SIZE)
{
ndx = MAX_PACKET_SIZE - 1;
}
}
else
{
checksum[ndx] = rc;

if (ndx >= CHECKSUM_CHAR_SIZE)
{
ndx = CHECKSUM_CHAR_SIZE - 1;
}
}

ndx++;
}
}
else
{
payload[ndx] = '\0';
recvInProgress = false;
ndx = 0;
lastPing = millis();
newData = true;
}
}
else if (rc == START_MARKER)
{
ndx = 0;
recvInProgress = true;
newData = false;
payloadData = false;
memset(payload, 0, MAX_PACKET_SIZE);
memset(checksum, 0, CHECKSUM_CHAR_SIZE);
}
}
}

void checkAndParseSerial1(Stream &port, char * payload, char * checksum, boolean &recvInProgress, boolean &newData, unsigned long &lastPing)
{
char rc;
static int ndx = 0;
static boolean payloadData = false;

while (port.available() > 0 && newData == false)
{
rc = port.read();

if (recvInProgress == true)
{
if (rc != END_MARKER)
{
if (rc == PARTITION_MARKER) // For crc match only, not used in this example, please ignore.
{
checksum[ndx] = '\0';
payloadData = true;
ndx = 0;
}
else
{
if (payloadData)
{
payload[ndx] = rc;

if (ndx >= MAX_PACKET_SIZE)
{
ndx = MAX_PACKET_SIZE - 1;
}
}
else
{
checksum[ndx] = rc;

if (ndx >= CHECKSUM_CHAR_SIZE)
{
ndx = CHECKSUM_CHAR_SIZE - 1;
}
}

ndx++;
}
}
else
{
payload[ndx] = '\0';
recvInProgress = false;
ndx = 0;
lastPing = millis();
newData = true;
}
}
else if (rc == START_MARKER)
{
ndx = 0;
recvInProgress = true;
newData = false;
payloadData = false;
memset(payload, 0, MAX_PACKET_SIZE);
memset(checksum, 0, CHECKSUM_CHAR_SIZE);
}
}
}

Thanks in advance!

you are getting closer to understanding object oriented programming!

struct Serial0Data
{
  boolean newData = false;
  unsigned long lastPing = 0;
  boolean recvInProgress = false;
  char payload[MAX_PACKET_SIZE];
  char checksum[CHECKSUM_CHAR_SIZE];
};

struct Serial1Data
{
  boolean newData = false;
  unsigned long lastPing = 0;
  boolean recvInProgress = false;
  char payload[MAX_PACKET_SIZE];
  char checksum[CHECKSUM_CHAR_SIZE];
};

Serial0Data serial0Data;
Serial1Data serial1Data;

could be just:

struct SerialData
{
  boolean newData = false;
  unsigned long lastPing = 0;
  boolean recvInProgress = false;
  char payload[MAX_PACKET_SIZE];
  char checksum[CHECKSUM_CHAR_SIZE];
};

SerialData serial0Data;
SerialData serial1Data;

then pass the object to your function:

checkAndParseSerial(Serial1, serial1Data);

another hint? the static local variables in the function could be part of the SerialData object and you could do both with one function... welcome to classes! (like the example I showed you above).

Thanks BulldogLowell

I'm into development, haven't got in touch with the C/CPP for past 15 years. Came to know about Arduino and raspberry very recently. Doing this all as a hobby project.

Ok, next thing I'll try is to merge those functions.

But the current issue I'm facing is with the large serial inputs which causes the outputs from serials to freeze. I'm quite sure it's something related to arduino buffer?

The serials are connected to ESP's and I have confirmed they both accept the large serial data from Arduino and ping back data to arduino. But my Arduino is not showing those data thereafter.

mithunrockey:
But the current issue I'm facing is with the large serial inputs which causes the outputs from serials to freeze. I'm quite sure it's something related to arduino buffer?

you may be overflowing the HardwareSerial buffers with the very large strings.

try once with the baud rates down at 9600 instead of 115200.