Serial buffer or faster processing?

Hello,

I am developing an hardware interface to be controlled through the serial port. The system is basically made of switches and leds. Led driver is the MAX7219 with the LedControl library. I have some speed issues in processing the commands sent on the COM port. Normally I can send a command as : "Lxyz=status:" where

L : tells to Arduino I'm going to manage a led
x : device id (number of the MAX7219 I'm going to operate)
y : led row
z : led column
status : "1" for turning on or "0" for turning off the led
":" : separation character

The following code works well if I drive few leds per time, but when I get longer strings (i.e. processing all the 64 leds: -> "L000=1:L001=1:.........:L076=1:L077=1") only the first ones are processed and the rest of the commands gets lost.

if (SerialUSB.available() > 0) {
    char1 = SerialUSB.read();
    if (char1 == 'L'){     // Codice "L" indica che voglio abilitare il led 
              delay (11);
              // leggo 4 caratteri (1 numero board, 2 numero riga, 3 numero colonna, 1 per indicare lo stato 0 o 1 per spento/acceso
              lednumber = "";
              lednumber += char(SerialUSB.read());
              lednumber += char(SerialUSB.read());
              lednumber += char(SerialUSB.read());
              SerialUSB.read(); // Legge il carattere '='
              lednumber += char(SerialUSB.read());
              SerialUSB.read(); // Legge il carattere ':' 
              boardindex = lednumber.charAt(0) - '0';
              row = lednumber.charAt(1) - '0';
              column = lednumber.charAt(2) - '0';
              if ((lednumber.charAt(3) - '0') == 1) {
                ledstatus = true;
                }
              else  {
                ledstatus = false; 
              }
               if (boardindex<devices) {
                  lc.setLed(boardindex, row, column, ledstatus); 
              }
              }
}

I have been trying lowering the Serial speed to 19.200 and some more leds are processed. I also tried to increase the buffer size to 256kb and more leds are processed). If I test it with Arduino due it works well with the native Serial port (I know the buffer is 512kb there) but I think there should be a way to optimize the code so that I don't have to use an Arduino Due for such a simple job, which uses only 5 pins on the board.

My question is : My code is taking from the serial stream one character per time, then processing, then reading the next one. If processing takes too long also reading takes too long. Would it be possible to stream the whole serial buffer into one or more Strings and then parse the strings after? Do you have any advice on how to do so?

I hope I explained it well.

Thanks in advance for your help,

Best Regards.

delay (11);

What's that doing in a thread with a discussion of poor performance?

How much data do you really need to send to control 64 LEDs? Hint: 8 bytes. You are sending WAY TOO MUCH data.

AWOL:

delay (11);

What's that doing in a thread with a discussion of poor performance?

I understand and fully agree with your point.
However, for some unknown (at least to me) reasons, without that delay it doesn't work.

However, for some unknown (at least to me) reasons, without that delay it doesn't work.

Serial data arrives slowly. If there is at least one character to be read, you read as many as 4 with just this snippet.

