2 buttons, 2 leds, blinking, non-blocking - broken.

I have a couple issues with the following code and to be honest, I do not understand why.

Desired outcome: 2 buttons, 1 red, 1 blue. If users presses blue button, blue led flashes for time set by blinkTime. If user press red button, red led flashes for time set. User should be able to press either button and they should blink independently.

1 - If uploaded as is the following code will allow user to press either button and the related led will begin flashing. However if user presses other button the first led blinks until timer is up then the second led starts blinking for the remainder of its time.

2 - If “// Serial.print(poles_led);” is uncommented the code almost words as desired however the leds are not blinking at the proper rate as defined by blinkRate. There is a delay happening.

I am using the pushbutton library from pololu http://pololu.github.io/pushbutton-arduino/

Thank you in advance.

#include <Pushbutton.h>

int poles_led[] = {4, 5};
int poles_button[] = {11, 10};
unsigned long  poles_start[] = {0, 0};
int poles_state[] = {0, 0};

int x;

Pushbutton red_pushbutton(10);
Pushbutton blue_pushbutton(11);

unsigned long blinkTime = 5000; // the time we need to wait
unsigned long blinkRate = 100; // the time we need to wait

unsigned long previousMillis = 0; // millis() returns an unsigned long.

void setup() {
  for (x = 0; x < 2; x++) {
    pinMode(poles_led[x], OUTPUT);
    digitalWrite(poles_led[x], LOW);
  }
  Serial.begin(9600);
}

void loop() {
  pole_check();
}

void pole_check()
{
  for (x = 0; x < 2; x++) {
    if (poles_state[x]  == 1) {
      if (millis() - poles_start[x] >= blinkTime) {
        Serial.println(" off ");
        poles_state[x]  = 0;
        poles_start[x] = 0;
        digitalWrite(poles_led[x], LOW);
      }
      else if (millis() - poles_start[x] <= blinkTime) {
      //  Serial.print(poles_led[x]);
        Serial.println(" blinky ");
        if ((unsigned long)(millis() - previousMillis) >= blinkRate) {
          previousMillis = millis();
          digitalWrite(poles_led[x], !digitalRead(poles_led[x]));  // Ooooh magic toggle code!
        }
      }
    }
    if (red_pushbutton.getSingleDebouncedPress())
    {
      poles_state[1] = 1;
      poles_start[1] = millis();
      Serial.println(" | red ");
    }
    if (blue_pushbutton.getSingleDebouncedPress())
    {
      poles_state[0] = 1;
      poles_start[0] = millis();
    }
  }
}

You have this code inside the “for” loop which doesn’t look right to me:

if (red_pushbutton.getSingleDebouncedPress())
    {
      poles_state[1] = 1;
      poles_start[1] = millis();
      Serial.println(" | red ");
    }
    if (blue_pushbutton.getSingleDebouncedPress())
    {
      poles_state[0] = 1;
      poles_start[0] = millis();
    }

Nick, as per your suggestion i moved the bit of code you quoted to the top of the pole
_check() function. However the leds are not flashing at the 100 interval set, they are flashing erratically. Is there a better method?

Please post your amended code.

There is no 

if (poles_state[x]  == 0)

check around your 

if (red_pushbutton.getSingleDebouncedPress())...

code (you could also use an else instead). This will keep moving your times.
#include <Pushbutton.h>

int poles_led[] = {4, 5};
int poles_button[] = {11, 10};
unsigned long  poles_start[] = {0, 0};
int poles_state[] = {0, 0};

int x;

Pushbutton red_pushbutton(10);
Pushbutton blue_pushbutton(11);

unsigned long blinkTime = 5000; 
unsigned long blinkRate = 100;

unsigned long previousMillis = 0;

void setup() 
{
  for (x = 0; x < 2; x++) 
  {
    pinMode(poles_led[x], OUTPUT);
    digitalWrite(poles_led[x], LOW);
  }
  //Serial.begin(9600);
}

void loop() 
{
  pole_check();
}

void pole_check()
{
 if (poles_state[1]  == 0) 
  {
    if (red_pushbutton.getSingleDebouncedPress())
    {
      poles_state[1] = 1;
      poles_start[1] = millis();
    }
  }
  if (poles_state[0]  == 0) 
  {
    if (blue_pushbutton.getSingleDebouncedPress())
    {
      poles_state[0] = 1;
      poles_start[0] = millis();
    }
  }

  for (x = 0; x < 2; x++) 
  {
    if (poles_state[x]  == 1) 
    {
      if (millis() - poles_start[x] >= blinkTime) 
      {
        poles_state[x]  = 0;
        poles_start[x] = 0;
        digitalWrite(poles_led[x], LOW);
      }
      else if (millis() - poles_start[x] <= blinkTime) 
      {
        if ((unsigned long)(millis() - previousMillis) >= blinkRate) 
        {
          previousMillis = millis();
          digitalWrite(poles_led[x], !digitalRead(poles_led[x]));  // Ooooh magic toggle code!
        }
      }
    }
  }
}

ajnin:

You posted new code, but you didn't say whether it worked or not.

Trying to several things at once often results in hard to read monolithic code. I think this problem would be better solved by designing two independent state machines. If you use my state state machine libraryyou can have multiple instances of the same machine.

#include <Streaming.h>
#include <SM.h>

SM pole[] = {SM(idle), SM(idle)};
int poles_count[2];
const int poles_led[] = {4, 5};
const int poles_button[] = {11, 10};
const int blinkRate = 100;
byte i;

void setup(){
  Serial.begin(115200);
  for(i = 0; i<2; i++){
    pinMode(poles_led[i], OUTPUT);
    pinMode(poles_button[i], INPUT_PULLUP);//active low inputs
  }//()
}//setup()

void loop(){
  for(i = 0; i<2; i++) EXEC(pole[i]);
}//loop()

State idle(){
  if(!digitalRead(poles_button[i])){//active low buttons
    digitalWrite(poles_led[i], 1);
    Serial<<"led :"<<i<<" on"<<endl;
    pole[i].Set(on);
    poles_count[i]++;
  }//if(button pressed)
}//idle(){}

State on(){
  if(pole[i].Timeout(blinkRate)){
    digitalWrite(poles_led[i], 0);
    Serial<<"led :"<<i<<" off"<<endl;
    if(poles_count[i] >= 25){
      pole[i].Set(idle);
      poles_count[i] = 0;
    }//if(poles_count)
    else pole[i].Set(off);
  }//if(timeout)
}//on()

State off(){
  if(pole[i].Timeout(blinkRate)){
    digitalWrite(poles_led[i], 1);
    Serial<<"led :"<<i<<" on"<<endl;
    pole[i].Set(on);
    poles_count[i]++;
  }//if(timeout)
}//off()

previousMillis was my problem. I hadn't declared two of them to handle both the timers for the blink rate. Thank you all for you assistance. Aside from using another library is there a better way, perhaps a more bulletproof way to handle the above?

Post the code of your working version and I am sure that you will get suggestions.

State machines are the most bulletproof way of approaching these kinds of problems. But they tend to lead to large monolithic code. One way around that is using function pointers. Have a look in the documentation of my library on how to use them if you don't want to use the library itself. But you will loose the inherent timing mechanism of the library.