0

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++;
                }
            }
7
  • 1
    Please define "not work as expected". What does it do?
    – Majenko
    Oct 22 '17 at 13:38
  • the program does not blink on input sequence match with the password and does no display the message to the lcd screen Oct 22 '17 at 13:41
  • Does not look like your key debouncing is very robust. Likely you are sensing numbers like "5555566677777777888".
    – st2000
    Oct 22 '17 at 13:49
  • my approach is based on the code in repl.it/NAeo/0 and I need it for small input sequences like say 200 inputs Oct 22 '17 at 13:58
  • @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.
    – user31481
    Oct 22 '17 at 14:05
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;
            }
        }
    }

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.