1

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!

3
  • 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. Commented Apr 12, 2017 at 15:38
  • I think the statement lcd.clear() turns all the LEDs off, that does not seem like a good idea Commented Apr 12, 2017 at 15:39
  • strongly suggest using a timer interrupt to handle the blinking, using a keypress interrupt for the cursor motion. Commented Apr 12, 2017 at 15:41

1 Answer 1

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.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.