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!

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

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.