Two button LCD menu problems

Using Arduino Programming Questions
1

I am having a huge amount of trouble getting a two button scroll menu to work on an LCD. First I cannot get the functions to work so I threw the whole thing in the "Loop" function.

I tried the

Menufunction(menu)

void Menufunction( int menu){

//Case statement
}

Nothing happens. The lcd locked up halfway through displaying "Menu Item 6" and just displayed "Item 6" without me being able to get anything out of it.

The next and largest problem is that I am using two normally closed momentary push buttons to scroll either up or down and I am trying to get the device to "De-bounce" or toggle on and off in a slow, noise free manner. I tried using the training de-bounce function with no avail and then I wrote my own which I've had better luck with.

The "Loop" function is going fast because everytime I push the button to go down on the menu It skips about 2-10 cases and looks horiible as if its on a really slow refresh rate. I am despirate for help on this one I have been working on this for two days and I cannot scroll up or down on a simple menu. what gives??

Thanks

Here's the code

#include <LiquidCrystal.h>

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

long hightimer;
long lowtimer;
const int UpButton = 6;
const int DownButton = 7;
const int LedState = 13;

int LEDstate = HIGH;
int UpButtonState = HIGH;
int DownButtonState = HIGH;

int timelow = 1000;
int menu = 1;


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

void loop() {
 int readingUP = digitalRead(UpButton);
 int readingDOWN = digitalRead(DownButton);
 
 if(readingUP != UpButtonState){
   lowtimer = millis();
 } 
 if ((millis() - lowtimer) > timelow){
   digitalWrite(UpButton, LOW);
   lowtimer = millis();
   }
   if(readingUP != LOW){
     digitalWrite(UpButton, HIGH);
     if(menu == 15){
       menu = 0;
     }
     else{
     menu = menu + 1;
     }
 }
 
  if(readingDOWN != DownButtonState){
   lowtimer = millis();
  }
 if ((millis() - lowtimer) > timelow){
   digitalWrite(DownButton, LOW);
   lowtimer = millis();
   }
   if(readingDOWN != LOW){
     digitalWrite(DownButton, HIGH);
     if(menu == 0){
       menu = 15;
     }
     else{
     menu = menu - 1;
     }
 }
 
    lcd.clear();
    switch (menu) {
    case 1:
      lcd.setCursor(0, 1);
      lcd.print("Menu Item 1");
      break;
    case 2:
      lcd.setCursor(0, 1);
      lcd.print("Menu Item 2");
      break;
    case 3:
      lcd.setCursor(0, 1);
      lcd.print("Menu Item 3");
      break;
    case 4:
      lcd.setCursor(0, 1);
      lcd.print("Menu Item 4");
      break;
    case 5:  
      lcd.setCursor(0, 1);
      lcd.print("Menu Item 5");
      break;
    case 6:
      lcd.print("Menu Item 6");
      break;
    case 7:
      lcd.setCursor(0, 1);
      lcd.print("Menu Item 7");
      break;
    case 8:
      lcd.setCursor(0, 1);
      lcd.print("Menu Item 8");
      break;
    case 9:
      lcd.setCursor(0, 1);
      lcd.print("Menu Item 9");
      break;
    case 10:
      lcd.setCursor(0, 1);
      lcd.print("Menu Item 10");
      break;
    case 11: 
      lcd.setCursor(0, 1);
      lcd.print("Menu Item 12");
      break;
    case 13:
      lcd.setCursor(0, 1);
      lcd.print("Menu Item 13");
      break;
    case 14:
      lcd.setCursor(0, 1);
      lcd.print("Menu Item 14");
      break;
    case 15:
      lcd.setCursor(0, 1);
      lcd.print("Menu Item 15");
      break;
    default:
      lcd.setCursor(0, 1);
      lcd.print("Error");
      }  }
2

In setup(), you do not declare a pin mode for the up and down switch pins.

In loop, you use digitalRead() and digitalWrite() to read from and write to the same pins. Since the default mode is INPUT, the read works. The write is turning the pull-up resistors off and on. I doubt that is what you intend to be doing.

You should explicitly declare the pinMode, and enable or disable the pull-up resistors in setup(), and never write to them again.

if(readingUP != UpButtonState){
   lowtimer = millis();
 } 
 if ((millis() - lowtimer) > timelow){
   digitalWrite(UpButton, LOW);
   lowtimer = millis();
   }

  if(readingDOWN != DownButtonState){
   lowtimer = millis();
  }
 if ((millis() - lowtimer) > timelow){
   digitalWrite(DownButton, LOW);
   lowtimer = millis();
   }

Low, High, Up, Down... You should be consistent in the selection of names, and in the use of variables. A separate set of variables for debouncing the down switch would be useful.

You are keeping track of whether a switch was pressed or not. Yet, you are displaying a (new) menu regardless of whether a switch was pressed or not. Why?

How come case 6 doesn't position the cursor? Is it really necessary to position the cursor the same way in all the cases? Why is there no case 12?

      }  }

Stop this. Right now. Every curly brace goes on a separate line, properly indented with it's opening curly brace.

3

Thank you for your help!