Problem with append to char array (from KEYPAD)

Hello guys, this is my second post here. I'm working on a project right now, a system that will accept a 4 digit pass, will turn on a piece of equipment, user will have the option to change the password. My code right now looks like this:

#include <Keypad_I2C.h>
#include <Keypad.h>
#include <Wire.h>
#include <Password.h>

#define I2CADDR 0x3F		//PCF8574 address

Password password = Password("1234");	//set default password

const byte ROWS = 4;		//four rows
const byte COLS = 4;		//three columns

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

byte rowPins[ROWS] = { 0, 1, 2, 3 };	//connect to the row pinouts of the keypad
byte colPins[COLS] = { 4, 5, 6, 7 };	//connect to the column pinouts of the keypad

Keypad_I2C kpd(makeKeymap(keys), rowPins, colPins, ROWS, COLS, I2CADDR, PCF8574);

void setup()
{
    Serial.begin(9600);
    Wire.begin();

    DDRB |= 1 << 0;
    DDRB |= 1 << 1;

    kpd.setDebounceTime(30);
    kpd.setHoldTime(40);

    kpd.begin(makeKeymap(keys));
    kpd.addEventListener(keypadEvent);
}

void loop()
{
    kpd.getKey();
}

void keypadEvent(KeypadEvent keyPress)
{
    switch (kpd.getState()) {
    case PRESSED:
	switch (keyPress) {
	case '#':
	    checkPassword();
	    break;

	case '*':
	    password.reset();
	    PORTB &= ~(1 << 0);
	    break;

	case 'A':
	    changePassword();
	    break;

	default:
	    if (!password.evaluate()) {
		password.append(keyPress);	//append to password only if is not authenticated yet
		Serial.print("+");
	    }
	}
    }
}

void checkPassword()
{
    if (password.evaluate())	//if password is correct
    {
	PORTB |= 1 << 0;
	tone(7, 440, 500);

    } else {
	tone(7, 500, 500);
	delay(510);
	noTone(7);
	tone(7, 600, 500);
	delay(510);

	PORTB |= 1 << 1;
	delay(100);
	PORTB &= ~(1 << 1);
	delay(100);
	PORTB |= 1 << 1;
	delay(100);
	PORTB &= ~(1 << 1);
	password.reset();	//reset password
    }
}

void changePassword()
{
    char pass[5];		//new password storage
    byte index;			//index for pass array

    if (password.evaluate()) {

	PORTB |= 1 << 0;
	delay(100);
	PORTB &= ~(1 << 0);

	index = 0;
	Serial.println("CP");	//print debug message "Change Pass"

	while (index <= 3) {	//check to see if you received 4 characters in buffer
	    if (kpd.getKey() != NO_KEY) {
		pass[index] = kpd.getKey();	//store character in buffer one by one 
		Serial.print(index);
		Serial.print(pass[index]);

		index = index + 1;	//increment index
	    }
	}
	//Serial.println(pass);
	Serial.println("Password changed");
    }
    password.reset();
    PORTB &= ~(1 << 0);
}

The problem is this: in the changePassword routine i'm waiting for 4 digits witch i put in an array (i increment the index), but when i try to print any element of the array ... every element appear blank ... like no character was append . Could you please ... put me on the right traks?
Also .. (after this problem will be solved) how could i make a string from array elements as i will need to set new password with password.set(char_from_array);

Thank you in advance.

The pass array should be null terminated in order that it can be recognised as a C style string. You have declared it with a size of 5 and the password is 4 characters long so you have room for the null in the array. Add the null to the array after you increment index and before you try to print any array element or the whole array.

        pass[index] = kpd.getKey();	//store character in buffer one by one 
        Serial.println(index);
        index++;	//increment index  
        pass[index] = 0;  //add the null terminator
        Serial.print(pass[index]);
	    PORTB &= ~(1 << 0);

What's your objection to digitalWrite? Do you want your code to be obscure?

    DDRB |= 1 << 0;
    DDRB |= 1 << 1;

And pinMode?

Thank you for your response, i've made the modification that you post:

while (index <= 3) { //check to see if you received 4 characters in buffer
if (kpd.getKey() != NO_KEY) {
pass[index] = kpd.getKey(); //store character in buffer one by one
Serial.print(index);
Serial.print(pass[index]);

index = index + 1; //increment index
}
}
pass[4]=0;
Serial.println(pass[1]);
Serial.println("Password changed");

But still i can't print any element of the array...

@Nick Gammon: no sir ... i just saw that i can reuse some space. Btw (now is a nice moment) thank you for all you work regarding Arduino (from your forum) excellent work.

i just saw that i can reuse some space

You're using barely half of even a 168's memory - premature optimisation should not be your concern now.

As well as printing index in the loop try printing the value returned from kpd.getKey(); by putting it in a variable first. What sort of value does it return ?

