Issue Displaying LCD Menu Correctly

I'm pretty new at Arduino, and I'm trying to make my own LCD menu (displayed on a 16x2). I've written some code that will move a custom cursor up and down with the press of either an "up" button or a "down" button. As of right now, I only have it set up for 3 menu items. Everything works fine for the top 2 items, but when I move to the 3rd menu item, the LCD only displays the 3rd item, even though the 2nd should still be visible above it. Once I move the cursor up again, the display returns to normal.

If someone could take a look at my code and help me figure out the problem, I'd really appreciate it.

#include <LiquidCrystal.h>

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

boolean bottom = false;

const int upPin = 6;
const int downPin = 7;

unsigned long prevTime = 0;

int upPinState = 0;
int downPinState = 0;
int prevUpPinState = 0;
int prevDownPinState = 2;

String menu1 = "Menu Item 1";
String menu2 = "Menu Item 2";
String menu3 = "Menu Item 3";
String menu4 = "Menu Item 4";

int cursorPosition = 1;

byte myCursor[8] = {
  B10000,
  B01000,
  B00100,
  B00010,
  B00010,
  B00100,
  B01000,
  B10000,
};

void setup() {
  pinMode(upPin, INPUT);
  pinMode(downPin, INPUT);
  
  lcd.createChar(1, myCursor);
  lcd.begin(16, 2);  
  lcd.setCursor(3,0); 
  lcd.print("Created By:");
  lcd.setCursor(7,1);
  lcd.print("JZ");
  delay(3000);
  lcd.clear();
  lcd.print("\x01" + menu1);
  lcd.setCursor(1, 1);
  lcd.print(menu2);
  delay(1000);
 
  Serial.begin(9600);
  Serial.println(cursorPosition);
}

void loop() {
  int signal = 0;
  
  upPinState = digitalRead(upPin);
  downPinState = digitalRead(downPin);
  
  if(upPinState != prevUpPinState && upPinState == HIGH) {
    signal = 1;
  }
    
  if(downPinState != prevDownPinState && downPinState == HIGH) {
    signal = 2;
  }
  
  if(cursorPosition == 1){
    switch(signal) {
      case 1: // up
          lcd.clear();
          lcd.setCursor(0,0);
          lcd.print("\x01" + menu1);
          lcd.setCursor(1, 1);
          lcd.print(menu2);
          cursorPosition = 1;
          Serial.println(cursorPosition);
          break;      
      case 2: // down
          lcd.clear();
          lcd.setCursor(1,0);
          lcd.print(menu1);
          lcd.setCursor(0,1);
          lcd.print("\x01" + menu2);
          cursorPosition = 2;
          Serial.println(cursorPosition);
          break;
    }
  }
  else if(cursorPosition == 2){
    switch(signal) {
      case 1: // up
          lcd.clear();
          lcd.setCursor(0,0);
          lcd.print("\x01" + menu1);
          lcd.setCursor(1, 1);
          lcd.print(menu2);
          cursorPosition = 1;
          Serial.println(cursorPosition);
          break;      
       case 2: // down
          lcd.clear();
          lcd.setCursor(1,1);
          lcd.print(menu2);
          lcd.setCursor(0,1);
          lcd.print("\x01" + menu3);
          cursorPosition = 3;       
          Serial.println(cursorPosition);
          break;
    }
  }
  else if(cursorPosition == 3){
    switch(signal) {
      case 1: //up
          lcd.clear();
          lcd.setCursor(0,0);
          lcd.print("\x01" + menu2);
          lcd.setCursor(1, 1);
          lcd.print(menu3);
          cursorPosition--;
          Serial.println(cursorPosition);
          break;      
    }
  }
  prevUpPinState = upPinState;
  prevDownPinState = downPinState; 
}

As of right now, I only have it set up for 3 menu items.

Those three items would be

String menu1 = "Menu Item 1";
String menu2 = "Menu Item 2";
String menu3 = "Menu Item 3";
String menu4 = "Menu Item 4";

Can't count, eh?

  lcd.print("\x01" + menu1);