if (SerialUSB.available() > 0) {
    char1 = SerialUSB.read();
    if (char1 == 'L'){     // Codice "L" indica che voglio abilitare il led 
              delay (11);
              // leggo 4 caratteri (1 numero board, 2 numero riga, 3 numero colonna, 1 per indicare lo stato 0 o 1 per spento/acceso
              lednumber = "";
              lednumber += char(SerialUSB.read());
              lednumber += char(SerialUSB.read());
              lednumber += char(SerialUSB.read());

The delay() is needed to make sure that there is something to read. It is, of course, NOT the way to read serial data.

Oh, and using Strings is far slower that using strings (or, even better, in your case, byte arrays).

It doesn't "work" without the delay() because you're only checking to see that you have at least one character to read.
The delay gives you time for the serial line to (hopefully) deliver the rest.
This isn't a good scheme to rely upon.

Well ok, I got the point, the way I'm using is not good.

I forgot to mention that actually I'm not able to control the data which is being sent from an external software (not made by me). This software enables and disables single leds by sending that code and therefore I can't control the excessive number of bytes being sent. I agree a binary code (8 chars) would fit it. However, I can't control this (the sender).

What could then be a possible way to correctly read from serial? I'm not a programmer and the quality of my code reflects it :slight_smile:

Thanks for your time

What could then be a possible way to correctly read from serial?

You need start and end markers on the packets. Then, you need some code like this:

#define SOP '<'
#define EOP '>'

bool started = false;
bool ended = false;

char inData[80];
byte index;

void setup()
{
   Serial.begin(57600);
   // Other stuff...
}

void loop()
{
  // Read all serial data available, as fast as possible
  while(Serial.available() > 0)
  {
    char inChar = Serial.read();
    if(inChar == SOP)
    {
       index = 0;
       inData[index] = '\0';
       started = true;
       ended = false;
    }
    else if(inChar == EOP)
    {
       ended = true;
       break;
    }
    else
    {
      if(index < 79)
      {
        inData[index] = inChar;
        index++;
        inData[index] = '\0';
      }
    }
  }

  // We are here either because all pending serial
  // data has been read OR because an end of
  // packet marker arrived. Which is it?
  if(started && ended)
  {
    // The end of packet marker arrived. Process the packet

    // Reset for the next packet
    started = false;
    ended = false;
    index = 0;
    inData[index] = '\0';
  }
}

In your case, L is the start of the packet and : is the end of the packet, and the packets are always the same size (7 chars; 4 of them are data). Parsing them is going to be trivial.

Thank you, this can run much faster I guess.

However, I have a standard EOP which is ":" but not a single SOP. The starting character will be an "L" if I have to drive leds but could be A,B,C,D.... etc if the software is driving various 7 segments displays (6 digits). I could modify this :

if(inChar == SOP)

in this :

if((inChar == 'L') || (inChar == 'A') || inChar == 'B'))

Do you think this could affect the speed significantly? The problem is I have no chance If I want to read every different information.

You can remove the started flag and the SOP variable. Any data that comes in when ended is false is considered part of the current packet.

Or, you can have multiple start of packet markers, and save data differently when each one arrives.

What is this external application, and why can't you ditch it? The LEDs can be either on or off. 8 bytes conveys all the information that you need. What do the other commands, A, B, etc. do?

I think if you separate the business of receiving commands from the business of responding to the commands it will make it much easier to sort out your problems. I would aim to reduce loop() to something like this

void loop() {
  receiveCommand();
  interpretCommand();
  switchLeds();
}

The receiveCommand() function would read in data until a ':' is received and save it somewhere together with a marker to say a new command has been received.

The interpretCommand() function would see that a new command was received and would figure out and save the new Led on/off situation. It would also update the marker to say the command had been dealt with.

The switchLeds() function would switch all the leds according to the current settings.

There should be ample time to do all this.

...R

Please post more information on the codes being sent.

Why have ABCD when you use X to select a device?
Does the stream end in an ':' or does that character only separate values?
Are all the codes the same length?

The external application is ProSim737 (http://prosim-ar.com/), a software to simulate systems of a Boeing 737.

The strings are not all of the same lenght unfortunately.
ABCD is used to manage 7 segments displays. If I get "A=123456" I will call a separate function to directly print the numbers, not lighting the single leds.
The ":" is separating different variables' values. It's not always determining the end of the stream. This is true only after the last string.

In the configuration file I can set for a certain led or a certain variable a string to be assigned. If I assign to the led n.1 the string L000 then the software will send the command L000=1 or L000=0. It manages the single led and does not group them, so I can't get a state variable from the software for all the leds inside. I will get single states when these get updated internally.
Of course I could assign "a" to the led 1, "b" to the led 2 etc etc.. , then I would get strings like "a=1:b=1:" but I will have to manage up to 800 leds when the software will be completed and therefore I decided to have a code also easy to understand for driving the Arduino.

The rest of the displays to be driven are 7 segments displays. Again here every value is associated with a string.
For instance :
Radio Frequency --> "A"
Latitude : "B"
I will get inputs from the software like "A=123456" "B=014020" etc. Unfortunately here a lot of bytes are used for the data as you can see and not only for managing the protocol.
Each of this inputs is redirected via I2C to a dedicated MAX7219, therefore I also need to know which device I'm going to operate (till now I have 5 MAX7219 cascaded).

Essentially, ':' is your end of line marker and '=' is your separator between key/value pairs, similar to the lines in an ini or conf file.
So, the high level processing should look like this;

  1. Buffer characters up to the line marker,
  2. Then process the buffer, splitting your key and value on the '='
  3. Then select a function according to the 'key'
  4. Then assign the 'value', by carrying out the operation/s the value corresponds to.
  5. Dispose of any key/value pairs your device does not respond to.
  6. goto 1.

You need to think about how the stream is synchronised between your host and your device.
Does your device request 'discrete' values?
Does the host software send values when states change (delta state)?
Does the host software stream the entire state, frame by frame?

This is true only after the last string.

I really wanted to know if ':' is the last character sent, seems it is.

I can see you have two commands that are not of equal length, however

Is the led command always 7 characters ( including ':' suffix ) ?
Is the 7 seg command always 9 characters ( including ':' suffix ) ?

Need specific details not just a hint. Or at least a link to documentation.

No intention to give only hints, I am trying to give you as much details as I can do. Sorry if something is not clear.

Yes you're right, ":" will always be the last character.

Led string will all be the same, 7 characters, correct.

Numerical values will have different length. I have values to display from 1 digit up to 8 digits. So, I could get "C=1:", 4 characters including ":" up to "D=12345678:" 11 characters including ":"

The software will not stream the entire state, every single variable is transmitted if the state changes.
I try to post what is in the manual : (http://prosim737.com/wp-content/uploads/2012/08/ProSim737_beta_manual-08-21-2012.pdf) page 27.

Using the Generic driver
ProSim737 contains a “Generic” driver that communicates in clear text over a COM port
or over a network through TCP port. It allows you to read and write states of switches and
indicators quickly and efficiently. The generic driver is available in the “Drivers” tab of the
configuration screen of ProSim737 as “Generic COM port/TCP driver”.
After enabling the driver, Switches, Indicators, Analog values and encoders can be
configured to work with driver. In the first drop down menu of the item, select either a COM
port or “Generic driver TCP”. Next, use the text field next to the dropdown box to enter a
label for this IO Element. The name must be unique.
Communication with the driver is either through the configured COM port or through the TCP
port. The driver listens on TCP port 8091 for incoming connections. The protocol is straight
ASCII and consists of reports separated by the newline character. Each report is in this
format:
[‘=’ ]
Where is the label that was configured in the configuration screen and is the
value of the item. If ‘=’ is omitted, the value is assumed to be ‘1’.
Specific instructions per IO element type:
Switches Configure each switch state. To change a switch, send the name of
the switch state that is currently active.
Indicators Indicators are reported with value ‘0’ for off, ‘1’ for on and ‘2’ for
bright.
Gates A ‘0’ is sent for off and a ‘1’ is sent for on.
Analog elements Send the value of the element to change it.
Numerical outputs The output is reported with its current value.
Encoders Send a ‘1’ for a clockwise rotation and a ‘-1’ for counter clockwise.
Send this value each time the encoder is rotated.

Hope it's clear

Thanks for the info, It seems like an interesting project. I have a small library I've been working on which can possibly provide a solution. I'll have a crack and see what I can come up with.

Fa77:
Yes you're right, ":" will always be the last character.
...
Hope it's clear

There is no mention of the ':' terminator in the documentation you quoted.
The documentation states the key/value is terminated by an end of line character.
So, which is it?

MattS-UK:
There is no mention of the ':' terminator in the documentation you quoted.
The documentation states the key/value is terminated by an end of line character.
So, which is it?

I know, and that's also the reason why at the beginning I lost many hours trying to understand what was wrong. Then by using a serial logger and browing the ProSim forum I learned every string was followed by a ":". Actually if I log the serial traffic this character is there.

Fa77:
I know, and that's also the reason why at the beginning I lost many hours trying to understand what was wrong.

Nice!

The code example below might save you a little time. I would have posted it yesterday but needed to hack the snippet out of a larger sketch, which I wrote to de-serialise .conf files.

/* MattS-UK Arduino example code
    Extract key value pairs from serial stream
*/  

//if you don't want to use the Streaming library
//  you need to change  the Serial << output statements
//  to use .print, .println methods
#include <Streaming.h>

void setup()
{
	Serial.begin(9600);
	Serial << "Extract key=value pairs from serial stream...\r\n";
}

#define MAX_BUFFER_LENGTH 64
#define MAX_KEY_LENGTH 32
#define MAX_VALUE_LENGTH 32

char messageBuffer[MAX_BUFFER_LENGTH +1] = {0};
char key[MAX_KEY_LENGTH +1] = {0};
char value[MAX_VALUE_LENGTH +1] ={0};
uint16_t bufferHead = 0;

void loop()
{
	bool pairAvailable = false;
	uint16_t bytesAvailable = Serial.available();

	//buffer overrun check
	while (bytesAvailable > MAX_BUFFER_LENGTH) {
		Serial.read();
	}
	
	//sanity check
	if (bufferHead > MAX_BUFFER_LENGTH) {
		//reset the buffer
		messageBuffer[0] = 0;
		bufferHead = 0;
		return;
	}

	//read incoming
	if(bytesAvailable) {
		for(uint8_t i = 0; i < bytesAvailable; i++) {
			char inChar = Serial.read();
			if (inChar != ':') {
				messageBuffer[bufferHead++] = inChar;
			}
			else {
				//end of message.
				//Terminate the buffer and extract pair.
				messageBuffer[bufferHead] = 0;
				if(messageToPair(messageBuffer, key, value)) {
					pairAvailable = true;
				}
				else {
					Serial << "malformed message\r\n";
				}
				//reset the buffer
				messageBuffer[0] = 0;
				bufferHead = 0;
			}
		}
	}

	//process new pair
	if(pairAvailable) {
		processPair(key, value);
	}
}

//extract key = value pair from message buffer
bool messageToPair(char* message, char* key, char* value) {
	uint8_t messageLength = strlen(message);
	uint8_t valueHead = 0;
	bool keyFound = false;

	//sanity check
	if (!messageLength) {
		//failed
		key[0] = 0;
		value[0] = 0;
		return false;
	}

	//split key = value pair
	for (uint8_t i = 0; i < messageLength; i++){
		if ((!keyFound) && (i < MAX_KEY_LENGTH)) {
			key[i] = message[i];
			if(message[i] == '='){
				keyFound = true;
				key[i] = 0;
			}
		}
		else {
			value[valueHead++] = message[i];
		}
	}
	value[valueHead] = 0;

	//sanity check
	if((!keyFound) || (strlen(value) > MAX_VALUE_LENGTH)) {
		//fail
		key[0] = 0;
		value[0] = 0;
		return false;
	}
	return true;
}

//example pair processing rule
void processPair(char* key, char* value) {
	Serial << "Key=" << key << "\tValue=" << value << "\r\n";

	if(key[0] = 'L') {
		//device is an LED
		uint8_t ledID = atoi(&key[1]);
		bool ledState = (bool) atoi(value);
		Serial << " ledID = " << ledID  << "\tstate = " << ledState << "\r\n";
		return;
	}
}