Another millis() Question

Hello All,

First time poster here, but I do know the frustration of asking a question that has already been answered. I am new to programming, but trying to build projects as they come and figure it out as I go.

I have a project where I have 6 contact switches (Level 1-6) that need to indicate to 6 LED's respectively. I also have a 5v buzzer as an audible alarm that one of the switches has closed. I have that much figured out.

Where I am getting into a land of frustration, is that I am trying to add a button as an acknowledge switch that would disable the buzzer for a set duration of time (somewhere between 10-15 minutes), while still allowing the other contact switches to indicate open/close with the LED's during that time
.
I have ran this code with the delay function successfully , until the blocking does not allow the other led's to operate correctly.

I believe I understand the concept of the millis() function, but I am having trouble applying it. As I understand it, If a button was pressed to turn on an LED at the currentMillis of, just say,100, and you programmed a wait time of 500, the LED would turn off at 600. With that logic, if the button was pressed again at 2000, with the same wait time the led would turn off at 2500.

Currently the LED's and buzzer operate correctly,
With the acknowledge button depressed, the buzzer does turn off
With the acknowledge button depressed, other LED's will indicate open/closed
When the acknowledge button is released, with a contact switched closed, the buzzer turns back on with no delay.

Thank you for the help,

const int LED1 = 3;
const int LED2 = 4;
const int LED3 = 5;
const int LED4 = 6;
const int LED5 = 7;
const int LED6 = 8;
int Level1 = A0;
int Level2 = A1;
int Level3 = A2;
int Level4 = A3;
int Level5 = A4;
int Level6 = A5;
int Buzzer = 2;
int Button = 9;
int buttonState = 0;
boolean BuzzerState = false;

unsigned long startMillis;
unsigned long currentMillis;
const unsigned long period = 4000;

void setup() {

  pinMode(LED1, OUTPUT);
  pinMode(LED2, OUTPUT);
  pinMode(LED3, OUTPUT);
  pinMode(LED4, OUTPUT);
  pinMode(LED5, OUTPUT);
  pinMode(LED6, OUTPUT);
  pinMode(Level1, INPUT);
  pinMode(Level2, INPUT);
  pinMode(Level3, INPUT);
  pinMode(Level4, INPUT);
  pinMode(Level5, INPUT);
  pinMode(Level6, INPUT);
  pinMode(Button, INPUT);
  pinMode(Buzzer, OUTPUT);
  Serial.begin(9600);

  startMillis = millis();
}

void loop() {

  Serial.println(digitalRead(Button));
  buttonState = digitalRead(Button);

  digitalWrite(Buzzer, LOW);
  BuzzerState = false;

  if (digitalRead(Level1) == HIGH) {
    digitalWrite(LED1, HIGH);
    BuzzerState = true;
  } else {
    digitalWrite(LED1, LOW);
  }
  if (digitalRead(Level2) == HIGH) {
    digitalWrite(LED2, HIGH);
    BuzzerState = true;
  } else {
    digitalWrite(LED2, LOW);
  }
  if (digitalRead(Level3) == HIGH) {
    digitalWrite(LED3, HIGH);
    BuzzerState = true;
  } else {
    digitalWrite(LED3, LOW);
  }
  if (digitalRead(Level4) == HIGH) {
    digitalWrite(LED4, HIGH);
    BuzzerState = true;
  } else {
    digitalWrite(LED4, LOW);
  }
  if (digitalRead(Level5) == HIGH) {
    digitalWrite(LED5, HIGH);
    BuzzerState = true;
  } else {
    digitalWrite(LED5, LOW);
  }
  if (digitalRead(Level6) == HIGH) {
    digitalWrite(LED6, HIGH);
    BuzzerState = true;
  } else {
    digitalWrite(LED6, LOW);
  }


  if (BuzzerState == true) {
    if (digitalRead(Button) == HIGH) {
      currentMillis = millis();
      if (currentMillis - startMillis >= period) {
        digitalWrite(Buzzer, LOW);
        startMillis = currentMillis;
      }
    } else {
      digitalWrite(Buzzer, HIGH);
    }
  }
}
  • Know when the "silence" button is pressed to record millis(). (startSilence = millis();)

  • Calculate fifteen minutes. (fifteenMins = 15 min * 60 sec/min * 1000UL ms/sec)

  • Set a "silence" flag. (silenceBuzzerFlag = 1)

  • If fifteen minutes time is up, clear the "silence" flag. (if millis() > startSilence + fifteenMins)... (silenceBuzzerFlag = 0;)

  • If the buzzer is to be sounded, examine the "silence" flag (if (soundBuzzer == true))

  • if the silence flag is clear (over 15 mins), sound the buzzer.(if (silenceBuzzerFlag == 0))

This is a bad advice to OP
Never calculate time interval in the future, it is not work while millis overflow.
The proper way is using substraction of timer points:
if (millis() -startSilence > fifteenMins)... (silenceBuzzerFlag = 0;

Better use millis() - startSilence >= fifteenMins as it handles millis overflow correct.

you might benefit from studying state machines. Here is a small introduction to the topic: Yet another Finite State Machine introduction

Otherwise there are many discussions about managing time and non blocking code - for extra information and examples look at

look this over.

#undef MyHW
#ifdef MyHW
const byte PinLeds [] = { 10, 11 };         // { 3, 4, 5, 6, 7, 8 };
const byte PinButs [] = { A2, A3 };         // { A0, A1, A2, A3, A4, A5 };
const byte PinBuz     = 13;
const byte PinAck     = A1;

#else
const byte PinLeds [] = {  3,  4,  5,  6,  7,  8 };
const byte PinButs [] = { A0, A1, A2, A3, A4, A5 };
const byte PinBuz     =  2;
const byte PinAck     =  9;
#endif

const int  Nbut = sizeof(PinButs);

enum { StOff, StActive, StAck };
int  stateBuz [Nbut];
bool stateLed [Nbut];

const unsigned long PeriodMsec = 4000;
      unsigned long ledMsec [Nbut];
      unsigned long buzMsec [Nbut];     // potentially turn back on later

enum { LedOff = HIGH, LedOn = LOW };
enum { BuzOff = HIGH, BuzOn = LOW };

char s [90];

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

    for (int n = 0; n < Nbut; n++)  {
        // check for led timeout
        if (stateLed [n] && msec - ledMsec [n] >= 500) {
            stateLed[n] = false;
            digitalWrite (PinLeds [n], LedOff);

            sprintf (s, "  led %d timeout", n);
            Serial.println (s);
        }

        // check for led button press
        if (! stateLed [n] && LOW == digitalRead (PinButs [n]))  {
            ledMsec [n] = msec;
            digitalWrite (PinLeds [n], LedOn);
            digitalWrite (PinBuz,      BuzOn);

            stateBuz[n] = StActive;
            stateLed[n] = true;

            sprintf (s, "  but %d pressed", n);
            Serial.println (s);
        }
    }

    if (LOW == digitalRead (PinAck))  {
        for (int n = 0; n < Nbut; n++)  {
            if (stateBuz [n] != StAck)  {
                stateBuz [n] = StAck;       // potentiall turned on later?
                buzMsec  [n] = msec;        // capture timeStamp - unused
                digitalWrite (PinBuz, BuzOff);

                sprintf (s, "    ack %d", n);
                Serial.println (s);
            }
        }
    }
}

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

    pinMode (PinAck, INPUT_PULLUP);
    pinMode (PinBuz, OUTPUT);
    digitalWrite (PinBuz, BuzOff);

    for (int n = 0; n < Nbut; n++)  {
        pinMode      (PinButs [n], INPUT_PULLUP);
        pinMode      (PinLeds [n], OUTPUT);
        digitalWrite (PinLeds [n], LedOff);
    }
}

don't understand what you have in mind for this,but i added variables to capture when the buzzer was activated for each LED

Hi @tward1 !

First of all it would be helpful if you post the wiring of your components!

  1. The use of pinMode(...,INPUT) with buttons and switches requires the use of external pullup or pulldown resistors to ensure stable potential while a button/switch is open.
  2. In every loop of your sketch you set the buzzer pin to LOW and
    if BuzzerState is true while the Button pin is LOW
    the buzzer pin is set to HIGH again.

See here for the second observation:

  if (BuzzerState == true) {

    if (digitalRead(Button) == HIGH) {
      currentMillis = millis();
      if (currentMillis - startMillis >= period) {
        digitalWrite(Buzzer, LOW);
        startMillis = currentMillis;
      }
    } else {
     // This happens when BuzzerState == true and Button state is LOW
      digitalWrite(Buzzer, HIGH);
    }

  }

If you use passive buzzer this creates a sound depending on timing of loop(), the quicker loop runs the higher the sound will be.

Do you have an active buzzer and does it switch ON when the Buzzer pin is set to LOW or to HIGH?

Some hints how to improve readability and traceability of a sketch:

  • Reduce complexity to ease testing and debugging
  • Use functions with parameters to avoid repeating the same code
  • Avoid nested if conditions wherever possible

Here is a version closely based on your code but less complex (and should work as intended):

constexpr byte LED1 = 3;
constexpr byte Level1 = A0;
constexpr byte Buzzer = 2;
constexpr byte Button = 9;
int buttonState = 0;
boolean BuzzerState = false;

unsigned long startMillis;
unsigned long currentMillis;
unsigned long period = 0;
const unsigned long wantedPeriod = 4000;

void setup() {

  pinMode(LED1, OUTPUT);
  pinMode(Level1, INPUT);
  pinMode(Button, INPUT);
  pinMode(Buzzer, OUTPUT);
  Serial.begin(115200);
  startMillis = millis();
}

void loop() {

  buttonState = digitalRead(Button);
  BuzzerState = false;

  if (digitalRead(Level1) == HIGH) {
    digitalWrite(LED1, HIGH);
    BuzzerState = true;
  } else {
    digitalWrite(LED1, LOW);
  }

 if (digitalRead(Button) == HIGH) {
        startMillis = millis();
        // This makes sure that the delay will become valid
        // after the first button press! 
        period = wantedPeriod;
  }

  
  if (millis() - startMillis < period) {
    BuzzerState = false;
  }

  if (BuzzerState == true) {
        digitalWrite(Buzzer, HIGH);
  } else {
       digitalWrite(Buzzer, LOW);
  }
}

Feel free to test it on Wokwi:
https://wokwi.com/projects/464988089266131969

I separated

  • checking the Button pin
  • evaluating the "silence period" and
  • handling the buzzer

If this works as intended there are only a few steps required to add more level switches and leds by using arrays and for-loops ...

Good luck!
ec2021

I understand your corrections... but...

It is not bad advice, it is conveying a concept.

You (in your young life) learned to add before you learned to subtract.

"then + 15" is the same point in time as "now - then > 15"...

... and teaching practices addition before subtraction because one is the easier concept.

Countdown clocks exist.

A great concept to teach after watching a working example walk before it runs.

I didn't code-dump this time, choosing pseudocode, because this isn't a person wanting "the codes, now," and shows aptitude.

Thanks for your understanding.

the concept is "duration" so you do have to subtract the start time from the current time to get the duration and compare it to see if you waited long enough, more than the threshold.

So actually millis() - startSilence > fifteenMins is the way to think about it...

Do you try to put a brave face on it?
Given this post, it looks like you need to relearn unsigned arithmetic...

Mentioning countdown clocks to future date/time is putting on a brave face?

No. Is not the same, at least in matter of overflow unsigned values.

You continue to pretend that there was no gross error in your advice, that's exactly what I meant when I spoke about a brave face.

If you truly don't understand this (as your comment about needing to restart the controller every forty days makes me suspect), then I advise you to search for the words "millis overflow" and carefully read the discussions.

aren't these algebraically idenical?

one adds the interval to the previous
the other subtracts the interval from the current

Adding oil to the fire ? :slight_smile:

trying to understand what the issue is

an example would help

the issue is that we are not in the beautiful infinite numbers world but operate modulo 232

if you do A > B + C then B + C can overflow and return to 0 and the test is evaluated as true whilst the duration (A - C) is not > B in reality.

A - C works fine modulo 232 so this is the way to go

That's exactly the point:

While

A - C > D

and

A > D +C

give identical results if we use numbers that never overflow, the world of 8-, 16-, 32-, or 64-bit integers overflows sooner or later.

The miracle of the modulo is difficult to explain to newcomers and to keep in mind. Therefore, the manner in which the comparison between "subtraction" and "interval" is handled is considered and communicated as "best and only practice".

Although there are lots (if not the most) sketches where a millis() overflow will never occur because the application does only run a few hours or days, it is told to be the only way to do it.

It is "best practice" as it does no harm in the short running sketches but avoids "long term" problems where it is required. We don't have to actively consider the expected runtime.

Best practices usually try to avoid possible (even rare) issues, but I don't see a danger that we run out of interesting technical problems in future ... unless that AI starts coding without any glitch ... Hmm ... :slight_smile:

It probably is when you have a datatype with unlimited bits.

But on real hardware we have limits.
uint32_t can only store number up to (2^32)-1.
The result is that millis() rolls over to 0 after 49.7 days.

Now set startSilence 10 minutes before the overflow and the addition breaks.

Is it millis() or millisCounter that rolls over?

On AVR millis() is only a getter function around the variable. So I think the answer is yes.