Troubles using "Serial." library

Hello everybody, I'm new to the forum and I'm also not expert with Arduino. I wrote a little code to control an RGB LED matrix but it seems that the Serial communication doesn't want to work (better to say I'm not good at writing codes probably!). Anyway, this is what I've done:

if (Serial.available() > 0)
   {
     for (int i = 0; i < 8; i++)
     {
       led[i].red = Serial.read() - '0';
       led[i].green = Serial.read() - '0';
       led[i].blue = Serial.read() - '0';
       
       Serial.print("red["); Serial.print(i); Serial.print("] = ");
       Serial.println(led[i].red);
        
       Serial.print("green["); Serial.print(i); Serial.print("] = ");
       Serial.println(led[i].green);
        
       Serial.print("blue["); Serial.print(i); Serial.print("] = ");
       Serial.println(led[i].blue);
      }
   }

   rest of the code...

This code should read the first 24 bytes stored in the buffer (I wrote the " - '0' "expression because I send bytes via Serial Monitor).

The result I get (as soon as I open the serial monitor) is a list of 24 variables (led[0].red, led[0].green, etc...) set to -49. It shouldn't be correct because if I write nothing the "if statement" should be avoided. What's wrong? :~

Thanks, Matt

This code should read the first 24 bytes stored in the buffer

Yes and you see that there is at least one byte to read and then you go ahead and read all 24 bytes when most of then have not arrived yet.

Sorry, you reply didn't help me. Could you be more specific please?

Thanks

Could you be more specific please?

The Serial.available() function returns the number of bytes waiting to be read. As soon as it reports that there is one byte available, you proceed to read 24 values. That causes most of the reads to fail, returning -1, which you treat as good data.

There is nothing hard to understand about that, is there?

PaulS:
The Serial.available() function returns the number of bytes waiting to be read. As soon as it reports that there is one byte available, you proceed to read 24 values. That causes most of the reads to fail, returning -1, which you treat as good data.

Yes I knew that, I tried to fix with:

if (Serial.available() >= 24)
{
 ...read data...
}
...rest of the code...

I still get the same problem, even if I type nothing in the Serial Monitor window and I really don't understand why.

I still get the same problem, even if I type nothing in the Serial Monitor window and I really don't understand why.

The, clearly, the problem isn't in the snippet you posted.

You have two choices. Post another snippet, and we'll try to guess what is wrong with the code you didn't post, or, post all of your code.

You could try my "ShowInfo" Sketch: Arduino Playground - ShowInfo there is a 'r' option to test the serial connection. It is a short loop, and during that loop you can type characters followed by enter, and see what it does.

I wrote a little code to control an RGB LED matrix but it seems that the Serial communication doesn't want to work (better to say I'm not good at writing codes probably!).

You may need to describe in more detail just what you are trying to do. First, do you have control of the application sending the data to the arduino so that it can be modified if needed? What exactly is the data format of the data being sent to the arduino? Is the data being sent to the arduino being sent to the arduino as single data packets one at a time for each the LEDs, or is all the LED data being sent as a single large data packet that needs to be parsed to recover the individual LED data?

I fixed it! You could never belive the mistake I made... That was my code (I won't post the whole one because it's not necessary):

#include <Led.h>

Led led[8];             // Object belonging to the Led class
                       
void setup()           
{
  Serial.begin(115200);    // Set the serial comunication speed
  pinMode(SER, OUTPUT);    // Set inputs/outputs
  pinMode(SHCLK, OUTPUT);
  pinMode(RCLK, OUTPUT);
}

void loop()
{
  
  if (Serial.available() > 23)[u];[/u]  // <-- the semicolon was the error!
  {
    // Reads the upcoming 24 bytes
    for (int i = 0; i < 8; i++)
    {
      led[i].red = Serial.read() - '0';
      led[i].green = Serial.read() - '0';
      led[i].blue = Serial.read() - '0';
      
      Serial.print("red["); Serial.print(i); Serial.print("] = ");
      Serial.println(led[i].red);
      
      Serial.print("green["); Serial.print(i); Serial.print("] = ");
      Serial.println(led[i].green);
      
      Serial.print("blue["); Serial.print(i); Serial.print("] = ");
      Serial.println(led[i].blue);
    }
  }

....
}

In my 1st post the semicolon didn't appear because I didn't copy-paste the text interly (I wrote the first 2 lines manually) but the mistake - into the sketch - was there!

Thanks for your help anyway

Well it is rare that a "what's wrong with this code" has an error that corrects the problem. We didn't stand a chance did we!

That is why we kept asking you to POST ALL YOUR CODE, you would have been forced to copy and paste it.

Grumpy_Mike:
That is why we kept asking you to POST ALL YOUR CODE, you would have been forced to copy and paste it.

That is true, I'll keep in mind this!

Grumpy_Mike:
Well it is rare that a "what's wrong with this code" has an error that corrects the problem.

Rare but not impossible, you can check by your own if you want that (incredibly, probably when I wrote those lines I was a bit careless, who knows!) the semicomma in that specific point didn't let the "if statament" do its work properly.

CODE UNCORRECT

#define SER       13   // Serial data on pin 13
#define RCLK      12   // 74HC595 register clock on pin 12
#define SHCLK     11   // 74HC595 shift clock on pin 11
#define ROWS      8    // Number of rows set by the designers - we are going to make this value changable by the operator to
                       // let him control different kind of hardware boards with different setups
#define COLS      8    // Numbers of columns set by the designer (same as above)
#define PWMbits   4    /* Every color has 4 bits, so that the number of color results to be: 2^4 * 2^4 * 2^4 = 4096 - we are going
                          to make this value changable by the operator in order to let him control the colors setup */

