Trouble with multiple millis() timers

[TheGuruOfNothing]

I am using an ESP32 Devkit V1 DoIt board with a wroom32 chipset and am using the Arduino IDE to program my board. I am new to coding but have done over 100 hours of research and debugging to try to solve this issue. I have run out of stuff to try and figured I should reach out for some assistance. My guess is that this is something stupid simple and I have looked at this too long to see it.

I am trying to create a controller for a package delivery vault and created the code that I will put below. I have numerous items I wanted timed like blinking LED's and lock, unlock and lockdown timeout operations. The entire project is going to be open source... if I can ever get the controller stable. What it is supposed to do:

Power it up and it will run the unlock solenoid for 6 seconds. (Startup) It will sit and wait for the lid to be opened and light up a green LED. (Ready). Once the lid is opened, it will blink a blue LED and wait for the lid to close. If during that time, the lid is left open for more than 2 minutes, it will send an MQTT message. (Lid Open). Once the lid has been closed, it will turn on a red LED and run a timer for 15 seconds. (Lockdown Timer). Once timed out, it runs the lock solenoid for 6 seconds and then blinks the red LED while waiting for a reset from MQTT. Pretty simple and done in an FSM.

What it DOESN'T do:
It starts up and the timer runs for 6 seconds and moves to ready state and the green LED works. It waits for the lid to open and when it opens, the blink_blue and ajar timers function properly. It sees the lid close, turns on the red light and moves to the lockdown timer state where it stays stuck in that loop and it appears that the timer does NOT increment... I think. I have put in a serial print to print that specific timer and it always stays zero. I have tried MANY MANY times to get that state to work but can't sort it out. It's set up the exact same way to the ones that work but doesn't here. I am out of ideas. Any suggestions you have would be GREATLY appreciated...

Code: [Select all] [Expand/Collapse]

