Menu driven 16x2 LCD display

Hi everyone!

This is my first Arduino project, so go easy on me. :wink:

I'm trying to get a menu driven display working to operate 4 hoists, each connected to a H-Bridge w/ enable motor controller. I've used the example "Hunt The Wumpus" as a base for the project.

1st - The issue that I'm having right now is that "void handle_menu()" is not detecting "clicked_buttons", however void input_move() which sets the state to handle_menu does see them correctly. You can see the result with serial monitor.

2nd - How do you Serial.print the current state and if it changes? I tried Serial.println(state); but it wouldn't compile like that.

3rd - I know in void move_hoist I will need to use something different than clicked_buttons. Clicked_buttons only triggers when releasing the button. The UP and DOWN pins will need to stay HIGH during the whole key press and go LOW when released.

Kind regards and Thanks!

// -------------------------------------------------------------------------------
// An Arduino sketch that controlls 4 hoists connected to the Arduino Digital PINs:
// Each hoist has a H bridge motor controller attached to it with an ENABLE PIN 
// as well as UP and DOWN PINs.  All hoists share the UP and DOWN PINs and are
// selected by enabling a specific hoist.
//
// PIN  8 - Hoist 1 Enable
// PIN  9 - Hoist 2 Enable 
// PIN 10 - Hoist 3 Enable
// PIN 11 - Hoist 4 Enable 
// PIN 12 - UP
// PIN 13 - DOWN
// --------------------------------------------------------------------------------
#include <Wire.h>
#include <Adafruit_MCP23017.h>
#include <Adafruit_RGBLCDShield.h>

uint8_t current_hoist;
const int hoist_up = 13;
const int hoist_down = 12;
const int hoist[] = {8, 9, 10, 11};

void (*state)() = NULL;
void (*last_state)() = NULL;
unsigned long last_state_change_time;
unsigned long time;
const uint8_t menu_col[4][2] = { {0,  3},
                                 {4,  7},
                                 {8,  11},
                                 {12, 15} };

uint8_t selected_menu_idx;
Adafruit_RGBLCDShield lcd = Adafruit_RGBLCDShield();
enum BackLightColor { RED=0x1, YELLOW=0x3, GREEN=0x2, TEAL=0x6, BLUE=0x4, VIOLET=0x5, WHITE=0x7 };
uint8_t clicked_buttons;


void setup() {
  Serial.begin(9600);
  Serial.println("Debug");
  delay(2000);
  
  pinMode(hoist_up, OUTPUT);
  pinMode(hoist_down, OUTPUT);
  pinMode(hoist[0], OUTPUT);
  pinMode(hoist[1], OUTPUT);
  pinMode(hoist[2], OUTPUT);
  pinMode(hoist[3], OUTPUT);
  
  lcd.begin(16, 2);
  int current_hoist = 0;
  state = begin_welcome;
}

void loop() {
  time = millis();
  
  if (last_state != state) {
    last_state = state;
    last_state_change_time = time;
    Serial.print("State change time: ");
    Serial.println(last_state_change_time);
  }
  read_button_clicks();
  state();
  delay(10);
}


void read_button_clicks() {
  static uint8_t last_buttons = 0;
  uint8_t buttons = lcd.readButtons();
  clicked_buttons = (last_buttons ^ buttons) & (~buttons);
  last_buttons = buttons;

}

void clear_current_menu() {
  Serial.print("In clear_current_menu: ");
  Serial.print(menu_col[selected_menu_idx][0], 1);  
  lcd.setCursor(menu_col[selected_menu_idx][0], 1);
  lcd.print(' ');
  Serial.print(" , ");
  Serial.println(menu_col[selected_menu_idx][1], 1);
  lcd.setCursor(menu_col[selected_menu_idx][1], 1);
  lcd.print(' ');
}

void highlight_current_menu() {
//  Serial.println("In highlight_current_menu");
  lcd.setCursor(menu_col[selected_menu_idx][0], 1);
  lcd.write(0x7E);
  lcd.setCursor(menu_col[selected_menu_idx][1], 1);
  lcd.write(0x7F);
}

