Blinking cursor on Arduino is inconsistent and misses inputs

Question

I am trying to write a part of a program that prints time and allows the user to move a cursor across it. I have the following code, but there are a few issues with it.

#include <Adafruit_RGBLCDShield.h>
#include <utility/Adafruit_MCP23017.h>
#include <Time.h>
#include <TimeLib.h>
Adafruit_RGBLCDShield lcd = Adafruit_RGBLCDShield();
void setup() {
 lcd.begin(16,2);
 setTime(12,30,15,0,0,0);
}
int col = 0;
void loop() {
 uint8_t buttons = lcd.readButtons(); //create readable buttons
 char time_out[16];
 lcd.setCursor(0,0);
 sprintf(time_out, "%02u:%02u:%02u",hour(),minute(),second());
 lcd.print(time_out);
 lcd.setCursor(col,0);
 lcd.blink();
 delay(100);
 if (buttons) {
 lcd.clear();
 if (buttons & BUTTON_RIGHT) { 
 col += 1;
 } 
 if (buttons & BUTTON_LEFT) {
 col -= 1; 
 } 
 } //buttons 
}

If you upload this to your board, you can tell that the blinking is highly irregular, not at all aesthetically pleasing.

You'll also notice that sometimes, when pressing left/right, the input isn't registered.

I suspect this has to do something with the delay() function used, which pauses the entire program.

I looked into the following method, but this does not work when needing to print something in a loop (as the time library requires).

Any idea how I could make this work better (regular blinking times and no miss of user input)?

Thanks a lot!

• user3629249: the code needs to check the variable col to assure it is inside the valid range before incrementing/decrementing. This is to avoid the cursor being set outside the valid range. I.E. if the valid range is 0...15 then check col >0 before decrementing and check col<15 before incrementing.
• user3629249: I think the statement lcd.clear() turns all the LEDs off, that does not seem like a good idea
• user3629249: strongly suggest using a timer interrupt to handle the blinking, using a keypress interrupt for the cursor motion.

Answer 1

I don't have the hardware to test but, it should be more or less like this; (I have also normalized the code into several functions for better readability.)

#include <Adafruit_RGBLCDShield.h>
#include <utility/Adafruit_MCP23017.h>
#include <Time.h>
#include <TimeLib.h>
Adafruit_RGBLCDShield lcd = Adafruit_RGBLCDShield();
unsigned long timerBlink; //Required for periodic blinking
int intervalBlink = 100;
void setup() {
 lcd.begin(16, 2);
 setTime(12, 30, 15, 0, 0, 0);
 timerBlink = 0;
}
int col = 0;
void loop() {
 readButtons();
 printDate();
 if (millis() - timerBlink > intervalBlink) {
 blink();
 }
}
void readButtons() {
 uint8_t buttons = lcd.readButtons(); //create readable buttons
 if (buttons) {
 lcd.clear();
 if (buttons & BUTTON_RIGHT) {
 col += 1;
 }
 if (buttons & BUTTON_LEFT) {
 col -= 1;
 }
 } //buttons 
}
void blink() {
 lcd.setCursor(col, 0);
 lcd.blink();
 timerBlink = millis();//Start timer for intervalBlink
}
void printDate() {
 char time_out[16];
 lcd.setCursor(0, 0);
 sprintf(time_out, "%02u:%02u:%02u", hour(), minute(), second());
 lcd.print(time_out);
}

Instead of blocking delay function, using timeout with timestamp is a better practice.