1. /*
2.   Package Vault Controller for ESP32
3.   Copyright 2019 theguruofnothing
4.   Board: ESP32 Devkit V1 (DoIt)
5.   This controller is the brain for "The Package Vault Project", an open source high(er) security package
6.   reception device. It was designed with the sole purpose of curtailing package theft. I want others to take
7.   this design and make it their own. However, I would hope that you would honor my efforts for the greater
8.   good and not use it for unbridaled commercial gain. Custom designed controller units are available at
9.   www.superhouse.tv and I hope you come by and check out the community on the Discourse channel there.
10.   Questions? You can reach me at TheGuruOfNothing@yahoo.com
11. */
12.  
13. #include <WiFi.h>
14. #include <PubSubClient.h>
15.  
16. WiFiClient Vault1;
17. PubSubClient client(Vault1);
18.  
19. //Timer Intervals
20. #define BLINK_INTERVAL        1000     // 800MS BLINK (RED OR BLUE LED)
21. #define BLINK_INTERVAL_TOGGLE 400     // EMERGENCY INDICATOR (R,G,B)
22. #define LID_OPEN_TIME_OUT     120000  // 2 MINUTES BEFORE LID_OPEN MESSAGE TO MQTT
23. #define LOCKDOWN_TIMER        5000   // 15 SECOND LOCKDOWN TIMER
24. #define RELAY_TIMER           6000    // TIME ACTUATOR RELAY SHOULD BE ACTIVE IN ANY DIRECTION
25. #define DEBOUNCE_DELAY        400     // MAG SWITCH DEBOUNCE TIMER
26.  
27. //States
28. #define STATE_STARTUP       0
29. #define STATE_READY         1
30. #define STATE_LID_OPEN      2
31. #define STATE_TIMER_RUNNING 3
32. #define STATE_LOCKING_DOWN  4
33. #define STATE_SECURED       5
34.  
35. //Pin Definitions
36. #define mag_sw       5  //Lid switch
37. #define relay_1      18 //Unlock
38. #define relay_2      19 //Lock
39. #define int_lights   21 //Interior light strip
40. #define pir          2  //Passive infrared sensor, internal to box (Panic/Safety)
41. #define panic_button 4  //Lit button, internal to box (Panic/Safety)
42. #define ledG         12 //Green LED
43. #define ledB         13 //Blue LED
44. #define ledR         14 //Red LED
45.  
46. bool lid_state = 0;
47. bool last_lid_state  = 0;
48.  
49. unsigned long time_now;
50. unsigned long last_debounce_time;
51. unsigned long one;
52. unsigned long two;
53. unsigned long three;
54. unsigned long blink_time = 0;
55. unsigned long ajar = 0;
56. uint16_t last_lid_time =0;
57.  
58. //Your Wifi and MQTT info here
59.  
60. int current_state = STATE_STARTUP;
61.  
62. void setup() {
63.   Serial.begin(115200); //Change this if using Arduino board
64.  
65.   // We set up the pinmode() and starting conditions
66.   pinMode(relay_1, OUTPUT);
67.   pinMode(relay_2, OUTPUT);
68.   pinMode(mag_sw, INPUT_PULLUP);
69.   pinMode(pir, INPUT_PULLUP);
70.   pinMode(panic_button, INPUT_PULLUP);
71.   pinMode(int_lights, OUTPUT);
72.   pinMode(ledG, OUTPUT);
73.   pinMode(ledB, OUTPUT);
74.   pinMode(ledR, OUTPUT);
75.  
76.   //Set the relay's to off to begin with or they float partially active...
77.   digitalWrite(relay_1, HIGH);    //Relay board requires these HIGH to be off
78.   digitalWrite(relay_2, HIGH);    //Relay board requires these HIGH to be off
79.   digitalWrite(int_lights, HIGH); //Relay board requires these HIGH to be off
80.  
81.   //Start up Wifi connection
82.   //Connecting to MQTT server and set callback
83.  
84. }
85.  
86. void read_lid()
87. {
88.   lid_state = digitalRead(mag_sw);
89.   if (millis() - last_debounce_time > DEBOUNCE_DELAY) {   //Debounce Timer
90.     last_debounce_time = millis();
91.   }
92.  
93.   if (lid_state != last_lid_state) {   //check if the   button is pressed or released
94.     last_lid_state = lid_state;
95.   }
96. }
97.  
98. void panic_check() { //This is a kill switch for the box. Forces an unlock and then locks the processor in an infinite loop
99.   // to prevent any possibility of resetting and relocking
100.   int PANIC1 = digitalRead(pir);
101.   int PANIC2 = digitalRead(panic_button);
102.  
103.   if (PANIC1 || PANIC2 == LOW) // Something has gone very wrong and box needs to be unlocked NOW!!!
104.   {
105.     unlock();
106.     Serial.println("Emergency Protocol Activated");
107.     Serial.println("A failure has occurred and vault has been disabled. Power must be cycled to reset");
108.     client.publish("vault/state", "Panic");
109.  
110.   while (true) {} //I want to kill the process so there is no way to restart the  main loop unless power cycled
111.     //so I created an infinite while() loop to do that. Probably a more elegant way to do it
112.     //but let's face it, if this protocol has to be enacted, shit HAS hit the fan...
113.   }
114. }
115.  
116. void unlock()
117. {
118.   time_now = millis();
119.   digitalWrite(relay_2, LOW);  //Starts actuator power
120.   // start up the relay run timer
121.   if ((time_now - one >= RELAY_TIMER) && (RELAY_TIMER >0))
122.    {
123.    one = time_now;
124.    Serial.println(one);
125.    digitalWrite(relay_2, HIGH);  //Stops actuator power
126.    digitalWrite(ledG, HIGH);
127.    }
128. }
129.  
130. void lock()
131. {
132.   time_now = millis();
133.   digitalWrite (int_lights, HIGH);
134.   digitalWrite (relay_1, LOW);  //Starts actuator power
135.   // start up the relay run timer
136.    if ((time_now - two > RELAY_TIMER) && (RELAY_TIMER >0))
137.    {
138.    two = time_now;
139.    digitalWrite (relay_1, HIGH);  //Stops actuator power
140.    }
141. }
142.  
143. void lockout_timer()
144. {
145.   time_now = millis();
146.   if ((time_now - three > LOCKDOWN_TIMER) && (LOCKDOWN_TIMER >0))
147.    {
148.    three = time_now;
149.    Serial.print("Timer has timed out now...");  
150.    }
151. }
152.  
153. void blink_blue()
154. {
155.    //Blink the BLUE LED at once per second...until the reset request is received
156.    if(millis() - blink_time > BLINK_INTERVAL)
157.    {
158.    blink_time = millis();// Update the time for this activity:
159.    digitalWrite(ledB, !digitalRead(ledB));
160.    }
161. }
162.  
163. void blink_red()
164. {
165.    //Blink the RED LED at once per second...until the reset request is received
166.    if(millis() - blink_time > BLINK_INTERVAL)
167.    {
168.    blink_time = millis();// Update the time for this activity:
169.    digitalWrite(ledR, !digitalRead(ledR));
170.    }
171. }
172.  
173. void lid_ajar()
174. {
175.   if (millis() - ajar > LID_OPEN_TIME_OUT) {
176.   ajar = millis();// Update the time for this activity:
177.   client.publish("vault/lid_ajar", "TRUE");
178.   }
179. }
180. void loop() {
181.  
182.   time_now = millis();  //Not sure if this is needed here, but it's there...
183.   read_lid();     // Update the lid status
184.   Serial.println (lid_state);
185.   client.loop();   //Loops connection with MQTT subscriptions
186.  
187.   switch (current_state) {
188.     case STATE_STARTUP:
189.         Serial.println("Startup In Progress...");
190.         Serial.println("______________________");
191.         unlock();
192.         if (one > RELAY_TIMER){
193.         current_state = STATE_READY;
194.         }
195.       break;
196.     case STATE_READY:
197.         Serial.println("Ready For Packages...");
198.         Serial.println("______________________");
199.         if (lid_state == 1)
200.         {
201.         current_state = STATE_LID_OPEN;
202.         }
203.       break;
204.     case STATE_LID_OPEN:
205.         Serial.println("The Lid Has Been Opened...");
206.         Serial.println("______________________");
207.         digitalWrite(ledG, LOW);
208.         if (lid_state == 1)
209.         {
210.          blink_blue();
211.          lid_ajar();
212.         }
213.         else
214.         {
215.         digitalWrite(ledB, LOW);
216.         digitalWrite (int_lights, LOW);
217.         digitalWrite(ledR, HIGH);
218.         current_state = STATE_TIMER_RUNNING;
219.         }
220.       break;
221.     case STATE_TIMER_RUNNING:  
222.         Serial.println ("Timer Operation Has Begun...");
223.         Serial.println ("______________________");
224.         if (three >= LOCKDOWN_TIMER){
225.         current_state = STATE_LOCKING_DOWN;
226.         }
227.       break;
228.     case STATE_LOCKING_DOWN:
229.         Serial.println("Locking The Box Now...");
230.         Serial.println("______________________");
231.         lock();
232.         if (two > RELAY_TIMER){
233.         current_state = STATE_SECURED;
234.         }
235.       break;
236.     case STATE_SECURED:
237.         Serial.println("The captain has extinguished the NO SMOKING sign...");
238.         Serial.println("You may now roam freely about the cabin...");
239.         Serial.println("Vault Secured...");
240.         Serial.println("______________________");
241.         blink_red();
242.         panic_check();  //Check if the panic indicators have been tripped after the lockdown has occurred
243.       break;
244.   }
245. }

