Auto Down counter using Arduino Uno with millis function and 16x2 LCD

Using Arduino Programming Questions
#1

Following is the code I have implemented, is this a correct way to do it. Or is there a better way to do auto countdown. Please suggest. Thanks.

#include <LiquidCrystal.h>
// initialize the library with the numbers of the interface pins
LiquidCrystal lcd(14, 15, 16, 17, 18, 19);//RS,EN,D4,D5,D6,D7

unsigned long prevMillis;
int count = 230;


void setup() {

  lcd.begin(16, 2);//initializing LCD
  lcd.setCursor(0, 0);
  lcd.print("Initializing...");
  prevMillis = millis();  // get the time at the start

}

void loop() {
  static byte countDoneFlag;
  static byte doOneTime;
  unsigned long currentMillis = millis();
  static long timecheck;

  if (currentMillis - prevMillis >= 1000) {
    if (countDoneFlag == 0) {
      count--;
    }
    prevMillis = currentMillis;
  }
  if (count == 0) {
    if ( doOneTime == 0) {
      timecheck = millis();//millis should be around 10k at this point
      lcd.clear();
      doOneTime = 1;
    }
    countDoneFlag = 1;
    lcd.setCursor(0, 0);
    lcd.print("Counting ended");
    lcd.setCursor(0, 1);
    lcd.print("time= ");
    lcd.print( timecheck);
    lcd.print("ms");
  } else {
    char displayText[17] = "";
    snprintf(displayText, sizeof(displayText), "PulseCount:%5d", count);
    lcd.setCursor(0, 0);
    lcd.print(displayText);
  }
}
#2 
prevMillis = currentMillis;

I would make that

prevMillis += 1000;

To keep any timing errors from accumulating.

#3

Sure I shall do that. Thanks Delta!! Anything else needs to be corrected or can be made better?

#4

Anything else needs to be corrected or can be made better?

Yes. Use boolean variables where they make more sense. countDoneFlag and doOneTime look like they should be true or false, not 0 or 1.

    char displayText[17] = "";
    snprintf(displayText, sizeof(displayText), "PulseCount:%5d", count);

Since you start at 230, and decrement from there, why is it necessary to use 5 digits in the displayed value?

#5

I''ll make sure I use them as boolean variables. The count 230 is merely an example to check whether the code works well or not. Actual count is a 5 digit number which will be taken as an input from the user via a 4x4 matrix keypad, which is I am going to implement next.

I have one more doubt. If here I change: if (currentMillis - prevMillis >= 1000) to may be 100 ms or anything else will I have to make change in the following statement as well.

prevMillis += 1000;

Thanks.

#6

will I have to make change in the following statement as well

Yes. Which is why you should have:

const unsigned long interval = 1000;

   if(currentMillis - prevMillis >= interval)

   prevMillis += interval;

If the interval changes, you only need to change one line of code.

#7

So for the boolean variables which I have declared in the void loop, should I initialize them there itself?

#8

So for the boolean variables which I have declared in the void loop, should I initialize them there itself?

Yes.

#9

As per the corrections suggested.

void loop() {
  static byte countDoneFlag = false;
  static byte doOneTime = false;
  unsigned long currentMillis = millis();
  static long timecheck;

  if (currentMillis - prevMillis >= interval) {
    if (countDoneFlag == false) {
      count--;
    }
    prevMillis += interval;
  }
  if (count == 0) {
    if ( doOneTime == false) {
      timecheck = millis();//millis should be around 10k at this point
      lcd.clear();
      doOneTime = true;
    }
    countDoneFlag = true;
    lcd.setCursor(0, 0);
    lcd.print("Counting ended");
    lcd.setCursor(0, 1);
    lcd.print("time= ");
    lcd.print( timecheck);
    lcd.print("ms");
  } else {
    char displayText[17] = "";
    snprintf(displayText, sizeof(displayText), "PulseCount:%5d", count);
    lcd.setCursor(0, 0);
    lcd.print(displayText);
  }
}
#10 
  static byte countDoneFlag = false;

A byte is not a boolean. Same size, same range of values, but there is a type called boolean for a reason.

  unsigned long currentMillis = millis();

If you decide that you need finer resolution, and need to use micros(), you look pretty silly assigning the value returned by micros() to currentMillis. It is easy to see, then, that Millis is NOT necessary in the name. Something meaningful, like now, would be a better name.

    if (countDoneFlag == false) {

A real programmer never uses == true or == false in an if statement.

    if (!countDoneFlag)
    {

      timecheck = millis();//millis should be around 10k at this point

Does that comment add any value? What happens when you change interval, and it no longer takes 10 seconds to get to this point? Are you going to change the useless comment?

    char displayText[17] = "";
    snprintf(displayText, sizeof(displayText), "PulseCount:%5d", count);

It is not necessary to supply an initial value for an array that you are going to step on immediately.

If you decide to count down for 20000 milliseconds, this statement will print "PulseCount:19999" (or some 5 digit number) the first time this statement is executed. Wouldn't "PulseCount: 19999" be better?

#11

PaulS:

    char displayText[17] = "";

snprintf(displayText, sizeof(displayText), "PulseCount:%5d", count);



It is not necessary to supply an initial value for an array that you are going to step on immediately.

If you decide to count down for 20000 milliseconds, this statement will print "PulseCount:19999" (or some 5 digit number) the first time this statement is executed. Wouldn't "PulseCount: 19999" be better?

@PaulS:
Count the characters again. Also keep in mind what kind of display is being used.

#12

Thanks PaulS for helping with all the corrections.

void loop() {
  static bool countDoneFlag;
  static bool doOneTime;
  unsigned long now = millis();
  static long timecheck;

  if (now - prevMillis >= interval) {
    if (!countDoneFlag) {
      count--;
    }
    prevMillis += interval;
  }
  if (count == 0) {
    if (!doOneTime) {
      timecheck = millis();//millis should be around 10k at this point
      lcd.clear();
      doOneTime = 1;
    }
#13