Input from buttons sporadic

I’ve done a sketch, with much coding input from Péter Esch.

It’s a countdown timer based on a Uno R3 and TM1638 unit.
The code in itself works fine, but sometimes the board does not respond to buttons being pushed.

#include <TM1638.h>
#include <Timer.h>

bool enabled = false; // counting is enabled or not
byte counter = 0; // the counter (for main loop, counter max = 1000 msec / delay)
byte hours = 0, minutes = 0, seconds = 0; // trivial
bool btnpressed = false; // using to on/off and brightness switch don’t repeat if pressed
byte animation = 0; // animations’ counter
byte brightness = 0; // brightness of the display (0…7)
bool DoIt = false;
int8_t clockEvent;
const int buzzer = 12; //buzzer to arduino pin 12
// define a regular module
TM1638 module(8,9,7); // data, clock, strobe
Timer t;
// initial setup when arduino starts
void setup() {
randomSeed(analogRead(0));
pinMode(buzzer, OUTPUT); // Set buzzer - pin 12 as an output
// initialize the timer to call clockHeartBeat function in every seconds
clockEvent = t.every(1000, clockHeartBeat);
// intialize TM1638 module
module.setupDisplay(true, brightness);
Initialize();
}
// initialize display and variables before use
void Initialize() {
enabled = false;
// test all LEDs and segments
writeByteToLEDs(255);
writeStrToDisplay(“88888888”);
delay(1000);
digitalWrite(12, HIGH);
delay(250); // …for 250 miliseconds
digitalWrite(12, LOW);
writeByteToLEDs(0);
writeStrToDisplay(" ");
}
// show the countdowning (format: hh-mm-ss)
void writeTimeToDisplay() {
char s[8];
sprintf(s, “%02d-%02d-%02d”, hours, minutes, seconds);
writeStrToDisplay(s);
}
// write 8 char to the display
void writeStrToDisplay(char s[8]) {
module.setDisplayToString(s);
}
// set output LEDs on or off
void writeByteToLEDs(byte led) {
module.setLEDs(led);
}
// main procedure when hours, minutes and seconds reached the bottom
void TimeIsOver() {
byte buttons = 0;
writeByteToLEDs(0);
InitializeAnimation();

while (buttons == 0) {
DoAnimation_HorizontalSnake();
delay(25);
buttons = module.getButtons();
}
// reinitialize the system
Initialize();
}
// initialize animations’ counter
void InitializeAnimation() {
animation = 0;
}
// animation: ___
void DoAnimation_HorizontalSnake() {
byte values[8] = {0,0,0,0,0,0,0,0};
switch (animation) {
case 0: values[0] = 8; break;
case 1: values[1] = 8; break;
case 2: values[2] = 8; break;
case 3: values[3] = 8; break;
case 4: values[4] = 8; break;
case 5: values[5] = 8; break;
case 6: values[6] = 8; break;
case 7: values[7] = 8; break;
case 8: values[7] = 4; break;
case 9: values[7] = 64; break;
case 10: values[6] = 64; break;
case 11: values[5] = 64; break;
case 12: values[4] = 64; break;
case 13: values[3] = 64; break;
case 14: values[2] = 64; break;
case 15: values[1] = 64; break;
case 16: values[0] = 64; break;
case 17: values[0] = 32; break;
case 18: values[0] = 1; break;
case 19: values[1] = 1; break;
case 20: values[2] = 1; break;
case 21: values[3] = 1; break;
case 22: values[4] = 1; break;
case 23: values[5] = 1; break;
case 24: values[6] = 1; break;
case 25: values[7] = 1; break;
case 26: values[7] = 2; break;
case 27: values[7] = 64; break;
case 28: values[6] = 64; break;
case 29: values[5] = 64; break;
case 30: values[4] = 64; break;
case 31: values[3] = 64; break;
case 32: values[2] = 64; break;
case 33: values[1] = 64; break;
case 34: values[0] = 64; break;
case 35: values[0] = 16; break;
}
module.setDisplay(values);
animation++;
if (animation > 35) {
animation = 0;
}
}
// heartbeat for the clock signal
void clockHeartBeat() {
DoIt = true;
}
// the main loop
void loop() {
byte buttons = module.getButtons();
// handling the buttons
if (buttons != 0) {
byte abutton = buttons >> 1;
switch (abutton) {
case 0: // start
if (btnpressed == false) {
if (enabled == true) {
enabled = false;
DoIt = false;
} else {
enabled = true;
DoIt = false;
// / t.Reset(clockEvent);
}
writeByteToLEDs(1);
}
btnpressed = true;
break;
case 1:
if (btnpressed == false) {
brightness++;
if (brightness > 7) {
brightness = 0;
}
module.setupDisplay(true, brightness);
writeByteToLEDs(2);
}
btnpressed = true;
break;
case 2:
if (hours > 0) {
hours–;
}
writeByteToLEDs(4);
break;
case 4:
if (hours < 99) {
hours++;
}
writeByteToLEDs(8);
break;
case 8:
if (minutes > 0) {
minutes–;
}
writeByteToLEDs(16);
break;
case 16:
if (minutes < 59) {
minutes++;
}
writeByteToLEDs(32);
break;
case 32:
if (seconds > 0) {
seconds–;
}
writeByteToLEDs(64);
break;
case 64:
if (seconds < 59) {
seconds++;
}
writeByteToLEDs(128);
break;
}
} else {
btnpressed = false;
}
// if counting is enabled
if ((enabled == true) && (DoIt == true)) {
DoIt = false;
if (seconds > 0) {
seconds–;
} else {
if (minutes > 0) {
minutes–;
seconds = 59;
} else {
if (hours > 0) {
hours–;
minutes = 59;
seconds = 59;
} else {
// if hours, minutes and seconds reached the bottom
hours = 0;
minutes = 0;
seconds = 0;
enabled = false;
digitalWrite(12, HIGH);
delay(5000);
digitalWrite(12, LOW);
TimeIsOver();
}
}
}
}
// always show time in the main loop during normal operation
writeTimeToDisplay();
// if a button has been pressed, turn off the attached LED
if (buttons != 0) {
writeByteToLEDs(0);
}
// updating the timer
if (enabled == true) {
t.update();
}
// delaying the main loop, debouncing button presses, etc.
delay(300);
}

I know there are several improvements to be made, that’s purely cosmetic.
My main issue is when pressing the buttons (S1 being Start, S2 Brightness, S3 -Hours, S4 +Hours, S5 -Minutes, S6 +Minutes, S7 -Seconds and S8 +Seconds), sometimes the requisite action does not take place until buttons have been pressed multiple times (the amount and timing of presses seem to be random)

Fairly new to Arduino and programming so please bear with me!

Any ideas?

  char s[8];
  sprintf(s, "%02d-%02d-%02d", hours, minutes, seconds);

2 digits, a dash, 2 digits, a dash, 2 digits, and a NULL terminator will not fit in an 8 element array.

I know there are several improvements to be made, that's purely cosmetic.

Nonsense. There are many improvements to be made. They are NOT "purely cosmetic".

The one above is not.

Getting rid of the stupid delay() at the end of loop() is NOT "purely cosmetic", either.