code only works when Serial.println() is present..??

Hey guys, i have a section of may code that only works if Serial.println() is in the code.
I tossed it in there to debug because it wasn't working, and for some reason having it in there causes it to work.

any ideas on why?

code is attached.

Slave_Nano.ino (2.71 KB)

code is attached.

Why? The code was small enough to post:

#include <Wire.h>

int red = 10;
int green = 9; 
int blue = 5; 
byte fromMaster[4];
int toLights[4];
int val;



void setup() {
  pinMode(red,OUTPUT);
  pinMode(green,OUTPUT);
  pinMode(blue,OUTPUT);
  Wire.begin(8);
  Wire.onReceive(receiveEvent);
  Serial.begin(9600);
}

void loop() {
  powerOff:
  while(toLights[0]==0){
    analogWrite(red,255);
    analogWrite(green,255);
    analogWrite(blue,255);
  }
  
  while(toLights[1]==0){
    if(toLights[1]==1){
      goto standard;
    }
    for(int i=0; i<=255; i++){
      analogWrite(red,i);
      analogWrite(green,255-i);
      analogWrite(blue,255);
      Serial.println("#1");
      Serial.println(toLights[1]);
      if(toLights[1]==1){
        goto standard;
      }
      else if(toLights[0]==0){
        goto powerOff;
      }
      delay(25);
    }
    for(int i=0; i<=255; i++){
      analogWrite(green,i);
      analogWrite(blue,255-i);
      analogWrite(red,255);
      Serial.println("#2");
      Serial.println(toLights[1]);   
        if(toLights[1]==1){
        goto standard;
        }
        else if(toLights[0]==0){
        goto powerOff;
        }
      delay(25);
    }
    for(int i=0; i<=255; i++){
      analogWrite(blue,i);
      analogWrite(red,255-i);
      analogWrite(green,255);
      Serial.println("#3");
      Serial.println(toLights[1]);
      if(toLights[1]==1){
        goto standard;
      }
      else if(toLights[0]==0){
        goto powerOff;
      }
      delay(25);
    }
  }

  standard:

  
  
  if(toLights[2]<255){
   analogWrite(red,toLights[2]);
   analogWrite(green,255-toLights[2]);
   analogWrite(blue,255);
   delay(toLights[3]);
   analogWrite(red,255);
   analogWrite(green,255);
   analogWrite(blue,255);
   delay(toLights[3]);
   if(toLights[0]==0){
     goto powerOff;   
   }
  }
  else if(toLights[2]>=255&&toLights[2]<510){
    toLights[2]=map(toLights[2],255,509,0,255);
    analogWrite(red,255);
    analogWrite(green,toLights[2]);
    analogWrite(blue,255-toLights[2]);
    delay(toLights[3]);
    analogWrite(red,255);
    analogWrite(green,255);
    analogWrite(blue,255);
    delay(toLights[3]);
    if(toLights[0]==0){
      goto powerOff;    
   }
  } 
  else if(toLights[2]>=510){
    toLights[2]=map(toLights[2],510,765,0,255);
    analogWrite(green,255);
    analogWrite(blue,toLights[2]);
    analogWrite(red,255-toLights[2]);
    delay(toLights[3]);
    analogWrite(red,255);
    analogWrite(green,255);
    analogWrite(blue,255);
    delay(toLights[3]);
    if(toLights[0]==0){
      goto powerOff;
   }
  } 
   
}

void receiveEvent(){
  while (Wire.available()){
    fromMaster[val]=Wire.read();
    val++;
    }
    val =0;
  for(int i=0; i<4; i++){
      toLights[i]=int(fromMaster[i]);
    }
    Serial.println(toLights[2]);
   
    
  }
byte fromMaster[4];
int toLights[4];

Why is toLights an int array? Since it is valued with bytes read from some device, it makes no sense to type it int.

      toLights[i]=int(fromMaster[i]);

The compiler is perfectly capable of storing a byte in an int element without your ham-fisted "help".

      goto standard;

What is this horsecrap? There is NO need for labels and gotos in properly written code.

