Switch case stuck

Hi, I'm trying to make a menu system but I'm having problems transitioning to cases.

currently I only made 3 cases:

  1. main menu (choose between, sales (to print total coins inserted), retail (to change price) wholesale (to change price) and timer (to change duration)
  2. retail menu
  3. timer menu

I have no problem going from case 1 to 2 and v.v. but when i go to case 3 the lcd just goes blank.

int minus = 4;
int plus = 5;
int ok = 6;
int ss_state;
unsigned long i;
int pointer = 1;
float price = 10;
unsigned long timer = 5000;

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

LiquidCrystal_I2C lcd(0x27, 20, 4);

uint8_t check[8] = {0x0, 0x1, 0x3, 0x16, 0x1c, 0x8, 0x0};

void setup() {
  // put your setup code here, to run once:

  pinMode(minus, INPUT);
  pinMode(plus, INPUT);
  pinMode(ok, INPUT);
  Serial.begin(9600);

  lcd.createChar(5, check);
#define printByte(args)  write(args);

  lcd.init();
  lcd.backlight();

  ss_state = 1;
}

void loop() {

  ss();

}


void ss() {
  switch (ss_state) {

    case 1:

      //lcd.setCursor(0,0);
      //lcd.print(pointer);
      //lcd.setCursor(1,0);
      //lcd.print(ss_state);

      lcd.setCursor(8, 0);
      lcd.print("MENU");

      lcd.setCursor(1, 1);
      lcd.print("SALES");

      lcd.setCursor(13, 1);
      lcd.print("RETAIL");

      lcd.setCursor(1, 3);
      lcd.print("WHOLESALE");

      lcd.setCursor(13, 3);
      lcd.print("TIMER");

      i++;


      if (digitalRead(minus) == HIGH && i >= 5) {
        pointer = pointer - 1;
        i = 0;
        delay(5);
      }

      if (digitalRead(plus) == HIGH && i >= 5) {
        pointer = pointer + 1;
        i = 0;
        delay(5);
      }


      if (pointer == 1) {
        lcd.setCursor(12, 3);
        lcd.print(" ");
        lcd.setCursor(0, 3);
        lcd.print(" ");
        lcd.setCursor(0, 1);
        lcd.printByte(5);
      }

      if (pointer == 2) {
        lcd.setCursor(0, 1);
        lcd.print(" ");
        lcd.setCursor(12, 1);
        lcd.print(" ");
        lcd.setCursor(0, 3);
        lcd.printByte(5);
      }

      if (pointer == 3) {
        lcd.setCursor(0, 3);
        lcd.print(" ");
        lcd.setCursor(12, 3);
        lcd.print(" ");
        lcd.setCursor(12, 1);
        lcd.printByte(5);
      }

      if (pointer == 4) {
        lcd.setCursor(0, 1);
        lcd.print(" ");
        lcd.setCursor(12, 1);
        lcd.print(" ");
        lcd.setCursor(12, 3);
        lcd.printByte(5);
      }

      if (pointer >= 5) {
        pointer = 1;
      }

      if (pointer <= 0) {
        pointer = 4;
      }


      if (digitalRead(ok) == HIGH && pointer == 3 && i >= 5) {
        lcd.clear();
        ss_state = 2;
        delay(250);

      }

      if (digitalRead(ok) == HIGH && pointer == 4 && i >= 5) {
        lcd.clear();
        ss_state = 3;
        delay(250);
      }

      break;


    case 2:
      i++;
      float one = 1;
      lcd.setCursor(2, 0);
      lcd.print("SET RETAIL PRICE");
      lcd.setCursor(8, 2);
      lcd.print(price);

      if (digitalRead(plus) == HIGH && i >= 5) {
        price = price + one;
        delay(250);
      }

      if (digitalRead(minus) == HIGH && i >= 5) {
        price = price - one;
        delay(250);
      }



      if (digitalRead(ok) == HIGH && i >= 5) {
        lcd.clear();
        ss_state = 1;
        delay(250);

      }
      break;

    case 3:
      i++;
      lcd.setCursor(6, 0);
      lcd.print("SET TIMER");
      lcd.setCursor(13, 2);
      lcd.print(timer / 1000);
      lcd.setCursor(15, 2);
      lcd.print("sec");

      if (digitalRead(plus) == HIGH && i >= 5) {
        timer = timer + 1000;
        delay(250);
      }

      if (digitalRead(minus) == HIGH && i >= 5) {
        timer = timer - 1000;
        delay(250);
      }

      if (digitalRead(ok) == HIGH && i >= 5) {
        lcd.clear();
        ss_state = 1;
        delay(250);

      }
      break;
  }
}

It doesn't compile for me because of this line:

    float one = 1;

The compiler doesn't like such definitions, to make the error go away enclose the entire case in braces.

Try adding some serial prints so you can see where code execution goes.

1 Like

idk what i did to my arduino but my serial monitor just reads ? but it still works though. i did try printing on the lcd the ss_state in the transition from case 1 to 3

if (digitalRead(ok) == HIGH && pointer == 4 && i >= 5) {
        lcd.clear();
        lcd.print(ss_state);
        ss_state = 3;
        delay(250);

ss_state does become 3 but it gets stuck there.

so is it

case 1: {

something;

break;

}

or

{case 1:

something;

break;

}

The first one - you just need to be sure that any variable declarations in the case are contained in braces.

1 Like

You're a life saver. works now! =D

Hi
I compiled the skectch from @spycbeef and LCD went blank.
I learned from you this way of putting the "command" of the case inside braces.
Thanks.
But I was left with a big doubt, and I would like to tell you about it.

Why does the compiler not "like" this type of definition "and where can I find more information about not using this type of definition?

Thanks in advance

RV mineirin

The error you can get is "error: crosses initialization of" which you could search for. Here's one random link I found.

Basically, it's a side effect of the fact that a switch statement looks structured, but isn't really. It's actually a bunch of labels and jumps behind the scenes and something that you intend to use in one case only will be available to all subsequent cases unless you use braces to restrict scope.

1 Like

Hi
thank you so much,
I will search the indicated link
mineirin

You have a series of if blocks testing the values 1 through 5. The way you currently have it written, if pointer does equal 1, your code then proceeds to check against pointer values 2 through 5 even though you know it's equal to 1. One way to avoid these redundant checks is to use a cascading if-else block:

if (pointer == 1) {
// first code block
} else {
if (pointer == 2) {
// second code block
} else {
if (pointner == 3) {

and so on. That way, when the match is found on pointer, the other tests are skipped. A better way is to use another switch block on pointer:

switch(pointer) {
case 1:
// first if block statements...
break;
case 2:
// second if block statements
break;

and so on. This is a better solution because the compiler generates a jump table that allows program control to branch immediately to the proper code block after a single compare on pointer.

1 Like

thanks for the tip! =)

Will try this. I'm new to arduino and thought of this menu system after watching a video about if cases thats why my code is so untidy haha but i will definitely apply your tip =)

I just noticed, the other IFs are inside the else statement? is that different from else if or just the same but a cleaner way of writing it?

also is that ok that rhere is no closing brackets for the ELSEs?

Yes, it is different. If the first test fails (e.g., pointer not equal to 1), then execution falls to the next if test (i.e., does pointer equal 2). This continues until a match is found or the if-else block is exhausted. If a match is found before the end of the cascading if-else block, its code is executed and the rest of the if tests are ignored.

Actually, I don't like cascading if-else blocks given another alternative. I prefer the switch/case block I mentioned earlier. If you're new to C, read the reviews for my Beginning C for Microcontrollers book on Amazon and see if you think that might be of help to you.

1 Like

Yes, I am. I will do that. Thank you so much, econjack! =D