Pissing away resources on Strings, so you can do this, IS going to bite you in the posterior before you are done.

The variable signal's name gives no clue what you think it is to contain/be used for. And, there are no comments that describe what you are doing with it.

Everything works fine for the top 2 items, but when I move to the 3rd menu item

How do you "move to the third item"?

What value does signal have when this happens? What value does cursorPosition have?

          Serial.println(cursorPosition);

Anonymous printing sucks.

          Serial.print("cursorPosition: ");
          Serial.println(cursorPosition);

conveys WAY more information.

The solution seems simple.

Put the menu item text in an array of chars rather than use Strings, position the lcd cursor at 0, 0 and print the array level to be on the top line, position the lcd cursor at 0, 1 and print the next array level.

When the up/down buttons become pressed adjust the array index appropriately and print the menu again. Put the output to the lcd in a function and call it with the current array level as a parameter.

@UKHeliBob I considered that before but just never tried that because I wasn't too familiar with arrays. BUT, I re-wrote the code using character arrays and it's working beautifully now! Thank you.

@PaulS I tried to comment all the lines now and got rid of the strings so I'd love to get some feedback on whether or not I'm coding this as efficiently as possible. What do you think?

Also, a question for both of you. I now have this set up so the the row number (x) determines which menu item I am on and the columns are used for accessing submenus.

For instance, in my code I have this ( x value for row, y value for column):

char topMenu[][3][16]= { 
{"Menu Item 1", "Sub Menu 1.1"}, 
{"Menu Item 2", "Sub Menu 1.2", "Sub Menu 2.1"}, 
{"Menu Item 3", "Sub Menu 1.3", "Sub Menu 2.2"},
{"Menu Item 4"},
};

// topMenu index; indicates position within menu
int x = 0; 
int y = 0;

As I move up and down, the x value either increments up or down, respectively. When I want to access a submenu, the y value will increment x + 1. So if I'm on "Menu Item 2" (x=1,y=0) and I move into the submenu, the index changes to (x=1, y=2) and then I'm on "Sub Menu 2.1."

My question now would be, what is the best way to now set a the boundaries of the submenus?

As I currently have it, if I move up from "Sub Menu 2.1", I will move to (x=0, y=2) which has no menu item.

Here's the full code:

#include <LiquidCrystal.h>

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

// creates menu
char topMenu[][3][16]= { 
{"Menu Item 1", "Sub Menu 1.1"}, 
{"Menu Item 2", "Sub Menu 1.2", "Sub Menu 2.1"}, 
{"Menu Item 3", "Sub Menu 1.3", "Sub Menu 2.2"},
{"Menu Item 4"},
};

// topMenu index; indicates position within menu
int x = 0; 
int y = 0;


int signal = 0;
char incomingByte = 0;

int upPinState = 0;
int prevUpPinState = 0;
int downPinState = 0;
int prevDownPinState = 0;

const int upPin = 6;
const int downPin = 7;

// custom cursor for LCD
byte myCursor[8] = {
  B10000,
  B01000,
  B00100,
  B00010,
  B00010,
  B00100,
  B01000,
  B10000,
};

void setup() {
  
  pinMode(upPin, INPUT);
  pinMode(downPin, INPUT);
  
  // send starting menu to lcd
  lcd.createChar(1, myCursor); 
  lcd.begin(16, 2);  
  lcd.print("\x01");
  lcd.print(topMenu[x][y]);
  lcd.setCursor(1,1);
  lcd.print(topMenu[x+1][y]);
  
  // for debugging
  Serial.begin(9600);
}

void loop() {
  
  upPinState = digitalRead(upPin); // read state of "up" button
  downPinState = digitalRead(downPin); // read state of "down" button
  
  //change signal value if "up" button is pressed
  if(upPinState != prevUpPinState && upPinState == HIGH) {
    signal = 1;
  }
  // change signal value if "down" button is pressed  
  if(downPinState != prevDownPinState && downPinState == HIGH) {
    signal = 2;
  }
  
  // use serial input for signal value (i.e. no hardware button available)
  if(Serial.available() > 0) {
    incomingByte = Serial.read();            
  }
    
  cursorPositionAction(); // move cursor in response to user input
  
  //remember last value
  prevUpPinState = upPinState; 
  prevDownPinState = downPinState;  
  signal = 0; //reset signal variable
  incomingByte = 0;

}

