Count down timer project locks up

I have been working on this timer project for a little while. At this point it seems to be working fine. The one thing I am have trouble with is the whole thing seems to lock up randomly (only happened a few times) usually at or near startup. It will quit responding to any inputs including the rotary encoder. The reset button on the board does not effect it. Only unplugging the board and plugging it back in fixes it. I'm using an Arduino Uno and everything is connected using a solderless breadboard. Not sure if the problem is in my code, or if maybe there is a problem with my Arduino board, or my breadboard setup. Really not sure.

#include <EEPROM.h>

#include <LiquidCrystal_I2C.h>

#include <Wire.h>

LiquidCrystal_I2C lcd(0x27, 16, 2);


#define clkPin 2 //clk, dt, and sw pins from the rotary encoder
#define dtPin 4
#define swPin 6
#define startPin 8 //start/pause button
#define memoryPinA 9 //saved time buttons
#define memoryPinB 10
#define longPressTime 1500
#define uvPin 11 //uv light
#define buzzerPin 12
#define buzzerFreq 440
#define buzzOnTime 750
#define buzzOffTime 250
#define NumberOfBuzzes 3
#define flashOnTime 400
#define flashOffTime 250
#define debounceTime 10
volatile int timeSel = 1;
volatile int hours;
volatile int minutes;
volatile int seconds;
bool startPinState = false;

void encoderISR() {
  static unsigned long lastInterruptTime = 0;
  unsigned long interruptTime = millis();
  if (interruptTime - lastInterruptTime > debounceTime) {
    if (digitalRead(dtPin) == HIGH) {
      if (timeSel == 0) {
        hours ++;
      }
      if (timeSel == 1) {
        minutes ++;
      }
      if (timeSel == 2) {
        seconds ++;
      }
    }
    else {
      if (timeSel == 0) {
        hours --;
      }
      if (timeSel == 1) {
        minutes --;
      }
      if (timeSel == 2) {
        seconds --;
      }
    }
    hours = max(0, hours);
    minutes = min(59, max(0, minutes));
    seconds = min(59, max(0, seconds));
    lastInterruptTime = interruptTime;
  }
}


void toggleHMS() {  //steps through the variable used to select housr, minutes, and seconds
  if (timeSel < 2) {
    timeSel ++;
  }
  else {
    timeSel = 0;
  }
  if (timeSel == 0) {
    flashHours();
    delay(flashOffTime);
    printTime();
    delay(flashOnTime);
    flashHours();
    delay(flashOffTime);
    printTime();
  }
  if (timeSel == 1) {
    flashMinutes();
    delay(flashOffTime);
    printTime();
    delay(flashOnTime);
    flashMinutes();
    delay(flashOffTime);
    printTime();
  }
  if (timeSel == 2) {
    flashSeconds();
    delay(flashOffTime);
    printTime();
    delay(flashOnTime);
    flashSeconds();
    delay(flashOffTime);
    printTime();
  }
  while (!digitalRead(swPin))
    delay(debounceTime);
}

void flashHours() {
  lcd.clear();
  lcd.setCursor(4, 0);
  lcd.print("  ");
  // lcd.print(hours);
  lcd.print(":");
  if (minutes < 10) {
    lcd.print("0");
  }
  lcd.print(minutes);
  lcd.print(":");
  if (seconds < 10) {
    lcd.print("0");
  }
  lcd.print(seconds);
}

void flashMinutes() {
  lcd.clear();
  lcd.setCursor(4, 0);
  if ( hours < 10) {
    lcd.print(" ");
  }
  lcd.print(hours);
  lcd.print(":");
  lcd.print("  ");
  lcd.print(":");
  if (seconds < 10) {
    lcd.print("0");
  }
  lcd.print(seconds);
}

void flashSeconds() {
  lcd.clear();
  lcd.setCursor(4, 0);
  if ( hours < 10) {
    lcd.print(" ");
  }
  lcd.print(hours);
  lcd.print(":");
  if (minutes < 10) {
    lcd.print("0");
  }
  lcd.print(minutes);
  lcd.print(":");
  lcd.print("  ");
}

void printTime() {
  lcd.clear();
  lcd.setCursor(4, 0);
  if ( hours < 10) {
    lcd.print(" ");
  }
  lcd.print(hours);
  lcd.print(":");
  if (minutes < 10) {
    lcd.print("0");
  }
  lcd.print(minutes);
  lcd.print(":");
  if (seconds < 10) {
    lcd.print("0");
  }
  lcd.print(seconds);
}