//! Check for left and right button clicks and update the menu index as needed.
void handle_menu() {
//    Serial.println("In handle_menu");
    
    if (clicked_buttons & BUTTON_UP) {
      Serial.println("H-UP ");
    }
    if (clicked_buttons & BUTTON_DOWN) {
      Serial.println("H-DOWN ");
    }
    if (clicked_buttons & BUTTON_LEFT) {
      Serial.println("H-LEFT ");
    }
    if (clicked_buttons & BUTTON_RIGHT) {
      Serial.println("H-RIGHT ");
    }
    if (clicked_buttons & BUTTON_SELECT) {
      Serial.println("H-SELECT ");
    }
    
  
  
  
  if (clicked_buttons & BUTTON_LEFT) {
    Serial.print("Clicked Left - New index: ");
    selected_menu_idx = (selected_menu_idx > 0) ? selected_menu_idx - 1 : 3;
    Serial.println(selected_menu_idx);
  } else if (clicked_buttons & BUTTON_RIGHT) {
    Serial.print("Clicked Right - New index: ");
    selected_menu_idx = (selected_menu_idx < 3) ? selected_menu_idx + 1 : 0;
    Serial.println(selected_menu_idx);
  } 
}


void begin_welcome() {
  lcd.clear();
  lcd.setBacklight(TEAL);
  lcd.print(F("Hoist Controller"));
  state = blink_select;
}

void blink_select() {
  static boolean blink = true;
  static unsigned long last_blink_time;
  
  if (time - last_blink_time >= 1000) {
    lcd.setCursor(0, 1);
    if (blink) {
      lcd.write(0x7E);
      lcd.print(F(" PRESS SELECT "));
      lcd.write(0x7F);
    } else {
      lcd.print(F("                "));
    }
    blink = !blink;
    last_blink_time = time;
  }
  
  if (clicked_buttons & BUTTON_SELECT) {
    state = start_menu;
  }
}


// -------------------------------------------------------------------------------
// Moving states / functions
// -------------------------------------------------------------------------------

void start_menu() {
  digitalWrite(hoist[0], LOW);
  digitalWrite(hoist[1], LOW);
  digitalWrite(hoist[2], LOW);
  digitalWrite(hoist[3], LOW);
  

  lcd.clear();
  lcd.setCursor(0, 0);
  lcd.print(F("Select Hoist #"));
  
  lcd.setCursor(0, 1);
  lcd.print(F(" 01  02  03  04"));

  selected_menu_idx = 0;
  lcd.setCursor(menu_col[selected_menu_idx][0], 1);
  lcd.print('[');
  lcd.setCursor(menu_col[selected_menu_idx][1], 1);
  lcd.print(']');

//  Serial.println("Goint to begin_input_move from start_menu");
  state = begin_input_move;
}

void begin_input_move() {
//  Serial.println("In begin_input_move");
  lcd.setCursor(0, 0);
  lcd.print(F("Select Hoist # "));
  lcd.print(' ');
  
  state = input_move;
}

void input_move() {
  if (clicked_buttons) {
    
    if (clicked_buttons & BUTTON_UP) {
      Serial.println("UP ");
    }
    if (clicked_buttons & BUTTON_DOWN) {
      Serial.println("DOWN ");
    }
    if (clicked_buttons & BUTTON_LEFT) {
      Serial.println("LEFT ");
    }
    if (clicked_buttons & BUTTON_RIGHT) {
      Serial.println("RIGHT ");
    }
    if (clicked_buttons & BUTTON_SELECT) {
      Serial.println("SELECT ");
    }
       
    clear_current_menu();
    if (clicked_buttons & BUTTON_SELECT) {
        current_hoist = selected_menu_idx;
        Serial.println("Goint to move_hoist from input_move");
        state = move_hoist;
      }
    } else {
//      Serial.println("Goint to handle_menu from input_move");
      handle_menu();
    }
    
    highlight_current_menu();
  }


void move_hoist () {
  // Enable Selected Hoist
  digitalWrite(hoist[current_hoist], HIGH);
  lcd.clear();
  lcd.setCursor(0, 0);
  lcd.print(F("Press UP or DOWN"));
  lcd.setCursor(0, 1);
  lcd.print(F("To Move Hoist"));

// Going to have to use something different than clicked_buttons
// clicked_buttons only captures on button release.

  if (clicked_buttons) {
    
    if (clicked_buttons & BUTTON_UP) {
      digitalWrite(hoist_up, HIGH);
    } else if ( clicked_buttons & BUTTON_DOWN) {
      digitalWrite(hoist_down, HIGH);
    } else if ( clicked_buttons & BUTTON_SELECT) {
      state = start_menu;
    }
  }
  
  digitalWrite(hoist_up, LOW);
  digitalWrite(hoist_down, LOW);
}
  int current_hoist = 0;

