Help me improve my code

Using Arduino Project Guidance
#1

Hello there!

I am trying to make two count down timers on LCD keypad shield.
The idea is when the first timer goes to 00:00 (MM:SS), the second one to start counting. I have made something so far. But there is a problem. When the first timer goes to 00:01 the second timer starts to count, i do not know what i have mistaken.

I am pretty shure the code can be made to look a lot more “elegant”.

Please assist me.

#include <LiquidCrystal.h>
LiquidCrystal lcd(8, 9, 4, 5, 6, 7);
int a;
int b;
int c;
int d;
void setup() {
  lcd.begin(16, 2);
  lcd.print("Timer 1");
  lcd.setCursor(9,0);
  lcd.print("Timer 2");
  lcd.setCursor(2,1);
  lcd.print(":");
   a=0; //minuti TMR1:
   b=10; //sekundiTMR1:
   c=10; //minuti TMR2:
   d=20; //sekundi TMR2:
   
   lcd.setCursor(0,1);
   lcd.print(a);
   lcd.setCursor(2,1);
   lcd.print(":");
   lcd.setCursor(3,1);
   lcd.print(b);
   lcd.setCursor(11,1);
   lcd.print(c);
   lcd.setCursor(13,1);
   lcd.print(":");
   lcd.setCursor(14,1);
   lcd.print(d);
   
}

void loop() { 
  if (a>20)
  {
    a=20;
  }
  if (a>0 || b>0)
  {
  if (a>=10)
  {
  lcd.setCursor(0,1);
  lcd.print(a);
  }
  else
  {
    lcd.setCursor(0,1);
    lcd.print("0");
    lcd.setCursor(1,1);
    lcd.print(a);
  }
  if (b>=10)
  {
  lcd.setCursor(3,1);
  lcd.print(b);
  }
  else
  {
    lcd.setCursor(3,1);
    lcd.print("0");
    lcd.setCursor(4,1);
    lcd.print(b);
  }
  if (b==0)
  {
    b=60;
    a--;
  }
  b--;
  }
  else
  {
    lcd.setCursor(0,1);
    lcd.print("00:00");
  }
if (a==0 && b==0)
{
  d--;
  if (d>=10)
  {
  lcd.setCursor(14,1);
  lcd.print(d);
  }
  else
  {
    lcd.setCursor(14,1);
    lcd.print("0");
    lcd.setCursor(15,1);
    lcd.print(d);
  }
if (d==0)
{
  c--;
  d=60; 
}
if (a>=10)
{
  lcd.setCursor(11,1);
  lcd.print(a);
}
else
{
  lcd.setCursor(11,1);
  lcd.print("0");
  lcd.setCursor(12,1);
  lcd.print(a);
}
}
 delay(1000);
}
#2

I find it impossible to make sense of your code because you have not used meaningful variable names. Instead of a, b, c etc give them names that reflect what the variable is for.

Have a look at Planning and Implementing a Program

...R

#3

How is that?

#include <LiquidCrystal.h>
LiquidCrystal lcd(8, 9, 4, 5, 6, 7);

int minTMR1; //the varriable holding the minutes for the first timer:
int secTMR1; //the variable holding the seconds for the first timer:
int minTMR2; //the varriable holding the minutes for the second timer:
int secTMR2; //the variable holding the seconds for the second timer:
void setup() {
  lcd.begin(16, 2);
  lcd.print("Timer 1");
  lcd.setCursor(9,0);
  lcd.print("Timer 2");
  lcd.setCursor(2,1);
  lcd.print(":");
   minTMR1=0; //minutes TMR1:
   secTMR1=10; //secondsTMR1:
   minTMR2=10; //minutes TMR2:
   secTMR2=20; //seconds TMR2:
   
   lcd.setCursor(0,1);
   lcd.print(minTMR1); //print minuters for TMR1:
   lcd.setCursor(2,1);
   lcd.print(":");
   lcd.setCursor(3,1);
   lcd.print(secTMR1); //print seconds for TMR1:
   lcd.setCursor(11,1);
   lcd.print(minTMR2); //print minutes for TMR2:
   lcd.setCursor(13,1);
   lcd.print(":");
   lcd.setCursor(14,1);
   lcd.print(secTMR2); //print seconds for TMR2:
   
}