void countDown() {
  if (seconds > 0) {
    seconds --;
  }
  else if (minutes > 0) {
    minutes --;
    seconds = 59;
  }
  else if (hours > 0) {
    hours --;
    minutes = 59;
    seconds = 59;
  }
}


void buzz() {
  for (int j = 0; j < NumberOfBuzzes; j++) {
    tone(buzzerPin, buzzerFreq);
    delay(buzzOnTime);
    noTone(buzzerPin);
    delay(buzzOffTime);
  }
}

void callMemoryA() {
  hours = EEPROM.read(0);
  minutes = EEPROM.read(1);
  seconds = EEPROM.read(2);
  printTime();
}

void callMemoryB() {
  hours = EEPROM.read(10);
  minutes = EEPROM.read(11);
  seconds = EEPROM.read(12);
  printTime();
}

void updateMemoryA() {
  EEPROM.update(0, hours);
  EEPROM.update(1, minutes);
  EEPROM.update(2, seconds);
  lcd.clear();
  delay(flashOffTime);
  printTime();
  delay(flashOnTime);
  lcd.clear();
  delay(flashOffTime);
  printTime();
}

void updateMemoryB() {
  EEPROM.update(10, hours);
  EEPROM.update(11, minutes);
  EEPROM.update(12, seconds);
  lcd.clear();
  delay(flashOffTime);
  printTime();
  delay(flashOnTime);
  lcd.clear();
  delay(flashOffTime);
  printTime();
}

void setup() {
  //Serial.begin(9600);
  lcd.init();
  lcd.backlight();
  pinMode(clkPin, INPUT);
  pinMode(dtPin, INPUT);
  pinMode(swPin, INPUT_PULLUP);
  pinMode(startPin, INPUT_PULLUP);
  pinMode(memoryPinA, INPUT_PULLUP);
  pinMode(memoryPinB, INPUT_PULLUP);
  pinMode(uvPin, OUTPUT);
  pinMode(buzzerPin, OUTPUT);
  attachInterrupt(digitalPinToInterrupt(clkPin), encoderISR, LOW);
  printTime();
}

void loop() {
  static unsigned long lastTime = 0;
  unsigned long currentTime = millis();
  int timeSum = (hours + minutes + seconds);
  static int lastTimeSum;

  if (timeSum == 0 && startPinState == true) {

    startPinState = false;
    digitalWrite(uvPin, LOW);
    printTime();
    buzz();

  }

  if (currentTime - lastTime >= 1000) {
    if (startPinState == true && timeSum > 0) {
      countDown();
      digitalWrite(uvPin, HIGH);
      lastTime = currentTime;
    }
    else {
      digitalWrite(uvPin, LOW);
    }
  }

  if ((!digitalRead(swPin))) {
    toggleHMS();
  }

  if ((!digitalRead(startPin))) {
    startPinState = !startPinState;
    while (!digitalRead(startPin))
      delay(debounceTime);
  }

  if (timeSum != lastTimeSum) {  //only updates the lcd if the time changes
    printTime();
    lastTimeSum = timeSum;
  }

  if ((!digitalRead(memoryPinA))) {
    unsigned long buttonStart = millis();
    unsigned long buttonEnd;
    delay(debounceTime);
    while ((!digitalRead(memoryPinA))) {
      buttonEnd = millis();
    }

    if (buttonEnd - buttonStart >= longPressTime) {
      updateMemoryA();
    }
    else {
      callMemoryA();
    }
    startPinState = false;
    delay(debounceTime);
  }

  if ((!digitalRead(memoryPinB))) {
    unsigned long buttonStart = millis();
    unsigned long buttonEnd;
    delay(debounceTime);
    while ((!digitalRead(memoryPinB))) {
      buttonEnd = millis();
    }

    if (buttonEnd - buttonStart >= longPressTime) {
      updateMemoryB();
    }
    else {
      callMemoryB();
    }
    startPinState = false;
    delay(debounceTime);
  }
}

If you see anything in the code that should be done differently, I would certainly appreciate the feedback. Still learning. Thanks in advance!

What shows on the LCD during the lock-up? I can not see how the lockup generator is connected, so please, show a wiring diagram.