Your while() has no exit condition also.

easy guys, super new to coding. not aware of all the subtle nuances of this yet.
and because i am receiving information over I2c, i assumed that it had to be translated back into int because I2c is byte transmission.

secondly, sorry about the attachment. didnt realize that it had to be downloaded, thought it just uploaded it. my bad.

the while() exit conditions are the if statements, i get it, its crap code...still learning, function over form for the moment.
and those if() statements exit the while() to the ever-so-hated goto locations. but only when the Serial.println() is in there.

If code starts behaving/misbehaving when you add or remove something 'innocent', it usually indicates memory issues.

Often writing outside the bounds of an array can be the cause, use of String (capital S) can also be the cause.

Can't look at your code as I'm using a cell phone.

Bjjohnson3782:
i have a section of may code

Which section?

Bjjohnson3782:
for some reason having it in there causes it to work.

Describe exactly what happens when you remove the Serial.println().

  while (Wire.available()) {
    fromMaster[val] = Wire.read();
    val++;
  }

Are you sure this will never write more than 4 times to fromMaster?

pert:
Which section?

while(toLights[1]==0){
if(toLights[1]==1){
goto standard;
}
for(int i=0; i<=255; i++){
analogWrite(red,i);
analogWrite(green,255-i);
analogWrite(blue,255);
Serial.println("#1");
Serial.println(toLights[1]);
if(toLights[1]==1){
goto standard;
}
else if(toLights[0]==0){
goto powerOff;
}
delay(25);
}
for(int i=0; i<=255; i++){
analogWrite(green,i);
analogWrite(blue,255-i);
analogWrite(red,255);
Serial.println("#2");
Serial.println(toLights[1]);
if(toLights[1]==1){
goto standard;
}
else if(toLights[0]==0){
goto powerOff;
}
delay(25);
}
for(int i=0; i<=255; i++){
analogWrite(blue,i);
analogWrite(red,255-i);
analogWrite(green,255);
Serial.println("#3");
Serial.println(toLights[1]);
if(toLights[1]==1){
goto standard;
}
else if(toLights[0]==0){
goto powerOff;
}
delay(25);
}
}

this section.

and when the serial is not there the board will enter the color loop (the section of code above mentioned), but will not exit it, and it will not call the powerOff.

when it is in the code, the color loop works fine, enters and exits without an issue, and the powerOff works as well.

I fixed some declarations and changed the code to not need 'goto'. The 'powerOff:' label was at the top of the loop() function so just doing a 'return;' does almost the same. The 'standard:' label was just after a bunch of 'for' loops. Breaking out of each loop gives the same effect.

In the receive event handler any input over 4 characters would overflow the buffer. Fixed that.

'toLights' can be changed in the receive event handler but was not marked 'volatile' so the compiler might assume that the variable only had to be fetched once.

#include <Wire.h>


const byte red = 10;
const byte green = 9;
const byte blue = 5;


volatile int toLights[4] = {0, 0, 0, 0};


void setup()
{
  pinMode(red, OUTPUT);
  pinMode(green, OUTPUT);
  pinMode(blue, OUTPUT);
  Wire.begin(8);
  Wire.onReceive(receiveEvent);
  Serial.begin(9600);
}


