[SOLVED] Blinking indicators!

I am building a control panel with several illuminated push-buttons. The indicator of the button needs to be in a different state/phase depending on how many times the button has been pressed, the phases are:-
0. (default state) the indicator must blink on and off every second,
1. (after 1st press) the indicator must stay illuminated until the 2nd press,
2. (after 2nd press) the indicator must extinguish for a predetermined amount of time before reverting back to phase 0, during this phase the loop() still needs to run.

The problem I am having is that currently I can only achieve phase 2 using a delay, which prevents the loop() from running, this is acceptable with short delays but on some buttons this delay will need to be up to a minute.

Code is below:-

#include <OneButton.h>                         // load OneButton library
const int indicator = 17;                      // define indicator pin
int phaseCount = 0;                            // define variables:
bool butStatePre = true;
bool butStateCur = true;
unsigned long millisPre = 0;
unsigned long millisCur;
unsigned long phaseStart;
const int phaseDelay = 3000;                   // delay for phase to timeout
OneButton but = OneButton(4, true, true);      // define OneButton instance

void setup() {                   // put your setup code here, to run once:
  pinMode(indicator, OUTPUT);     // initialise indicator pin
  but.attachClick(butClick);      // attach OneButton state event
  Serial.begin(9600);             // initialise the serial monitor
}

void loop() {                      // put your main code here, to run repeatedly:
  millisCur = millis();             // update current time
  but.tick();                       // query button
  while (phaseCount == 0) {         // blink indicator when required (phase 0 case):
    digitalWrite(indicator, (millis() / 1000) % 2);     
    but.tick();
  }
}

void butClick() {                               // button process flow:
  if (butStateCur != butStatePre) {              // test if button state has changed
    butStatePre = butStateCur;                    // update current button state
    phaseCount ++;                                // increment phase counter
    } if (phaseCount == 0) {                      // phase 1 case:
        digitalWrite(indicator, HIGH);              // illuminate indicator
        phaseCount ++;                              // increment phase counter
        Serial.println("Button pressed Phase = 1"); // print message to serial monitor
      } else if (phaseCount == 1) {                // phase 2 case:
          phaseStart = millis();                     // store current time
          digitalWrite(indicator, LOW);              // extinguish indicator
          Serial.println("Button pressed Phase = 2"); // print message to serial monitor
          //if (phaseStart - millis() >= phaseDelay) {// WHEN TIMER EXPIRES (WILL NEED 							  TO BE UP TO A MINUTE ON OTHER BUTTONS)
          delay(phaseDelay);                          // loop() STILL NEEDS TO RUN WHILE 									THIS DELAY IS BEING EXECUTED
          phaseCount = 0;                             // reset phase counter
          Serial.println("Timeout Phase = 0");        // print message to serial monitor
          //}
        }
}

The part I am having trouble with (CAPS in comments) is allowing the loop() to run whilst the indicator is being held low before the timer expires.
I don't know if it's relavent or not, but the microcontroller is a Teensy 2.0.

Any suggestions or comments would be greatly appreciated.

Look up ‘state machines’ and ‘non-blocking’ timing. (millis)

You’re already well on the way, and these techniques are nit that hard to pull together.

The only thing that might catch you out is ‘debouncing’ the button presses to select the right mode reliably.

1 Like

If you can think of phaseCount as the "state" of a state machine, consider changing this "while" to an "if":

and then you can add an "if" statement to handle phase 2. Inside of that you can check if(millisCur - phaseStart > predeterminedMillis) and transition to phase 0.

Thanks for nudging me in the right direction. I haven't had any debouncing issues yet (I think OneButton handles this) but I shall keep it in mind as further buttons are added.

Thank you for your help.
I had also realised the counter wasn't incrementing from phase 1 to phase 2. As usual the soloution was right in front of me, but I couldn't see the wood for the trees! Re-arranging the code was the chainsaw that I needed.

1 Like

