Problems with atoi()

Hello everyone,

I haven’t been using Arduino for very long so please forgive me in advance if the solution to this problem turns out to be obvious.

When I use the atoi function it seems to take the array and turn it into a single character / symbol.

I have no idea why it is doing this as I have used atoi before and not had this problem.

Please find below, the full extract from my code. Also, I have attached the output on the LCD for referencing.

Thank you.

#include<Keypad.h>
#include<LiquidCrystal.h>

LiquidCrystal lcd(A0,A1,A2,A3,A4,A5);

const byte rows = 4, cols = 4;

const int doorLock = 10, start = 11;

int arrayTracker = 0, check = 0;

char keys[rows][cols] = {

{‘1’,‘2’,‘3’,‘a’},
{‘4’,‘5’,‘6’,‘b’},
{‘7’,‘8’,‘9’,‘c’},
{’*’,‘0’,’#’,‘d’}
};

byte rPins[rows] = {9,8,7,6}, cPins[cols] = {5,4,3,2};

Keypad keypad = Keypad(makeKeymap(keys),rPins,cPins,rows,cols);

void setup(){

pinMode(doorLock, OUTPUT);

pinMode(start, INPUT);

lcd.begin(16,2);

lcd.print(“Enter Code:”);

lcd.setCursor(0,1);
}

void loop(){

digitalWrite(doorLock, HIGH);

char storage[3];

char input = keypad.getKey();

if(input){

storage[arrayTracker] = input;

lcd.setCursor(arrayTracker, 1);

lcd.print(storage[arrayTracker]);

arrayTracker++;
}

if(digitalRead(start) == HIGH){

check = atoi(storage);

if(check == 1234){

lcd.clear();

lcd.print(“Code Accepted.”);

digitalWrite(doorLock, LOW);

exit(0);
}

else{

lcd.clear();

lcd.print(storage);

lcd.setCursor(0,1);

lcd.print(“Incorrect Code!”);

exit(0);
}
}
}

lcd failure.PNG

lcd failure.PNG

Not possible to say for sure with the information given but my suggestions are:

  1. Move the declaration of the storage array from loop() to global scope and allocate a larger array, say 13 bytes for luck.
int arrayTracker = 0, check = 0;
char storage[13];
  1. Check if there is room in the array before adding characters to it and always write a null terminator.
   if ( arrayTracker < 12 ) {
      storage[arrayTracker] = input;
      storage[arrayTracker + 1] = '\0';
    }

pass-codes are strings, not integers. you should treat them as such.

The buffer being a local variable to the loop function it means it is reset at every loop... not what you need

Why the “h***” do you define a size of 3 when you expect 1234 as a code... 1234 needs 4 chars and to deal with cstrings you need a trailing null character (‘\0’).. ensure the user cannot try to enter more data...

What do you expect from your exit(0)?? That’s a bad idea if you want the loop to loop...

———————
Please correct your post above and add code tags around your code:
[code]`` [color=blue]// your code is here[/color] ``[/code].

It should look like this:// your code is here
(Also press ctrl-T (PC) or cmd-T (Mac) in the IDE before copying to indent your code properly)

I have executed your codes (totally unchanged) on UNO-LCD, and the following are the screen images of the results; it appears that the program is working? (Check that you have 2k2 pull-down resistor with DPin-11. Also check for loose connections and broken jumper wires!)


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

LiquidCrystal_I2C lcd(0x27, 16, 2);

const byte rows = 4, cols = 4;

const int doorLock = 10, start = 11;

int arrayTracker = 0, check = 0;

char keys[rows][cols] = {
 
 {'1','2','3','a'},
 {'4','5','6','b'},
 {'7','8','9','c'},
 {'*','0','#','d'}
};

byte rPins[rows] = {9,8,7,6}, cPins[cols] = {5,4,3,2};

Keypad keypad = Keypad(makeKeymap(keys),rPins,cPins,rows,cols);

void setup()
{
 
 pinMode(doorLock, OUTPUT);
 
 pinMode(start, INPUT);
 
 lcd.init();
 lcd.backlight();
 
 lcd.print("Enter Code:");
 
 lcd.setCursor(0,1);

 //HR: goto HR;
}

void loop()
{
 
 digitalWrite(doorLock, HIGH);
 
 char storage[3];
 
 char input = keypad.getKey();
 
 if(input)
 {
 
   storage[arrayTracker] = input;
   
   lcd.setCursor(arrayTracker, 1);
   
   lcd.print(storage[arrayTracker]);
   
   arrayTracker++;    
 }


 if(digitalRead(start) == HIGH)
 {

  check = atoi(storage);
 
  if(check == 1234)
  {
   
   lcd.clear();
   
   lcd.print("Code Accepted.");
   
   digitalWrite(doorLock, LOW);
   
   exit(0);
 }
 
 else
 {
   
   lcd.clear();
       
   lcd.print(storage);
   
   lcd.setCursor(0,1);
   
   lcd.print("Incorrect Code!");
   
   exit(0);
 }
 }
}

BulldogLowell:
pass-codes are strings, not integers. you should treat them as such.

To illustrate why you should treat pass-codes as strings, think about the two different pass-codes 12 and 0012. If you treat them as integers, they would look the same to your code.

The Flow Chart given below describes the program logic of the OP. From this Flow Chart, it is possible to recognize the necessity of the exit(0); instructions. If this code has not been added, the program would go back to the beginning of the loop() function and make the variable doorLock again HIGH. (doorLock was made from HIGH to LOW when keycode was matched.)

Pass codes (1, 2, 3, 4) are coming from the keypad as discrete data items. When 1 pressed from the keypad, the corresponding ASCII code (0011 0001 = 0x31) is stored into location storage[0] (thanks to the smart keypad library), and so on. At the end of the entry of the digits 1, 2, 3, and 4; we have the following scenario of the array named storage.

storage[0] contains = 0x31 = 0011 0001
storage[1] contains = 0x32 = 0011 0010
storage[2] contains = 0x33 = 0011 0011
storage[3] contains = 0x34 = 0011 0100

The OP has wanted to compare his expected received keycode with integer 1234 (0x04D2). This requires that the content of the array (storage) has to be turned into: (BCD) 0001 0010 0011 0100 ----> (Binary) 0x4D2. The atoi(storage) function deposits 0x04D2 into the integer variable check.

GolamMostafa:
Pass codes (1, 2, 3, 4) are coming from the keypad as discrete data items. When 1 pressed from the keypad, the corresponding ASCII code (0011 0001 = 0x31) is stored into location storage[0] (thanks to the smart keypad library), and so on. At the end of the entry of the digits 1, 2, 3, and 4; we have the following scenario of the array named storage.

regardless, OP should be managing the passcode as a string. It is foreseeable that as @cristop pointed out, passcode may be misinterpreted using math.

GolamMostafa:
Pass codes (1, 2, 3, 4) are coming from the keypad as discrete data items. When 1 pressed from the keypad, the corresponding ASCII code (0011 0001 = 0x31) is stored into location storage[0] (thanks to the smart keypad library), and so on. At the end of the entry of the digits 1, 2, 3, and 4; we have the following scenario of the array named storage.

storage[0] contains = 0x31 = 0011 0001
storage[1] contains = 0x32 = 0011 0010
storage[2] contains = 0x33 = 0011 0011
storage[3] contains = 0x34 = 0011 0100

The OP has wanted to compare his expected received keycode with integer 1234 (0x04D2). This requires that the content of the array (storage) has to be turned into: (BCD) 0001 0010 0011 0100 ----> (Binary) 0x4D2. The atoi(storage) function deposits 0x04D2 into the integer variable check.

This is total BS.... it just works out if luck.

When nothing is pressed the loop loops and the storage allocated on the stack is released.
When you press 1, indeed the ‘1’ is stored in the first location for storage and then the loop exits and storage is discarded. The loops loops and storage is allocated again. Because of luck and nothing else has happened in between Storage is allocated at the same spot on the stack and because local variables are not zeroed like global ones the ‘1’ is still there in memory.

After a few thousands loops (and being thousands of times lucky) you press 2 and it gets to the next storage byte and now you have ‘1’ and ‘2’ in memory

The same repeats and now you ente ‘3’ so you are still lucky and now you have ‘1’ ‘2’ and’3’ in the storage array.

At that point the array is full. There is no more memory allocated for any extra character but the code does not know better...

So you hit ‘4’ and it gets to overwrite the next byte on the stack.. again you are super lucky because there was nothing else happening and so the byte at the end of the array is not used by any other variable... so you don’t trash anything, your bug goes stealth.. and memory is now holding ‘1’ ‘2’ ‘3’ ‘4’

Then after a few thousand loops again and luckily no memory mess on the stack you use atoi() ... this functions works on cstring and cStrings needs to be null terminated to indicate the end of the cString.... again out of sheer luck, your arduino stack has been zeroed when you reset the arduino, so the next byte on the stack is zero and the function sees the properly formatted cstring “1234\0” which can be converted and bingo it’s a match...

So by now you had 3 bugs and still things appear to work.... your are a lucky but really bad programmer :)... add a couple variables on the stack, a few more functions and suddenly all will fail