This local variable goes out of scope almost immediately. You might as well delete this line. Having global and local variables with the same name is a bad idea, generally. It is especially bad when the variables have different types.

  if (last_state != state) {
    last_state = state;
    last_state_change_time = time;
    Serial.print("State change time: ");
    Serial.println(last_state_change_time);
  }
  read_button_clicks();

Last state of what? Doesn't the current state depend on which switch was selected?

  state();

I suppose that you think digitalRead() should have been called fred(), too.

2nd - How do you Serial.print the current state and if it changes?

The current state is the address of a function to call. You could print the address, but that would not mean anything to you. You cause all the changes to state. You can print a message when you assign a new value to state.

3rd - I know in void move_hoist I will need to use something different than clicked_buttons. Clicked_buttons only triggers when releasing the button. The UP and DOWN pins will need to stay HIGH during the whole key press and go LOW when released.

You need to provide a link to the library you are using. The statement "clicked_buttons only triggers" makes no sense. clicked_buttons is a variable. The value in the variable only changes when a key is released because that is what you are checking for. The value returned by lcd.readButtons() shows the current state of each of the switches, using one bit per switch. You could use a mask to determine if the switch you think of as UP is currently being pressed. Ditto for the switch you think of as DOWN.

PaulS:
Hi PaulS,

Thank you for taking the time to respond! Let me expand on my prior comment, not only is this my first Arduino project it's the first time I've worked with C++. I'm trying to learn as I go; it's turing out to be an advantageous first project.

I used Hunt_The_Wumpus (Overview & Parts | Arduino "Hunt The Wumpus" | Adafruit Learning System) as my starting point and is where 90% of the code came from. I'm trying to figure out what it's doing to modify it to suit my needs.

Using Arduino Uno R3 w/ Adafruit's RGB LCD Shield Kit (RGB LCD Shield Kit w/ 16x2 Character Display - Only 2 pins used! [POSITIVE DISPLAY] : ID 716 : Adafruit Industries, Unique & fun DIY electronics and kits). The button's are on the RGB LCD Shield and uses the Adafruit_MCP23017.h & Adafruit_RGBLCDShield.h libraries.

If there is a better way to go about this, please let me know.

  int current_hoist = 0;

This local variable goes out of scope almost immediately. You might as well delete this line. Having global and local variables with the same name is a bad idea, generally. It is especially bad when the variables have different types.

Removed, Thank you!

  if (last_state != state) {

last_state = state;
    last_state_change_time = time;
    Serial.print("State change time: ");
    Serial.println(last_state_change_time);
  }
  read_button_clicks();



Last state of what? Doesn't the current state depend on which switch was selected?



state();



I suppose that you think digitalRead() should have been called fred(), too.

This is came from Hunt The Wumpus, I thought it was part of the state switching that it used. That's why I was trying to Serial.print(state); to figure out what state it was in, but it wouldn't compile.




> 2nd - How do you Serial.print the current state and if it changes?


The current state is the address of a function to call. You could print the address, but that would not mean anything to you. You cause all the changes to state. You can print a message when you assign a new value to state.

That's what I ended up doing.




> 3rd - I know in void move_hoist I will need to use something different than clicked_buttons. Clicked_buttons only triggers when releasing the button. The UP and DOWN pins will need to stay HIGH during the whole key press and go LOW when released.


You need to provide a link to the library you are using. The statement "clicked_buttons only triggers" makes no sense. clicked_buttons is a variable. The value in the variable only changes when a key is released because that is what you are checking for. The value returned by lcd.readButtons() shows the current state of each of the switches, using one bit per switch. You could use a mask to determine if the switch you think of as UP is currently being pressed. Ditto for the switch you think of as DOWN.

Yes, I knew I had to address this.

The issue I'm working on is that void handle_menu never sees a button press, so I'm not able to move the cursor around.

Thanks again!