Serial communication with arduino/error in my code

I've been trying to fix this for hours now and I'm really at a loss.

My project is to use C# and send data over serial to arduino, which will end up being a pattern in an led matrix using the FastLED library.

Not only is the data painfully slow to send (each bit taking 1 second ~3 mins in total), but it doesn't even work.
I get to the last bit and then...nothing. It just sits there.

Any help would be appreciated. (I'm aware the code is probably quite dodgy)
Also, the code in c# takes a png from my desktop and turns it into an array.

C# Code:

using System;
using System.Drawing;
using System.IO.Ports;
using System.Threading;
using System.Threading.Tasks;

namespace Net_Framework_C_Sharp
{
    class Program
    {
        static SerialPort _serialPort;

        static void send(string bit)
        {
            _serialPort.Write(bit);
            Console.WriteLine($"Sent: {bit}\nWaiting..");
            while (_serialPort.BytesToRead < 0)
            {
                Console.Write('.');
            }
            Console.WriteLine($"\nRecieved: {_serialPort.ReadLine()}\n");
        }
        static void Main(string[] args)
        {
            int x, y;
            Bitmap image1;
            string[,] colours = new string[64, 3];

            _serialPort = new SerialPort();
            _serialPort.PortName = "COM4";
            _serialPort.BaudRate = 115200;
            _serialPort.WriteTimeout = 500;

            image1 = new Bitmap(@"[PATH TO FILE]", true);

            for (x = 0; x < image1.Width; x++)
            {
                for (y = 0; y < image1.Height; y++)
                {
                    Color pixelColor = image1.GetPixel(x, y);
                    colours[(x * 8) + y, 0] = pixelColor.R.ToString();
                    colours[(x * 8) + y, 1] = pixelColor.G.ToString();
                    colours[(x * 8) + y, 2] = pixelColor.B.ToString();
                }
            }

            _serialPort.Open();
            for (int z = 0; z < 64; z++)
            {
                for (int a = 0; a < 3; a++)
                {
                    send(colours[z, a]);
                    Console.WriteLine(z);
                }
            }
        }
    }
}

Arduino Code:

#include "FastLED.h"
const int data = 6, numLED = 64;
int ledPattern[63][2];
void setup() {
  Serial.begin(115200);
  pinMode(data, OUTPUT);
}
void printLED() {
  CRGB leds[numLED];
  FastLED.addLeds<NEOPIXEL, data>(leds, numLED);
  for(int x = 0; x < 64; x++){
    leds[x] = CRGB(ledPattern[x][0], ledPattern[x][1], ledPattern[x][2]);
    Serial.println("assigned " + (String)x);
  }
  FastLED.show();
}
void loop() {
  if (Serial.available() > 0) {
    for(int a = 0; a < 64; a++){
      for(int b = 0; b < 3; b++){
        while (Serial.available() < 1) {
          delay(100);
        }
        ledPattern[a][b] = Serial.parseInt();
        Serial.println("ack");
      }
    }
    printLED();
  }
}

did you test your hardware with a simple testcode if sending data with the fastled-library works in principle?

You have a delay of 0,1 seconds in your inner for loop this means transferring a single byte needs 01, seconds.

I don't know C# well. The C#Sharp-code seems to wait for something to receive and has a timeout of 0,5 seconds.
But your arduino-code is never sending something back to the C#-device.

So it might be that this is slowing down for each byte to 0,6 seconds.

Did you do a quick copy & paste from different sources ?
So in this hours of modifying your code without really understanding it you made the experience it did not work.
You should reduce the possible sources of bugs to a minimum through reducing the code.

The next 3 hours you should spend on writing a very basic test-programs:

  • a small testcode that receives a few bytes from the serial monitor
  • a small C#-sharp program that just repeats sending a "Hello" every two seconds

If this works to receive the "Hello" in your Arduino-device
Change the code to send the data for one single LED
and test this.

best regards Stefan

Hi StefanL38,
Thanks for your reply, and sorry for the delay getting back to you.

I have not tested the code for just the FastLED library and I currently don't have time to until tomorrow morning so I'll let you know how that goes soon. If anything, I have used very similar code in a different sketch and it worked fine.

The delay in that loop is for the Arduino to wait a bit until there is serial data from the C# code, admittedly that could be reduced to something like:

delay(10);

I did try to get the Arduino to send a string of: "ack" back to the C# program so it doesn't wait the whole 500 milliseconds, but there could be something wrong with this line of the C#:

while (_serialPort.BytesToRead < 0)

I wasn't too sure on that. If it is wrong, then the delay is understandable.
In fact, I don't think I copied any sources at all when making this. Although, I was using the Microsoft website for the C# references and class documentations.

I will definitely try to do some debugging and I'll give your basic program tips a go. Again, this won't be until tomorrow (I'm currently writing this on my phone) but I'll keep you updated.

Cheers
-Niall

I can't say if it's wrong, but it makes no sense:

while (_serialPort.BytesToRead < 0)

How can there ever be less than zero bytes to read?

niall895:
The delay in that loop is for the Arduino to wait a bit until there is serial data from the C# code, admittedly that could be reduced to something like:

You don't want any delay()s in an Arduino that is receiving serial data.

Have a look at the examples in Serial Input Basics - simple reliable non-blocking ways to receive data. There is also a parse example to illustrate how to extract numbers from the received text.

The technique in the 3rd example will be the most reliable. It is what I use for Arduino to Arduino and Arduino to PC communication.

You can send data in a compatible format with code like this (or the equivalent in any other programming language)

Serial.print('<'); // start marker
Serial.print(value1);
Serial.print(','); // comma separator
Serial.print(value2);
Serial.println('>'); // end marker

In your PC program you need to ensure that it opens the Serial port, allows time for the Arduino to reset before sending the first data and then keeps the serial port open until it is completely finished with the Arduino.

...R

Good spot aarg, I'm just gonna go ahead and change that to a one. I've also made a little update to this method for aesthetics (It now features an iteration tracker!):

static void send(string bit, int iteration)
{
    _serialPort.Write(bit);
    Console.Write($"Sent: {bit} ({iteration}/192)\nWaiting..");
    while (_serialPort.BytesToRead < 1)
    {
        Console.Write('.');
        Thread.Sleep(200);
    }
    Console.WriteLine($"\nRecieved: {_serialPort.ReadLine()}\n");
}

And with that, my waiting dots are working!

Robin2, I've almost entirely reworked my Arduino code based on your example (I wish I had found it sooner it's very useful :slight_smile: )

I've tried to keep it brief like you said in your post, here is the updated code (the printLED function is unchanged):

void loop() {
  recieve();
}

void recieve() {
  while (Serial.available() > 0) {
    data = Serial.read();
    if (rcv == true && data != '>') {
      rcvData[count] = data;
      count++;
    }
    else if(data == '<') {
      rcv = true;
    }
    else if (data == '>' && rcv == true) {
      rcv = false;
      count = 0;
      //end of transmission
      for (int x = 0; x < 192; x++) {
        Serial.print(rcvData[x]);
        rcvData[x] = '\0';
      }
    }
  }
}

I'm happy to say it works a treat! (At least when I tested it in the Arduino serial monitor). One thing I will say is that it doesn't accept characters like \n but that isn't needed anyway and wouldn't be hard to implement.

Now for the C#.
I've removed the waiting feature from the send() method since Arduino doesn't respond any more, that also means there is no delay in sending the data. I hope this doesn't overload Arduino's buffer since I'm actually sending 192 bits here, but I guess there's one way to find out. Here is the modified method:

static void send(string bit, int iteration)
{
    _serialPort.Write(bit);
    Console.Write($"Sent: {bit} ({iteration}/64)\n");
}

I've also made some changes to the part of the code which starts looping. Before the loop a "<" is sent and aftwards a ">" is sent. It now waits for arduino to be ready aswell after opening the port (thanks Robin2). After that, it just reads whatever Arduino sends back.
And the code is...

_serialPort.Open();
Console.Write("Waiting for Arduino..");
while (_serialPort.BytesToRead < 1)
{
    Console.Write(".");
    Thread.Sleep(200);
}
Console.WriteLine("Ready");
_serialPort.Write("<");
Console.WriteLine("Sent start marker: <");
for (int z = 0; z < 64; z++)
{
    for (int a = 0; a < 3; a++)
    {
        send(colours[z, a], z);
    }
}
_serialPort.Write(">");
Console.WriteLine("Sent end marker: >\nReading...");

while (true)
{
    Console.WriteLine(_serialPort.ReadLine());
}

Update, after some testing I've removed the part of C# that reads from arduino after the transmission and before, since this wasn't working properly - it wasn't even reading the "Ready" from arduino.

So the code still doesn't work, even after adding a 100 millisecond delay in the C# code to slow the transmission slightly. I know the arduino is recieving since the Rx light is flashing. But after nothing happens and the leds do not turn on.

What's interesting is that if I send the bits manually through the serial monitor, I see Arduino send "Ready" afterwards which means it is resetting itself, right?

I'm not really sure where to go from here...

EDIT: I forgot to mention a change in the arduino code to call printLED() instead of Serial.println(). It looks like this:

//end of transmission
for (int x = 0; x < 64; x++) {
    for (int y = 0; y < 3; y++) {
        ledPattern[x][y] = rcvData[x * 3 + y];
        rcvData[x] = '\0';
    }
}

I have also made some progress: sending short transmissions like <11111> makes the first few LEDs turn on, which is great! But I still dont know why Arduino is resetting.

int ledPattern[63][2];

Your ledPattern array does not have an element with index 63 and no element with index 2 in the other dimension; so below is incorrect and might result in undefined behaviour or crash your Arduino application

for (int x = 0; x < 64; x++) {

for (int y = 0; y < 3; y++) {
        ledPattern[x][y] = rcvData[x * 3 + y];
        rcvData[x] = '\0';
    }
}

  CRGB leds[numLED];

FastLED.addLeds<NEOPIXEL, data>(leds, numLED);

The ususal approach is to have the constructor as a global variable and the second line in setup(); any reason why you don't follow the example?

Be aware that FastLed.show() disables interrupts while updating the strip. Any serial data that comes in during that time is lost, except for the last byte.

I advise to send one byte at a time from your C# application and echo it back from the Arduino. Only when the C# application receives the echo,it should send the next byte.

Hi sterretje,

sterretje:

  1. Your ledPattern array does not have an element with index 63 and no element with index 2 in the other dimension; so below is incorrect and might result in undefined behaviour or crash your Arduino application

You're right! And I noticed this not that long ago in the compilier (you almost got it word for word).

sterretje:
2) The ususal approach is to have the constructor as a global variable and the second line in setup(); any reason why you don't follow the example?