As to the necessity of exit() it’s usually a bad idea because the programs dies and now the only way to get something going again is to press reset on the board or power cycle the arduino... poor user experience...

atoi() expects a string, not (just) a char array. It is critically important to understand the difference between a string and a char array.

This is total BS.... it just works out if luck.

What is happening below? Here is an 8-bit oriented array named char storage[];; there is no null-charctaer inserted neither by the user nor by the compiler.

void setup() 
{
 
  Serial.begin(9600);
  char storage[4];
  storage[0] = 0x31;// = 0011 0001
  storage[1] = 0x32;// = 0011 0010
  storage[2]  = 0x33;// = 0011 0011
  storage[3] = 0x34;// = 0011 0100
  int check = atoi(storage);
  Serial.print(check, HEX); //prints: 0x4D2
  Serial.println(check);    //prints 1234
}

void loop() {
  // put your main code here, to run repeatedly:

}

BTW: The function prototype of atoi():

Prototype: int atoi(const char *string);
Header File: stdlib.h (C) or cstdlib (C++)
Explanation: This function accepts a string and converts it into an integer. For example, if "1234" is passed into the function, it will return 1234, an integer.

Here. the pointer variable string is referring to an array whose location can hold signed 8-bit data (char).

1. The Geologists say that they can interpret well-log better because they know the geology better. The Engineers say that they interpret well-log better because they know the principles better.

