Vertical "moving" text on a LCD-display

Question

Following idea:

You have a LCD-display with 2 rows.

1. You write the first line of a "text" to the second row. Then you clean the screen.

2. After a delay you write the first line to the first row and a the second line of the text to the second row. Then follows another clean and delay.

3. Then you write the second line to the first row and the third line to the second row. And so on ...

When the end of the text is reached the whole process starts again from the beginning.

That way the impression of an "vertical moving" text is accomplished.

The code I have written:

#include <LiquidCrystal.h>

LiquidCrystal lcd(12, 11, 5, 4, 3, 2);

void setDisplay(String lineValue, int lineNumber, int waitUntilClear, boolean clearScreen) {
  lcd.setCursor(0, lineNumber),
  lcd.print(lineValue); 

  delay(waitUntilClear);

  if (clearScreen == true) {
   lcd.clear(); 
  }
}

void setup() {
  lcd.begin(16, 2);
}

void loop() {
  const int DELAY = 3000;
  const int COUNT_ELEMENTS = 12;

  String messages[COUNT_ELEMENTS] = {
    "",
    "Furniture Store",
    " MEYER & MILLER",
    "",
    "  Contact us!",
    "",
    " -- Email ----- ",
    "[email protected]",
    "",
    " -- Phone ----- ",
    " 000 123456",
    ""
  };

  for (int i = 0; i < COUNT_ELEMENTS - 1; i++) {
    setDisplay(messages[i], 0, 0, false);
    setDisplay(messages[i + 1], 1, DELAY, true);
  } 
}

It works fine and by using an additional function I could avoid code redundancies mostly. If someone has an idea how to improve my code or even doing it completely different in a better way, then I would appreciate his/her comment or answer.

Answer 1

waitUntilClear is always 0 when clearScreen is false then boolean flag is redundant:

void setDisplay(String lineValue, int lineNumber, unsigned long msToWaitBeforeClear) {
  lcd.setCursor(0, lineNumber),
  lcd.print(lineValue); 

  if (msToWaitBeforeClear > 0) {
   delay(msToWaitBeforeClear);
   lcd.clear(); 
  }
}

Note that delay() accepts unsigned long instead of int then our parameter should match its type (does it make sense to have a negative delay?)

Now is (should be) clearer that we're calling setDisplay() twice but it's easier to simply call it with the index of the current line:

void displayPage(String messages[], size_t startIndex) {
  for (int8_t rowIndex = 0; rowIndex < NUMBER_OF_DISPLAY_LINES; ++rowIndex) {
      lcd.setCursor(0, rowIndex),
      lcd.print(messages[startIndex + rowIndex]); 
  }

   delay(DELAY_BETWEEN_PAGES_IN_MS);
   lcd.clear(); 
}

Used as:

for (size_t i = 0; i < COUNT_ELEMENTS - 1; ++i) {
  displayPage(messages, i);
} 

Now it's time to remove those magic numbers in setup() and lcd declaration. I'd also suggest to move all global constants to the very top, outside functions (probably in a separate file). It's what you want to change to configure your display and it's better if you don't have to search them buried inside code.

One small note: if you manually unroll the loop over NUMBER_OF_DISPLAY_ROWS then you can skip the first call to setCursor() because clear() already do it for you:

void displayPage(String messages[], size_t startIndex) {
    lcd.print(messages[startIndex]); 
    lcd.setCursor(0, 1),
    lcd.print(messages[startIndex + 1]); 

    delay(DELAY_BETWEEN_PAGES_IN_MS);
    lcd.clear(); 
}