Display not toggling correctly

Hello, can someone please tell me where my code is going wrong?
I have a single button toggling 3 different pages displayed on an LCD.
However, each page is displaying automatically one after the other and continuously.
I wish each page to be selected only on each button press.
Thanks for your help :slight_smile:

#include <SPI.h>
#include <SdFat.h>
#include <SFEMP3Shield.h>
#include <CapacitiveSensor.h>
#include <Wire.h>
#include <LiquidCrystal_I2C.h>

int switchPin = 10;   
int Display = 0;
int Threshold = (80); //Threshold of triggered mp3
int Multiply = (0);  //increases or decreases the overal sensitivity

SdFat sd;
SFEMP3Shield MP3player;

CapacitiveSensor   cs_5_3 = CapacitiveSensor(5, 3);      
CapacitiveSensor   cs_5_4 = CapacitiveSensor(5, 4);
CapacitiveSensor   cs_5_14 = CapacitiveSensor(5, 14);
CapacitiveSensor   cs_5_15 = CapacitiveSensor(5, 15);
CapacitiveSensor   cs_5_16 = CapacitiveSensor(5, 16);

LiquidCrystal_I2C lcd(0x27, 20, 4); // set the LCD address to 0x27 for a 16 chars and 2 line display


void setup()
{
  // initialize the lcd
  lcd.init();
  lcd.backlight();
  digitalWrite(switchPin, HIGH);

  sd.begin(SD_SEL, SPI_HALF_SPEED);
  MP3player.begin();
  MP3player.setVolume(0, 0);                 // set volume of the mp3 player ( 0 = loudest )
  Serial.begin(9600);

}

void loop()
{
  Multiply = map(analogRead(A3), 0, 1023, 5, 100);

  long total1 =  cs_5_3.capacitiveSensor(Multiply);
  long total2 =  cs_5_4.capacitiveSensor(Multiply);
  long total3 =  cs_5_14.capacitiveSensor(Multiply);
  long total4 =  cs_5_15.capacitiveSensor(Multiply);
  long total5 =  cs_5_16.capacitiveSensor(Multiply);

  {
    delay(10);

    if (total1 > Threshold)
    {
      MP3player.playTrack(1);

    }
    else  if (total2 > Threshold)
    {
      MP3player.playTrack(2);

    }
    else  if (total3 > Threshold)
    {
      MP3player.playTrack(3);

    }
    else  if (total4 > Threshold)
    {
      MP3player.playTrack(4);

    }
    else  if (total5 > Threshold)
    {
      MP3player.playTrack(5);

    }

  }

  if (digitalRead(switchPin) == HIGH) {
    delay(500);                        // delay to debounce switch

    Display ++;
    if (Display > 3) {
      lcd.clear();
      Display = 1;
    }
  }
  switch (Display) {
    case 1: {
        lcd.setCursor(0, 0);
        lcd.print("      "); // write blank space over the previous
        lcd.setCursor(0, 0); //value so there are no artefacts from the previous output
        lcd.print(total1);
        lcd.setCursor(10, 0);
        lcd.print("      ");
        lcd.setCursor(10, 0);
        lcd.print(total2);
        break;
      }

    case 2: {
        lcd.clear();
        lcd.setCursor(0, 1);
        lcd.print("      ");
        lcd.setCursor(0, 1);
        lcd.print(total3);
        lcd.setCursor(10, 1);
        lcd.print("      ");
        lcd.setCursor(10, 1);
        lcd.print(total4);
        break;
      }

    case 3: {
        lcd.clear();
        lcd.setCursor(0, 2);
        lcd.print("      ");
        lcd.setCursor(0, 2);
        lcd.print(total5);
        lcd.setCursor(0, 3);
        lcd.print("      ");
        lcd.setCursor(0, 3);
        lcd.print(Multiply);
        break;
      }
  }
}
int switchPin = 10;   // momentary switch on 10, other side connected to ground

When a button is wired this way:
HIGH == button is not pressed
LOW == button is pressed

Sorry, that comment is misleading and shouldn’t be there.
on my button I have one side to 5v and the other side connected to pin 10 with a 10k resistor to ground.

This piece of code works perfectly with my setup though…

#include <LiquidCrystal.h>
#include <Wire.h>
#include <LiquidCrystal_I2C.h>
#include <LiquidCrystal.h>
LiquidCrystal_I2C lcd(0x27, 20, 4); 

int switchPin = 10;   
int Display = 0;

void setup()
{
  lcd.init();
  lcd.backlight(); pinMode(switchPin, INPUT);
  digitalWrite(switchPin, HIGH);      
  Serial.begin(9600);

}