Because I use a function, and unfortunatly I don't have the know-how to send an array, especially whatever CRGB does to it, to a function. In short, the array won't be in scope.

sterretje:
3) Be aware that FastLed.show() disables interrupts while updating the strip. Any serial data that comes in during that time is lost, except for the last byte.

Thanks for the tip. However, I am manually starting and stopping the program so I'll just keep that in mind :wink:

You actually caught me just before I was about to post my solution. I've made too many changes to the original code to keep track now, but I have created a working version! To which I am very greatfull of the help I've gotten from this forum. If anybody is interested, here it is:

C# code:

using System;
using System.Drawing;
using System.IO.Ports;
using System.Threading;
using System.Threading.Tasks;
namespace Net_Framework_C_Sharp
{
    class Program
    {
        static SerialPort _serialPort;
        static void Main(string[] args)
        {
            int x, y;
            Bitmap image1;
            int[,] colours = new int[64, 3];
            string str = "";
            _serialPort = new SerialPort();
            _serialPort.PortName = "COM4";
            _serialPort.BaudRate = 115200;
            _serialPort.WriteTimeout = 500;
            image1 = new Bitmap(@"[PATH TO FILE]", true);
            for (x = 0; x < image1.Width; x++)
            {
                for (y = 0; y < image1.Height; y++)
                {
                    Color pixelColor = image1.GetPixel(x, y);
                    colours[(x * 8) + y, 0] = pixelColor.R;
                    colours[(x * 8) + y, 1] = pixelColor.G;
                    colours[(x * 8) + y, 2] = pixelColor.B;
                }
            }
            _serialPort.Open();
            Console.Write("Waiting for Arduino..");
            while (_serialPort.BytesToRead < 1)
            {
                Console.Write(".");
                Thread.Sleep(200);
            }
            Console.WriteLine("Ready");
            Thread.Sleep(1000);
            str += "<";
            for (int z = 0; z < 64; z++)
            {
                for (int a = 0; a < 3; a++)
                {
                    if (colours[z, a] > 1)
                    {
                        str += "1";
                    }
                    else
                    {
                        str += "0";
                    }
                }
            }
            str += ">";
            _serialPort.Write(str);
            Console.WriteLine($"Sent: {str}");
        }
    }
}

