Arduino Micro: 69 is supposed to show up when I press 1 on the keypad, however some weird number (15 in this case) is showing up

Question

// **I AM ALSO GETTING WEIRD CLUELESS DELAYS WHEN I PRESS BUTTONS ON THE MODE MENU**

/*
   Jebediah's Launch Control System for Kerbal Space Program
   Alpha Build 0.70
   An Open-Source Project by John Seong
*/

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

const byte ROWS = 4;
const byte COLS = 3;

char keys[ROWS][COLS] = {
  {'1', '2', '3'},
  {'4', '5', '6'},
  {'7', '8', '9'},
  {'*', '0', '#'}
};

int timeV, thrustV, massV, gravityV;

byte rowPins[ROWS] = {12, 11, 10, 9};
byte colPins[COLS] = {8, 7, 6};

Keypad keypad = Keypad(makeKeymap(keys), rowPins, colPins, ROWS, COLS);

bool keyDetect = false;
bool menuKeyDetect = false;
bool goHomeDetect = false;

bool countDownDetect = false;
bool countDownOutput = false;
bool twrCalDetect = false;

String timeValue, thrustValue, massValue, gravityValue;

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

void setup() {
  Serial.begin(9600);
  lcd.begin(16, 2);
  lcd.clear();

  Startup();

  timeV = constrain(timeV, 15, 120);
}

void loop() {
  PressKey();

  if (menuKeyDetect == true) {
    PressMenuKey();
  }

  if (goHomeDetect == true) {
    GoHome();
  }

  if (countDownDetect == true) {
    CountDownSequence();
  }

  if (countDownOutput == true) {
    CountDownOutputSequence();
  }
}

// Pages & Sections
void Startup() {
  lcd.print("Jeb's Launch");

  lcd.setCursor(0, 1);
  lcd.print("Control System");

  delay(3000);

  lcd.clear();
  lcd.print("Alpha Build 0.70");

  lcd.setCursor(0, 1);
  lcd.print(" STUDIO HORIZON");

  delay(3000);

  lcd.clear();
  lcd.print("Welcome, Kerman");

  lcd.setCursor(0, 1);
  lcd.print("Press any key...");

  keyDetect = true;
}

void ModeMenu() {
  menuKeyDetect = true;

  lcd.clear();
  lcd.print("1. Countdown");

  lcd.setCursor(0, 1);
  lcd.print("2. TWR");
}

void CountDown() {
  lcd.print("Time (sec): ");

  lcd.setCursor(0, 1);
  lcd.print("# to continue...");

  goHomeDetect = true;
  countDownDetect = true;
}

void TwrCal() {
  lcd.print("Thrust: ");

  lcd.setCursor(0, 1);
  lcd.print("# to continue...");

  goHomeDetect = true;
}

// Actions & Behaviours
void PressKey() {
  char key = keypad.getKey();
  if (key) {
    Serial.println(key);
    if (keyDetect == true) {
      lcd.clear();
      ModeMenu();
      keyDetect = false;
    }
  }
}

void PressMenuKey() {
  char key = keypad.getKey();
  if (key == '1') {
    Serial.println(key);
    if (menuKeyDetect == true) {
      lcd.clear();
      CountDown();
      menuKeyDetect = false;
    }
  } else if (key == '2') {
    Serial.println(key);
    if (menuKeyDetect == true) {
      lcd.clear();
      TwrCal();
      menuKeyDetect = false;
    }
  }
}

void GoHome() {
  char key = keypad.getKey();
  if (key == '*') {
    lcd.clear();
    ModeMenu();
  }
}

void CountDownSequence() {
  char key = keypad.getKey();

  if (key == '0') {
    timeV = timeV + 0;
  }

  if (key == '1') {
    timeV = 69;
  }

  countDownDetect = false;
  countDownOutput = true;
}

void CountDownOutputSequence() {
    lcd.setCursor(13, 0);
    Serial.println(timeV);
    lcd.print(timeV);
}

Edgar Bonet · Accepted Answer · 2020-07-09T12:44:46.707

According to the documentation, the getKey() function is non blocking. This means it does not wait for a key to be pressed: it instead returns immediately whether a key has been pressed or not. If no key has been pressed (i.e. most of the time), it returns a NUL character, which has the numeric value zero and evaluates to false in a boolean context. Hence the idiom:

char key = keypad.getKey();
if (key) {  // if a key has actually been pressed
    // handle it
}

The CountDownSequence() function fails to account for this:

void CountDownSequence() {
  char key = keypad.getKey();  // no check for NUL
if (key == '0') {
    timeV = timeV + 0;  // this has no effect at all
  }
if (key == '1') {
    timeV = 69;  // this is what you would expect to be executed
  }
countDownDetect = false;  // these two lines tell the rest of the
  countDownOutput = true;   // program that we are done.
}

Since key will almost certainly be '\0', the program will move to the next step without ever initializing timeV.