The code itself is not how I would write it but there is nothing that would create what you describe (what's not great is not protecting the use of the volatile variables within the loop with a critical section but issues there would not create the behaviour you describe)

this part is weird.

I'd be tempted to say that there is a short circuit somewhere and when you unplug and plug again you shake the board / wires enough to "fix" it... check the wires and soldering.

Also if you remove physically the buzzer - do you still see the issue ?

PS: the code could be easier if you were using the encoder library and one of the numerous button library such as Button in easyRun or OneButton or Toggle or EasyButton or Bounce2, ...

Once that I can remember it showed the hours and the colon that follows them, at least once it did not show anything, and at least once it displayed the time as it was set.

As far as posting the wiring diagram, should I draw something up by hand and just take a picture? Or is there a program I can use to illustrate it? I know I've seen several diagrams that look like they're done in some kind of CAD. Tinkercad maybe?

drawing something by hand that ressembles your exact wiring is good. it does not need to be photo realistic, a rectangle with numbers for the pins you use for example would be good enough for the arduino.

Have you double checked your wires ?

did you do any soldering job yourself ?

a in focus photo of the gig could also help

I have not tried it without the buzzer connected. I can try that when I get home.

I will certainly check everything for shorts.

I'm not sure what you mean concerning protecting the volatile variables within the loop.

You have those variables

and you update them within the ISR

the rest of the code is also using them. You could see weird results happening if an interrupts happens right in the middle of your code reading those ints

typically you would have at the beginning of the loop a piece of code where you disable interrupts, create a copy of those variables and then re-enable the interrupts. Then the rest of the loop plays with the copy. This way there is no risk of the main loop working with bytes that might be changed as you proceed.

But as I wrote, that would just mess with the time, not create the issue you described.

So:

noInterrupts();
//coppy variables
interrupts();

That's what you're saying?

I will get something drawn up as soon as I can.

Yes, that would make the code more robust.

before drawing the wiring, check without the buzzer (what type of buzzer is this?) and double check all the wires for short or loose connections

It is a Piezo buzzer that comes with one of the Sunfounder starter kits. One pin is connected to the buzzer pin of the Arduino and the other pin is connected to ground through a 220 Ohm resistor.

Pretty sure that's too much going on in an ISR.

Generally, an ISR should be as short and fast as possible. If your sketch uses multiple ISRs, only one can run at a time, other interrupts will be executed after the current one finishes in an order that depends on the priority they have. millis() relies on interrupts to count, so it will never increment inside an ISR. Since delay() requires interrupts to work, it will not work if called inside an ISR. micros() works initially but will start behaving erratically after 1-2 ms. delayMicroseconds() does not use any counter, so it will work as normal.

Plus

Inside the attached function, delay() won’t work and the value returned by millis() will not increment. Serial data received while in the function may be lost. You should declare as volatile any variables that you modify within the attached function. See the section on ISRs below for more information.

source:

Nope - that’s globally fine. It’s a lot of lines but just a few tens of microseconds in execution

Millis won’t update but you can read the value so it’s fine

Ok then.

Ok so I've hooked up this circuit using an Uno R3 and all the exact same starter kit parts.
So on startup,

  • the lcd lights up and displays 0:00:00
  • nothing until I do something else, so
  • I press start button and hear three beeps
  • the clock stays at 0:00:00
  • I turn the rotary encoder one click and lcd reads 0:01:00
  • I press start button again, clock counts down to 0:00:00
  • buzzer then beeps three times again
    Ok, so what else is it supposed to do? There are two other buttons, memory A and memory B.
    What are they for?
    The flow I described above without touching the memory buttons unnecessarily to save writes to my EEPROM seems to work fine.
    I can't duplicate your error.
    Can you tell me what or if I've missed anything?

As you kinda said

touching the memory buttons to save writes to my EEPROM

Long press saves what’s on display, short press brings memory back

Hey, you never can be too cautious with these countdown-clock-type of projects :upside_down_face:

Also, @Beef5upreme , I can't replicate your issue. Seems to work great.

Yeah, I suspect the real issue is with the board or with my setup. Everything is just plugged into a solderless breadboard and connected with jumper wires so a lot of room for error I suppose.

Only one thing to do then: move it to different hardware. I still wouldn't rule out what @J-M-L was saying in post #7 and test further, probably a great optimization in any case since it's coming from a forum guru (I don't understand the deeper stuff like he does), @J-M-L is orders of magnitude more skilled at all this stuff than I am.

Good luck in any case, and if you solve the issue, maybe pop back in and let us know what you discovered.

And of course, one good code deserves another, since I'm blatantly going to use this code in a Hallowe'en prop (I have a dozen or so maglocks kicking around looking for a home), so if you're looking for any prop/parlor game type code, I might already have something that's close to what you're after, let me know!

I went ahead and edited the post to include a wiring diagram. Hopefully it's clear enough.

1 Like