Question about a piece of code[SOLVED]

So i made this sketch that reads some Serial data consisting of a Start of packet marker, some integer and a end of packet marker. This data should be recieved as an integer, and it does, as long as i add a 5ms delay in the function to read the serial data. I would love to have it running at full speed, but that doesnt seem to work for some odd reason. Removeing the delay resuslts in me recieveing overflown integers. So what do you think this might be caused by?

Are you using serial.available to check that data is there before you read each character?

So what do you think this might be caused by?

Without seeing your code? Could be nearly anything.

Sry forgot the code -_-

int rPin = 9;
int gPin = 10;
int bPin = 11;

int R;
int G;
int B;

char rPre = 'R';
char rSuf = 'r';
char gPre = 'G';
char gSuf = 'g';
char bPre = 'B';
char bSuf = 'b';

void setup()
{
  Serial.begin(9600);
  pinMode(rPin, OUTPUT);
  pinMode(gPin, OUTPUT);
  pinMode(bPin, OUTPUT);
}

long getSerial(char x, char z)
{
  long y = 0;
  boolean readSerial = false;
  delay(5);
  if(Serial.peek() == x)
  {
    Serial.read();
    readSerial = true;
  }
  
  while(readSerial)
  {
    if(Serial.peek() == z)
    {
      Serial.read();
      readSerial = false;
    }
    else
    {
      //Serial.println(Serial.peek());
      y = y*10 + Serial.read()-'0';
    }
  }
  return y;
}

void loop()
{
   R = getSerial(rPre, rSuf);
   G = getSerial(gPre, gSuf);
   B = getSerial(bPre, bSuf);
  
  if(R != 0)
  {  
    Serial.print("Red value: ");
    Serial.println(R);
    Serial.println("--------------------");
  }
  if(G != 0)
  {
    Serial.print("Green value: ");
    Serial.println(G);
    Serial.println("--------------------");
  }
  if(B != 0)
  { 
    Serial.print("Blue value: ");
    Serial.println(B);
    Serial.println("--------------------");
  }
  /*
  analogWrite(rPin, R);
  analogWrite(gPin, G);
  analogWrite(bPin, B);
  */
}

As suspected, this:

      y = y*10 + Serial.read()-'0';

Is reading the serial port regardless of whether there is anything there. Delay masks this, but ideally, you need to check before you read

so

if(Serial.peek() != -1)
{
Serial.read() stuff?
}

oh yeah, and using Serial.available() caused issues, only the R123r thing works

for some reason, using other pre/suf-fixes made it run the while(Serial.available()) indefinently

oh yeah, and using Serial.available() caused issues

Issues, huh? Well, those are easily fixed. All you need to do is <Hidden, along with your “issues”>

Please use descriptive variable names, one letter variables are meanless, and with no comments either, you are really making life harder for everyone including yourself a few months down the road.

Love the idea of an “overflown integer”!!

Suggest set the baudrate higher if you can (115200 for instance), and yes, always use Serial.available() before any peek()/read() call, otherwise your code’s behaviour is undefined.

The variables are pretty self explanitory when you know the project ;) its an RGB controller so R G anf B are Red Green and Blue.

also, when i used the Serial.available(), i could get the values from the Rxxr pre/suf-fixed one, and it would get stuck inside the while(Serial.available()) loop

just taking a look at the code again now.

So here's the code with a Serial.available() while loop, if you would try it out, and see what it does, i would be really happy.

int rPin = 9;
int gPin = 10;
int bPin = 11;

int R;
int G;
int B;

char rPre = 'R';
char rSuf = 'r';
char gPre = 'G';
char gSuf = 'g';
char bPre = 'B';
char bSuf = 'b';

void setup()
{
  Serial.begin(9600);
  pinMode(rPin, OUTPUT);
  pinMode(gPin, OUTPUT);
  pinMode(bPin, OUTPUT);
}

long getSerial(char x, char z)
{
  long y = 0;
  boolean readSerial = false;
  while(Serial.available())
  {
    if(Serial.peek() == x)
    {
      Serial.read();
      readSerial = true;
    }
  
    while(readSerial)
    {
      if(Serial.peek() == z)
      {
        Serial.read();
        readSerial = false;
      }
      else
      {
        //Serial.println(Serial.peek());
        y = y*10 + Serial.read()-'0';
      }
    }
    return y;
  }
}

void loop()
{
   R = getSerial(rPre, rSuf);
   G = getSerial(gPre, gSuf);
   B = getSerial(bPre, bSuf);
  
  if(R != 0)
  {  
    Serial.print("Red value: ");
    Serial.println(R);
    Serial.println("--------------------");
  }
  if(G != 0)
  {
    Serial.print("Green value: ");
    Serial.println(G);
    Serial.println("--------------------");
  }
  if(B != 0)
  { 
    Serial.print("Blue value: ");
    Serial.println(B);
    Serial.println("--------------------");
  }
  /*
  analogWrite(rPin, R);
  analogWrite(gPin, G);
  analogWrite(bPin, B);
  */
  delay(1500);
}

