1

I'm making an LED spectrum analyzer with an Arduino Due and my PC. The audio processing is done on the PC, and then sent to the Arduino. The exact data sent is a 'coordinate', or more specifically, a number representing a particular band, and another number representing the maximum amplitude of that band. There are 11 bands. Each coordinate has a x and a y value, separated by a comma, and then sent on a new line.

This is my code as of now:

void loop() {
  if (Serial.available()) {
    xCoord = Serial.parseInt();
    yCoord = Serial.parseInt();
    if (xCoord != 0) {
      int r, g, b;
      for (int i = 0; i < 8; i++) {
        int fromCoordToIndex = ((xCoord - 1) * 8) + i;
        //
        //do some calculations for the color
        //
        strip.setPixelColor(fromCoordToIndex, strip.Color(r, g, b));
        strip.show();
      }
    }
  }
}

The problem is parseInt seems to be very very slow (it's not the timeout issue, it's set to 1). From what I can see, it seems to be skipping some data, so every now and then a band gets missed out. The only way I can fix this is to insert a delay between each coordinate being sent. I found that 40 ms works ok, but remember there are 11 bands, so that means about half a second to refresh the whole board, which feels like a slide show...

I've tried inputting individual characters and then using toInt, I've tried parseInt obviously, I've tried native USB (it's a Due), and I've tried fiddling with baud rates. None seems to have had any effect.

dda
  • 1,595
  • 1
  • 12
  • 17
mr-matt
  • 147
  • 1
  • 5
  • 15

6 Answers6

2

The annoying thing about parseInt() is not its speed, it's the fact that it relies on a timeout. If the timeout is too long, it slows you down. If it's too short, it may induce errors.

A safer way to parse the sort of messages you are using is to buffer all the incoming characters until you find a carriage return, then parse the whole line. For example:

// Just echo the messages with a different separator.
void parse(char *buffer)
{
    char *s = strtok(buffer, ",");
    int x = atoi(s);
    s = strtok(NULL, ",");
    int y = atoi(s);
    Serial.print(x);
    Serial.print(':');
    Serial.println(y);
}

void loop()
{
    static char buffer[BUFFER_SZ];
    static size_t lg = 0;
    while (Serial.available()) {
        char c = Serial.read();
        if (c == '\r') {        // carriage return
            buffer[lg] = '\0';  // terminate the string
            parse(buffer);
            lg = 0;             // get ready for next message
        }
        else if (lg < BUFFER_SZ - 1) {
            buffer[lg++] = c;
        }
    }
}

Note that you should add some error checking when you call strtok().

Edgar Bonet
  • 45,094
  • 4
  • 42
  • 81
1
#include <SoftwareSerial>
SoftwareSerial s1(5,6);

void setup {
s1.begin(9600);
s1.setTimeout(50); //sets the timeout of parseint() to 50ms

}

Please note that lowering the timeout too much can cause errors, i would not go below 10.

Python Schlange
  • 426
  • 1
  • 4
  • 18
0

Premature optimization is the root of all evil (or at least most of it) in programming. Donald Knuth.

I don't know how fast/slow parseInt() is. Neither do you. You have to time it first.

OK. Maybe it's really slow, but is it the slowest part of your code? Slow compared to what?

Is parseInt what you need? parseInt doesn't convert negative values and it can time-out (but still return zero ... Duh!).

If you read the whole line as a String and then parse the values, you avoid the time-out problem and you can also verify that the data is correct.

This sketch show how to process your data:

void setup() {
  Serial.begin(9600);
  while(!Serial);

  String lines[] = {
    "1,1",
    "10,20",
    "11,1000",
  };
  int channel;
  int value;

  for (int i=0; i < 3; i++) {      
    sscanf(lines[i].c_str(), "%d,%d", &channel, &value);
    Serial.print("channel="); Serial.print(channel);
    Serial.print(" value="); Serial.println(value);
  }
}

void loop() { 
}

Well, I'm using sscanf, that's slower than parseInt for sure, but it get the job done. I doubt you (or me) can do it better, and there more useful thing to do with our time.

Edit:

I profiled

sscanf("0001,0001", "%d,%d", &channel, &value);

with an Arduino UNO and it took 147us to convert a single par of values.

0

Instead of using parseint, send the data for the 11 bands in a very specific form to the Arduino so the Arduino can 'parse' it very easily.

Maybe instead of sending text, send 33 bytes for RGB (3) times 11 bands are the levels (0-255) of the consecutive channels. Than there is no conversion needed (except for reading the bytes).

Michel Keijzers
  • 13,014
  • 7
  • 41
  • 58
0

Your data has 11 channels and a single value for each channel. The values will be presented in graphic format, so it fits in the range 0-255 without degrading the presentation. You always can scale your readings to your display's resolution.

You then send these 11 values in binary form. One byte per channel plus one byte to separate one data set from another. You don't need to send the channel number, just the values.

At the Arduino end you read 12 bytes and then loop thru it converting the bytes to ints.

#define CHANNELS 11

void setup()
{
  Serial.begin(9600);

  while(!Serial);
}

void loop()
{
  char packet[CHANNELS + 1];
  int values[CHANNELS];

  // Read 12 bytes (11 bytes of data + 1 byte delimiter)
  int nBytes = Serial.readBytes(packet, sizeof(CHANNELS + 1));

  if(nBytes == CHANNELS + 1) {
    // Convert bytes to ints;
    for(int i=0; i<CHANNELS; i++) {
      values[i] = packet[i];
    }
  } else {
     // Error. 
     }

  // Here you display your graph.
}

This code doesn't have any error-detection. For that you can add an extra byte with the checksum of the whole dataset.

dda
  • 1,595
  • 1
  • 12
  • 17
0

I seem to have fixed it by lowering the baud rate from 115200 to 9600. This answer inspired the change. Apparently, the higher the baud the faster the information is sent, but the higher the likelihood of an error, and the lower the baud rate, the slower it is sent, but the more reliable it is.

mr-matt
  • 147
  • 1
  • 5
  • 15