Eventual implementation, for anyone that is interested:-

#include <OneButton.h>                         // load OneButton library
const int indicator = 17;                      // define indicator pin
int phaseCount = 0;                            // define variables:
bool butStatePre = true;
bool butStateCur = true;
unsigned long millisPre = 0;
unsigned long millisCur;
unsigned long phaseStart;
const int phaseDelay = 3000;                   // delay for phase to timeout
OneButton but = OneButton(4, true, true);      // define OneButton instance

void setup() {                    // put your setup code here, to run once:
  pinMode(indicator, OUTPUT);     // initialise indicator pin
  but.attachClick(butClick);      // attach OneButton state event
  Serial.begin(9600);             // initialise the serial monitor
}

void loop() {                                           // put your main code here, to run repeatedly:
  millisCur = millis();                                 // update current time
  but.tick();                                           // query button
  if (phaseCount == 0) {                                // blink indicator when required (phase 0 case):
    digitalWrite(indicator, (millis() / 1000) % 2);     // I don't know how this magic works, but a single line to blink an LED is rather elegant!
  } if (phaseCount == 2) {                              // extinguish indicator until timer elapses (phase 2 case):
      phase2();
    }
}

void butClick() {                                         // button process flow:
  if (butStateCur != butStatePre) {                       // test if button state has changed
    butStatePre = butStateCur;                            // update current button state
    phaseCount ++;                                        // increment phase counter
    } if (phaseCount == 0) {                              // phase 1 case:
        digitalWrite(indicator, HIGH);                    // illuminate indicator
        phaseCount ++;                                    // increment phase counter
        Serial.println("Button pressed, Phase = 1");      // print message to serial monitor
      } else if (phaseCount == 1) {                       // phase 2 case:
          phaseStart = millis();                          // store current time
          digitalWrite(indicator, LOW);                   // extinguish indicator
          phaseCount ++;                                  // increment phase counter
          Serial.println("Button pressed, Phase = 2");    // print message to serial monitor
        }
}

void phase2() {                                   // phase 2 process flow:
  if (millisCur - phaseStart > phaseDelay) {      // test for elapsed time
    phaseCount = 0;                               // reset phase counter
    Serial.println("Timeout, Phase = 0");         // print message to serial monitor
  }
}
1 Like

Hello JamesJRovira

Welcome to the best Arduino forum ever :slight_smile:

How many indicators do you will have for the project?

Hi @JamesJRovira,

welcome to the arduino-forum.

well done posting your code as a codection in your first posting.

Of course to any kind of code-behaviour there are always more than one solution to achive this behaviour.

If you want to post a solution you should post the complete sketch. From the very first to the very last line.

Inside the code that you have posted I did not find a line where phaseCount is resetted to zero.

The behaviour of a state-machine can be relalised with if-conditions but there is a different way that reduces the number of if-conditions to a minimum.

This is done by using the switch-case-break;-statement.

switch-case-break is a special kind of if-condition

the basic structure is

millisCur = millis();

switch (phaseCount)

  case 0: // blink LED and wait for first  button-press
    digitalWrite(indicator, (millis() / 1000) % 2);

    if ( "transition from unpressed to pressed") {                 
      digitalWrite(indicator, HIGH);
      phaseCount = 1;                                       
	}  	
    break;


  case 1: // wait for second button-press
    if ( "transition from unpressed to pressed") {	  
	  phaseStart = millis();
      digitalWrite(indicator, LOW);
      phaseCount = 2;
	}
	
    break;


  case 2: // after time-out return to state 0
	if (millisCur - phaseStart > phaseDelay) {      // test for elapsed time
		phaseCount = 0;                             
		Serial.println("Timeout, Phase = 0");       
	  }
    break;

} // end of switch

The code above has a very clear structure and is easy to maintain / to modify.
To me personal thi skind of code is much easier to read than nested if-conditions

The code-section above does not yet work because you did not post your complete sketch.