GeSHi © Codebox Plus Extension

[mikemoy]

your "case STATE_TIMER_RUNNING: "
never calls lockout_timer(), so it has no way to increment

[TheGuruOfNothing]

Yep... I knew it was something stupid. I missed that when I moved the operation code to functions from the case states.

[boarchuz]

I wrote a similar comment yesterday but you deleted the post! I see you fixed a couple things (not what mike rightly pointed out though). I think most of this is still relevant:

The "one" "two" "three" variables are really confusing and difficult to follow. I think you might even be confusing yourself. Perhaps give them more useful names to make it clear why/when/what they're doing.

eg.

Code: Select all

digitalWrite (relay_1, LOW);  //Starts actuator power
if ((time_now - two > RELAY_TIMER) && (RELAY_TIMER >0))
{
  two = time_now;
  digitalWrite (relay_1, HIGH);  //Stops actuator power
}

First time around, that will start, immediately stop (two is still 0), then set two=time_now which will immediately meet the condition in STATE_LOCKING_DOWN:

Code: Select all

if (two > RELAY_TIMER){
  current_state = STATE_SECURED;
}

Very odd. I think the fundamental issue is that your current_state variable doesn't match the actual current state of your routine. Generally you should enter that state and set current_state at the same time, not set current_state to the next desired state and try to handle initialising and polling it in the same context.
eg. When LOCKDOWN_TIMER expires, it should immediately prepare the next state.