As a side note, I recommend you try to rewrite your program as a finite state machine. A single state variable would make it less confusing and easier to understand than the current collections of booleans.

Edit: Expanding on the idea of the finite state machine. I tried an implementation of your concept based on it.

For handling a menu system, where the user can navigate through several on-screen “pages”, the most obvious first approach is to have one state per page displayed. For this program, these states could be:

MENU, which offers the choice to move to either of the following two pages
SET_COUNT allows the user to set the countdown duration
TWR allows him to compute the thrust/weight ratio
COUNT_DOWN actually runs the countdown timer

In the following, I added an extra transitional state called START that comes right before MENU. This is not required, and its only purpose is to avoid code repetition by having a single transition (START → MENU) responsible for actually drawing the menu.

When implementing a finite state machine, my first recommendation would be to start by drawing a state diagram like this:

In this diagram, each transition is labeled with the event that triggers it. The events are key presses, save for “finished” which means that the countdown is done. The state diagram can become significantly more complex than this as you add new functionality. It is, however, a very good way of designing your system from the point of view of user interaction.

Once you are happy with the state diagram, the coding is just a matter of translating it to a switch/case construct, with one case per state. Here is my take at it:

/*
   Jebediah's Launch Control System for Kerbal Space Program
   Alpha Build 0.70
   An Open-Source Project by John Seong
*/
#include <Wire.h>
#include <LiquidCrystal_I2C.h>
#include <Keypad.h>
const byte ROWS = 4;
const byte COLS = 3;
char keys[ROWS][COLS] = {
  {'1', '2', '3'},
  {'4', '5', '6'},
  {'7', '8', '9'},
  {'*', '0', '#'}
};
byte rowPins[ROWS] = {12, 11, 10, 9};
byte colPins[COLS] = {8, 7, 6};
Keypad keypad=Keypad(makeKeymap(keys), rowPins, colPins, ROWS, COLS);
LiquidCrystal_I2C lcd(0x3F, 2, 1, 0, 4, 5, 6, 7, 3, POSITIVE);
// Show a two-line message on the LCD.
void lcdShow(const char line0, const char line1) {
  lcd.clear();
  lcd.print(line0);
  lcd.setCursor(0, 1);
  lcd.print(line1);
}
void setup() {
  Serial.begin(9600);
  lcd.begin(16, 2);
  lcdShow("Jeb's Launch", "Control System");
  delay(3000);
  lcdShow("Alpha Build 0.70", " STUDIO HORIZON");
  delay(3000);
  lcdShow("Welcome, Kerman", "Press any key...");
  while (!keypad.getKey()) ;  // wait for key press
}
void loop() {
  static enum {START, MENU, SET_COUNT, COUNT_DOWN, TWR} state = START;
  static uint32_t last_second;  // millis() value on last full second
  static int count;             // countdown value
char key = keypad.getKey();
switch (state) {
    case START:  // transitional state
      lcdShow("1. Countdown", "2. TWR");
      state = MENU;
      /* fallthrough /
    case MENU:
      if (key == '1') {  // Countdown
        lcdShow("Enter time", "seconds: ");
        count = 0;
        state = SET_COUNT;
      } else if (key == '2') {  // TWR
        lcdShow("Thrust:", "# to continue...");
        state = TWR;
      }
      break;
    case SET_COUNT:
      if (key >= '0' && key <= '9' && count <= 99) {
        lcd.print(key);
        count = 10  count + (key - '0');
      } else if (key == '#') {
        lcdShow("Take off in", "     seconds");
        // Force a refresh on entering COUNT_DOWN:
        last_second = millis() - 1000;
        count++;
        state = COUNT_DOWN;
      } else if (key == '') {
        state = START;
      }
      break;
    case COUNT_DOWN:
      if (millis() - last_second >= 1000) {
        last_second += 1000;
        count--;
        if (count == 0) {
          Serial.println("Take off!");
        } else if (count < 0) {
          state = START;
          break;
        }
        lcd.setCursor(1, 1);
        lcd.print(count<10 ? "  " : count<100 ? " " : "");  // pad
        lcd.print(count);
      } else if (key == '') {
        state = START;
      }
      break;
    case TWR:
      if (key == '*' || key == '#') {
        state = START;
      }
      break;
  }
}

Note the comment “fallthrough” between START and MENU. This indicates that the lack of break at this place is intended: the processing of the MENU case will follow right after the processing of the START case, in the same loop iteration. The purpose of the fall through is to not loose the current key press, if any. If this is not important, you could put a regular break here and defer the handling of MENU to the next loop iteration.

Note also that the key presses are checked with tests of the form if (key == SOME_VALUE), which means there is no need for an extra if (key) to test whether there was an actual key press.

Arduino Micro: 69 is supposed to show up when I press 1 on the keypad, however some weird number (15 in this case) is showing up

1 Answers1