For making it work the if-condition must be specified and this code must be able to
detect the transition from unpressed to pressed.

It is not enough to detect "button pressed"
The code would run so fast that without checking for
unpressed => pressed => unpressed => pressed
the code would immidiately jump into mode 1 with the first button-press

It is unvisible if your button is pressed when

digitalRead(buttonPin == LOW)

or

digitalRead(buttonPin == HIGH)

which must be known to write the code for the transition-detection

So really:
it is always a good idea to post your complete sketch.

Thanks for your interest.
The project (when complete) will have 10 illuminated pushbuttons, 8x momentary and 2x latching, also 1x non-illuminated momentary.

Thanks, some interesting points to bear in mind.

not clear if the latching buttons will be mechanically latching or if the latching should be done in software

Shall all buttons work this way independant from each other?

Adafruit has a tutorial-section - where in multiple steps - you can learn how to create a template with a certain functionality.

The adafruit-example demonstrates it on a flashing LED.
But this coding.rechnique can be used for any kind of code.
Which means it can be used for your behaviour button-presses combined with on/off/blinking too

This template can then be used to create multiple independant blinking leds with a few lines of code.

This template is called a "class"
The tutorial starts here

and here is the part that makes use of a C+-class

look this over

struct Indicator {
    const byte PinBut;
    const byte PinLed;

    byte       butState;
    byte       mode;
    unsigned long msec;
}
ind [] = {
 //   but  led
    {  A1,  13 },
    {  A2,  12 },
    {  A3,  11 },
};
const int Nind = sizeof(ind)/sizeof(Indicator);

const unsigned long MsecPeriod = 1000;

enum { Off = HIGH, On = LOW };
enum { M_Blink, M_On, M_Off, M_Last };

char s [90];

// -----------------------------------------------------------------------------
void
loop ()
{
    unsigned long msec = millis ();

    for (int n = 0; n < Nind; n++)  {
        byte but = digitalRead (ind [n].PinBut);
        if (ind [n].butState != but)  {     // state change
            ind [n].butState  = but;
            delay (20);                     // debounce

            if (LOW == but)  {
                if (M_Last <= ++ind [n].mode)
                    ind [n].mode = 0;

                sprintf (s, " %d: mode %d", n, ind [n].mode);
                Serial.println (s);
            }
        }

        switch (ind [n].mode)  {
        case M_Off:
            digitalWrite (ind [n].PinLed,  Off);
            break;

        case M_On:
            digitalWrite (ind [n].PinLed,  On);
            break;

        case M_Blink:
            if (msec - ind [n].msec >= MsecPeriod)  {
                ind [n].msec = msec;
                digitalWrite (ind [n].PinLed,
                            ! digitalRead (ind [n].PinLed));
            }
            break;
        }
    }
}

// -----------------------------------------------------------------------------
void
setup ()
{
    Serial.begin (9600);

    for (int n = 0; n < Nind; n++)  {
        pinMode      (ind [n].PinLed,  OUTPUT);
        digitalWrite (ind [n].PinLed,  Off);
        pinMode      (ind [n].PinBut, INPUT_PULLUP);
        ind [n].butState = digitalRead (ind [n].PinBut);
    }
}

I expressed @JamesJRovira's sketch parts a bit differently, using two language features that come in handy.

I jettisoned the library. I don't mind button libnraries, they don't all suck, but it is important at some point to understand how they function. This was failrly simple state change detection.

I added an anonymous enum just to give names to the states.

I used a switch/case instead of chained if/else statements; in state machines it makes for more readable code.

I moved any writing to the LED to after the switch/case mechanism. All other control is performed by setting a variable. This mightn't be important, but it does move any output to its own section of code. As a benefit it allows one place where we can adhere to the API which states that digital outputs can only be written HIGH or LOW. Yeah, I know. Sue me.

I changed a few things for the UNO and to make it work faster. Life too short.