Arduino code:

#include "FastLED.h"
const int dataPin = 6, numLED = 64, brightness = 5;  //brightness 0-255
int ledPattern[63][2];
int count = 0, one = 0, two = 0, three = 0;
char rcvData[192], data = '\0';
bool rcv = false;
void setup() {
  Serial.begin(115200);
  pinMode(dataPin, OUTPUT);
  Serial.print("Ready");
}
void loop() {
  recieve();
}
void recieve() {
  while (Serial.available() > 0) {
    data = Serial.read();
    if (rcv == true && data != '>') {
      rcvData[count] = data;
      count++;
    }
    else if(data == '<') {
      rcv = true;
    }
    else if (data == '>' && rcv == true) {
      rcv = false;
      count = 0;
      //end of transmission
      printLED();
    }
  }
}
void printLED() {
  CRGB leds[numLED];
  FastLED.addLeds<NEOPIXEL, dataPin>(leds, numLED);
  for (int x = 0; x < 192; x+=3) {
    if (rcvData[x] != '0') {
      one = brightness;
    }
    if (rcvData[x+1] != '0') {
      two = brightness;
    }
    if (rcvData[x+2] != '0') {
      three = brightness;
    }
    leds[x/3] = CRGB(one, two, three);
    Serial.println("leds[" + (String)(x/3) + "]: " + (String)one + ", " + (String)two + ", " + (String)three); //for debugging
    rcvData[x/3] = '0';
    Serial.println("rcvData[" + (String)x + "]: " + rcvData[x]); //for debugging
    one = 0;
    two = 0;
    three = 0;
  }
  FastLED.show();
  for (int x = 0; x < 64; x++) {
    leds[x] = CRGB(0, 0, 0);
  }
}

