Millis not reliable?

Projects Programming
1

Hi all,

I experience problems handling the time with millis.
In general, it works as expected, but sometimes not.
It is hard to retrace, as the function fails in seldom cases only.

Partial code:

// light things
const long LightTime = 10000;
unsigned long TimerStart = 0;


void TimerSet() {
  TimerStart = millis();
  digitalWrite(Light, LOW);
}


void TimerCheck() {
  unsigned long Now = millis();
  if (IsMoving == true) TimerSet();
  if (Now - TimerStart >= LightTime) digitalWrite(Light, HIGH);
}

TimerSet is called in this case when a door opens.
TimerCheck is called at the beginning of the main loop.

Sometimes the light switches off before the time is up (10000 msec).

Any ideas what could be wrong?
Or is there an alternative way to handle the time?

Thanks.

2

Can you post the whole code as I do not see the problem in this snippet.

3

millis() is extremely reliable. So the problem is not millis(), it is your code.

As usual, snippets aren't helpful because the problem isn't there.

4

try this

 if ((Now - TimerStart) >= LightTime) digitalWrite(Light, HIGH);

And post your whole code.

5

I thought the problem is on my side...

The whole sketch:

//---assign pins--------------------------------------------------

// order buttons
const int CommandButton_0 = 2;
const int CommandButton_1 = 3;
const int CommandButton_2 = 4;

// position switches
const int FloorSwitch_0 = 5;
const int FloorSwitch_1 = 6;
const int FloorSwitch_2 = 7;

// door opener
const int DoorButton_0 = 8;
const int DoorButton_1 = 9;
const int DoorButton_2 = 10;

// savety
const int Door = 11;


const int DoorOpener_0 = A2;
const int DoorOpener_1 = A3;
const int DoorOpener_2 = A4;




// motor
const int Motor_up = A0;
const int Motor_down = A1;

// other
const int Light = A5;

// light things
const long LightTime = 10000;
unsigned long TimerStart = 0;
// --------------------------------------------------

// --------------------------------------------------
// positions
const int Floor_0 = 0;
const int Floor_1 = 1;
const int Floor_2 = 2;
const int Floor_Nowhere = 999;

// movement
boolean IsMovingUp = false;
boolean IsMovingDown = false;
boolean IsMoving = false;

// where we are and where to go
int TargetStation = Floor_Nowhere;
int CurrentStation = Floor_Nowhere;

// --------------------------------------------------
void setup() {

  Serial.begin(9600);

  pinMode(FloorSwitch_0, INPUT);
  pinMode(FloorSwitch_1, INPUT);
  pinMode(FloorSwitch_2, INPUT);

  pinMode(CommandButton_0, INPUT);
  pinMode(CommandButton_1, INPUT);
  pinMode(CommandButton_2, INPUT);

  pinMode(Door, INPUT);

  pinMode(DoorOpener_0, OUTPUT);
  pinMode(DoorOpener_1, OUTPUT);
  pinMode(DoorOpener_2, OUTPUT);

  pinMode(DoorButton_0, INPUT);
  pinMode(DoorButton_1, INPUT);
  pinMode(DoorButton_2, INPUT);


  pinMode(Motor_up, OUTPUT);
  pinMode(Motor_down, OUTPUT);

  pinMode(Light, OUTPUT);

  MoveStop();

  digitalWrite(Light, HIGH);

  digitalWrite(DoorOpener_0, HIGH);
  digitalWrite(DoorOpener_1, HIGH);
  digitalWrite(DoorOpener_2, HIGH);

  //digitalWrite(FloorSwitch_2,HIGH);

}