Code: Select all

if (millis() - three >= LOCKDOWN_TIMER){
  Serial.println("Locking The Box Now...");
  Serial.println("______________________");
  lock();
  //Update state
  current_state = STATE_LOCKING_DOWN;
}

where lock() does something like this:

Code: Select all

void lock() //maybe beginLocking() would be more descriptive
{
  time_now = millis();
  digitalWrite (int_lights, HIGH);
  digitalWrite (relay_1, LOW);  //Starts actuator power
  // start up the relay run timer
  two = time_now;
}

Now in STATE_LOCKING_DOWN, two will have a useful value:

Code: Select all

case STATE_LOCKING_DOWN:
  if (time_now - two > RELAY_TIMER){
     digitalWrite (relay_1, HIGH);  //Stops actuator power
    Serial.println("The captain has extinguished the NO SMOKING sign...");
    Serial.println("You may now roam freely about the cabin...");
    Serial.println("Vault Secured...");
    Serial.println("______________________");
    current_state = STATE_SECURED;
  }
  break;

The other issue you have, and you'll see how I've corrected it above, is that you're interchangeably using these "one" "two" "three"s as timestamps and intervals.
Consider this:

Code: Select all

two = time_now;
//...
if (two > RELAY_TIMER)

If two has been set to millis(), which is basically just the time since the ESP32 started, then two will always be > RELAY_TIMER assuming the ESP32 has been running that long. You probably mean to check the difference instead:

Code: Select all

unsigned long relay1StartTime =0; //tells us a lot more than "two" does
//...
relay1StartTime = time_now;
//...
if (time_now - relay1StartTime > RELAY_TIMER)

[TheGuruOfNothing]

@boarchuz... You are correct in your assumptions of what I was trying to accomplish and thank you for your time wading through my coding swamp! I bit off more than I could chew initially on this project, being as it was my first foray into coding. I didn't actually delete the first post. I tried to make an edit to it as I had changed a couple things and because I am a new poster to this board, it had to go to review so it got pulled. That was a bummer. You pointed out some (now) very obvious issues that 6 hours ago I didn't have my head wrapped around. I was working through this code and had posted a request for help on the superhouse.tv forum where all this project grew from and because it is a pretty small group, I was struggling to get pointed in the right direction. And honestly,, I am dense as a post at times Out of the blue, one of the members there who does this stuff blindfolded with one hand tied behind his back took pity on me and grabbed my code and did a massive rework on it to correct my myriad of noobish mistakes and schooled me on the how's and why's of where I went off the rails. As I am sure you are aware, THAT list was long! LOL.

So now I am studying the crap out of the code that works and it is pretty obvious what I had all wrong from the beginning. Funny enough, in the first couple versions of my sketch, I actually was somewhat close with the elements of my code. Then as I tried to debug it and ended up using uneducated guesses to revise it, I got farther and farther away from where it should have been. Uckkk.