2. The Computer Science Graduates say that they can study and analysis algorithms better because they know the theory better. The Engineering Graduates say that they can implement algorithms in better ways because they know where and how to make engineering compromises.

GolamMostafa:
What is happening below?

void setup() 

{
  Serial.begin(9600);
  byte storage[4];
  storage[0] = 0x31;// = 0011 0001
  storage[1] = 0x32;// = 0011 0010
  storage[2]  = 0x33;// = 0011 0011
  storage[3] = 0x34;// = 0011 0100
  int check = atoi(storage);
  Serial.print(check, HEX); //prints: 0x4D2
  Serial.println(check);    //prints 1234
}

void loop() {
  // put your main code here, to run repeatedly:

}

Similar luck and hidden bug, you are missing the trailing null char denoting the end of the cstring but are lucky memory was zeroed out before your program runs, so the byte in memory just after then end of your array is 0 and thus print and atoi work

Setup only runs once and as your array is local to setup you could not use it in loop() and memory will be released once the setup() is completed

This would be correct

void setup() 
{
  Serial.begin(9600);
  char storage[5]; // 4 chars and trailing null
  storage[0] = ‘1’;// = 0011 0001
  storage[1] = ‘2’;// = 0011 0010
  storage[2] = ‘3’;// = 0011 0011
  storage[3] = ‘4’;// = 0011 0100
  storage[4] = ‘\0’;// = 0000 0000 IS THE END OD THE CSTRING
  int check = atoi(storage);
  Serial.print(check, HEX); //prints: 0x4D2
  Serial.println(check);    //prints 1234
}