void loop() {

  CurrentStationRead();  // where are we?
  TargetRead();  // read command
  DoorClosed();  // are the doors closed?
  TimerCheck(); // time to switch off the light?

  DoorOpen();//door buttons


  if (TargetStation == Floor_0 )
  {
    switch (CurrentStation) {
      case Floor_0:
        MoveOver(69);
        MoveStop();
        break;
      case Floor_1:
        MoveDown();
        break;
      case Floor_2:
        MoveDown();
        break;
      case Floor_Nowhere:
        MoveDown();
        break;
    }
  }

  if (TargetStation == Floor_1 )
  {
    switch (CurrentStation) {
      case Floor_0: // ist unten
        MoveUp();
        break;
      case Floor_1:// ist schon da
        if (IsMovingDown == true) MoveOver(74);
        if (IsMovingUp == true) MoveOver(69);
        MoveStop();
        break;
      case Floor_2: // ist oben
        MoveDown();
        break;
      case Floor_Nowhere:
        /*
        on reset, when position is unknown, we can't move to Floor_1
        this is impossible as we must know where we are
        if (MovingUp == true) MoveUp();
        if (MovingDown == true) MoveDown();
        if (IsMoving == false) MoveUp();
        */
        break;
    }
  }


  if (TargetStation == Floor_2 )
  {
    switch (CurrentStation) {
      case Floor_0:
        MoveUp();
        break;
      case Floor_1:
        MoveUp();
        break;
      case Floor_2:
        MoveOver(74);
        MoveStop();
        break;
      case Floor_Nowhere:
        MoveUp();
        break;
    }
  }

  // ---END-----------------------------------------------
}

//--------------------------------------------------
void MoveStop() {
  digitalWrite(Motor_up, HIGH);
  digitalWrite(Motor_down, HIGH);
  TargetStation = Floor_Nowhere;
  IsMovingUp = false;
  IsMovingDown = false;
  IsMoving = false;
}
//--------------------------------------------------

//--------------------------------------------------
void MoveUp() {
  digitalWrite(Motor_up, LOW);
  IsMovingUp = true;
  IsMoving = true;
}
//--------------------------------------------------

//--------------------------------------------------
void MoveDown() {
  digitalWrite(Motor_down, LOW);
  IsMovingDown = true;
  IsMoving = true;
}
//--------------------------------------------------

//--------------------------------------------------
void DoorClosed() {
  if (digitalRead(Door) == LOW) //door is open
  {
    TargetStation = Floor_Nowhere;
    digitalWrite(Motor_up, HIGH);
    digitalWrite(Motor_down, HIGH);
    IsMovingUp = false;
    IsMovingDown = false;
    IsMoving = false;
    TimerSet();
  }
}
//--------------------------------------------------

//--------------------------------------------------
void TargetRead() {
  if (IsMoving == false)
  {
    if (digitalRead(CommandButton_0) == HIGH) TargetStation = Floor_0;
    if (digitalRead(CommandButton_1) == HIGH) TargetStation = Floor_1;
    if (digitalRead(CommandButton_2) == HIGH) TargetStation = Floor_2;
    delay(50);
    if (TargetStation < Floor_Nowhere)
    {
      Serial.print("Commanded Target: ");
      Serial.println(TargetStation);
    }
  }
}
//--------------------------------------------------

//--------------------------------------------------
void CurrentStationRead() {
  CurrentStation  = Floor_Nowhere;
  if (digitalRead(FloorSwitch_0) == HIGH) CurrentStation = Floor_0;
  if (digitalRead(FloorSwitch_1) == HIGH) CurrentStation = Floor_1;
  if (digitalRead(FloorSwitch_2) == HIGH) CurrentStation = Floor_2;
}
//--------------------------------------------------

//--------------------------------------------------
void MoveOver(short mm) {
  short msec_per_mm = 3.036;
  delay(mm * msec_per_mm);
}
//--------------------------------------------------

//--------------------------------------------------
void TimerSet() {
  TimerStart = millis();
  digitalWrite(Light, LOW);
}
//--------------------------------------------------

//--------------------------------------------------
void TimerCheck() {
  unsigned long Now = millis();
  if (IsMoving == true) TimerSet();
  if (Now - TimerStart >= LightTime) digitalWrite(Light, HIGH);
}
//--------------------------------------------------