Try it here:


Wokwi_badge blinking indicators


The code
// https://forum.arduino.cc/t/solved-blinking-indicators/1327297
// https://wokwi.com/projects/415999787798480897

const int indicator = 11;
const int myBuuton = 4;

//...
enum {PHASE0, PHASE1, PHASE2};

int thePhase = PHASE0;

bool butStateCur = true;
bool butStatePre;

bool ledOn;

unsigned long millisPre = 0;
unsigned long millisCur;
unsigned long phaseStart;
const int phaseDelay = 2000;           // delay for phase to timeout

void setup() {
  Serial.begin(115200);
  Serial.println("\nJello Whirled!\n");

  pinMode(indicator, OUTPUT);
  pinMode(myBuuton, INPUT_PULLUP);

  butStatePre = digitalRead(myBuuton) == LOW;
}

void loop() {
  millisCur = millis();                

//... let us do our own switch handling
  bool switchChange = false;    // until proven otherwise
  butStateCur = digitalRead(myBuuton) == LOW;
  if (butStatePre != butStateCur) {
    butStatePre = butStateCur;

    switchChange = true;
    delay(20);    // debounce
  }

//...
  switch (thePhase) {
  case PHASE0 :
    ledOn = (millis() / 500) % 2;
    if (switchChange) {
      ledOn = true;
      Serial.println("Phase = 1");
      thePhase = PHASE1;
    }

    break;
    
  case PHASE1 :
    if (switchChange) {
      ledOn = false;
      Serial.println("Phase = 2");
      thePhase = PHASE2;
      phaseStart = millisCur;
    }

    break;

  case PHASE2 :
    if (millisCur - phaseStart > phaseDelay) {
      thePhase = PHASE0;
      Serial.println("Timeout, Phase = 0");
    }

    break;
  }

  digitalWrite(indicator, ledOn ? HIGH : LOW);  // C/C++ ternary operator
}

One more trick, which is to move any printing to the output section. This is the same sketch with logic to print the state if it is newly entered.

See that here:


Wokwi_badge also blinking indicators


More code
// https://forum.arduino.cc/t/solved-blinking-indicators/1327297
// https://wokwi.com/projects/416000716783497217
const int indicator = 11;
const int myBuuton = 4;

//...
enum {PHASE0 = 0, PHASE1, PHASE2};
char *tags[] = {"phase zero", "phase one", "phase two", };

int thePhase = PHASE0;

bool butStateCur = true;
bool butStatePre;

bool ledOn;

unsigned long millisPre = 0;
unsigned long millisCur;
unsigned long phaseStart;
const int phaseDelay = 3000;           // delay for phase to timeout

void setup() {
  Serial.begin(115200);
  Serial.println("\nJello Whirled!\n");

  pinMode(indicator, OUTPUT);
  pinMode(myBuuton, INPUT_PULLUP);

  butStatePre = digitalRead(myBuuton) == LOW;
}

void loop() {
  millisCur = millis();                

//... let us do our own switch handling
  bool switchChange = false;    // until proven otherwise
  butStateCur = digitalRead(myBuuton) == LOW;
  if (butStatePre != butStateCur) {
    butStatePre = butStateCur;

    switchChange = true;
    delay(20);    // debounce
  }

//...
  switch (thePhase) {
  case PHASE0 :
    ledOn = (millis() / 500) % 2;
    if (switchChange) {
      ledOn = true;
      thePhase = PHASE1;
    }

    break;
    
  case PHASE1 :
    if (switchChange) {
      ledOn = false;
      thePhase = PHASE2;
      phaseStart = millisCur;
    }

    break;

  case PHASE2 :
    if (millisCur - phaseStart > phaseDelay) {
      thePhase = PHASE0;
    }

    break;
  }

  digitalWrite(indicator, ledOn ? HIGH : LOW);  // C/C++ ternary operator

  static int printedPhase = -1;  // no it does not!
  if (printedPhase != thePhase) {
    Serial.print("entered ");
    Serial.println(tags[thePhase]);

    printedPhase = thePhase;
  }
}

