LCD display menu

Using Arduino Programming Questions
#1

hello guys,
i made this code. works so far but I believe it can be shorter. Any tips ? the button also sometimes doesn’t work well.
here is my code
(Please use code tags, </> button on the menu. Thank you - Moderator)

#include <LiquidCrystal.h>

const int rs = 12, en = 11, d4 = 7, d5 = 6, d6 = 5, d7 = 4;
LiquidCrystal lcd(rs, en, d4, d5, d6, d7);

int leds[] = {A0, A1, A2, A3};

int sw1 = 2;
int sw2 = 3;
int counter = 0;
int menu = 0;
int  collect = 0;

bool aan = false, confirm = false;;

String HoofdMenu[] = {"Individuele Led", "Kleur"};
String IndividueleLed[] = {"1", "2", "3", "4", "Terug"};
String Kleur[] = {"Rood", "Groen", "Terug"};
String OnOff[] = {"Aan", "Uit"};
char arrow = '>';
void setup() {
 // put your setup code here, to run once:
 for (int i = 0; i < 4; i++)
 {
   pinMode(leds[i], OUTPUT);
 }
 attachInterrupt(digitalPinToInterrupt(sw1), isrSwitch2, RISING);
 attachInterrupt(digitalPinToInterrupt(sw2), isrSwitch, RISING);
 lcd.begin(16, 5);

}

void loop() {
 // put your main code here, to run repeatedly:
 switch (menu)
 {
   case 0:
     HoofdMenuAfdrukken();
     while (menu == 0)
     {
       if (aan)
       {
         lcd.clear();
         counter++;
         if (counter > 1)
         {
           counter = 0;
         }
         HoofdMenuAfdrukken();
         delay(500);
         aan = false;
       }
       if (confirm)
       {
         if (counter == 0)
         {
           menu = 1;
         }
         else
         {
           menu = 3;
         }
         delay(500);
         confirm = false;
       }
     }
     break;
   case 1:
     while (menu == 1)
     {
       if (confirm == false)
       {
         lcd.clear();
         lcd.setCursor(0, 0);
         lcd.print(arrow);
         lcd.setCursor(1, 0);
         lcd.print(IndividueleLed[counter]);
         lcd.setCursor(1, 1);
         lcd.print(IndividueleLed[counter + 1]);
         delay(500);
       }
       if (aan)
       {
         counter++;
         if (counter > 4)
         {
           counter = 0;
         }
         delay(500);
         aan = false;
       }
       if (confirm)
       {
         lcd.clear();
         if (counter == 4)
         {
           menu = 0;
         }
         else
         {
           collect = counter;
           menu = 2;
         }
       }
       confirm = false;
     }
     break;
   case 2:
     counter = 0;
     while (menu == 2)
     {
       lcd.clear();
       lcd.setCursor(0, counter);
       lcd.print(arrow);
       lcd.setCursor(1, 0);
       lcd.print(OnOff[0]);
       lcd.setCursor(1, 1);
       lcd.print(OnOff[1]);
       delay(500);
       if (aan)
       {
         counter++;
         if (counter > 1)
         {
           counter = 0;
         }
         delay(500);
         aan = false;
       }
       if (confirm)
       {
         if (counter == 0)
         {
           digitalWrite(leds[collect], HIGH);
         }
         else
         {
           digitalWrite(leds[collect], LOW);
         }
         delay(500);
         menu = 0;
         confirm = false;
       }
     }
     break;
   case 3:
     counter = 0;
     while (menu == 3)
     {
       lcd.clear();
       lcd.setCursor(0, 0);
       lcd.print(arrow);
       lcd.setCursor(1, 0);
       lcd.print(Kleur[counter]);
       lcd.setCursor(1, 1);
       lcd.print(Kleur[counter + 1]);
       delay(500);
       if (aan)
       {
         counter++;
         if (counter > 2)
         {
           counter = 0;
         }
         delay(500);
         aan = false;
       }
       if (confirm)
       {
         if (counter == 2)
         {
           menu = 0;
         }
         else
         {
           collect = counter;
           menu = 2;
         }
         confirm = false;
       }
     }
     break;
 }
}
void isrSwitch()
{
 aan = true;
}
void isrSwitch2()
{
 confirm = true;
}
void HoofdMenuAfdrukken()
{
 lcd.setCursor(0, counter);
 lcd.print(arrow);
 lcd.setCursor(1, 0);
 lcd.print(HoofdMenu[0]);
 lcd.setCursor(1, 1);
 lcd.print(HoofdMenu[1]);
}
#2

this is not readable, noticed how your text turned into italic ?

Please correct your post above and add code tags around your code:
[code]`` [color=blue]// your code is here[/color] ``[/code].

It should look like this:// your code is here
(Also press ctrl-T (PC) or cmd-T (Mac) in the IDE before copying to indent your code properly)

(and read forum rules)

————
side note: your ISR variables should be volatile

#3

it’s a little hard without an description of how your menu code works

instead of having those blocks of lcd calls, have a routine that is passed two strings (either can be NULL) to be displayed on the LCD.

i would make the switch statement in loop() a separate sub-function called menu(). It would be invoked when “aan” or “confirm” are true (and set false on return from menu().

i would think the value of menu would determine what is displayed and would sequence thru the menus with each press (?) of “aan”.

when “confirm” is pressed, the menu action is invoked.

i can’t really follow the relationship between counter and menu. I think it’s better if menus follow a consistent set of rules.

not sure if you need a sub-menu which becomes active when confirm is pressed allowing “aan” to sequence thru some choice until confirm is pressed again

#4

Do I get right that

  • You have a 2 line LCD
  • You have two buttons.

You always display 2 lines of the menu on the LCD. The one on the first line gets a '>' symbol in front

One button is used to circle the menu choices.
One button is used to validate the choice (being the one on the first line, with the '>' in front)

is that correct ?

  • if you want the code to be reactive, you can't have calls to delay() at all.
  • if you want to handle the buttons in a simple way, take a button library. That will make your code easier

there would be ways to make this very generic so that you don't have to code all the interactions. Just declare the menu text and some callbacks functions for example and have a little state machine handle the management of the menu

#5