void cursorPositionAction(){
  
  //for use with hardware input
  switch(signal) {
    
    // move up
    case 1:
      if(x > 0) {
        x--;
        lcd.clear();
        lcd.print("\x01");
        lcd.print(topMenu[x][y]);
        lcd.setCursor(1,1);
        lcd.print(topMenu[x+1][y]);
        Serial.print("x value: ");
        Serial.println(x);
        Serial.print("y value: ");
        Serial.println(y);
      }
    break;
    
    // move down
    case 2:
      if(x < 3) {
        x++;
        lcd.clear();
        lcd.setCursor(1,0);
        lcd.print(topMenu[x-1][y]);
        lcd.setCursor(0,1);
        lcd.print("\x01");
        lcd.print(topMenu[x][y]);
        Serial.print("x value: ");
        Serial.println(x);
        Serial.print("y value: ");
        Serial.println(y);
      }
    break;
  }
  
    //for use with serial input
    switch(incomingByte) {
      
      // select menu item
      case 's':
        y = y + 1 + x;
        for (int positionCounter = 0; positionCounter < 13; positionCounter++) {
          lcd.scrollDisplayLeft();
          delay(50);
        }
        lcd.clear();
        lcd.print("\x01");
        lcd.print(topMenu[x][y]);
        lcd.setCursor(1,1);
        lcd.print(topMenu[x+1][y]);
        Serial.print("x value: ");
        Serial.println(x);
        Serial.print("y value: ");
        Serial.println(y);
      break;
      
      // go back 
      case 'b':
        y = y - 1 - x;
        for (int positionCounter = 0; positionCounter < 13; positionCounter++) {
          lcd.scrollDisplayRight();
          delay(50);
        }
        lcd.clear();
        lcd.print("\x01");
        lcd.print(topMenu[x][y]);
        lcd.setCursor(1,1);
        lcd.print(topMenu[x+1][y]);
        Serial.print("x value: ");
        Serial.println(x);
        Serial.print("y value: ");
        Serial.println(y);
      break;
      }
}

As I currently have it, if I move up from "Sub Menu 2.1", I will move to (x=0, y=2) which has no menu item.

Technically, there is a NULL value for the pointer at that location. That is easy to recognize.

One of the challenges of programming is translating requirements into code. The far bigger challenge, though, is dealing with incomplete requirements. As an example, you are asking how to write the code for the situation above, but you have not defined what should happen. Damned hard to write the code when you don't know what it should do.

So if I'm on "Menu Item 2" (x=1,y=0) and I move into the submenu, the index changes to (x=1, y=2)

I don't follow that. If you increment once, how does y get from 0 to 2?

There are several menu libraries that you might want to look at. As you are discovering, menus are not square items. They are actually binary trees. Representing a binary tree in an n-dimensional array is very difficult.

Thanks for the follow up. The reason y goes from 0 to 2 is that when I move "forward" (from top menu item to sub menu), it increments by a value that is equal to the current x value + 1.

So if I was on x = 0 and y = 0, then moving "forward" would increment y by x +1, which would make y = 1.

If I was on x = 1 and y = 0, then moving forward would make y = 2.

Using this approach, I was able to list subsequent menu items in subsequent columns. The format would be:

Column 1: Column 2: Column 3:

TopMenu 1 Submenu1.1

TopMenu 2 Submenu1.2 Submenu2.1

However, I couldn't really foresee this approach easily handling any menu items below the first sublevel.

I did some research and found a really good tutorial that someone had used and then I tweaked it and cleaned it up and it works beautifully. No limits on depth and navigates easily.

Here's the code:

#include "menuEntry.h"

#include <LiquidCrystal.h>

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

