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.
get keypress, print value of keypress
... that will show you what you are dealing with