Because I use a function, and unfortunatly I don't have the know-how to send an array, especially whatever CRGB does to it, to a function. In short, the array won't be in scope.

Look at the FASTED examples. The all declare the CRGB array in the global space (before setup). If declared as a global variable, there will not be a scope issue as every function knows about them and you don't have to pass it as an argument.

    while ( _serialPort.BytesToRead < 1)

is non-intuitive and not straightforward expression of "are there any", so use

    while (_serialPort.BytesToRead == 0)

sterretje, when I put those lines of code at the top like so:

#include "FastLED.h"

const int dataPin = 6, numLED = 64, brightness = 5;  //brightness 0-255
int ledPattern[63][2];
int count = 0, one = 0, two = 0, three = 0;
char rcvData[192], data = '\0';
bool rcv = false;

CRGB leds[numLED];
FastLED.addLeds<NEOPIXEL, dataPin>(leds, numLED);

All I get is this:

exit status 1
'FastLED' does not name a type

I'm actually not sure why.
aarg, will do.

FastLED.addLeds<NEOPIXEL, dataPin>(leds, numLED);

That should be in setup(), not in the global section above setup().

Right you are:

CRGB leds[numLED];

void setup() {
  Serial.begin(115200);
  pinMode(dataPin, OUTPUT);
  FastLED.addLeds<NEOPIXEL, dataPin>(leds, numLED);
  Serial.print("Ready");
}

It works flawlessly!
Thanks everyone.

This topic was automatically closed 120 days after the last reply. New replies are no longer allowed.