Incrementing value / LED problem; causing lockup?

Hey, guys!

I'm a total noob, so please be gentle. I am having a problem that is totally dumbfounding me. I am using an Arduino Uno. I have a function written that cycles through various RGB values to send to a RGB LED. In this particular case, the trigger passes no value resulting in 'eyeColor' being incremented by 1 every time it is called. The problem is that it the entire program locks up when it hits case 8 where it uses the values in the assigned array. The serial printout shows me what I am supposed to see...but that's it. Any ideas? This may be painfully simple but I am beyond stumped. Please help. Thanks.

int ledRed=9;
int ledGreen=10;
int ledBlue=11;

int eyeColor;
int eyeColorMax=9;
int eyeRGB[3]={100,150,200};

void setup(){
  pinMode(ledRed,OUTPUT);
  pinMode(ledGreen,OUTPUT);
  pinMode(ledBlue,OUTPUT);
}

void loop(){
  /* contains code that calls 'functionEyeColor(0)' function */
}

void functionEyeColor(int value){
  if (value){
    eyeColor=value;
  } else {
    eyeColor++;
  }

  if (eyeColor>=eyeColorMax)
    eyeColor=0;

  switch(eyeColor){
    case 0:
      tempRed=0;
      tempGreen=0;
      tempBlue=0;
    break;
    case 1:
      tempRed=255;
      tempGreen=0;
      tempBlue=0;
    break;
    case 2:
      tempRed=0;
      tempGreen=255;
      tempBlue=0;
    break;
    case 3:
      tempRed=0;
      tempGreen=0;
      tempBlue=255;
    break;
    case 4:
      tempRed=255;
      tempGreen=255;
      tempBlue=0;
    break;
    case 5:
      tempRed=255;
      tempGreen=0;
      tempBlue=255;
    break;
    case 6:
      tempRed=0;
      tempGreen=255;
      tempBlue=255;
    break;
    case 7:
      tempRed=255;
      tempGreen=255;
      tempBlue=255;
    break;
    case 8: /* I have also tried hard coding the values below with the same result. */
      tempRed=eyeRGB[0];
      tempGreen=eyeRGB[1];
      tempBlue=eyeRGB[2]; 
    break;
  }

  analogWrite(ledRed,tempRed);
  analogWrite(ledGreen,tempGreen);
  analogWrite(ledBlue,tempBlue);

  Serial.print(F("\r\neyeColor: "));
  Serial.print(eyeColor);
  Serial.print(F("\ttempRed: "));
  Serial.print(tempRed);
  Serial.print(F("\ttempGreen: "));
  Serial.print(tempGreen);
  Serial.print(F("\ttempBlue: "));
  Serial.print(tempBlue);
}

This may be painfully simple but I am beyond stumped.

So are we.

void loop(){
  /* contains code that calls 'functionEyeColor(0)' function */
}

Clearly that comment is wrong, because loop() contains no such code.

Seriously?!?!? The code that calls the function is correct and works. It's just a simple if...then. I am not sure what part of that you needed to know. I didn't want to bore anybody with code that is not relevant. However, here is the part of the program in question.

#include <SPI.h>
#include <PS3BT.h>

USB Usb;
BTD Btd(&Usb);
PS3BT PS3(&Btd);

int ledRed=9;
int ledGreen=10;
int ledBlue=11;

int eyeColor;
int eyeColorMax=9;
int eyeRGB[3]={100,150,200};
int tempBlue;
int tempGreen;
int tempRed;

void functionEyeColor(int value){
  if (value){
    eyeColor=value;
  } else {
    eyeColor++;
  }

  if (eyeColor>=eyeColorMax)
    eyeColor=0;

  switch(eyeColor){
    case 0:
      tempRed=0;
      tempGreen=0;
      tempBlue=0;
    break;
    case 1:
      tempRed=255;
      tempGreen=0;
      tempBlue=0;
    break;
    case 2:
      tempRed=0;
      tempGreen=255;
      tempBlue=0;
    break;
    case 3:
      tempRed=0;
      tempGreen=0;
      tempBlue=255;
    break;
    case 4:
      tempRed=255;
      tempGreen=255;
      tempBlue=0;
    break;
    case 5:
      tempRed=255;
      tempGreen=0;
      tempBlue=255;
    break;
    case 6:
      tempRed=0;
      tempGreen=255;
      tempBlue=255;
    break;
    case 7:
      tempRed=255;
      tempGreen=255;
      tempBlue=255;
    break;
    case 8:
      tempRed=100;
      tempGreen=150;
      tempBlue=200;
    break;
  }

  analogWrite(ledRed,tempRed);
  analogWrite(ledGreen,tempGreen);
  analogWrite(ledBlue,tempBlue);

  Serial.print(F("\r\neyeColor: "));
  Serial.print(eyeColor);
  Serial.print(F("\ttempRed: "));
  Serial.print(tempRed);
  Serial.print(F("\ttempGreen: "));
  Serial.print(tempGreen);
  Serial.print(F("\ttempBlue: "));
  Serial.print(tempBlue);
}

