Arduino program to Blink L LED when input matches a set pattern

Question

I have written a code that is supposed to blink the LED on PB5 pin, when a sequence of input matches a 4 digit password input from a keypad.But the program doesn't appears to be working as expected.

keypadGetKeyPressed() returns a char output of the keypad and I want to blink led on PB5 /13 PIN

lcd.h has used PORTB is it because of that

 #include <avr/io.h>
 #include <util/delay.h>
 #include "lcd.h"
 #include "keypad.h"
 #include <string.h>
 int main(void) {
 // Password
 char password[4]="5678";
 char keyPressed, oldKeyPressed = 0;
 lcdInit();
 keypadSetupPins();
 lcdWriteCommand(DISPLAY_ON | BLINK | UNDERLINE);
 lcdWriteCommand(CLEAR_DISPLAY);
 char p1 = '1';
 char p2 = '2';
 char p3 = '3';
 char p4 = '4';
 char p5 = '5';
 //Sets the digital pin Pb5 or 13 as Output
 DDRB |= (1<<PB5);
 //Sets LED initially as ON, put ~ before the ( to set it off initially
 // DDRB |= (1<<PB5);
 // to count matched character
 int count=0;
 char message[]="Logged In";
 while (1) {
 keyPressed = keypadGetKeyPressed();
 if(keyPressed==password[count]){
 count+=1;
 }
 else{
 //if next char doesn't match reset counter
 count= (keyPressed==password[0])?1:0;
 }
 if(count==4){
 //Call the required function to toggle LED
 count=0;
 PORTB |= ~(1<<PB5);
 _delay_ms(1000);
 // Turn led off by setting corresponding bit low in the PORTB register.
 PORTB |= ~(1<<PB5);
 _delay_ms(500);
 lcdWriteString(message);
 }
 if (keyPressed != 0 && (keyPressed != oldKeyPressed))
 {
 lcdWriteChar(keyPressed);
 }
 oldKeyPressed = keyPressed;
 _delay_ms(20);
 if (keyPressed == p1 ) {
 lcdWriteString("done");
 }
 }
 }

keypad.h file

 #ifndef KEYPAD_H_
 #define KEYPAD_H_
 #include <avr/io.h>
 #define NUM_COLS 3
 #define NUM_ROWS 4
 #define COL_DIR DDRB
 #define COL_PORT PORTB
 #define COL_1_PIN 2
 #define COL_2_PIN 3
 #define COL_3_PIN 4
 //NB it is important that the row pins all trigger the same
 //Pin change interrupt - see p70 of the datasheet
 #define ROW_DIR DDRC
 #define ROW_INPUT PINC
 #define ROW_PORT PORTC //for pullups
 #define ROW_1_PIN 0
 #define ROW_2_PIN 1
 #define ROW_3_PIN 2
 #define ROW_4_PIN 3
 /*
 * Set up the keypad pin directions, enable the pin change interrupt
 * and drive the column pins low.
 */
 void keypadSetupPins(void);
 /*
 * Scan the keypad to determine which key is pressed.
 * Returns 0 if no key pressed and returns ascii code if key pressed.
 * Sets all the cols to low when done.
 */
 char keypadGetKeyPressed(void);
 #endif /* KEYPAD_H_ */