void loop()
{
  if (digitalRead(switchPin) == HIGH) {
    delay(500);                        // delay to debounce switch

    Display ++;
    if (Display > 3) {
      lcd.clear();
      Display = 1;
    }

    switch (Display) {
      case 1: {
          lcd.setCursor(0, 0);
          lcd.print("page 1");

          break;
        }

      case 2: {
          lcd.clear();
          lcd.setCursor(1, 0);
          lcd.print("Page 2");
          break;
        }

      case 3: {
          lcd.clear();
          lcd.setCursor(1, 1);
          lcd.print("Page 3");
          break;
        }
    }
  }
}
digitalWrite(switchPin, HIGH);

then you should not set that pin HIGH since that turns on the internal pullup resistor (when the pin is an input).

I don’t think that the other issues I see are causing your problem, but here they are:

The capacitiveSensor() function takes a byte input parameter. you have Multiply set as an int

long total5 =  cs_5_16.capacitiveSensor(Multiply);

you have a lot of extra {}brackets that are not needed.

the ones surrounding this block of code are not necessary.

  {
    delay(10);

    if (total1 > Threshold)
    {
      MP3player.playTrack(1);

    }
    else  if (total2 > Threshold)
    {
      MP3player.playTrack(2);

    }
    else  if (total3 > Threshold)
    {
      MP3player.playTrack(3);

    }
    else  if (total4 > Threshold)
    {
      MP3player.playTrack(4);

    }
    else  if (total5 > Threshold)
    {
      MP3player.playTrack(5);

    }

  }

Also you do not need them for a case statements.

   case 1: {
          lcd.setCursor(0, 0);
          lcd.print("page 1");

          break;
        }

There is a lot of added code between those 2 sketches.
Did you test each piece after adding it or did you add all that code and only now testing it.

If its the latter then you should start with the code that works and add in one piece at a time and make sure that works before adding more. That way you will have better idea what could be causing your problem

Thanks Hutkikz! upon your suggestion previously I did try changing the code for the button but it acted exactly the same way…

#include <SPI.h>
#include <SdFat.h>
#include <SFEMP3Shield.h>
#include <CapacitiveSensor.h>
#include <Wire.h>
#include <LiquidCrystal_I2C.h>

const int buttonPin = 2;
int buttonState = 0;         // variable for reading the pushbutton status

int Display = 0;
int Threshold = (80); //Threshold of triggered mp3
int Multiply = (0);  //increases or decreases the overal sensitivity

SdFat sd;
SFEMP3Shield MP3player;

CapacitiveSensor   cs_5_3 = CapacitiveSensor(5, 3);       // 10M resistor between pins 4 & 2, pin 2 is sensor pin, add a wire and or foil if desired
CapacitiveSensor   cs_5_4 = CapacitiveSensor(5, 4);
CapacitiveSensor   cs_5_14 = CapacitiveSensor(5, 14);
CapacitiveSensor   cs_5_15 = CapacitiveSensor(5, 15);
CapacitiveSensor   cs_5_16 = CapacitiveSensor(5, 16);

LiquidCrystal_I2C lcd(0x27, 20, 4); // set the LCD address to 0x27 for a 16 chars and 2 line display


void setup()
{
  // initialize the lcd
  lcd.init();
  lcd.backlight();
  pinMode(buttonPin, INPUT);     

  sd.begin(SD_SEL, SPI_HALF_SPEED);
  MP3player.begin();
  MP3player.setVolume(0, 0);                 // set volume of the mp3 player ( 0 = loudest )
  Serial.begin(9600);

}

void loop()
{
  Multiply = map(analogRead(A3), 0, 1023, 5, 100);

  long total1 =  cs_5_3.capacitiveSensor(Multiply);
  long total2 =  cs_5_4.capacitiveSensor(Multiply);
  long total3 =  cs_5_14.capacitiveSensor(Multiply);
  long total4 =  cs_5_15.capacitiveSensor(Multiply);
  long total5 =  cs_5_16.capacitiveSensor(Multiply);

  {
    delay(10);

    if (total1 > Threshold)
    {
      MP3player.playTrack(1);

    }
    else  if (total2 > Threshold)
    {
      MP3player.playTrack(2);

    }
    else  if (total3 > Threshold)
    {
      MP3player.playTrack(3);

    }
    else  if (total4 > Threshold)
    {
      MP3player.playTrack(4);

    }
    else  if (total5 > Threshold)
    {
      MP3player.playTrack(5);

    }

  }
  buttonState = digitalRead(buttonPin);

  if (buttonState == HIGH) {     
    delay(500);                        // delay to debounce switch

    Display ++;
    if (Display > 3) {
      lcd.clear();
      Display = 1;
    }
  }
  switch (Display) {
    case 1: {
        lcd.setCursor(0, 0);
        lcd.print("      "); // write blank space over the previous
        lcd.setCursor(0, 0); //value so there are no artefacts from the previous output
        lcd.print(total1);
        lcd.setCursor(10, 0);
        lcd.print("      ");
        lcd.setCursor(10, 0);
        lcd.print(total2);
        break;
      }

    case 2: {
        lcd.clear();
        lcd.setCursor(0, 1);
        lcd.print("      ");
        lcd.setCursor(0, 1);
        lcd.print(total3);
        lcd.setCursor(10, 1);
        lcd.print("      ");
        lcd.setCursor(10, 1);
        lcd.print(total4);
        break;
      }

    case 3: {
        lcd.clear();
        lcd.setCursor(0, 2);
        lcd.print("      ");
        lcd.setCursor(0, 2);
        lcd.print(total5);
        lcd.setCursor(0, 3);
        lcd.print("      ");
        lcd.setCursor(0, 3);
        lcd.print(Multiply);
        break;
      }
  }
}