void loop() {
  // put your main code here, to run repeatedly:
}

which is the same as doing

void setup() 
{
  char storage[] = “1234”; // 4 chars and trailing null, let compiler calculate the size needed
  Serial.begin(115200); // no need to go slow on the serial console your pc is fast
  int check = atoi(storage);
  Serial.println(check, HEX); //prints: 4D2
  Serial.println(check);    //prints 1234
}

void loop() {
  // put your main code here, to run repeatedly:
}

Why the “h***” do you define a size of 3 when you expect 1234 as a code… 1234 needs 4 chars and to deal with cstrings you need a trailing null character (’’)… ensure the user cannot try to enter more data…

What is happening in the following C codes executed in the PC using scanf(); function? The declaration of under-size value in the array dimension does not restrict the entry of any number of characters from the standard input device (the keyboard) of the PC upto the return key. In spite of array size being 3, the user can enter any number of characters from the keyboard upto the return key, and they are well taken and printed correctly. The keypad of the OP of this thread is almost the same as the keyboard of the PC except that the keypad does not have the ‘Return Key’ and not adaptive with scanf(); function. However, the ‘Return Key (ASCII Code = 0x00)’ could be easily conceived (for the keypad) as the value of the input when no key is has been pressed.

#include<stdio.h>
#include<string.h>
#include<conio.h>

void main(void)
{
 char str1[3];
 clrscr();
 scanf("%s", str1);  //Entering: Golam
 printf("%sn", str1);    //Golam
 printf("%x", str1[5]);   //0x00  scanf() automatically put ''.
 getch();
}

Then after a few thousand loops again and luckily no memory mess on the stack you use atoi() … this functions works on cstring and cStrings needs to be null terminated to indicate the end of the cString… again out of sheer luck, your arduino stack has been zeroed when you reset the arduino, so the next byte on the stack is zero and the function sees the properly formatted cstring “1234” which can be converted and bingo it’s a match…

The OP was not lucky enough to have received awareness from his intellect that he is required to insert the instruction storage[arrayTracker]=input; just after detecting HIGH level at the Start pin. We have been lucky, due to bad luck of the OP, to enjoy such an interesting analysis (due to @J-M-L) of the OP’s program that contains hidden bug.

