LED PMW & Millis() Help

Hi all,

I wrote a simple snippet of code to control and fade the cabin lights in my airplane. The idea is that when both doors are closed, the computer waits for 30 seconds and then fades the lights down. When the doors are opened the lights fade up immediately.

The program was easy to write and everything works great except for the part where the computer waits 30 seconds. I'm a big time newbie and have been spending hours trying to figure out what the problem is. These have been my attempts:

  1. My first code tried to use a delay() function which slowed the fade to a crawl, even when I tried to isolate the delay from the fade function.

  2. After much reading I tried various iterations of the millis() function. It either ends ups slowing down the fade to a crawl or crashing the whole thing so that the lights won't do anything.

I've tried while(), if(), multiple if()s and other things, all to no avail. Can someone please put me out of my misery and show me how to do it right? Thanks

We need to see your code.

This is what I have that works. I had already deleted my attempts to delay it because they just messed it up.

//Cabin dome lights
int pilotdoor = 4;
int copilotdoor = 5;
int domelight = 3;
int value = 0;
int fadeamount = 5;

void setup() {
  pinMode(pilotdoor, INPUT);
  pinMode(copilotdoor, INPUT);
  pinMode(domelight, OUTPUT);
 }

void loop(){
  int state = digitalRead(pilotdoor);
  int state2 = digitalRead(copilotdoor);
  if (state && state2 == HIGH)
  //If BOTH pilot & copilot doors are latched
  {
  dimLights(); //Calls the function to dim the lights
}
 else //If the doors aren't closed, or one or both is reopened...
{
  raiseLights(); //Calls the function to raise the lights via fade.
}}

void dimLights() {
    
    value --;//decreases the value of the light by one
    value = constrain(value, 0, 255);//constrains the value between 0 & 255
    analogWrite(domelight, value);//turns on the light and sets at "value"
    delay(10);}//waits 10 milisec before going thru the loop again

void raiseLights() {
    value ++;//increases the value of by one.
   value = constrain(value, 0, 255);
   analogWrite(domelight, value);
   delay(20);}

Moderator edit:
</mark> <mark>[code]</mark> <mark>

</mark> <mark>[/code]</mark> <mark>
tags added.

Here is one of the solutions I attempted:

void loop(){
  int state = digitalRead(pilotdoor);
  int state2 = digitalRead(copilotdoor);
  if (state && state2 == HIGH)
  //If BOTH pilot & copilot doors are latched
  {
    unsigned long time;
    time = millis();
    while((time+30000)>=millis())
    {}
  dimLights(); //Calls the function to dim the lights
}

Moderator edit:
</mark> <mark>[code]</mark> <mark>

</mark> <mark>[/code]</mark> <mark>
tags added.

How about inserting the delay here:

void loop(){ int state = digitalRead(pilotdoor); int state2 = digitalRead(copilotdoor); if (state && state2 == HIGH) //If BOTH pilot & copilot doors are latched { delay(30000); dimLights(); //Calls the function to dim the lights }

Would this do what you want? The only thing I can think of is that we haven't consider the case where you latch the doors and and then open one again before the 30seconds are up.

Thanks for responding.

That was one of the first things I tried. What it does its ends up slowing down the whole fade effect, just as if I had gone down to the dimlights function and increased the delay there.

Ok, so here is something new…

I added the timer to the code:

void loop(){
  int state = digitalRead(pilotdoor);
  int state2 = digitalRead(copilotdoor);
  if (state && state2 == HIGH)
  //If BOTH pilot & copilot doors are latched
  {
    unsigned long time;
    time = millis();
    while((time+5000)>=millis())
    {}
  dimLights(); //Calls the function to dim the lights
}

–And I changed the dimlights function:

void dimLights() {
    
  while(value!=0){  
  value --;//decreases the value of the light by one
    value = constrain(value, 0, 255);//constrains the value between 0 & 255
    analogWrite(domelight, value);//turns on the light and sets at "value"
    delay(10);}}//waits 10 milisec before going thru the loop again

–That seemed to do the trick! If both buttons are pushed the light waits 5 seconds and then dims normally. If one or both of the buttons is released, it raise normally. However, there is a gotchya. When the lights raise, they also wait 5 seconds! Below is the code for the raise lights:

void raiseLights() {
    value ++;//increases the value of by one.
   value = constrain(value, 0, 255);
   analogWrite(domelight, value);
   delay(20);}

Moderator edit: ditto.

I'm glad I've gotten the timer to work now, but I must still not be understanding something right. I played with the breadboard a little more. If I push the buttons once it locks the code into running the dimlights function. I added another qualifier to my code to have the while() function check the buttons as well. This didn't help.

while((state && state2 == HIGH)&&(time+5000)>=millis())

So basically now I have another problem that my timer is acting like a delay() anyways. Can someone explain a better way to utilize millis()? I've read lots of reference articles, searched the forums, etc., but can't seem to find the answer I'm looking for. I'm betting this is probably something simple that I'm just not grasping...

Any help would be appreciated.

This:

    time = millis();
    while((time+5000)>=millis())
    {}

is just a longer way of writing this: delay(5000);

You don't seem to understand the blink without delay example.

I agree. I did more reading, played around with the examples even further and came up with this:

unsigned long currentMillis = millis();
if(currentMillis<(millis()-5000)){
dimLights();

This didn’t work. I’ve tried keeping it simple, just sketching an example where I push a button and the computer waits 5 seconds, then fades the LED (NOT using delay()).

int button = 5;
int light = 3;
int value = 0;
int fadeamount = 5;

void setup() {
  pinMode(button, INPUT);
  pinMode(light, OUTPUT);
}

void loop(){
  int state = digitalRead(button);
  if(state == HIGH){
    unsigned long currentMillis = millis();
    if(currentMillis < (millis()-5000)){
      while(value!=0){
        value --;
        value = constrain(value, 0, 255);
        analogWrite(light, value);
        delay(10);}
    }
  }
  else{
    digitalWrite(light, HIGH);
  }
}

This still does not work and I think I know why. I think the problem is that when I push the button, the “currentmillis” is set to the current millis(), every single time it loops. But how do I get around that? I need to mark when the button was pushed so that the Arduino can know when 5 seconds have expired. If I set the current millis at the beginning of the loop (before the “IF” statement) wouldn’t that do the exact same thing?

This still does not work and I think I know why. I think the problem is that when I push the button, the "currentmillis" is set to the current millis(), every single time it loops.

Yes, it is.

But how do I get around that?

You need to write down how YOU would perform the task that you want the Arduino to perform. You have a watch, a pad of paper and a pencil. When you can define HOW the Arduino is to perform the task, translating that into code is nearly trivial.

Forget about the code part for now. Just list the steps YOU would perform.

I give up. Can someone please give me a straight answer on how to handle the millis() function and button pushes? I've been trying to figure this out and keep hitting dead ends. I'd like to know so I can learn.

I went back to basics...I mean really basic. I wrote a code to turn on the light when I push the button. Everytime I try to add something to bring the timer element to it it doesn't work. Clearly, I am not understanding the blink with out delay example and so on. How do I do this????

Firstly you need to get clear in your head what states your program needs to be in and what conditions switch between the various states.

States are things like “waiting 30secs”, “fading lights down”, “fading lights up”, “normal case”. Conditions are things like the doors opening and closing, 30 seconds having elapsed, lights finishing fading.

In loop you need to look at the current state, and depending on what it is look for any pertinent conditions that could cause the state to change - then you change the state if appropriate. Use a variable to represent the state by name (define named states with enum or #defines). On entering or leaving a state there may be associated actions to be performed - use a function call for each action.

When changing to a state that is waiting, you need to record the current time into an “unsigned long” variable specific to that wait (so that the new state can test for that wait being over).

There are usually lots of boring cases to consider, you need to list them all and check you’ve covered every case. This technique is called “implementing a state machine”.

And lastly when checking to see if a wait has finished, use tests of this form:

  if (millis () - wait_starttime >= WAIT_DURATION)
  {
    ..
  }

It must be like this because millis() returns unsigned values and can wrap round. So long as WAIT_DURATION is less than 2^32 this test will always work correctly.

It you write the test like this:

  if (millis () >= wait_starttime + WAIT_DURATION)
  {
    ..
  }

Then it won’t always work - in fact it will fail every 50 days or so in bizarre ways because the addition overflows. (or every 72 minutes if using micros() instead of millis())

CStence: Clearly, I am not understanding the blink with out delay example and so on. How do I do this????

http://www.gammon.com.au/blink

Guys thanks so much for your candid responses. This gives me a bit to chew on!

The reference material was very helpful. The sticking point I keep running across is that I can't seem to resolve how to start a timer when a button is pushed without running into a paradox. In loop, I monitor the state of a button. When the button is pushed I make some state equal the millis() (so I can later subtract it from millis() and see if it has been long enough). I inevitably run into the paradox that just as soon as loop sees that the button is HIGH and "sets" the timer to millis(), it will keep on setting it to millis() every time it loops. If I put the function outside of loop (ie gotoTimer) it will still keep resetting it. I was thinking maybe I could write an if statement that would somehow tell when the button was first pushed and then not reset the timer after the initial loop when it found that the button was high:

if(state = high)&&(millis() - timesincebuttonpush != 0){
timesincebuttonpush = millis()

//but then I don't know how many milliseconds it might be until the loop comes around...that could mess it up.

//or maybe this? >>>

if (state = high && value == 0){
     value++;
     timesincebuttonpush = millis();

How confusing. Does anyone have a link to some code where I can see how someone else has done this so I can learn from seeing?

Below is something I tried but I stopped once I realized I was facing the same paradox.

int light = 3
int button = 4
 unsigned long howlongsincebuttonpush
 unsigned long buttonPushwait = 5000UL


void setup(){
 pinMode (light, OUTPUT);
 pinMode (button, INPUT);
 howlongsincebuttonpush = millis();
 digitalWrite(light, HIGH);
}



void loop(){
  int state = digitalRead(button);
  if (state = HIGH){
  gotoTimer();
  }
}

gotoTimer(){
  howlongsincebuttonpush = millis();
  if (millis() - howlongsincebuttonpush) >= buttonPushwait)
   dimlight();
}

dimlight(){
  howlongsincebuttonpush = millis();
  while(value != 0)
    value --;//decreases the value of the light by one
    value = constrain(value, 0, 255);//constrains the value between 0 & 255
    analogWrite(light, value);//turns on the light and sets at "value"
    delay(10);
    
}[code/]
void loop(){
  int state = digitalRead(button);
  if (state = HIGH){
  gotoTimer();
  }
}

First, exorcise the word “goto” from your mind. You don’t need it.

Second, this:

if (state = HIGH)

That sets state to HIGH. It doesn’t test it. So it will definitely be high. You mean:

if (state == HIGH)

I inevitably run into the paradox that just as soon as loop sees that the button is HIGH and “sets” the timer to millis(), it will keep on setting it to millis() every time it loops.

You need to detect a transition. Something like:

if (state == HIGH && prevState == LOW)

So you know that the state is now HIGH, but last time around it was LOW.

Nick, thanks for the reminder on the == sign. I had just posted something before I saw you had responded...my code was failing for that very reason. Below is the code I wrote that seems to do what I want minus two things: if I tap the button, it counts during each of my taps. Is there a better way to write this? Nick, I'll mull over what you said "if (state == HIGH && prevState == LOW)." The other thing I haven't squared away is the PWM stuff to dim the light instead of just turning it off. I tried to use the while function but it isn't working like I wanted.

Otherwise it basically does the right thing. I push the button and hold for 5 seconds and the light goes out. If I let up at any point it will turn back on immediately.

int light = 3;
int button = 4;
 unsigned long howlongsincebuttonpush;
 unsigned long buttonPushwait = 5000UL;
 int timervalue;
 int value;


void setup(){
 pinMode (light, OUTPUT);
 pinMode (button, INPUT);
 howlongsincebuttonpush = millis();
 digitalWrite(light, HIGH);
 int timervalue = 0;
}



void loop(){
  
  int state = digitalRead(button);
  if (state == HIGH && timervalue == 0){
    timervalue++;
    howlongsincebuttonpush = millis();
  }
  if (state == HIGH && timervalue != 0){
   startTimer();
    }
  if (state == LOW){
    digitalWrite(light, HIGH);
  }
}

void startTimer(){
  
  if ((millis() - howlongsincebuttonpush) >= buttonPushwait){
   dimlight();
  }
}

void dimlight(){
  timervalue = 0;
  digitalWrite (light, LOW);
  /*while(value != 0){
    value --;//decreases the value of the light by one
    value = constrain(value, 0, 255);//constrains the value between 0 & 255
    analogWrite(light, value);//turns on the light and sets at "value"
    delay(10);
  }*/
    
}
void setup(){
 pinMode (light, OUTPUT);
 pinMode (button, INPUT);
 howlongsincebuttonpush = millis();
 digitalWrite(light, HIGH);
 int timervalue = 0;
}

timervalue is a local variable that goes out of scope immediately. How useful is that?

You are still not detecting transitions (the switch is pressed now but wasn't last time (just pressed) or the switch is not pressed now but was last time (just released)). You will not get your code to do what you want until you do.

// Globals
int currState;
int prevState = LOW;

In loop:

  currState = digitalRead(button); // buttons are for shirts; I'm positive you are not using a button
  if(currState != prevState)
  {
    // A transition occurred. Is it interesting?
    if(currState == HIGH)
    {
       // Switch was just pressed
    }
    else
    {
       // Switch was just released
    }
  }
  prevState = currState;

You are not using the internal pullup resistors. Therefore, you must have external ones. Do you? How ARE the switches wired? Are we chasing a hardware problem or a software problem?

Paul, thanks for the constructive feedback. Regarding your last question, yes it was/is a software issue. If you saw my last post you've seen that I was able to come up with a way to basically do what I want. However, I'm intrigued by what you said regarding the switch state. I plan on playing with that a little more to see if I can write the code that way, instead of the way I have.