a7

I think your code has a slight problem when you switch in PHASE2. In this case I guess you should directly go to PHASE1 instead.

Here is a totally different approach - for those who are interested: blinkingIndicators pa - Wokwi ESP32, STM32, Arduino Simulator

I'm always happy to have a motive for looking at any code even especially my own to discover and corrdct any flaws.

I added my version to your proto_activities version. I added two switches and another LED and fixed them to unused i/o pins.

I call my setupA7() from setup(), I call my loopA7() from loop(). I did have to remove the redundant call to Serial.being(). Since your sketch runs free and has plenty of resources and there were no identifier conflicts it was a fairly easy cut and paste hack.

One switch was wired in parallel to close a switch each from your code and mine - wokwi does basic electricity well enough.

Using that one switch that rules both one of your LEDs and mine, I saw no difference.

So I don't see a problem slight or otherwise.

a7

That’s a nice way to test two implementations against each other.

Maybe try the following: switch on, then off and then while the led is off switch quickly on again.

Time's up.

Yours does what it does when you do that, mine does what it does. If yours is the same as the OP's then there you go.

I did not test that, so.

a7

We were looking at OneButton with an eye towards finding any reason not to toss it in the trash, and my beach buddy finally saw what has mislead me. And took some pleasure in reminding me of rule 2: autoformat any code you did not write before talking about it. or working with it.

(Rule 1 is delete all comments.)

void butClick() {                                         
  if (butStateCur != butStatePre) {                       
    butStatePre = butStateCur;                            
    phaseCount ++;                                        
    } if (phaseCount == 0) {                              
        digitalWrite(indicator, HIGH);                    
        phaseCount ++;                                    
        Serial.println("Button pressed, Phase = 1");      
      } else if (phaseCount == 1) {                       
          phaseStart = millis();                          
          digitalWrite(indicator, LOW);                   
          phaseCount ++;                                  
          Serial.println("Button pressed, Phase = 2");    
        }
}

The first if statement is completely irrelevant. The variables butStatePre and butStateCur are nothing but noise.

Add to that that however @JamesJRovira has used the library, only a brief button press qualifies as a click.

If you don't have a callback for a double click, you get two single clicks. With both single and double callbacks, holding the button long enough results in no callback. I suppose if you have callbacks for the other things OneButton handles you would get them.

OneButton -> trash. Learn how to handle buttons, find a library if you must, one that does not suck.

Whoever wrote the state recognition pattern in butClick() did not know how to use the library.

I said my time was up. I'll see if she might ever fix my sketches to account for the fact that the leading OR trailing edge of the button presses is all what counts. I'm not calling in any markers over it. If I had any. :expressionless:

a7

Here's my mashup of @frameworklabs's version and my second take, changed so that the button acts like a button.

A commented line in PHASE2 would prematurely exit if the button is pressed during.

See it here


Wokwi_badge Two in One


The code
// https://forum.arduino.cc/t/solved-blinking-indicators/1327297
// https://wokwi.com/projects/416041824859474945

#include "proto_activities.h"

// activities

pa_activity (ButtonRecognizer, pa_ctx_tm(), int pin, bool& was_pressed, bool& was_released) {
  pinMode(pin, INPUT_PULLUP);

  pa_repeat {
    pa_await (digitalRead(pin) == LOW);
    was_pressed = true;
    pa_pause;
    was_pressed = false;
    pa_delay_ms (20); // debounce button press
    
    pa_await (digitalRead(pin) == HIGH);
    was_released = true;
    pa_pause;
    was_released = false;
    pa_delay_ms (20); // debounce button release
  }
} pa_end

pa_activity (Blink, pa_ctx_tm(), bool& show) {
  pa_every_ms (500) {
    show = !show;
  } pa_every_end
} pa_end