if(digitalRead(start) == HIGH)
 {
    storage[arrayTracker] = input;   //Op was not lucky to include this line here
    check = atoi(storage);
    if(check == 1234)
    {
        lcd.clear();
        lcd.print("Code Accepted.");
   
        digitalWrite(doorLock, LOW);
        exit(0);
    }

The buffer being a local variable to the loop function it means it is reset at every loop… not what you need

The OP has not initialized the array; he has just declared it with the statement char storage[3];. Yes! In the loop, it is being declared again and again; does this re-declaration affect its contents?

Finally, the OP could have the following bug free (thanks+ to J-M-L) program:

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

LiquidCrystal_I2C lcd(0x27, 16, 2);

const byte rows = 4, cols = 4;
char storage[5];
const int doorLock = 10, start = 11;

int arrayTracker = 0, check = 0;

char keys[rows][cols] = {
 
 {'1','2','3','a'},
 {'4','5','6','b'},
 {'7','8','9','c'},
 {'*','0','#','d'}
};

byte rPins[rows] = {9,8,7,6}, cPins[cols] = {5,4,3,2};

Keypad keypad = Keypad(makeKeymap(keys),rPins,cPins,rows,cols);

void setup()
{
 
 pinMode(doorLock, OUTPUT);
 
 pinMode(start, INPUT);
 
 lcd.init();
 lcd.backlight();
 
 lcd.print("Enter Code:");
 
 lcd.setCursor(0,1);

 //HR: goto HR;
}

void loop()
{
 
 digitalWrite(doorLock, HIGH);
// char storage[3];
 
 char input = keypad.getKey();
 
 if(input)
 {
 
   storage[arrayTracker] = input;
   lcd.setCursor(arrayTracker, 1);
   lcd.print(storage[arrayTracker]);
   arrayTracker++;    
 }


 if(digitalRead(start) == HIGH)
 {
    
    storgae[arrayTracker] = input;  //null-character (no key press = 0x00) being inserted
    check = atoi(storage);              //atoi() is happy now to have a null-byte terminated array
    if(check == 1234)
    {
        lcd.clear();
        lcd.print("Code Accepted.");
   
        digitalWrite(doorLock, LOW);
        exit(0);
    }
 
    else
    {
        lcd.clear();
        lcd.print(storage);
        lcd.setCursor(0,1);
        lcd.print("Incorrect Code!");
        exit(0);
    }
 }
}

BTW: @J-M-L has opined that there are 3 bugs in the OP’s program:
(a) incorrect size of the array,
(b) declaration of the array at the wrong place and the resetting of the array, and
(c) non insertion of the null-character when using atoi() function.

The first two are certainly bad programming practices; but, they are not going to be the causes of program failure. The third one is certainly a bug, and it will cause the program to fail at the very unlucky moment when the location storage[4] will not come up with 0x00 (the null-byte).

@golam

Your comments are wrong... sorry...

BTW: @J-M-L has opined that there are 3 bugs in the OP's program:
(a) incorrect size of the array,
(b) declaration of the array at the wrong place and the resetting of the array, and
(c) non insertion of the null-character when using atoi() function.

The first two are certainly bad programming practices; but, they are not going to be the causes of program failure. The third one is certainly a bug, and it will cause the program to fail at the very unlucky moment when the location storage[4] will not come up with 0x00 (the null-byte).

I confirm the 3 ARE bugs waiting to bite you

(a) incorrect size of the array,

The memory allocated need to match the memory you will use (or be greater). You cannot allocate 3 bytes and expect to use 5 without consequences. This is a BUG

(b) declaration of the array at the wrong place and the resetting of the array, and

Assuming that local storage allocated on the stack will keep its value for the next call is a programming bug. In order to preserve values you need to declare the variables static which will make them safe to use across calls to the function.

(c) non insertion of the null-character when using atoi() function.

a cString ends with a nul char. there is no exception to this... that's how C has been designed....

I’m just on my phone - but try do add a couple of long int local variables before and after the array and do some maths with those and print them out you’ll see that going beyond the end of an array IS a bug...

If I have understood your instruction correctly, the following is the program face containing unsigned long and float data before and after the local array. I have done arithmetic operations on the data and have printed the result on the Serial Monitor. The results are continuously appearing on the monitor as the loop is repeating. The program also polling the keypad for the digits 1, 2, 3, and 4. There is a match and the LCD shows Code Accepted.

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

LiquidCrystal_I2C lcd(0x27, 16, 2);

const byte rows = 4, cols = 4;
//char storage[3];
const int doorLock = 10, start = 11;

int arrayTracker = 0, check = 0;

char keys[rows][cols] = {
 
 {'1','2','3','a'},
 {'4','5','6','b'},
 {'7','8','9','c'},
 {'*','0','#','d'}
};

byte rPins[rows] = {9,8,7,6}, cPins[cols] = {5,4,3,2};

Keypad keypad = Keypad(makeKeymap(keys),rPins,cPins,rows,cols);

void setup()
{
 
 Serial.begin(9600);
 pinMode(doorLock, OUTPUT);
 
 pinMode(start, INPUT);
 
 lcd.init();
 lcd.backlight();
 
 lcd.print("Enter Code:");
 
 lcd.setCursor(0,1);

 //HR: goto HR;
}

void loop()
{
 
 digitalWrite(doorLock, HIGH);
 unsigned long x1 = 0x12345678;
 unsigned long x2 = 0x12345678;
 unsigned long x3 = 0x12345678;
 float z1=1.23;
 
 char storage[3];
 
 unsigned long x4 = 0xABCDEF12;
 unsigned long x5 = 0x12345678;
 unsigned long x6 = 0x12345678;
 float z2 = 4.57;
 unsigned long x7 = x1 + x2 + x3 + x4 + x5 + x6;
 Serial.println(x7, HEX);
 Serial.println(z1+z2, 2);
 
 char input = keypad.getKey();
 
 if(input)
 {
 
   storage[arrayTracker] = input;
   lcd.setCursor(arrayTracker, 1);
   lcd.print(storage[arrayTracker]);
   arrayTracker++;    
 }


 if(digitalRead(start) == HIGH)
 {
    
    check = atoi(storage);
    if(check == 1234)
    {
        lcd.clear();
        lcd.print("Code Accepted.");
   
        digitalWrite(doorLock, LOW);
        exit(0);
    }
 
    else
    {
        lcd.clear();
        lcd.print(storage);
        lcd.setCursor(0,1);
        lcd.print("Incorrect Code!");
        exit(0);
    }
 }
}

The compiler is getting rid of your variables as all is constant so computed at compile time

Try making them volatile and add a call to random and add millis() too for example in the calculation- also do declare one if them in the if statement and do some math before and after writing in the array with a call to random

(Easier way would be to print the address in memory of the array and the variables and you would know which one overlaps the end of the array)

GolamMostafa:
If I have understood your instruction correctly, the following is the program face containing unsigned long and float data before and after the local array. I have done arithmetic operations on the data and have printed the result on the Serial Monitor. The results are continuously appearing on the monitor as the loop is repeating.

I came back near an Arduino - so consider the following code

void setup() {
  Serial.begin(115200);
}

void loop() {
  char bug1[3] = "56";
  char storage[3];

  Serial.println(F("------------------------"));
  Serial.print(F("3 bytes of storage at memory address = "));
  Serial.println((uint16_t) storage);

  Serial.print(F("Extra Bug1 storage at memory address = "));
  Serial.println((uint16_t) bug1);

  storage[0] = '1';
  Serial.print(F("Storage = \""));
  Serial.print(storage);
  Serial.println(F("\""));
  delay(1000);

  storage[1] = '2';
  Serial.print(F("Storage = \""));
  Serial.print(storage);
  Serial.println(F("\""));
  delay(1000);

  storage[2] = '3';
  Serial.print(F("Storage = \""));
  Serial.print(storage);
  Serial.println(F("\""));
  delay(1000);

  storage[3] = '4';
  Serial.print(F("Storage = \""));
  Serial.print(storage);
  Serial.println(F("\""));

  delay(2000);
}

the output in the console is

[sub][color=purple]
------------------------
3 bytes of storage at memory address = [color=red]2294[/color]
Extra Bug1 storage at memory address = [color=red]2297[/color]
[color=blue]Storage = "1D56"  // Should print "1", missing trailing null, we see the next string in memory
Storage = "1256" // Should print "12", missing trailing null,  we see the next string in memory
Storage = "12356"// Should print "123", missing trailing null,  we see the next string in memory
Storage = "12346" // we see that the '5' from the second string got overwritten by the '4'[/color]
------------------------
3 bytes of storage at memory address = [color=red]2294[/color]
Extra Bug1 storage at memory address = [color=red]2297[/color]
[color=teal]Storage = "12356" //the '5' has been copied again and we have the previous loop data
Storage = "12356"
Storage = "12356"
Storage = "12346" // here we overflow again, the '5' is overwritten by the '4'[/color]
------------------------
3 bytes of storage at memory address = [color=red]2294[/color]
Extra Bug1 storage at memory address = [color=red]2297[/color]
Storage = "12356"
Storage = "12356"
Storage = "12356"
Storage = "12346"
[/color][/sub]

--> one can see that throughout the loop the memory is allocated at the same place every time. but you can also see that when I fill in the storage array without making it a proper cstring, I print wrong information and you can see that as the bug1 variable is just after storage in memory (and is a proper cstring null terminated) the storage buffer basically oveflowing into the next variable, destructing the "56" string I had copied there...

@golam it's not clear to me what you have been trying to do/prove but if you are trying to "prove" that cstring does not need to be null terminated or check for input bound, you need to start questioning your programming skills

arduino_new:
@golam it's not clear to me what you have been trying to do/prove but if you are trying to "prove" that cstring does not need to be null terminated or check for input bound, you need to start questioning your programming skills

it's OK.

OP, seeing the last dozen or so posts done R-U-N-N-O-F-T.