Arduino is incorrectly reading serial input from C# program

Question

I am trying to create a program to operate my arduino based LED controller. I'm pretty sure that my C# program is outputting to serial port correctly as I double checked the output in Debug.WriteLine(). The program is supposed to send information to the arduino in the following format (newlines included):

mode
number of colors
blend speed
beats until color change
Red   <--- 
Green <---
Blue  <---
Brightness <---

the last 4 are the only ones that matter for now as I am trying to get the LED's to show up one solid color first before worrying about other modes or patterns. The debug writeline loop output of my last test looks like this

1
1
0
1
209
255
255
219

This should be translating to the relevant arduino variables the same way but instead, I only get white light that seems to randomly brighten or darken when I mess with the color/brightness sliders

My code for the arduino is below. I am very new to this so it's probably not even close to right. getData() at the bottom is the function that most likely is the culprit, but I'll put the rest in just in case:

#include <SPI.h>
#include <Adafruit_DotStar.h>

#define NUMPIXELS 60

Adafruit_DotStar strip = Adafruit_DotStar(NUMPIXELS, DOTSTAR_BRG);

// Variables
int mode;
int numColors;
int fadeSpeed;
int BBCC;
int R;
int G;
int B;
int Bright;
String buff;


void setup() {
  strip.begin();
  strip.show();
  //Serial.begin(9600);
}

void loop() {
  getData();


  if (mode == 1)
  {
    //strip.setPixelColor(0, R, G, B);
    strip.setBrightness(Bright);
    strip.show();
  }
}

void getData()
{
 // if (Serial.available() > 0)
 // {
    buff = Serial.read();
    int result = buff.toInt();
   mode = result;
   if (mode != -1)
   {
    buff = Serial.read();
    int result = buff.toInt();
    numColors = result;
    buff = Serial.read();
    result = buff.toInt();
    fadeSpeed = result;
    buff = Serial.read();
    result = buff.toInt();
    BBCC = result;
    buff = Serial.read();
    result = buff.toInt();
    R = result;
    buff = Serial.read();
    result = buff.toInt();
    G = result;
    buff = Serial.read();
    result = buff.toInt();
    B = result;
    buff = Serial.read();
    result = buff.toInt();
    Bright = result;//Serial.read();
    strip.setPixelColor(0, R, 0, 0);
    //strip.setBrightness(Bright);
    strip.show();
   }
   else
   {
    strip.setPixelColor(0, R, 0, 0);
    //strip.setBrightness(Bright);
    strip.show();
   }
//  }
}

Answer 1

Your Serial receive code is not correct in some ways:

1. You should only read from Serial, if there is really something to read. For the first read, that is not the problem, since you checked against the return value being -1 (which is the return code for "there is no new data in the buffer). But after that, you read without ever checking, if there is something to read. You need to understand, that the data is really send serially. The bytes come in one after another and there is no concept as messages on the Serial interface (which is actually called UART in non-Arduino environments). More over, there might be small pauses somewhere in the transmission, since you cannot foresee, when exactly the PC will send the data (the PC has a bunch of other tasks to do, too). So it might be, that you have received one byte, which will lead you into the if statement, but the rest of the data will arrive later. All data is then read as -1, since there is no data in the buffer.

   You have to check, if there is actually data to read, before reading. This can be done with Serial.available(). It will return the number of new data bytes in the buffer. For example, if you want to read 8 bytes, you should wait, until at least 8 bytes are inside the buffer by using if(Serial.available >= 8).

2. You are sending the data from the PC as ASCII encoded text. That means, that every number above 9 consists of multiple data bytes. The 4 values, that you are interested in, are all over 200, which means, that you need to read 3 bytes to get them. But you only call Serial.read() once for every value. That will give you the first digit, which is a 2 for all values in the shown data. More over, since you only read 1 byte, where there is the whole number (which may be multiple bytes) plus 1 or 2 bytes for the new line (depending on the chosen line ending \n or "\n\r"), The whole reading gets misaligned to the data structure. Now the read data (if there is any) is also dependent on how much data was send for the values before.

   Keep in mind, that with ASCII encoded text, you may need multiple Serial.read() calls to actually read a full number. I will describe the way to go here in the next point.

3. As I already mentioned above, there is no message protocol on the Serial interface. Lost data would lead to misalignment and thus garbled data. But you can impose an own protocol onto the Serial stream of data. It is a good practice, to first receive a full message (maybe with multiple data values inside it) and then process it. For that I would suggest a new data structure

   mode|number of colors|blend speed|beats until color change|Red|Green|Blue|Brightness\n
   

   The new line is here realized only with a newline character (\n). The values are separated with a vertical dash. You should put an if(Serial.available()>0) into your loop function. Then inside that if statement, you read one character and add it to a buffer. If you read the \n character, you start to process the buffer. You can then convert one value after another from the buffer and save them into variables. After that, you clear the buffer. You can see this style in the SerialEvent example, that comes with the Arduino IDE. You should also read Majenko's blog article about reading the Serial interface on the Arduino.

---

That said, you should be cautious with using String object on the Arduino. Sure, Strings are convenient, but they dynamically allocate the needed memory during the runtime. On microcontrollers, which normally have very limited RAM memory, that can lead to heap fragmentation (heap is the memory area for dynamic memory allocation). When using String objects, the underlying buffer get's often recreated. As buffers get allocated and deleted later, that can lead to small memory areas, which are unused, but to small to actually be used by the buffer. You can fill up your RAM very fast with that.

It is better to not use String, but directly a char array with a fixed length. Majenko also wrote an article about the String problem: The Evils of Arduino Strings

Answer 2

First off this is NOT how to read from the serial USART!

buff = Serial.read();
int result = buff.toInt();

Serail.read returns a single byte, you are treating it as a string, then trying to parse it; you will never see anything other than 0-9 if your lucky (its very unlikely that buff contains a valid string, chances are it will just keep going till it finds a \0, and return some random number, or just fail.

EDIT: It seems I was mistaken, the 'read()' returns an int, nominally only a byte is returned, but a 16-bit -1 (0xffff) is returned for an error; the documentation could be clearer here.

Second the arduino serial library has a huge flaw, -1 is used as a failure to read, but -1 (i.e. 255) could be a valid piece of data, this makes the return value useless, as you don't know if 255 is the correct value, or an error. The only way around this is with Serial.available so you must use this to control your code or you won't know whats going on.

Finally, you can write binary data in c# with .Write(byte[] ...) and then read it in binary back, so you don't have to parse it.