void loop() { 
  if (minTMR1>20)
  {
    minTMR1=20;
  }
  if (minTMR1>0 || secTMR1>0)
  {
  if (minTMR1>=10)
  {
  lcd.setCursor(0,1);
  lcd.print(minTMR1);
  }
  else
  {
    lcd.setCursor(0,1);
    lcd.print("0");
    lcd.setCursor(1,1);
    lcd.print(minTMR1);
  }
  if (secTMR1>=10)
  {
  lcd.setCursor(3,1);
  lcd.print(secTMR1);
  }
  else
  {
    lcd.setCursor(3,1);
    lcd.print("0");
    lcd.setCursor(4,1);
    lcd.print(secTMR1);
  }
  if (secTMR1==0)
  {
    secTMR1=60;
    minTMR1--;
  }
  secTMR1--;
  }
  else
  {
    lcd.setCursor(0,1);
    lcd.print("00:00");
  }
if (minTMR1==0 && secTMR1==0)
{
  secTMR2--;
  if (secTMR2>=10)
  {
  lcd.setCursor(14,1);
  lcd.print(secTMR2);
  }
  else
  {
    lcd.setCursor(14,1);
    lcd.print("0");
    lcd.setCursor(15,1);
    lcd.print(secTMR2);
  }
if (secTMR2==0)
{
  minTMR2--;
  secTMR2=60; 
}
if (minTMR2>=10)
{
  lcd.setCursor(11,1);
  lcd.print(minTMR2);
}
else
{
  lcd.setCursor(11,1);
  lcd.print("0");
  lcd.setCursor(12,1);
  lcd.print(minTMR2);
}
}
 delay(1000);
}
#4

You should indent your code carefully so you can see how the IFs and ELSEs relate to each other. Use the AutoFormat tool and you should get something like this

#include <LiquidCrystal.h>
LiquidCrystal lcd(8, 9, 4, 5, 6, 7);

int minTMR1; //the varriable holding the minutes for the first timer:
int secTMR1; //the variable holding the seconds for the first timer:
int minTMR2; //the varriable holding the minutes for the second timer:
int secTMR2; //the variable holding the seconds for the second timer:
void setup() {
    lcd.begin(16, 2);
    lcd.print("Timer 1");
    lcd.setCursor(9,0);
    lcd.print("Timer 2");
    lcd.setCursor(2,1);
    lcd.print(":");
     minTMR1=0; //minutes TMR1:
     secTMR1=10; //secondsTMR1:
     minTMR2=10; //minutes TMR2:
     secTMR2=20; //seconds TMR2:
     
     lcd.setCursor(0,1);
     lcd.print(minTMR1); //print minuters for TMR1:
     lcd.setCursor(2,1);
     lcd.print(":");
     lcd.setCursor(3,1);
     lcd.print(secTMR1); //print seconds for TMR1:
     lcd.setCursor(11,1);
     lcd.print(minTMR2); //print minutes for TMR2:
     lcd.setCursor(13,1);
     lcd.print(":");
     lcd.setCursor(14,1);
     lcd.print(secTMR2); //print seconds for TMR2:
     
}

void loop() {
    if (minTMR1>20)
    {
        minTMR1=20;
    }
    if (minTMR1>0 || secTMR1>0)
    {
        if (minTMR1>=10)
        {
            lcd.setCursor(0,1);
            lcd.print(minTMR1);
        }
        else
        {
            lcd.setCursor(0,1);
            lcd.print("0");
            lcd.setCursor(1,1);
            lcd.print(minTMR1);
        }
        if (secTMR1>=10)
        {
            lcd.setCursor(3,1);
            lcd.print(secTMR1);
        }
        else
        {
            lcd.setCursor(3,1);
            lcd.print("0");
            lcd.setCursor(4,1);
            lcd.print(secTMR1);
        }
        if (secTMR1==0)
        {
            secTMR1=60;
            minTMR1--;
        }
        secTMR1--;
    }
    else
    {
        lcd.setCursor(0,1);
        lcd.print("00:00");
    }
    
    if (minTMR1==0 && secTMR1==0)
    {
        secTMR2--;
        if (secTMR2>=10)
        {
            lcd.setCursor(14,1);
            lcd.print(secTMR2);
        }
        else
        {
            lcd.setCursor(14,1);
            lcd.print("0");
            lcd.setCursor(15,1);
            lcd.print(secTMR2);
        }
        if (secTMR2==0)
        {
            minTMR2--;
            secTMR2=60;
        }
        if (minTMR2>=10)
        {
            lcd.setCursor(11,1);
            lcd.print(minTMR2);
        }
        else
        {
            lcd.setCursor(11,1);
            lcd.print("0");
            lcd.setCursor(12,1);
            lcd.print(minTMR2);
        }
    }
    delay(1000);
}

Having don that, and having the better variable names I still can’t follow the program. Some comments would be useful.

However if it was my program I would make two significant changes which would IMHO make it all much easier.

First (and simplest) I would do all the calculation in seconds and just convert to minutes for display purposes. Indeed, I see no reason not to do it in milliseconds and just use the millis() function.

Second, I would completely separate the time calculations from the display code. I would use the time calculation function to put appropriate values into variables and use the display function to display whatever happens to be in those variables. Your code could be something like this

// global variables to hold the values to be displayed

void loop() {
  currentMillis = millis();
  updateTimeCalcs();
  displayData();
}

void updateTimeCalcs() {
   // check the value in currentMillis and, update variables as appropriate
}

void displayData() {
   // if (say) 1 second has passed, update the LCD display
}

This is the style from Planning and Implementing a Program. It also illustrates the use of millis() to manage timing.

…R

#5