So here’s the code with a Serial.available() while loop

The point with Serial.available() is that you need to call it before calling Serial.read() or Serial.peek(). It can be used to wait for a character…

while(Serial.available() == 0)
{
   // Do nothing but wait
}

So, instead of assuming that all the data you need is available, you should use this method to wait for each byte to become available.

So this gives me a large first value, and the next two values are correct. it Deffinently has something to do with time :confused:

int rPin = 9;
int gPin = 10;
int bPin = 11;

int R;
int G;
int B;

char rPre = 'R';
char rSuf = 'r';
char gPre = 'G';
char gSuf = 'g';
char bPre = 'B';
char bSuf = 'b';

void setup()
{
  Serial.begin(115200);
  pinMode(rPin, OUTPUT);
  pinMode(gPin, OUTPUT);
  pinMode(bPin, OUTPUT);
}

long getSerial(char x, char z)
{
  long y = 0;
  boolean readSerial = false;
    if(Serial.peek() == x)
    {
      Serial.read();
      readSerial = true;
    }
  
    while(readSerial)
    {
      if(Serial.peek() == z)
      {
        Serial.read();
        readSerial = false;
      }
      else
      {
        //Serial.println(Serial.peek());
        y = y*10 + Serial.read()-'0';
      }
    }
    return y;
}

void loop()
{
  while(!Serial.available())
  {}
    R = getSerial(rPre, rSuf);
    G = getSerial(gPre, gSuf);
    B = getSerial(bPre, bSuf);
  
    if(R != 0)
    {  
      Serial.print("Red value: ");
      Serial.println(R);
      Serial.println("--------------------");
    }
    if(G != 0)
    {
      Serial.print("Green value: ");
      Serial.println(G);
      Serial.println("--------------------");
    }
    if(B != 0)
    { 
      Serial.print("Blue value: ");
      Serial.println(B);
      Serial.println("--------------------");
    }
    /*
    analogWrite(rPin, R);
    analogWrite(gPin, G);
    analogWrite(bPin, B);
    */
}

You have to wait for each byte, or change the while loop in loop() to wait for all the characters.

Thanks a bunch :D the code now works awesome :D

now, do you know a nice place to ask for java help? :D nah, just gonna continue with learning it, then i can ask :D ;)

but thanks alot, will comment the code and throw it in here.

so far its here uncommented:

int rPin = 9;
int gPin = 10;
int bPin = 11;

int R;
int G;
int B;

char rPre = 'R';
char rSuf = 'r';
char gPre = 'G';
char gSuf = 'g';
char bPre = 'B';
char bSuf = 'b';

void setup()
{
  Serial.begin(115200);
  pinMode(rPin, OUTPUT);
  pinMode(gPin, OUTPUT);
  pinMode(bPin, OUTPUT);
}

long getSerial(char x, char z)
{
  long y = 0;
  boolean readSerial = false;
  while(!Serial.available())
  {Serial.println("1");}
    if(Serial.peek() == x)
    {
      Serial.read();
      readSerial = true;
    }
  
    while(readSerial)
    {
      while(!Serial.available())
      {Serial.println("2");}
      if(Serial.peek() == z)
      {
        Serial.read();
        readSerial = false;
      }
      else
      {
        //Serial.println(Serial.peek());
        y = y*10 + Serial.read()-'0';
      }
    }
    return y;
}

void loop()
{
  while(!Serial.available())
  {}
    R = getSerial(rPre, rSuf);
    G = getSerial(gPre, gSuf);
    B = getSerial(bPre, bSuf);
  
    if(R != 0)
    {  
      Serial.print("Red value: ");
      Serial.println(R);
      Serial.println("--------------------");
    }
    if(G != 0)
    {
      Serial.print("Green value: ");
      Serial.println(G);
      Serial.println("--------------------");
    }
    if(B != 0)
    { 
      Serial.print("Blue value: ");
      Serial.println(B);
      Serial.println("--------------------");
    }
    /*
    analogWrite(rPin, R);
    analogWrite(gPin, G);
    analogWrite(bPin, B);
    */
}
      while(!Serial.available())

Serial.available() returns the number of bytes available to read. It is not a boolean. It would be more correct to have:

while(Serial.available() == 0)

yeah i know, but i have seen this line used elsewhere multiple times, and it has worked every time for me, so i stick with it :)

Code:

while(!Serial.available())

Serial.available() returns the number of bytes available to read. It is not a boolean.

@PaulS: I don't see your point here.

Zero (false) /non-zero (true) is at the heart of C conventions, and was inherited from at least B.

The point is that code should be as clear as possible - people have to read it, not just compilers! Its especially good to be as clear as possible for newcomers to programming. Most languages don't conflate integer and boolean types - some day may want to port the code to another language, it helps to be as language-neutral...