keypad.c file

 * Set up the keypad pin directions, pull up resistors
 * and drive the column pins low.
 */
 void keypadSetupPins(void)
 {
 //Set column pins as output pins
 COL_DIR |= ( (1<<COL_1_PIN) | (1<<COL_2_PIN) | (1<<COL_3_PIN) );
 //Set rows pins to be inputs
 ROW_DIR &= ~( (1<<ROW_1_PIN) | (1<<ROW_2_PIN) | (1<<ROW_3_PIN) | (1<<ROW_4_PIN) );
 //Enable Pullups on row pins
 ROW_PORT |= ( (1<<ROW_1_PIN) | (1<<ROW_2_PIN) | (1<<ROW_3_PIN) | (1<<ROW_4_PIN) );
 //Send out three 0's on COLs
 COL_PORT &= ~( (1<<COL_1_PIN) | (1<<COL_2_PIN) | (1<<COL_3_PIN) );
 }
 /*
 * Scan the keypad to determine which key is pressed.
 * Returns 0 if no key pressed and returns ascii code if key pressed.
 * Sets all the cols to low when done.
 */
 void passwordString(char string[])
 {
 int i = 0;
 }
 char keypadGetKeyPressed(void)
 {
 char retVal = 0;
 uint8_t rowPins;
 //Send 011 on the cols
 //Send out 0 on col 1
 COL_PORT &= ~(1<<COL_1_PIN);
 //Send out 11 on cols 2 and 3
 COL_PORT |= ( (1<<COL_2_PIN) | (1<<COL_3_PIN) );
 //CRUCIAL - delay to allow values to propagate to the rows
 _delay_us(1);
 //Read PIN reg and copy into var
 rowPins = ROW_INPUT;
 //Test row 1
 if( !(rowPins & (1<<ROW_1_PIN)) ) //if the ROW_1_PIN in the ROW_INPUT reg is NOT high
 {
 //there is a 0 on row 1 pin => Key 1 is pressed
 retVal = '1';
 }
 //Test row 2
 if( !(rowPins & (1<<ROW_2_PIN)) ) //if the ROW_2_PIN in the ROW_INPUT reg is NOT high
 {
 //there is a 0 on row 1 pin => Key 4 is pressed
 retVal = '4';
 }
 //Test row 3
 if( !(rowPins & (1<<ROW_3_PIN)) ) //if the ROW_1_PIN in the ROW_INPUT reg is NOT high
 {
 //there is a 0 on row 1 pin => Key 7 is pressed
 retVal = '7';
 }
 //Test row 4
 if( !(rowPins & (1<<ROW_4_PIN)) ) //if the ROW_1_PIN in the ROW_INPUT reg is NOT high
 {
 //there is a 0 on row 1 pin => Key 7 is pressed
 retVal = '*';
 }
 COL_PORT &= ~(1<<COL_2_PIN);
 //Send out 011 on cols 2 and 3
 COL_PORT |= ( (1<<COL_1_PIN) | (1<<COL_3_PIN) );
 //CRUCIAL - delay to allow values to propagate to the rows
 _delay_us(1);
 //Read PIN reg and copy into var
 rowPins = ROW_INPUT;
 //Test row 1
 if( !(rowPins & (1<<ROW_1_PIN)) ) //if the ROW_1_PIN in the ROW_INPUT reg is NOT high
 {
 //there is a 0 on row 1 pin => Key 1 is pressed
 retVal = '2';
 }
 //Test row 2
 if( !(rowPins & (1<<ROW_2_PIN)) ) //if the ROW_2_PIN in the ROW_INPUT reg is NOT high
 {
 //there is a 0 on row 1 pin => Key 4 is pressed
 retVal = '5';
 }
 //Test row 3
 if( !(rowPins & (1<<ROW_3_PIN)) ) //if the ROW_1_PIN in the ROW_INPUT reg is NOT high
 {
 //there is a 0 on row 1 pin => Key 7 is pressed
 retVal = '8';
 }
 //Test row 4
 if( !(rowPins & (1<<ROW_4_PIN)) ) //if the ROW_1_PIN in the ROW_INPUT reg is NOT high
 {
 //there is a 0 on row 1 pin => Key 7 is pressed
 retVal = '0';
 }
 COL_PORT &= ~(1<<COL_3_PIN);
 //Send out 011 on cols 2 and 3
 COL_PORT |= ( (1<<COL_1_PIN) | (1<<COL_2_PIN) );
 //CRUCIAL - delay to allow values to propagate to the rows
 _delay_us(1);
 //Read PIN reg and copy into var
 rowPins = ROW_INPUT;
 //Test row 1
 if( !(rowPins & (1<<ROW_1_PIN)) ) //if the ROW_1_PIN in the ROW_INPUT reg is NOT high
 {
 //there is a 0 on row 1 pin => Key 1 is pressed
 retVal = '3';
 }
 //Test row 2
 if( !(rowPins & (1<<ROW_2_PIN)) ) //if the ROW_2_PIN in the ROW_INPUT reg is NOT high
 {
 //there is a 0 on row 1 pin => Key 4 is pressed
 retVal = '6';
 }
 //Test row 3
 if( !(rowPins & (1<<ROW_3_PIN)) ) //if the ROW_1_PIN in the ROW_INPUT reg is NOT high
 {
 //there is a 0 on row 1 pin => Key 7 is pressed
 retVal = '9';
 }
 //Test row 4
 if( !(rowPins & (1<<ROW_4_PIN)) ) //if the ROW_1_PIN in the ROW_INPUT reg is NOT high
 {
 //there is a 0 on row 1 pin => Key 7 is pressed
 retVal = '#';
 }
 return retVal;
 }

lcd.h file

 #ifndef LCD_H_
 #define LCD_H_
 #include <avr/io.h>
 #include <util/delay.h>
 //If nibble mode is enabled it is assumed that D4->D7 on the LCD
 //are connected to pins 0->3 of the data port.
 #define USE_NIBBLE_MODE 0 //0 for false, 1 for true
 #define LINE_MODE 0 //0 for ONE line mode, 1 for TWO line mode
 #define LCD_CONTROL_PORT PORTB
 #define LCD_CONTROL_DIR DDRB
 #define RS_PIN PORTB0
 #define E_PIN PORTB1
 #define LCD_DATA_PORT PORTD
 #define LCD_DATA_DIR DDRD
 #define CLEAR_DISPLAY 0x01
 #define DISPLAY_CURSOR_HOME 0x02
 #define DISPLAY_ON 0x0C
 #define DISPLAY_OFF 0x08
 #define UNDERLINE 0x02 //OR this with DISPLAY_ON
 #define BLINK 0x01 //OR this with DISPLAY_ON
 #define ONE_LINE_MODE 0x34
 #define TWO_LINE_MODE 0x38
 #define SET_ADDRESS 0x80 //You must OR the appropriate address with this
 #define DISPLAY_SHIFT_LEFT 0x18
 #define DISPLAY_SHIFT_RIGHT 0x1C
 #define CURSOR_SHIFT_LEFT 0x10
 #define CURSOR_SHIFT_RIGHT 0x14
 void lcdInit(void);
 void lcdWriteCommand(uint8_t command);
 void lcdWriteChar(char ch);
 void lcdWriteString(char string[]);
 #endif /* LCD_H */

lcd.c file

 #include "lcd.h"
 /***********************************************************
 * This function sets th direction of the pins that are
 * connected to the LCD.
 * The code makes use of the #defined names in lcd.h so that
 * if the pins connected to the LCD change only lcd.h needs
 * to be changed and not the code in this file.
 *
 * This function is complete.
 **********************************************************/
 void lcdInit(void)
 {
 //set direction of control pins to be outputs
 LCD_CONTROL_DIR |= (1<<RS_PIN);
 LCD_CONTROL_DIR |= (1<<E_PIN);
 //Set direction of data port pins to be outputs
 LCD_DATA_DIR |= 0xFF;
 }
 /****************************************************
 * Sends a a single command to the LCD
 * Takes care of the register selection,
 * placing data on the data pins and toggle of E pin
 *****************************************************/
 void lcdWriteCommand(uint8_t command)
 {
 PORTB &= ~(1<<RS_PIN);
 PORTD = command;
 PORTB |= (1<<E_PIN);
 _delay_us(2); //lcd boot delay
 PORTB &= ~(1<<E_PIN);
 _delay_ms(2);
 }
 /****************************************************
 * Sends a a single ascii char to the LCD
 * Takes care of the register selection,
 * placing data on the data pins and toggle of E pin
 *****************************************************/
 void lcdWriteChar(char ch)
 {
 PORTB |= (1<<RS_PIN);
 PORTD = ch;
 PORTB |= (1<<E_PIN);
 _delay_us(2); //lcd boot delay
 PORTB &= ~(1<<E_PIN);
 _delay_ms(2);
 }
 /****************************************************
 * Writes a string to the LCD
 * Steps through the string passed as a parameter and
 * calls lcdWriteChar() to send each character of the
 * string
 *****************************************************/
 void lcdWriteString(char string[])
 {
 int i = 0;
 while (string[i] != '0円')
 {
 lcdWriteChar(string[i]);
 i++;
 }
 }

• Majenko: Please define "not work as expected". What does it do?
• Shubham Johar: the program does not blink on input sequence match with the password and does no display the message to the lcd screen
• st2000: Does not look like your key debouncing is very robust. Likely you are sensing numbers like "5555566677777777888".
• Shubham Johar: my approach is based on the code in repl.it/NAeo/0 and I need it for small input sequences like say 200 inputs
• user31481: @ShubhamJohar. The code your are based on is PC-centric, where OS take care of physical devices, debouncing and buffering. With Arduino you have none of these: you have to debounce your keys yourself. See library Bounce2.

Answer 1

The following proposed code:

1. corrects the worst of the logic problems
2. eliminates unused/unnecessary local variables
3. performs the desired function
4. keeps data declarations local to where they are used
5. expects the keypad input function to perform the majority of the key debouncing.

and now the proposed code:

 #include <avr/io.h>
 #include <util/delay.h>
 #include "lcd.h"
 #include "keypad.h"
 #include <string.h>
 int main(void)
 {
 // Password
 char password[5]="5678"; // 5 to allow for trailing NUL byte
 char oldKeyPressed = 0;
 lcdInit();
 keypadSetupPins();
 lcdWriteCommand(DISPLAY_ON | BLINK | UNDERLINE);
 lcdWriteCommand(CLEAR_DISPLAY);
 //Sets the digital pin Pb5 or 13 as Output
 DDRB |= (1<<PB5);
 //Sets LED initially as ON, put ~ before the ( to set it off initially
 // DDRB |= (1<<PB5);
 size_t count = 0;
 for( size_t i=0; i< strlen( password ); i++ )
 {
 char keyPressed = keypadGetKeyPressed();
 if(keyPressed==password[count])
 {
 count+=1;
 }
 else
 {
 count = 0;
 }
 if( count == strlen( password ) )
 {
 char message[] = "Logged In";
 //Call the required function to toggle LED
 PORTB |= ~(1<<PB5);
 _delay_ms(1000);
 // Turn led off by setting corresponding bit low in the PORTB register.
 PORTB &= ~(1<<PB5);
 _delay_ms(500);
 lcdWriteString(message);
 }
 // this code block should (probably) only be for debug
 if (keyPressed != 0 && (keyPressed != oldKeyPressed))
 {
 lcdWriteChar(keyPressed);
 }
 // debounce the keypad
 oldKeyPressed = keyPressed;
 _delay_ms(20);
 // method of exiting the program before fully logged in
 if (keyPressed == '1' ) // so '1' cannot be part of the password
 {
 lcdWriteString("done");
 break;
 }
 }
 }