pa_activity (Delay_s, pa_ctx_tm(), int s) {
  pa_delay_s(s);
} pa_end

pa_activity (StateMachine, pa_ctx_tm(pa_use(Blink); pa_use(Delay_s)), bool was_pressed, bool was_released, bool& show) {
  pa_repeat {
    // Phase 1
    if (!was_pressed) {
      pa_when_abort (was_pressed, Blink, show);
    }

    // Phase 2
    show = true;
    pa_await (was_released);

    // Phase 3
    show = false;
    pa_when_abort (was_pressed, Delay_s, 3);
  }
} pa_end

pa_activity (Indicator, pa_ctx(), int pin, bool show) {
  pinMode(pin, OUTPUT);

  pa_always {
    digitalWrite(pin, show);
  } pa_always_end
} pa_end

pa_activity (Handler, pa_ctx(pa_co_res(3); pa_use(ButtonRecognizer); pa_use(StateMachine); pa_use(Indicator); bool was_pressed; bool was_released; bool show), 
                      int button, int indicator) {
  pa_co(3) {
    pa_with (ButtonRecognizer, button, pa_self.was_pressed, pa_self.was_released);
    pa_with (StateMachine, pa_self.was_pressed, pa_self.was_released, pa_self.show); 
    pa_with (Indicator, indicator, pa_self.show); 
  } pa_co_end
} pa_end

pa_activity (Main, pa_ctx(pa_co_res(2); pa_use_as(Handler, H1); pa_use_as(Handler, H2))) {
  pa_co(2) {
    pa_with_as (Handler, H1, 4, 11);
    pa_with_as (Handler, H2, 3, 10);
  } pa_co_end
} pa_end

pa_use(Main);

// setup and loop

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

  setupA7();
}

void loop() {
  pa_tick(Main);
  delay(10);  // ??

  loopA7();
}

const int indicator = A0;
const int myBuuton = A1;

//...
enum {PHASE0 = 0, PHASE1, PHASE2};
char *tags[] = {"phase zero", "phase one", "phase two", };

int thePhase = PHASE0;

bool butStateCur = true;
bool butStatePre;

bool ledOn;

unsigned long millisPre = 0;
unsigned long millisCur;
unsigned long phaseStart;
const int phaseDelay = 3000;           // delay for phase to timeout

void setupA7() {
//  Serial.begin(115200);
  Serial.println("\nJello Whirled!\n");

  pinMode(indicator, OUTPUT);
  pinMode(myBuuton, INPUT_PULLUP);

  butStatePre = digitalRead(myBuuton) == LOW;
}

void loopA7() {
  millisCur = millis();                

//... let us do our own switch handling
  bool buttonPressed = false;    // until proven otherwise
  butStateCur = digitalRead(myBuuton) == LOW;
  if (butStatePre != butStateCur) {
    if (butStateCur)
      buttonPressed = true;
    
    delay(20);    // debounce
  }

  butStatePre = butStateCur;

//...
  switch (thePhase) {
  case PHASE0 :
    ledOn = (millis() / 500) % 2;
    if (buttonPressed) {
      ledOn = true;
      thePhase = PHASE1;
    }

    break;
    
  case PHASE1 :
    if (buttonPressed) {
      ledOn = false;
      thePhase = PHASE2;
      phaseStart = millisCur;
    }

    break;

  case PHASE2 :
//    if (buttonPressed) thePhase = PHASE0;    // should the button cut short PHASE2?

    if (millisCur - phaseStart > phaseDelay) {
      thePhase = PHASE0;
    }

    break;
  }

  digitalWrite(indicator, ledOn ? HIGH : LOW);  // C/C++ ternary operator

  static int printedPhase = -1;  // no it does not!
  if (printedPhase != thePhase) {
    Serial.print("entered ");
    Serial.println(tags[thePhase]);

    printedPhase = thePhase;
  }
}

a7

1 Like