Ok problem solved regarding changing the password, i've managed to read the keypad (took small steps and it wasn't so hard after all). Thank you again for your help.

#include <Keypad_I2C.h>
#include <Keypad.h>
#include <Wire.h>
#include <Password.h>

#define I2CADDR 0x3F		//PCF8574 address

Password password = Password("1234");	//set default password

const byte ROWS = 4;		//four rows
const byte COLS = 4;		//three columns

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

byte rowPins[ROWS] = { 0, 1, 2, 3 };	//connect to the row pinouts of the keypad
byte colPins[COLS] = { 4, 5, 6, 7 };	//connect to the column pinouts of the keypad

Keypad_I2C kpd(makeKeymap(keys), rowPins, colPins, ROWS, COLS, I2CADDR, PCF8574);

void setup()
{
    Serial.begin(9600);
    Wire.begin();

    DDRB |= 1 << 0;
    DDRB |= 1 << 1;

    kpd.setDebounceTime(30);
    kpd.setHoldTime(40);

    kpd.begin(makeKeymap(keys));
    kpd.addEventListener(keypadEvent);
}

void loop()
{
    kpd.getKey();
}

void keypadEvent(KeypadEvent keyPress)
{
    switch (kpd.getState()) {
    case PRESSED:
	switch (keyPress) {
	case '#':
	    checkPassword();
	    break;

	case '*':
	    password.reset();
	    PORTB &= ~(1 << 0);
	    break;

	case 'A':
	    changePassword();
	    break;

	default:
	    if (!password.evaluate()) {
		password.append(keyPress);	//append to password only if is not authenticated yet
		Serial.print("+");
	    }
	}
    }
}

void checkPassword()
{
    if (password.evaluate())	//if password is correct
    {
        Serial.println("Access Granted");
	PORTB |= 1 << 0;
	tone(7, 440, 500);
    } else {
        Serial.println("Access denied");
	tone(7, 500, 500);
	delay(510);
	noTone(7);
	tone(7, 600, 500);
	delay(510);

	PORTB |= 1 << 1;
	delay(100);
	PORTB &= ~(1 << 1);
	delay(100);
	PORTB |= 1 << 1;
	delay(100);
	PORTB &= ~(1 << 1);
	password.reset();	//reset password
    }
}

void changePassword()
{
  byte index;
  char st_pass[5];
  
  Serial.println("Change password");
  index=0;
  while(index<=3){
    char key_pressed=kpd.getKey();
    if (key_pressed!=NO_KEY){
    Serial.println(key_pressed);
    st_pass[index]=key_pressed;
    index++;
    st_pass[index]='\0';
    }
    
  }
  Serial.println(st_pass);
  password.set(st_pass);
  Serial.println("Password Changed");
  password.reset();
}

Now i would like to change the password, with password.set(var_name), according to my test until now ... is not working like this: password.set(st_pass) - i guess because st_pass is char array and password.set expects a string. Is there any way to solve this without String library? Or i'm missing something?

Thank you again.

i guess because st_pass is char array and password.set expects a string.

Or i'm missing something?

Yes, you are missing something. An understanding of exactly what a string is. It is a NULL terminated array of chars. Which is what you have.

Now, what proof have you that password.set() is not working? You know, I hope, that you reset it to the default value every time you reset the Arduino.

If you don't want to do that, you need to persist the new password in EEPROM, and read it from EEPROM in setup(), rather than hard-coding a value.

Thank you PaulS for your answer, so the way to go is: strip the null character?

And yes, the final version will read the pass from EEPROM and if the pass is not present then i will set a default password like 1234.

so the way to go is: strip the null character?

No. The NULL character is a STOP sign. It must be there, so that string functions know where to stop reading from the array.

My question was this: Is there any way to transform a char array to string without String library? Because i heard that there is a slight problem with Strings, and as far as i could see the password.set() accept only strings.

Your use of capitals is confusing.
Forget about Strings and just concentrate on C strings.

Ok problem solved regarding string - the null char was put inside the loop, when it should be outside:

#include <Keypad_I2C.h>
#include <Keypad.h>
#include <Wire.h>
#include <Password.h>

#define I2CADDR 0x3F		//PCF8574 address

Password password = Password("1234");	//set default password

const byte ROWS = 4;		//four rows
const byte COLS = 4;		//three columns

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

byte rowPins[ROWS] = { 0, 1, 2, 3 };	//connect to the row pinouts of the keypad
byte colPins[COLS] = { 4, 5, 6, 7 };	//connect to the column pinouts of the keypad

Keypad_I2C kpd(makeKeymap(keys), rowPins, colPins, ROWS, COLS, I2CADDR, PCF8574);