Here is the button and LCD code on its own that I introduced and it works perfectly…

#include <LiquidCrystal.h>
#include <Wire.h>
#include <LiquidCrystal_I2C.h>
#include <LiquidCrystal.h>
LiquidCrystal_I2C lcd(0x27, 20, 4); // set the LCD address to 0x27 for a 16 chars and 2 line display

int switchPin = 10;   
int Display = 0;

void setup()
{
  lcd.init();
  lcd.backlight(); pinMode(switchPin, INPUT);
  digitalWrite(switchPin, HIGH);      // turn on pullup resistor
  Serial.begin(9600);

}

void loop()
{
  if (digitalRead(switchPin) == HIGH) {
    delay(500);                        // delay to debounce switch

    Display ++;
    if (Display > 3) {
      lcd.clear();
      Display = 1;
    }

    switch (Display) {
      case 1: {
          lcd.setCursor(0, 0);
          lcd.print("page 1");

          break;
        }

      case 2: {
          lcd.clear();
          lcd.setCursor(1, 0);
          lcd.print("Page 2");
          break;
        }

      case 3: {
          lcd.clear();
          lcd.setCursor(1, 1);
          lcd.print("Page 3");
          break;
        }
    }
  }
}

I’ll try you other suggestions now…

on my button I have one side to 5v and the other side connected to pin 10 with a 10k resistor to ground.

if (buttonState == HIGH) {     
    delay(500);                        // delay to debounce switch

    Display ++;

There is something not right about your wiring, and the description of the problem indicates that the unpressed state is reading HIGH and you are cycling automatically through the display screens.

Can you verify the inputs with a multimeter? Can you provide a photo of the switch and the wiring. On the most common button there are switched legs and connected legs. Connecting the two leads diagonally across the switch ensures you pick up switched legs.

I don't understand why the code on pin 10 was working? Is it the same button, same wiring, and same position on breadboard?

I thought it would an issue to do with the code in the loop. I have the button wired the same way in the button example from the arduino tutorial... https://www.arduino.cc/en/tutorial/button

and it works with the code for button and LCD which i posted so I'm quite confused.
I'm just going to strip the code back a little and see where she's going wrong...

Sorry, I must add that the first button I was using was reading high in its unpressed state but still working correctly with button LCD code. I've switched it for a correctly functioning button and now the LCD screen is blank... Pressing the button doesn't show anything on the screen now

Damn, lcd screen wasn't showing anything because I was tinkering with the code.

I've just tried my original code that I posted here and it's the same... 3 pages showing in a loop automatically and continuously!

I've just tried my original code that I posted here and it's the same... 3 pages showing in a loop automatically and continuously!

Button is not wired properly to read HIGH when pressed, LOW when not pressed.

It is more simple to use the internal pullup resister, engaged by using pinMode(pin, INPUT_PULLUP);

If wired diagonally across the switch from the input pin to ground, the pin will read HIGH when not pressed, and LOW when pressed.

Cool Catledog, I'll give that a shot!

  if (digitalRead(switchPin) == HIGH) {

delay(500);                        // delay to debounce switch

Display ++;

500 milliseconds isn't a debounce delay. That's like several months in Arduino time. It could have done 8 million things in that time except you chose to make it blind to all inputs.

Humans are slow, remember? It usually takes them more than 500ms to take their fat fingers off buttons they press. So the button is still held down after 500ms.

The digitalRead() tells you that the button IS high. Not "It just recently became high". Follow any button tutorial (preferably one without any delay()) that shows you how to detect that the button just became pressed and doesn't continuously increment Display while the button remains pressed.

Edit: fixed missing [/code] tag

Changing digitalWrite(switchPin, HIGH); to

  pinMode(10, INPUT_PULLUP);

and changing

if (digitalRead(switchPin) == HIGH){

to

if (digitalRead(switchPin) == LOW){

and now everything is working thanks catledog.

Thanks MorganS. I realised that delay was going to have to change and I should be using Millis instead of delay. I'll endeavor to improve that now thanks! :slight_smile: