Millis() is wayy off

I'm trying to implement a function in my code that waits for 5 seconds to pass (5000 millis) but millis() in my script is so off that I have to write 300000 (300 seconds) for it to wait 5 seconds. (No, I am not accidentally using micros()) I tried creating a blank sketch that outputs millis() and that sketch returns the correct number, so we know it's not a hardware issue. Here's the file (I'm updating the value on line 51, but other parts of the program also rely on the correct millis() time.) Thanks!
Bathroom_Alarm.ino (1.6 KB)

The easier you make it to read and copy the code the more likely it is that you will get help

Please follow the advice given in the link below when posting code , use code tags and post the code here

Welcome to the forums. Please take a moment and read the sticky post at the top about how to post. It helps people help you. Specifically, include your code so people can easily see it.

The first thing I would suggest, is look at the blink without Delay example in the IDE (File->examples->02.digital->Blink WIthout Delay) It will show you the proper way to track elapsed time. The method you are using is flawed. This statement

        unsigned long fakeDelay = millis();
        while (fakeDelay + 8000 > millis()) {}

is no different than delay(8000); since you are blocking

Second, get rid of all your while() statements. It is much better to structure your code more like a state machine and let loop() execute as many times as possible.

But what is causing the millis() to count way more milliseconds than actually pass?

A flaw in your code. The logic is difficult to follow. What exactly are you trying to accomplish? What is the 'contact' pin? Is it some momentary push button or a relay or what? My guess is that you may want to detect when the contact is closed, not if. To do that, look at the State Change Example in the IDE (File->examples->02.digitial->StateChangeDetection)

It's essentially a door sensor that detects if a door is open or closed. There will be a contact on the door and the wall and when they connect, contact will return LOW. I'll check out StateChangeDetection.

Why don't you try to use

Delay()

It is the exact same as mils() [i think]
But maybe it could work??

Maybe just try it!

Or maybe the Arduino is overloaded?

Just saying :slight_smile:

1 Like

@Imploded42, next time please insert your code using code tags in a post. This is your code, properly posted. Just easier for everybody else.

#include <Volume.h>

const int buzzer = 5; //pin for speaker
const int contact = 2; //contactIn is the ground pin

boolean madeFirstContact = false;
boolean alreadyLostContact = false;
unsigned long lostContactAt = 0;

Volume vol;

void setup() {
  vol.begin();
  pinMode(contact, INPUT_PULLUP);
  Serial.begin(9600);
  Serial.println("SUBSCRIBE TO SACROSAUNT");
}

void loop() {
  int volume = 0;
  int buzTone = 300;
  unsigned long currentMillis = millis();
  vol.noTone();
  noTone(buzzer);
  while (stillConnected()) {
    if (currentMillis + 2000 < millis()) { // 600000 actual, 200000 for testing (delay after contact to start alarm)
      while (stillConnected()) {
        vol.tone(buzTone, volume);
        //tone(buzzer, 3000);
        if (buzTone < 3500) {
          buzTone += 10;
        }
        if (volume <= 250) {
          volume += 1;
        }

        unsigned long fakeDelay = millis();
        while (fakeDelay + 8000 > millis()) {}
      }
    }
  }
}
boolean stillConnected() {
  if (digitalRead(contact) == LOW) { //LOW means current closed, HIGH means current opened
    Serial.println("CONTACT");
    alreadyLostContact = false;
    madeFirstContact = true;
    lostContactAt = 0;
    return true;
  }
  else if (alreadyLostContact && (millis() > (lostContactAt + 300000))) {
    Serial.println("NO CONTACT FOR MORE THAN 5 SECONDS");
    return false;
  }
  else {
    if (!alreadyLostContact) {
      alreadyLostContact = true;
      lostContactAt = millis();
    }

    if (madeFirstContact) {
      return true;
    } else {
      return false;
    }
  }
}

Sorry about that. First time poster.

I found the problem! It appears to be a fault in the volume library. I commented out the lines using the library and millis is now working correctly.

Sounds like to definitely want StateChangeDetection. You should also consider implementing this like a state machine... You could have something like DOOR_OPEN, DOOR_CLOSED, ALARM_ON, ...

Then, depending on which state you are in, you do whatever you need (e.g. if DOOR_OPEN but we just detected it closed, turn off alarm, set state to DOOR_CLOSED) This way, you can track elapsed time from when a state began (e.g. if DOOR_OPEN and millis() - startTime > maxDuration, then make alarm really loud...)

You are very mistaken if you think that millis() is the same as delay()

Oh, but then what is the difference?

I am kinda new

If you use delay(), the processor sits inside that statement until the required delay time has elapsed. This means it cannot do anything else while it is executing delay(). The experts here call that "blocking" the processor.

If you use millis(), you can set a timestamp in the future, eg:

futureTimeStamp = millis() + requiredDelay;

Then, the processor can get on with other things (e.g. scanning a keyboard, updating a display), PROVIDED it comes back every so often to see if millis() has reached or exceeded the required delay time.

It's more complex, but allows for more flexibility in how your code is structured. Having said that, if your code is simple and never needs to do anything else during the delay period, then delay() is absolutely fine. Better, actually, because it is simpler and it is never good to make a solution more complicated than it needs to be. Most of my code uses state machines with millis() everywhere, but sometimes I use delay(), if it's a simple job.

Ok Thanks a lot!

The simple version :

  • delay() stops the program from doing anything else for the period of the delay

  • millis() returns the number of milliseconds since the last time the processor was powered up or reset

No, do not do that. It is bad code and will eventually bite you if this is long running code since milis() rolls over to 0 every 49.xx days. The only guaranteed way to do this is

startTimeStamp = millis();
...
if ( millis() - startTimeStamp >= desiredDelay) { ...

Perfectly correct, and that was for the next lesson! As I wrote it, it is much easier to understand, and for the sake of demonstration, it works fine. As I said, I think the OP needs to learn this stuff in tiny bites.

So what happens when millis roll over just before if condition?

It still works, because of the way negative numbers are represented in integers.