void setup()
{
    Serial.begin(9600);
    Wire.begin();

    DDRB |= 1 << 0;
    DDRB |= 1 << 1;

    kpd.setDebounceTime(30);
    kpd.setHoldTime(40);

    kpd.begin(makeKeymap(keys));
    kpd.addEventListener(keypadEvent);
}

void loop()
{
    kpd.getKey();
}

void keypadEvent(KeypadEvent keyPress)
{
    switch (kpd.getState()) {
    case PRESSED:
	switch (keyPress) {
	case '#':
	    checkPassword();
	    break;

	case '*':
	    password.reset();
            Serial.println("Password reseted");
	    PORTB &= ~(1 << 0);
	    break;

	case 'A':
	    changePassword();
	    break;

	default:
	    if (!password.evaluate()) {
		password.append(keyPress);	//append to password only if is not authenticated yet
		Serial.print(keyPress);
	    }
	}
    }
}

void checkPassword()
{
    if (password.evaluate())	//if password is correct
    {
        Serial.println("-Access Granted");
	PORTB |= 1 << 0;
	tone(7, 440, 500);
     } else {
        Serial.println("-Access denied");
	tone(7, 500, 500);
	delay(510);
	noTone(7);
	tone(7, 600, 500);
	delay(510);

	PORTB |= 1 << 1;
	delay(100);
	PORTB &= ~(1 << 1);
	delay(100);
	PORTB |= 1 << 1;
	delay(100);
	PORTB &= ~(1 << 1);
	password.reset();	//reset password
    }
}

void changePassword()
{
  byte index;
  char st_pass[5];
  
  Serial.println("Change password");
  index=0;
  while(index<=3){
    char key_pressed=kpd.getKey();
    if (key_pressed!=NO_KEY){
    Serial.println(key_pressed);
    st_pass[index]=key_pressed;
    index++;
    }
    st_pass[4]='\0'; //this was the mistake now is ok
 }
  
  password.set(st_pass); 
  
  Serial.print("Pass set: ");
  Serial.println(st_pass);

}

But it seems to be a problem with Password library ... or with me... and i really, this time see what i'm doing wrong. It seems that after the password change, the new password is not accepted anymore and so the old password.

As you can see in the print screen: 1234 is the initial password, i type it, access granted, then i type 'A' to go to change password option, i wait for 4 digits, i get them, then i add \0 at the end. I set the new password, and try to use it ... as you can see... is not working, am i doing something wrong around here, because i can't see what.

Thank you in advance.

screen.jpg

	PORTB |= 1 << 1;
	delay(100);
	PORTB &= ~(1 << 1);
	delay(100);
	PORTB |= 1 << 1;
	delay(100);
	PORTB &= ~(1 << 1);

I still wish you wouldn't do this. It's incredibly confusing to read, and one has to look carefully to see if you are addressing the right pins in the right way. Just use digitalWrite. Saves everyone a lot of effort. There doesn't seem to be much point "saving space" using PORTB like this, when you use a password library.

As for your problem, you have a lot of Serial prints. How about sharing them? Copy and paste.

DeX_TeR:
@Nick Gammon: ... Btw (now is a nice moment) thank you for all you work regarding Arduino (from your forum) excellent work.

Thanks! :slight_smile:

@Nick Gammon: those are just some actions to be done, it's not so important regarding the code. I will modify the code. Regarding the Serial prints ... all are there in the post number 12 in the code. The print screen that is attached is there to demonstrate the flow of the code and the fact that password.set() is not working as it should ... as far as i can understand and code...
Aside from my wicked code style (my port and pin declarations) do you ... see anything else wrong with my code?! or with my code logic?!

Best regards and many thanks.

Can you give a link to the password library you used please?

After changing the password try resetting it as well (on your keypad) and then entering the new one.

This is the library link: Arduino Playground - Password Library

And i also tried that... no use...same symptoms. Tried to reset it after set it, reset set then reset ... and any combination that you could possibly think of... no use.

Library snippets:

class Password {
public:
	Password(char* pass);
...
	
private:
	char* target;
	char guess[ MAX_PASSWORD_LENGTH ];
	byte currentIndex;
};
//set the password
void Password::set(char* pass){
	target = pass;
}

Your code:

void changePassword()
{
  byte index;
  char st_pass[5];
  
  Serial.println("Change password");
  index=0;
  while(index<=3){
    char key_pressed=kpd.getKey();
    if (key_pressed!=NO_KEY){
    Serial.println(key_pressed);
    st_pass[index]=key_pressed;
    index++;
    }
    st_pass[4]='\0'; //this was the mistake now is ok
 }
  
  password.set(st_pass); 
  
  Serial.print("Pass set: ");
  Serial.println(st_pass);
}

You are calling password.set with a local variable (st_pass) which goes out of scope.

Move st_pass to global scope. ie;.

char st_pass[5];  // <---- move up here

void changePassword()
{
  byte index;
...

That will help.