Problems with making lcd menu

Hey guys, I’m trying to make menu for my project with 16x2 LCD and buttons, but my code doesn’t work.
I know that I can make it without interrupts but I’m going to expand the project soon and they will be usefull. I’m a newbe so don’t be mad if I made some silly mistakes.

#include <Wire.h>
#include <LiquidCrystal_I2C.h>

volatile unsigned long time = 0;
volatile bool position = 0;
byte customChar[] = {
  B11000,
  B01100,
  B10110,
  B01011,
  B01011,
  B10110,
  B01100,
  B11000
};

LiquidCrystal_I2C lcd(0x27, 2, 1, 0, 4, 5, 6, 7, 3, POSITIVE); 

void up(){
  if(millis() - time>100){
    position = 1;
    Serial.println(position);
    time= millis();
    arrow();
  }
}



void down(){
  if(millis() - time>100){
    position = 0;
    Serial.println(position);
    time= millis();
    arrow();
  }
}
void arrow(){
  if(position == 0){
    lcd.clear();
    lcd.setCursor(0,0);
    lcd.write((uint8_t)0);}
    else{
    lcd.clear();
    lcd.setCursor(0,1);
    lcd.write((uint8_t)0);}
    }
   
}


void setup() {
  Serial.begin(9600);
  
  lcd.begin(16, 2);
  lcd.createChar(0, customChar);
  
  
  pinMode(2,INPUT_PULLUP); //up button
  pinMode(3,INPUT_PULLUP); //down button
  
  attachInterrupt(digitalPinToInterrupt(2), up, LOW);  
  attachInterrupt(digitalPinToInterrupt(3), down, LOW);

  time = millis();
  arrow();
}

void loop() {
  // put your main code here, to run repeatedly:

}

don't print or do anything long in an interrupt. if you really want your buttons to be interrupts, set a flag that the up or down buttons were pressed and handle the flag at the next round in the loop

you could either use a button library if you don't want to deal with lower level stuff or just check them in your main loop

I’ve changed my code to this, and now it works great, thank you so much!

#include <Wire.h>
#include <LiquidCrystal_I2C.h>

volatile unsigned long time = 0;
volatile bool position = 1;
bool oldposition = 0;
byte customChar[] = {
  B11000,
  B01100,
  B10110,
  B01011,
  B01011,
  B10110,
  B01100,
  B11000
};

LiquidCrystal_I2C lcd(0x27, 2, 1, 0, 4, 5, 6, 7, 3, POSITIVE); 

void up(){
  if(millis() - time>100){
    position = 1;
    time= millis();
    
  }
}



void down(){
  if(millis() - time>100){
    position = 0;
    
    time= millis();
    
  }
}
void arrow(){
  if(position == 0){
    lcd.clear();
    lcd.setCursor(0,0);
    lcd.write((uint8_t)0);}
    else{
    lcd.clear();
    lcd.setCursor(0,1);
    lcd.write((uint8_t)0);}
    }
   



void setup() {
  Serial.begin(9600);
  
  lcd.begin(16, 2);
  lcd.createChar(0, customChar);
  
  
  pinMode(2,INPUT_PULLUP); //up button
  pinMode(3,INPUT_PULLUP); //down button
  
  attachInterrupt(digitalPinToInterrupt(2), up, LOW);  
  attachInterrupt(digitalPinToInterrupt(3), down, LOW);

  time = millis();
  
}

void loop() {
  if(position!=oldposition){
    arrow();
    oldposition=position;
  }

}

gitarzysta123:
I'm a newbe so don't be mad if I made some silly mistakes.

Trying to use interrupts as a newbe is silly.

Using interrupts to detect button presses is silly. Since you'll need to pick up a flag in the main code, you might as well just poll the input in the main code.

Using LOW or HIGH triggered interrupts is generally silly as they will continually fire while the button is pressed. This will bring the execution of your code to its knees.

Doing Serial I/O in an ISR is silly.

In next versions in my project will be many long processes, and interrupts will be necessary. Thank you for your reply, I missed that IRS are triggered by LOW, not FALLING :slight_smile:

gitarzysta123:
In next versions in my project will be many long processes, and interrupts will be necessary. Thank you for your reply, I missed that IRS are triggered by LOW, not FALLING :slight_smile:

you'll get notified the button is pressed but you likely won't be able to handle the press from within the ISR (if it touches anything like Serial or other stuff requiring interrupts to work)... you'll have to wait the end of your long running process to check the flag...

You should think about the architecture of your "long processes", make them happen in small chunks and test buttons and other events in between chunks

Thanks, I will have it in mind.