void setup(){
  Serial.begin(115200);

  if (Usb.Init()==-1){
    Serial.print(F("\r\nOSC did not start"));
    while (1);
  }

  Serial.print(F("\r\nPS3 Bluetooth Library Started"));

  pinMode(ledRed,OUTPUT);
  pinMode(ledGreen,OUTPUT);
  pinMode(ledBlue,OUTPUT);
}

void loop(){
  Usb.Task();

  if (PS3.PS3Connected){
    if (PS3.getButtonClick(CIRCLE))
      functionEyeColor(0);
  }
}

right before your switch statement, do a serial print

Serial.println(eyeColor);

see what happens ....

tzankoff:
The problem is that it the entire program locks up when it hits case 8 where it uses the values in the assigned array.

I don't understand "locks up". Do you mean that you get no further output from your println statements at all? Or do you mean that it stays in state 8?

In the final output you see, are you seeing the colour you specify in your array (100,150,200) or are you seeing colour 7 (255,255,255)?

If you are seeing (100,150,200) getting printed out, then the problem is not in functionEyeColor() - something in that loop behaves badly. At a guess, you have an off-by-one bug relating to eyecolourmax holding the maximum value rather than a length.

What does it do if you replace the loop with this:

void loop(){
  Usb.Task();

  delay(250);
  functionEyeColor(0);
}

?

I didn't want to bore anybody with code that is not relevant.

You have yet to prove that the program "locks up" in functionEyeColor() (which is a stupid name for a function; it's apparent that its a function. What is does is not color an eye), so you are in no position to decide that is relevant.

tzankoff:
I didn't want to bore anybody with code that is not relevant.

We aren't easily bored. We are however easily enraged.

http://snippets-r-us.com/

Qdeathstar:
right before your switch statement, do a serial print
Serial.println(eyeColor);
see what happens ....

The value increments like it is supposed to. However, it still locks up at 8.

PaulMurrayCbr:
I don't understand "locks up". Do you mean that you get no further output from your println statements at all? Or do you mean that it stays in state 8?

It stays in 8 and the program stops responding to other button presses (which there is code for but none of which has any impact on this function. To make sure of that, I will remove all functions and put them back one at a time to see if their presence, or lack thereof, has any impact or not.).

PaulMurrayCbr:
In the final output you see, are you seeing the colour you specify in your array (100,150,200) or are you seeing colour 7 (255,255,255)?

That is another issue that I am having. For some reason, the LED itself is not behaving as I thought it would. The pinMode is setup correctly and I tried using a Fading LED sketch to make sure I had hooked up the wiring properly (which I did), but now my analogWrite statements do not seem to be having the appropriate effect. However, I should still be able to see appropriate serial monitor output which I do but for some reason, the conunter does not rest after 8 which is what I am trying to figure out.

PaulMurrayCbr:
If you are seeing (100,150,200) getting printed out, then the problem is not in functionEyeColor() - something in that loop behaves badly. At a guess, you have an off-by-one bug relating to eyecolourmax holding the maximum value rather than a length.

The 'eyeColorMax' variable is set to 9 and the code says if eyeColor>=eyeColorMax then reset to 0. I notice that if I set eyeColorMax to 8, it cycles through just fine (i.e: it goes to 7 then resets)

PaulMurrayCbr:
What does it do if you replace the loop with this:

void loop(){

Usb.Task();

delay(250);
 functionEyeColor(0);
}

How interesting. It cycles through all 8 and then resets...like it's supposed to.

PaulS:
You have yet to prove that the program "locks up" in functionEyeColor() (which is a stupid name for a function; it's apparent that its a function. What is does is not color an eye), so you are in no position to decide that is relevant.

I am not sure how I am supposed to prove something. That is that function that was called when the locking up (or freezing or whatever you want to call it) happens. As for the name, stupid or not, it's called that for a reason.

it's called that for a reason.

And that reason is?

I am not sure how I am supposed to prove something.

You add Serial.print() statements before and after the call. You add Serial.print() statements in the function. You show the Serial Monitor output.

tzankoff:
How interesting. It cycles through all 8 and then resets...like it's supposed to.

  switch(eyeColor){
    case 0:
    case 1:
    case 2:
    case 3:
    case 4:
    case 5:
    case 6:
    case 7:
    case 8:
  }

All nine, you mean?

PaulS:
And that reason is?

It's just a name. I'm not going to argue over it.

PaulS:
You add Serial.print() statements before and after the call. You add Serial.print() statements in the function. You show the Serial Monitor output.

BEFORE: 7 <-- eyeColor value before function is called
Increment: 8 <-- eyeColor value after being incremented (should equal BEFORE+1)
Reset?: 8 <-- if eyeColor >= eyeColorMax (9) then reset to 0...which is not the case here.
CaseDone: 8 <-- eyeColor value after going through case statements
LedSet: 8 <-- eyeColor value after analogWrite statements
eyeColor: 8 tempRed: 100 tempGreen: 150 tempBlue: 200 <--- final readout
AFTER: 8 <-- eyeColor value after exiting function

...and it got stuck again after this.

PaulMurrayCbr:
What does it do if you replace the loop with this:

void loop(){

Usb.Task();

delay(250);
 functionEyeColor(0);
}




How interesting. It cycles through all 8 and then resets...like it's supposed to.

[quote author=Nick Gammon date=1438204451 link=msg=2336291]


switch(eyeColor){
    case 0:
    case 1:
    case 2:
    case 3:
    case 4:
    case 5:
    case 6:
    case 7:
    case 8:
  }




All **nine**, you mean?

[/quote]

Whoops! Yes, all NINE of them.

Add to setup
Serial.begin( your baud rate )

Add to switch(eyeColor)

in case 0:

Serial.print(eyeColor);
delay( x seconds )

case 8:
after
default:
Serial.print(eyeColor);
delay( x seconds )

What do you see on COM port output?

switch(eyeColor){
case 0:
case 1:
case 2:
case 3:
case 4:
case 5:
case 6:
case 7:
case 8:
}

I'm having trouble believing the code you posted got stuck, so please post the code you are running.

tzankoff:
Seriously?!?!? The code that calls the function is correct and works. It's just a simple if...then. I am not sure what part of that you needed to know. I didn't want to bore anybody with code that is not relevant.

I suspect the boring part is the part with the issue.

I would have posted the entire code to begin with but it exceeds the 9000 character limit which is why I removed everything that I did not think had to do with this issue. However, the entire code can be found at http://www.tzankoff.com/arduino.txt.

Vaclav:
case 8:
after
default:
Serial.print(eyeColor);
delay( x seconds )

I'm not quite sure what you mean by "after default". Please clarify.

Add "default:" after case 8:

case 8:
{
...
break;
}
// add here
default:
{
... if the logic / flow is correct you should never get here

}
} end of switch

I would also add Serial.print to check "value" and "eyeColor" to make sure they are as you expect and get set / reset correctly.

Generally any app with input, most if not all, can benefit if you emulate the input and check the code flow instead of waiting for some external event.
You already using Serial so add some more to find this "lock - up" problem.

tzankoff:

What does it do if you replace the loop with this:

    void loop(){

Usb.Task();

delay(250);
      functionEyeColor(0);
    }

How interesting. It cycles through all 8 and then resets...like it's supposed to.

BINGO.

So this means that the problem isn't in your eyecolor function - it's somewhere in that slab of code that looks like this:

void loop(){
  /* contains code that calls 'functionEyeColor(0)' function */
}

And this is why people get annoyed when people post snippets and incomplete code. We are all looking at the code you posted, trying to work out whats wrong - and there is nothing wrong to find. The problem is somewhere else, somewhere in code that you didn't post here.

On a different note: this might show you something about how to debug code. Isolate the bits and check that those bits do what you think they should do.

Ok. Now that we have esatblished that your propblem is not in the eyecolor function, I would like you to take the original sketch (with its existing loop function) and replace the functionEyeColor function with this:

void functionEyeColor(int value){

  if(value < 0) {
    Serial.print(F("\r\nWTF? Someone is trying to set the value to "));
    Serial.print(value);
    Serial.print(F(" and I have no way of handling that!"));
  }

  if (value){
    eyeColor=value;
  } else {
    eyeColor++;
  }

  if (eyeColor>=eyeColorMax)
    eyeColor=0;

  switch(eyeColor){
    case 0:
      tempRed=0;
      tempGreen=0;
      tempBlue=0;
    break;
    case 1:
      tempRed=255;
      tempGreen=0;
      tempBlue=0;
    break;
    case 2:
      tempRed=0;
      tempGreen=255;
      tempBlue=0;
    break;
    case 3:
      tempRed=0;
      tempGreen=0;
      tempBlue=255;
    break;
    case 4:
      tempRed=255;
      tempGreen=255;
      tempBlue=0;
    break;
    case 5:
      tempRed=255;
      tempGreen=0;
      tempBlue=255;
    break;
    case 6:
      tempRed=0;
      tempGreen=255;
      tempBlue=255;
    break;
    case 7:
      tempRed=255;
      tempGreen=255;
      tempBlue=255;
    break;
    case 8: /* I have also tried hard coding the values below with the same result. */
      tempRed=eyeRGB[0];
      tempGreen=eyeRGB[1];
      tempBlue=eyeRGB[2];
    break;
  }

  analogWrite(ledRed,tempRed);
  analogWrite(ledGreen,tempGreen);
  analogWrite(ledBlue,tempBlue);

  Serial.print(F("\r\neyeColor: "));
  Serial.print(eyeColor);
  Serial.print(F("\ttempRed: "));
  Serial.print(tempRed);
  Serial.print(F("\ttempGreen: "));
  Serial.print(tempGreen);
  Serial.print(F("\ttempBlue: "));
  Serial.print(tempBlue);
}

Do you get any interesting serial output when you run your original sketch with this modified eyecolor function?