void loop()
{
  while (toLights[0] == 0)
  {
    analogWrite(red, 255);
    analogWrite(green, 255);
    analogWrite(blue, 255);
  }


  while (toLights[1] == 0)
  {
    for (int i = 0; i <= 255; i++)
    {
      if (toLights[1] == 1)
        break;


      analogWrite(red, i);
      analogWrite(green, 255 - i);
      analogWrite(blue, 255);
      Serial.println("#1");
      Serial.println(toLights[1]);


      if (toLights[0] == 0)
        return;


      delay(25);
    }


    for (int i = 0; i <= 255; i++)
    {
      if (toLights[1] == 1)
        break;


      analogWrite(green, i);
      analogWrite(blue, 255 - i);
      analogWrite(red, 255);
      Serial.println("#2");
      Serial.println(toLights[1]);


      if (toLights[0] == 0)
        return;


      delay(25);
    }


    for (int i = 0; i <= 255; i++)
    {
      if (toLights[1] == 1)
        break;


      analogWrite(blue, i);
      analogWrite(red, 255 - i);
      analogWrite(green, 255);
      Serial.println("#3");
      Serial.println(toLights[1]);


      if (toLights[0] == 0)
        return;


      delay(25);
    }
  }


  if (toLights[2] < 255)
  {
    analogWrite(red, toLights[2]);
    analogWrite(green, 255 - toLights[2]);
    analogWrite(blue, 255);
    delay(toLights[3]);
    analogWrite(red, 255);
    analogWrite(green, 255);
    analogWrite(blue, 255);
    delay(toLights[3]);
    if (toLights[0] == 0)
      return;
  }
  else if (toLights[2] >= 255 && toLights[2] < 510)
  {
    toLights[2] = map(toLights[2], 255, 509, 0, 255);
    analogWrite(red, 255);
    analogWrite(green, toLights[2]);
    analogWrite(blue, 255 - toLights[2]);
    delay(toLights[3]);
    analogWrite(red, 255);
    analogWrite(green, 255);
    analogWrite(blue, 255);
    delay(toLights[3]);
    if (toLights[0] == 0)
      return;
  }
  else if (toLights[2] >= 510)
  {
    toLights[2] = map(toLights[2], 510, 765, 0, 255);
    analogWrite(green, 255);
    analogWrite(blue, toLights[2]);
    analogWrite(red, 255 - toLights[2]);
    delay(toLights[3]);
    analogWrite(red, 255);
    analogWrite(green, 255);
    analogWrite(blue, 255);
    delay(toLights[3]);
    if (toLights[0] == 0)
      return;
  }
}


void receiveEvent(int count)
{
  int i = 0;
  while (Wire.available())
  {
    byte received = Wire.read();
    if (i < 4)  // Don't write past the end of the array
      toLights[i] = received;
    i++;
  }


  Serial.println(toLights[2]);
}

i assumed that it had to be translated back into int because I2c is byte transmission.

I2C IS byte transmission. The values being sent are bytes. There is no reason to waste memory storing bytes in ints.

PaulS:
I2C IS byte transmission. The values being sent are bytes. There is no reason to waste memory storing bytes in ints.

your right. not sure why that didn't register. overthinking it i guess.

Bjjohnson3782:

pert:
Which section?

johnwasser:
I fixed some declarations and changed the code to not need 'goto'. The 'powerOff:' label was at the top of the loop() function so just doing a 'return;' does almost the same. The 'standard:' label was just after a bunch of 'for' loops. Breaking out of each loop gives the same effect.

In the receive event handler any input over 4 characters would overflow the buffer. Fixed that.

'toLights' can be changed in the receive event handler but was not marked 'volatile' so the compiler might assume that the variable only had to be fetched once.

thank you very much for this, this helps resolve many of the other issues i was having, and gives me some insight into how to better write from here on out. I appreciate it!!

There seems to be some confusion about the values in toLights[2]. It is initialized from an unsigned byte fetched from Wire but it is tested for values over 255:

  else if (toLights[2] >= 255 && toLights[2] < 510) {

How is toLights[2] supposed to get a value over 255 (the largest value a 'byte' can hold)?

johnwasser:
There seems to be some confusion about the values in toLights[2]. It is initialized from an unsigned byte fetched from Wire but it is tested for values over 255:

  else if (toLights[2] >= 255 && toLights[2] < 510) {

How is toLights[2] supposed to get a value over 255 (the largest value a 'byte' can hold)?

solid question, and to answer that, it cant. at least not like this.

im revising the code so that rather than what you see, it will be a switch case, where toLights[2] =1,2 or 3(case 1,2,3.) and i will be adding in a toLights[4] that will send a value that is betweeen 0 and 255

Hello Bjjohnson3782

Bjjohnson3782:
i have a section of may code that only works if Serial.println() is in the code.

In the original code : Which Serial.print makes the code working or not working ?

Regards,
bidouilleelec