Millis...alternate flash

Question

I thought I had the Millis() command sorted but i'm stumped on the one. Aim is to get 2 led's alternately dimming, they work great with delay(); of course....but i can't figure where i'm going wrong with Millis(). Please have a look and point out what i've got wrong. Thanks Doug

int LED3 = 5;
int LED4 = 6;

// each "event" (LED) gets their own tracking variable
unsigned long previousMillisLED3=0;
unsigned long previousMillisLED4=0; 

int intervalLED3 = 500;
int intervalLED4 = 500;

void setup() {
    pinMode(LED3, OUTPUT);
    pinMode(LED4, OUTPUT);

      digitalWrite(LED3,LOW);
      digitalWrite(LED4,LOW);

}

void loop() {
  //unsigned long currentMillis = millis();
      for(int P=0;P<6;P++){
        //if ((unsigned long)(currentMillis - previousMillisLED3) >= intervalLED3) {
            analogWrite(LED3, 127);
            analogWrite(LED4, 254);             
        delay(500);
              //previousMillisLED3 = currentMillis;

      //if ((unsigned long)(currentMillis - previousMillisLED4) >= intervalLED4) {
            analogWrite(LED3, 254);
            analogWrite(LED4, 127);
       delay(500); 
             //previousMillisLED4 = currentMillis;
      }    
}

Sorry, problems inputting code, but its all there.
I realize that there are some closing brackets missing in the LOOP() section for millis but it is set for delay() to show that it works.

Answer 1

Well, the for loop is useless. Then you need to plan your time cycle. For instance what you want is

time(ms)  LED3  LED4
   0       127   254
 500       254   127
1000 -> back to 0

So implement this. One way can be the one suggested by st2000:

int LED3 = 5;
int LED4 = 6;

// just one tracking, since there is only one sequence
unsigned long previousMillis=0;

int endOfPhase1 = 500;
int endOfPhase2 = 1000;

void setup() {
    pinMode(LED3, OUTPUT);
    pinMode(LED4, OUTPUT);
    digitalWrite(LED3,LOW);
    digitalWrite(LED4,LOW);
}

void loop() {
    unsigned long currentMillis = millis();

    if ((currentMillis - previousMillis) < endOfPhase1) {
        analogWrite(LED3, 127);
        analogWrite(LED4, 254);
    }
    else if ((currentMillis - previousMillis) < endOfPhase2) {
        analogWrite(LED3, 254);
        analogWrite(LED4, 127);
    }
    else
        previousMillis += endOfPhase2;
}

Just a small note about this: I tend to prefer summing the period instead of setting previousMillis to millis because this way small errors do not accumulate.

This way is the simplest, but the state machine oriented is the one I suggest you, since it is more powerful and you will reuse this a lot of times. Moreover it will detect the "change" of state, thus avoiding the continuous analogWrite (in the previous code the analogWrite instructions are executed a lot of times).

Just define a state machine as two states (state 1, where LED3 is 127 and LED4 is 254, and state 2, where LED3 is 254 and LED5 is 127). The transitions will be after 500ms in each state, but I'll make two separate constants so you can trim them later. Usually each state has a function to be performed when you enter the state, a function to run continuously, a function when you exit and one or more transitions. The way to perform this is

1. calculate if there is a state change
2. if there is, perform the exit functions of the previous state and the enter functions of the next state
3. advance to the new state
4. perform the actions to be done continuously in the new state

In our case just state transition, enter and advance should be implemented. Usually you use a switch case on a variable holding an index for the state. In this case, for instance,

int LED3 = 5;
int LED4 = 6;

// just one tracking, since there is only one sequence
unsigned long previousMillis=0;

int state1duration = 500;
int state2duration = 500;

uint8_t state;

void setup() {
    pinMode(LED3, OUTPUT);
    pinMode(LED4, OUTPUT);
    digitalWrite(LED3,LOW);
    digitalWrite(LED4,LOW);

    state = -1; // Invalid state, used to force a transition at startup
}

void loop() {
    unsigned long currentMillis = millis();

    // Check the transition
    uint8_t nextState = state;
    switch (state)
    {
    case 1: // Transition for state 1
        if ((currentMillis - previousMillis) >= state1duration)
        {
            nextState = 2;
            previousMillis += state1duration;
        }
        break;
    case 1: // Transition for state 2
        if ((currentMillis - previousMillis) >= state2duration)
        {
            nextState = 1;
            previousMillis += state2duration;
        }
        break;
    default: // When unknown state, go to state 1
        nextState = 1;
        break;      
    }

    // If there is a change, perform the enter functions
    if (nextState != state)
    {
        switch (nextState) // enter functions of the next state
        {
        case 1: // Entering state 1
            analogWrite(LED3, 127);
            analogWrite(LED4, 254);
            break;
        case 2: // Entering state 2
            analogWrite(LED3, 254);
            analogWrite(LED4, 127);
            break;  
        }
    }

    // Advance to the new state
    state = nextState;
}

This can be more complicated, but believe me, it is not. Once you get what this means you will be able to solve a lot of problems this way.

According to my experience, never try to simplify this to your particular problem, since it will make things more complicated. Just add, when needed, the exit functions (before switch(nextState) put another switch on state) and the execution (after the advancing, put another switch on state) if needed.

Answer 2

You are trying to use millis() exactly like delay(). The method delay() stops the program from progressing forward. It blocks the program. This is frowned upon as the processor could be doing something useful during this time.

The method millis() does not stop the program. Instead it is like telling you what time it is. It is your code that then need to know at what time you want to do something.

But in your code, you are doing something odd. You are switching the brightness of the LEDs then IMMEDIATELY switching them back. You are using the same time for the two things you want to do. Instead try to initialize previousMillisLED4 to 500, intervalLED3 to 1000 & intervalLED4 to 1000.

Answer 3

The final sub routine...I did move the counter to get a better result. The variable "ditchFlag" is set in an IF statement further up in the sketch. The duration of the subroutine is a combination of the timers and the number in "count".

void ditchDim() {    
while (ditchFlag == 1) {            //set to 1 when conditions are met

unsigned long currentMillis = millis();

if ((currentMillis - previousMillis) < endOfPhase1) {
    analogWrite(LED3, 127);
    analogWrite(LED4, 254);
}
else if ((currentMillis - previousMillis) < endOfPhase2) {
    analogWrite(LED3, 254);
    analogWrite(LED4, 127);
}else{
  count = (count+1);  
    previousMillis += endOfPhase2;             
    }
      if (count == 5) {
        count = 0;
        ditchFlag = 0;
        return;
    }
  }
}

thanks Doug