#include <Led.h>

Led led[8];             // Object belonging to the Led class
uint32_t value = 0;     // Variable where to store the whole 4 bytes that have to be sent at the shift registers
void enableOuts (uint32_t Out);
                       
void setup()           
{
  Serial.begin(115200);    // Set the serial comunication speed
  pinMode(SER, OUTPUT);    // Set inputs/outputs
  pinMode(SHCLK, OUTPUT);
  pinMode(RCLK, OUTPUT); 
}

void loop()
{
  
  if (Serial.available() > 23);   
  {
    // Reads the upcoming 24 bytes
    for (int i = 0; i < 8; i++)
    {
      led[i].red = Serial.read() - '0';
      led[i].green = Serial.read() - '0';
      led[i].blue = Serial.read() - '0';
      
      Serial.print("red["); Serial.print(i); Serial.print("] = ");
      Serial.println(led[i].red);
      
      Serial.print("green["); Serial.print(i); Serial.print("] = ");
      Serial.println(led[i].green);
      
      Serial.print("blue["); Serial.print(i); Serial.print("] = ");
      Serial.println(led[i].blue);
    }
  }
    
  // Shift the colors in the correct order and store the data into value
  for (int row = 0; row < pow(2, ROWS); row *= 2)
  {
    for (int j = 0; j < pow(2, PWMbits); j++)
    {
      for (int col = COLS; col > 0; col--)
      {
       value = ((led[col].blue && 1) << (32 - col)) | ((led[col].green && 1) << (24 - col)) | ((led[col].red && 1) << (16 - col)) | row;
       
      (((led[col].blue && 1) > 0) ? led[col].blue-- : led[col].blue = 0);
      (((led[col].green && 1) > 0) ? led[col].green-- : led[col].green = 0);
      (((led[col].red && 1) > 0) ? led[col].red-- : led[col].red = 0);   
      
      }
      
      enableOuts(value);
    }
  }
  
}

void enableOuts(uint32_t Out)
{
  for (byte c = 0; c < 32; c++)
  {
    (((Out & (1 << c)) == 1) ? digitalWrite(SER, HIGH) : digitalWrite(SER, LOW));
    digitalWrite(SHCLK, HIGH);
    digitalWrite(SHCLK, LOW);
  }
  
  digitalWrite(RCLK, HIGH);
  digitalWrite(RCLK, LOW);
}

CORRECT CODE

#define SER       13   // Serial data on pin 13
#define RCLK      12   // 74HC595 register clock on pin 12
#define SHCLK     11   // 74HC595 shift clock on pin 11
#define ROWS      8    // Number of rows set by the designers - we are going to make this value changable by the operator to
                       // let him control different kind of hardware boards with different setups
#define COLS      8    // Numbers of columns set by the designer (same as above)
#define PWMbits   4    /* Every color has 4 bits, so that the number of color results to be: 2^4 * 2^4 * 2^4 = 4096 - we are going
                          to make this value changable by the operator in order to let him control the colors setup */

#include <Led.h>

Led led[8];             // Object belonging to the Led class
uint32_t value = 0;     // Variable where to store the whole 4 bytes that have to be sent at the shift registers
void enableOuts (uint32_t Out);
                       
void setup()           
{
  Serial.begin(115200);    // Set the serial comunication speed
  pinMode(SER, OUTPUT);    // Set inputs/outputs
  pinMode(SHCLK, OUTPUT);
  pinMode(RCLK, OUTPUT); 
}

void loop()
{
  
  if (Serial.available() > 23)    
  {
    // Reads the upcoming 24 bytes
    for (int i = 0; i < 8; i++)
    {
      led[i].red = Serial.read() - '0';
      led[i].green = Serial.read() - '0';
      led[i].blue = Serial.read() - '0';
      
      Serial.print("red["); Serial.print(i); Serial.print("] = ");
      Serial.println(led[i].red);
      
      Serial.print("green["); Serial.print(i); Serial.print("] = ");
      Serial.println(led[i].green);
      
      Serial.print("blue["); Serial.print(i); Serial.print("] = ");
      Serial.println(led[i].blue);
    }
  }
    
  // Shift the colors in the correct order and store the data into value
  for (int row = 0; row < pow(2, ROWS); row *= 2)
  {
    for (int j = 0; j < pow(2, PWMbits); j++)
    {
      for (int col = COLS; col > 0; col--)
      {
       value = ((led[col].blue && 1) << (32 - col)) | ((led[col].green && 1) << (24 - col)) | ((led[col].red && 1) << (16 - col)) | row;
       
      (((led[col].blue && 1) > 0) ? led[col].blue-- : led[col].blue = 0);
      (((led[col].green && 1) > 0) ? led[col].green-- : led[col].green = 0);
      (((led[col].red && 1) > 0) ? led[col].red-- : led[col].red = 0);   
      
      }
      
      enableOuts(value);
    }
  }
  
}

void enableOuts(uint32_t Out)
{
  for (byte c = 0; c < 32; c++)
  {
    (((Out & (1 << c)) == 1) ? digitalWrite(SER, HIGH) : digitalWrite(SER, LOW));
    digitalWrite(SHCLK, HIGH);
    digitalWrite(SHCLK, LOW);
  }
  
  digitalWrite(RCLK, HIGH);
  digitalWrite(RCLK, LOW);
}

As you can see the "if" only executes a NULL when 24+ bytes are incoming.

Thanks for your tips anyway (;

Matt

PS: now I need to improve the algorithm, in case I have problems I'll post here

the semicomma in that specific point didn't let the "if statament" do its work properly.

Yes an unsurprisingly it is the first thing I looked for in your code when I first read the post.

Anyway good luck with the rest of it.