Trouble with Menus and Analog buttons.

Hey Guys.

I have the UNO with the DFRobot LCD Sheild and all I want to do is make a little up/down Scroll through menu system.
My code so far works but at a really fast pace and it pretty hard to control it that way.So i devised a piece of code that times it WITHOUT delays.
Problem is I cant seem to find what I am doing wrong but I have been sitting here for quite some time looking at the code, I bet the problem is staring me in the face.
Anyway if I press the UP button with the time code implemented it doesn’t change menus like I wanted instead it does the following (Will Explain after my MAIN code):

#include <LiquidCrystal.h>

LiquidCrystal lcd(8, 9, 4, 5, 6, 7);

char TrackUP = 0;
unsigned long InitTimeUP=0;
 
// define some values used by the panel and buttons
int lcd_key     = 0;
int adc_key_in  = 0;
#define btnRIGHT  0
#define btnUP     1
#define btnDOWN   2
#define btnLEFT   3
#define btnSELECT 4
#define btnNONE   5

int MenuState = 0; 

 
void setup()
{
 lcd.begin(16, 2);              // start the library
 lcd.setCursor(0,0);
 lcd.print("Push the buttons"); // print a simple message
}
  
void loop()
{
 lcd.setCursor(9,1);            // move cursor to second line "1" and 9 spaces over
 lcd.print(millis()/1000);      // display seconds elapsed since power-up

 read_LCD_buttons(); 
 
 switch (MenuState)               // depending on which button was pushed, we perform an action
 {
   
   case 0:
     {
       lcd.setCursor(0,0);
       lcd.print("Test Menu 1     ");
       lcd.setCursor(9,1);
       lcd.print(millis()/1000);
       break;
     }
     
     case 1:
     {
       lcd.setCursor(0,0);
       lcd.print("Test Menu 2     ");
       lcd.setCursor(9,1);
       lcd.print(millis()/1000);
       break;
     }
     
     case 2:
     {
       lcd.setCursor(0,0);
       lcd.print("Test Menu 3     ");
       lcd.setCursor(9,1);
       lcd.print(millis()/1000);
       break;
     }
   
     case btnNONE:
     {
     lcd.print("NONE  ");
     break;
     }
 }
 
}

// read the buttons
void read_LCD_buttons()
{
 adc_key_in = analogRead(0);      // read the value from the sensor 
 
 if (adc_key_in > 1000) {}; //0 = btnNONE ; 1 = btnRight ; 2 = btnUP ; 3 = btnDOWN ; 4 = bntLEFT ; 5 = btnSELECT ; btnNONE.
 if (adc_key_in < 50)   ;  
 if (adc_key_in < 195) {BtnUp();}; 
 if (adc_key_in < 380) {BtnDown();}; 
 if (adc_key_in < 555) {BtnUp();} ; 
 if (adc_key_in < 790)  ;   
}

void BtnUp()
  {
     if (TrackUP==0)
          {
          InitTimeUP = millis();
          TrackUP=1;
          }
          
           unsigned long RealTimerUP = millis();
            if(RealTimerUP -  InitTimeUP < 350) 
            {
               lcd.setCursor(1,15);
                lcd.print("Under");
             
            }
            else
            {
             
                lcd.setCursor(1,15);
                lcd.print("Over");
                    if (MenuState < 3)
                      {
                        MenuState++ ;
                      }
                      else
                      {
                        MenuState = 0;
                      }
     
             TrackUP =0;
            
            } 

}

So with the millis delay thing I have in function BtnUP() it doesnt seem to do anything accept for the

 lcd.setCursor(1,15);
lcd.print("Under");

which will really be nothing accept I made something for troubleshooting.

This however doesn’t seem to be executing at all.

                lcd.setCursor(1,15);
                lcd.print("Over");
                    if (MenuState < 3)
                      {
                        MenuState++ ;
                      }
                      else
                      {
                        MenuState = 0;
                      }
     
             TrackUP =0;

All I want it to do is when I press the UP button on the sheild I dont want it to respond at clock speed I just want it to chill out on that menu for 500ms and then respond again WITHOUT delays.

Thanks and regards.
Ard

General problems: - There is no information about the external hardware (how are buttons connected) - Code style could be better and leads to visually wrong assumtions. All in all code reviews get difficult with the current code layout. - A single procedure has to do the debouncing, buttons state detection (which is missing and btw is probably at least one of the problems) and menu cursor change. This also makes separate testing nearly impossible.

Specific problems: - If your button press will produce the adc value 900, then nothing will happen. - Similar, if the adc value 100 is produced, BtnUp() will be executed twice, which is probably not intended. - As mentioned above, there is no key state handling: You do not wait until the key is not pressed any more. Menu cursor is increased every 350ms because of this. It was not described if this is an intended behavior. - BtnDown() is not part of the code, but it might get called. So how do BtnDown and BtnUp interact with each other (for example if the adc value is 100, the sequence is BtnUp, BtnDown, BtnUp). Do these procs share variables? Can these procs influence each other? - As long as the button is pressed, the text "Over" will be overwritten immediatly by "Under". This would explain the visual feedback you explained and is a result of the missing key state handling. Probably there is an "else" missing in the "BtnUp" procedure for the "TrackUP == 0" condition.

Oliver

void read_LCD_buttons()
{
 adc_key_in = analogRead(0);      // read the value from the sensor 
 
 if (adc_key_in > 1000) {}; //0 = btnNONE ; 1 = btnRight ; 2 = btnUP ; 3 = btnDOWN ; 4 = bntLEFT ; 5 = btnSELECT ; btnNONE.
 if (adc_key_in < 50)   ;  
 if (adc_key_in < 195) {BtnUp();}; 
 if (adc_key_in < 380) {BtnDown();}; 
 if (adc_key_in < 555) {BtnUp();} ; 
 if (adc_key_in < 790)  ;   
}

First two thing you must absolutely learn:

  1. where to place comments
  2. how to indent code (CTRL-T is your friend)

Now about the code: you need to detect when the keypad goes from “no key pressed” state to “key X is pressed” state.

IMHO you should first change the read_LCD_buttons() to return an int representing the currently pressed key, or a special value if no key is pressed, let’s call that NO_KEY.

Every -say- 50 ms you execute this read_LCD_buttons(), and check its return value against the value it returned in the previous cycle.
If the previous value was NO_KEY, and the current one is -say- 3, then you know key 3 has been pressed, so you can run the relevant code.

PS: if you’re thinking about using a delay(50), please stop and study the “blink without delay” example.