// value for hardware/serial input
char signal = 0;
// char for current item
unsigned char selected = 0;
// menu items
const char menu1[] = "Menu Item 1"; // 0
const char menu2[] = "Menu Item 2"; // 1
const char menu3[] = "Menu Item 3"; // 2
const char menu4[] = "Menu Item 4"; // 3

const char subMenu101[] = "Sub Menu 1.1"; // 4
const char subMenu102[] = "Sub Menu 1.2"; // 5
const char subMenu103[] = "Sub Menu 1.3"; // 6
const char subMenu104[] = "Back";         // 7

const char subMenu201[] = "Sub Menu 2.1"; // 8
const char subMenu202[] = "Sub Menu 2.2"; // 9
const char subMenu203[] = "Back";         // 10

// structure provides attributes to menu items
menuEntry menu[] = {
  {menu1, 1, 0, 4, 0, 1, 4, 0},
  {menu2, 1, 0, 4, 0, 2, 7, 0},
  {menu3, 1, 0, 4, 1, 3, 2, 0},
  {menu4, 1, 0, 4, 2, 3, 3, 0},
 {subMenu101, 2, 4, 4, 4, 5, 4, 0},
 {subMenu102, 2, 4, 4, 4, 6, 5, 0},
 {subMenu103, 2, 4, 4, 5, 7, 6, 0},
 {subMenu104, 2, 4, 4, 6, 7, 0, 0},
  {subMenu201, 3, 8, 3, 8, 9, 8, 0},
  {subMenu202, 3, 8, 3, 8, 10, 9, 0},
  {subMenu203, 3, 8, 3, 9, 10, 1, 0},
};

// custom cursor
byte myCursor[8] = {
  B10000,
  B01000,
  B00100,
  B00010,
  B00010,
  B00100,
  B01000,
  B10000,
};

void setup() {
  // initiate lcd and print start menu
  lcd.createChar(1, myCursor); 
  lcd.begin(16, 2);  
  lcd.print("\x01");
  lcd.print(menu[selected].text);
  lcd.setCursor(1,1);
  lcd.print(menu[selected + 1].text);
  
  // for debugging
  Serial.begin(9600);  
}

void loop() {
  // need this to navigate LCD
  browse_menu();
  
}

void show_menu() {
  unsigned char top;
  unsigned char bottom;  
  // calculate bottom and top values for current menu/submenu
  bottom = (menu[selected].top_value + menu[selected].num_menuPoints) - 1;
  top = menu[selected].top_value;
    
  switch(signal) {
    // how LCD responds to "up" signal
    case 'u': // currently responds to 'u' from Serial input
      if(selected > top) {
        selected = menu[selected].up;
        lcd.clear();
        lcd.print("\x01");
        lcd.print(menu[selected].text);
        lcd.setCursor(1,1);
        lcd.print(menu[selected + 1].text);
      }
    break;  
    // how LCD responds to "down" signal
    case 'd': // currently responds to 'd' from Serial input
      if(selected < bottom) {
        selected = menu[selected].down;
        lcd.clear();
        lcd.setCursor(1,0);
        lcd.print(menu[selected - 1].text);
        lcd.setCursor(0, 1);
        lcd.print("\x01");
        lcd.print(menu[selected].text);
      }
    break;
    // how LCD responds to "select" signal
    case 's': // currently responds to 's' from Serial input
      selected = menu[selected].select;
      lcd.clear();
      lcd.print("\x01");
      lcd.print(menu[selected].text);
      lcd.setCursor(1,1);
      lcd.print(menu[selected + 1].text);
    break;
      }
}
      
void browse_menu() {
  // where to get input action from (can be hardware or Serial)
  if(Serial.available() > 0) {
    signal = Serial.read(); 
        
    show_menu();
    signal = 0;
  }
}

Here's the header file:

#ifndef menuEntry_h
#define menuEntry_h

typedef struct menu{
 const char *text;
 unsigned char menu_group;
 unsigned char top_value;
 unsigned char num_menuPoints;
 unsigned char up;
 unsigned char down;
 unsigned char select;
 int (*fp) (void);
} menuEntry;

#endif

How does it look to you now?