//--------------------------------------------------
void DoorOpen() {

  if (IsMoving == true)
  {
    digitalWrite(DoorOpener_0, HIGH);
    digitalWrite(DoorOpener_1, HIGH);
    digitalWrite(DoorOpener_2, HIGH);
    return;


  }

  switch (CurrentStation) {
    case Floor_0:

      if (digitalRead(DoorButton_0) == HIGH) //door button is pressed
      {
        digitalWrite(DoorOpener_0, LOW);
        TimerSet();
      }
      else
      {
        digitalWrite(DoorOpener_0, HIGH);
      }


      break;

    case Floor_1:

      if (digitalRead(DoorButton_1) == HIGH) //door button is pressed
      {
        digitalWrite(DoorOpener_1, LOW);
        TimerSet();
      }
      else
      {
        digitalWrite(DoorOpener_1, HIGH);
      }

      break;
    case Floor_2:

      if (digitalRead(DoorButton_2) == HIGH) //door button is pressed
      {
        digitalWrite(DoorOpener_2, LOW);
        TimerSet();
      }
      else
      {
        digitalWrite(DoorOpener_2, HIGH);
      }



      break;
    case Floor_Nowhere:

      digitalWrite(DoorOpener_0, HIGH);
      digitalWrite(DoorOpener_1, HIGH);
      digitalWrite(DoorOpener_2, HIGH);

      break;
  }



}
//--------------------------------------------------
6

Just to verify, you've wired your LED so that LOW will turn it on and HIGH will turn it off?

7

Yes.
A relay board requires to do so.
The whole application works ok since several months, only obvious problem is the time.

8 
short msec_per_mm = 3.036;

you realise "short" is an integer?

9

TimerSet is called in several places, not just when a door is open.

Your code is a potential minefield as you are not detecting changes in button
switch state, just probing the current value again and again. Thus many functions
are being called repeatedly while the relevant button is pressed or whatever.

First you need to debounce your switches and buttons and then you need to
detect them changing in value, and trigger your events from the changes.
Otherwise its going to be really hard to see the wood for the trees and debug
the code.

Have a look at the StateChangeDetection example perhaps?

Having said that I can't see any obvious way the light timeout can happen early,
but you should put in some debugging println's to check when TimerSet and
TimerCheck are actually being called.

10

Gregor77:
A relay board requires to do so.

And we magically knew you were using a relay, how? What does Light mean? Have you spliced up an AC mains line to turn on a CFL bulb?

There could be some kind of EMI interference that might be tripping the relay or the transistor driver (i'm assuming there is one) that is driving the relay.

Gregor77:
The whole application works ok since several months, only obvious problem is the time.

Well that's obvious. Your code mixes unsigned and signed longs.

So I would be surprised if it worked for 49 days and then behaved funny.

11

AWOL:

short msec_per_mm = 3.036;

you realise "short" is an integer?

Oh.
No, I didn't realize this, I need to fix this.
Interestingly, the MoveOver works as expected as it is near 3.
But I agree, that is an error to fix.

12

Did you really mean to comment out these 3 lines?

     case Floor_Nowhere:
        /*
        on reset, when position is unknown, we can't move to Floor_1
        this is impossible as we must know where we are
        if (MovingUp == true) MoveUp();
        if (MovingDown == true) MoveDown();
        if (IsMoving == false) MoveUp();
        */
13

[quote author=James C4S date=1433019186 link=msg=2255303]
And we magically knew you were using a relay, how?[/quote]
I thought this would be useless info, now I know better.

What does Light mean? Have you spliced up an AC mains line to turn on a CFL bulb?

It is a standard bulb.
I also thought of interference, so I replaced the bulb by a battery-driven LED, same effect.

Well that's obvious. Your code mixes unsigned and signed longs.

Hmm, can you tell me where (except in MoveOver)?

So I would be surprised if it worked for 49 days and then behaved funny.

49 days?
I don't understand.

14

aarg:
Did you really mean to comment out these 3 lines?

Yes, because I realized it can't work.
I should have deleted them before posting the code, but I wanted it show it unaltered.

15

MarkT:
Have a look at the StateChangeDetection example perhaps?

I will definitely have a look.
Thanks for the hint.

Having said that I can't see any obvious way the light timeout can happen early,
but you should put in some debugging println's to check when TimerSet and
TimerCheck are actually being called.

The problem is, the function fails seldom only.
It is so seldom that I can't sit there for days or weeks to watch what happens...

The light never fails to switch on, but sometimes switches off to early.
In any case it switches off.

16

I see now.
LightTime must be unsigned long as well?

17

LightTime must be unsigned long as well?

Yes anything to do with time must be.

18

Grumpy_Mike:
Yes anything to do with time must be.

No. Intervals like LightTime (but only intervals) can be any type that can contain the desired interval value. In this case the value is 10000, so even an int can be safely used.