-1

I'd like to make some basic calculator. Currently, I'm struggling with inputs. I wanted to get from user some numerical input, then, I'd like to have an information, which type of calculation he is about to do. I'm not sure why, but I can't get any numerical input from user, because the program "jumps" ahead to the operation type. Here's some code:

void loop(){
int a = getNumber();

if(a != NO_KEY){
    do{
    operationType = getOperationType();
    }while(!operationType);
  }

}

getNumber

  int getNumber(){
  char userInput = customKeypad.getKey();
  int value;

  if (userInput == '0' || userInput == '1' || userInput == '2' || userInput == '3' || userInput == '4' || userInput == '5' || 
  userInput == '6' || userInput == '7' || userInput == '8' || userInput == '9'){
      value = userInput - '0';
      Serial.println("value");
      Serial.println(value);
      
    }
  return value;
  }

getOperationType

char getOperationType(){
char userInput = customKeypad.getKey();
 

  switch (userInput){

      case '+':
//      add(a , b);
        Serial.println("add");
        break;
      case '-':
//      subtract(a , b);  
        Serial.println("subtract");  
        break;
      case '*':
//      multiply(a , b);
        Serial.println("multiply");
        break;
      case '/':
//      divide(a , b);
        Serial.println("Divide");
        break;  
      case 'C':
//      TODO
        break;
    }
    return userInput;
  } 

Can I have any hint, why I can't just get simple inputs from the keyboard?

1
  • your program logic assumes that your program waits for a key to be pressed .... reduce your code to get keypress, print value of keypress ... that will show you what you are dealing with
    – jsotola
    May 16 at 19:39

1 Answer 1

1

You need to watch out for what you return and if your variables are initialized. So, what happens here:

In loop() you call getNumber(). There you declare two variables:

  • userInput, which you initialize to the return value of customKeypad.getKey(). So this variable will have the value NO_KEY if no key is pressed.
  • value, which you don't initialize. Thus its value is whatever was there in the variables memory space before it was allocated. This can be anything, but it is not random in most cases. It depends on what your program does prior to this, but you don't have any control over it.

Then you check for userInput being an ASCI digit. As you didn't press a key this obviously is false, so the variable value gets never written. And then you return that variable.

In loop() you check if the return value of getNumber() is unequal to NO_KEY. And since you didn't initialize value, it probably is. Thus the program goes directly to the operation type part.


What to do now?

First: Always initialize every variable that you declare in a function. Global variables get initialized implicitly, but not local variables. To be sure that I don't make an error, I just always initialize ALL my variables when declaring them.

Second: Currently value will not be NO_KEY since you don't assign that value to the variable. But doing so wouldn't fit with your program logic. value is the real numeric value, not the ASCI representation of it. And NO_KEY is defined in the Keypad library as \0 (the null character, with numeric value zero). So NO_KEY is actually equal to 0. And zero is also a valid input to your calculator.

Instead I would suggest returning -1 if no key was pressed and then in loop() checking for that. So something like this:

int getNumber(){
  char userInput = customKeypad.getKey();
  int value;

  if (userInput == '0' || userInput == '1' || userInput == '2' || userInput == '3' || userInput == '4' || userInput == '5' || userInput == '6' || userInput == '7' || userInput == '8' || userInput == '9'){
    value = userInput - '0';
    Serial.println("value");
    Serial.println(value);    
  } else {
    return -1;
  }
  return value;
}

void loop(){
  int a = getNumber();

  if(a != -1){
    do{
      operationType = getOperationType();
    }while(!operationType);
  }
}

That said, I have some suggestions for you for going further:

  • For checking if the returned key is an ASCII digit there is actually a simple function for that: isDigit(). So instead of your chained equal conditions you can just use

      if(isDigit(userInput))
    
  • If you go on in this style you will find yourself wrapping multiple loops in one another as you need to loop for waiting on a keypress. That can get rather messy and it gets difficult to add additional functionality. Instead you could implement a Finite State Machine (FSM). Here the code in loop() consists mainly out of a switch case structure, where every case is one state of your program. For example you could have the states NUMBER1_INPUT, OPERATION_INPUT, NUMBER2_INPUT, DISPLAY_RESULT. The switch structure uses a single int variable as a state marker. For example when you set that state variable to NUMBER1_INPUT it will stay there and loop. When the user as done the number input you can change the state variable to OPERATION_INPUT and let it continue there. It is an important concept and it will change your way of Arduino coding for the better.

    There are many resource for learning how to program a FSM. I've once written a longer answer about this, but you can search for something like "Finite State Machine" on this site or the web to find out more.

1
  • Thank you so much for such explanation. I will take a look on your answer about FSM. I really appreciate your effort.
    – user851242
    May 17 at 11:14

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.