Variable value not changing

Using Arduino Programming Questions
#1

In this function, I try to gradually light a set of LED eyes from being off up to full brightness. Once they reach full brightness, I want them to flash on and off a few times. You can see from my “Serial.print” commands that I’ve been trying to figure this out.

Is there a problem with mixing variable types? My delayTime variable I called a byte, and I add it to lastTime, which is unsigned long.

I print the value of variable “eyeBrightness” to the serial monitor, and it’s always 0.
I also print the value of variable “lastTime,” and it seems to be updating properly.

I really have no clue why not – if it’s a scope problem, or variable type problem, or syntax. I’ve been trying to fix this for a while and hope someone can spot a problem.

The “idleBeast” function is just a bunch of digitalWrite(LOW) commands to turn off all the outputs before I start this sequence. Sometimes I treat this PWM pin as a digital output and sometimes as an analog, so I am mixing up digitalWrite and analogWrite to that pin. I’m not sure if that can cause problems or not.

[color=#CC6600]void[/color] throbTheEyes()
{
  [color=#CC6600][b]Serial[/b][/color].[color=#CC6600]println[/color]([color=#006699]"You are at the start of throbTheEyes function"[/color]);
  idleBeast();    [color=#7E7E7E]//This function includes a delay to give time to release button[/color]
  [color=#CC6600][b]Serial[/b][/color].[color=#CC6600]println[/color]([color=#006699]"Still in throb function, idleBeast was called and you have returned."[/color]);
      [color=#7E7E7E]//Next are some variables to track time.[/color]
  [color=#CC6600]unsigned[/color] [color=#CC6600]long[/color] lastTime=[color=#CC6600]millis[/color]();
  [color=#CC6600][b]Serial[/b][/color].[color=#CC6600]print[/color]([color=#006699]"Millis value is "[/color]);
  [color=#CC6600][b]Serial[/b][/color].[color=#CC6600]println[/color](lastTime);
  
  [color=#CC6600]byte[/color] delayTime=14;    [color=#7E7E7E]//ms to wait before brightening eyes[/color]
      [color=#7E7E7E]//Next is a variable to track brightness of eyes[/color]
  [color=#CC6600]byte[/color] eyeBrightness=0;
  
  [color=#CC6600]int[/color] blinkTimer=1250;    [color=#7E7E7E]//When eyes blink, 1250 milliseconds on, 1250 ms off[/color]
  
      [color=#7E7E7E]//What follows is a loop to run the eye throbbing sequence.[/color]
  [color=#CC6600]do[/color]{  [color=#7E7E7E]//Starts infinite loop.[/color]
    [color=#CC6600][b]Serial[/b][/color].[color=#CC6600]println[/color]([color=#006699]"You made it to the do loop."[/color]);
    [color=#CC6600][b]Serial[/b][/color].[color=#CC6600]print[/color]([color=#006699]"eyeBrightness variable is "[/color]);
    [color=#CC6600][b]Serial[/b][/color].[color=#CC6600]println[/color](eyeBrightness);
    [color=#CC6600][b]Serial[/b][/color].[color=#CC6600]print[/color]([color=#006699]"lastTime variable is "[/color]);
    [color=#CC6600][b]Serial[/b][/color].[color=#CC6600]println[/color](lastTime);
    lastTime=[color=#CC6600]millis[/color]();      [color=#7E7E7E]//updates the last time we changed eyes[/color]
    checkInputs();
    
    [color=#CC6600]if[/color] (eyeBrightness<255 && [color=#CC6600]millis[/color]()>=lastTime + delayTime)  [color=#7E7E7E]//1st stage is for brightening the eyes.[/color]
      {
      [color=#CC6600]analogWrite[/color](LEDpin,eyeBrightness);
      lastTime=[color=#CC6600]millis[/color]();
      eyeBrightness=eyeBrightness++;
      }
    
    [color=#CC6600]if[/color] (eyeBrightness==255){
          lastTime=[color=#CC6600]millis[/color]();
          [color=#CC6600]byte[/color] blinkCounter=0;
          [color=#CC6600]digitalWrite[/color](LEDpin,[color=#006699]HIGH[/color]);
          eyesLit=[color=#CC6600]true[/color];
          [color=#CC6600]do[/color]{
              checkInputs();    [color=#7E7E7E]//Checks to see if a button is pressed on remote.[/color]
              
              [color=#CC6600]if[/color] ([color=#CC6600]millis[/color]()>=lastTime+blinkTimer && eyesLit==[color=#CC6600]true[/color]){
                [color=#CC6600]digitalWrite[/color](LEDpin,[color=#006699]LOW[/color]);
                eyesLit=[color=#CC6600]false[/color];
                lastTime=[color=#CC6600]millis[/color]();
                blinkCounter++;
              }
              
              [color=#CC6600]if[/color] ([color=#CC6600]millis[/color]()>=lastTime+blinkTimer && eyesLit==[color=#CC6600]false[/color]){
                [color=#CC6600]digitalWrite[/color](LEDpin,[color=#006699]HIGH[/color]);
                eyesLit=[color=#CC6600]true[/color];
                lastTime=[color=#CC6600]millis[/color]();
                blinkCounter++;
              }
          } [color=#CC6600]while[/color] (blinkCounter<8);
          
          eyeBrightness=0;    [color=#7E7E7E]//Reset variable to work right in the brightening sequence.[/color]
    }
  } [color=#CC6600]while[/color](alwaysTrue==[color=#CC6600]true[/color]); [color=#7E7E7E]//end of infinite loop[/color]
}
#2

First of all, a few pointers. To include code, use the code block notation in the future. Consider the next line:

eyeBrightness=eyeBrightness++;

This is not the way to use the ++ operator. Just use

eyeBrightness++;

Also note that while(alwaysTrue==true); can be written as while(true);

The real cause of the error is this line:

lastTime=millis();  

It is inside the loop, and therefore the following IF statement is never true. You could have spotted this error by adding your Serial.Println statements at the correct level.

#3

RobvdVeer:
First of all, a few pointers. Consider the next line:

This is not the way to use the ++ operator. Just use

eyeBrightness++;

I had just come back to edit my post, since just after posting, I figured out the extra "lastTime=millis" was probably causing the problem. Between fixing the syntax for the eyeBrightness++ and that, it now works the way that I want it to! Thanks!!!

Also note that while(alwaysTrue==true); can be written as while(true);

Right now I have a global variable in the program that is

const boolean alwaysTrue = true;

So you're saying that I don't need a variable at all for that? Good to know!

The real cause of the error is this line:

lastTime=millis(); 

It is inside the loop, and therefore the following IF statement is never true. You could have spotted this error by adding your Serial.Println statements at the correct level.

As I said, I had just figured this out about 5 minutes after posting. I had to fix that line && fix the eyeBrightness++ though to get it to work, so I actually had two critical errors.

Thanks so much for your help!

#4

You're most welcome.

#5

Buffalo_Chip:

Also note that while(alwaysTrue==true); can be written as while(true);

Right now I have a global variable in the program that is

const boolean alwaysTrue = true;

So you're saying that I don't need a variable at all for that? Good to know!

Well, let's not jump all the fences at once. If you never need to change alwaysTrue to false then you don't need a global variable. But if you need to be able to change it to break out of the WHILE then you do need a variable. The code could still be simplified a little to while (alwaysTrue) {

...R

#6

Well, let's not jump all the fences at once. If you never need to change alwaysTrue to false then you don't

. . . want to be calling the variable "alwaysTrue". :wink:

#7

Robin2:
Well, let's not jump all the fences at once. If you never need to change alwaysTrue to false then you don't need a global variable. But if you need to be able to change it to break out of the WHILE then you do need a variable. The code could still be simplified a little to while (alwaysTrue) {

...R

While you're at it, prepare your program for every future change you'll ever be needing. ]:smiley:
Keep it simple, do not try to overengineer your piece of code. That's what Java enterprise architects do. This is embedded programming. Use only what you need, when you need it. Just do while(true) and change it when you need it.

Especially DONT call it alwaysTrue when a) it's a non constant variable and b) when it isn't. This will cause only confusion. If you must, change the name to 'keepRunning'.

#8

RobvdVeer:
Especially DONT call it alwaysTrue when a) it's a non constant variable and b) when it isn't. This will cause only confusion. If you must, change the name to 'keepRunning'.

I agree completely. But based on my experience of many other Threads here I was concerned that the OP might not really mean it to behave like its name suggested. :slight_smile:

...R

#9

May i suggest 'alwaysTrueExceptWhenItIsnt'

#10