Is that the right way to check if an existing password is stored?

Question

I'm creating an application with 2 ATMega16 micro controller.

The second micro should check if a password is stored in the external EEPROM then send this information to the first one. So, if there were a stored password user will be ask to log-in, else the user should set a new password and send it to the second micro to save it in the external EEPROM.

The code doesn't run in a right way; can you help me to understand what happens?

Note: all driver used are tested and all of them works correctly.

First Micro Code

#define PASSWORD_LENGTH 5
#define PASSWORD_ADDRESS 0x0311
#define HMI_READY 0
#define CONTROL_READY 1
#define IS_PASSWORD_EXIST 6
#define PASSWORD_EXISTS 7
#define PASSWORD_NOT_EXISTS 8

#include "lcd.h"
#include "keypad.h"
#include "uart.h"

void HMI_init(void) ;
void HMI_set_new_password(uint8 *a_ptrPassword) ;
void HMI_send_password(uint8 *a_ptrPass) ;

uint8 g_password[PASSWORD_LENGTH] = {0} ;

int main(void)
{
    HMI_init() ;
    UART_sendByte(HMI_READY) ;
    LCD_clearDisplay() ;
    LCD_displayString("Stuck Here : ( ") ; /*<<<<<<<<<<<<<<<<<<<<<*/
    if(UART_receiveByte() == PASSWORD_NOT_EXISTS)
    {
        LCD_sendCommand(CLEAR_DISPLAY) ;
        LCD_displayString("N O ") ;

    }
    else if(UART_receiveByte() == PASSWORD_EXISTS) ;
    {
        LCD_sendCommand(CLEAR_DISPLAY) ;
        LCD_displayString("Y E S ") ;
        while(1)
        {

        }
    }

}




/* Functions Definitions */
void HMI_init(void)
{
    LCD_init() ;
    LCD_sendCommand(CLEAR_DISPLAY) ;
    LCD_sendCommand(CURSOR_OFF) ;
    LCD_displayStringRowCol(0,4,"WELCOME") ;
    LCD_displayStringRowCol(1,1,"TO DOOR LOCKER") ;
    UART_init();
    SREG |=(1<<7) ;
}

void HMI_set_new_password(uint8 *a_ptrPassword)
{
    uint8 i = 0 ;
    uint8 key = 0 ;
    uint8 temp_pass[PASSWORD_LENGTH] = {0} ;
    uint8 confirm_flag = 0 ;

    while(confirm_flag == 0)
    {
        i = 0 ;
        LCD_sendCommand(CLEAR_DISPLAY) ;
        LCD_sendCommand(CURSOR_ON) ;
        LCD_displayString("SET A PASSWORD : ") ;
        LCD_goToRowCol(1,0) ;
        while(i<PASSWORD_LENGTH)
        {
            key = KeyPad_getPresssedKey() ;
            if(key>=0 && key<=9)
            {
                a_ptrPassword[i] = key ;
                LCD_displayCharacter('*') ;
                i++ ;

            }else
            {

            }
            _delay_ms(2000) ;
        }
        LCD_sendCommand(CLEAR_DISPLAY) ;
        LCD_sendCommand(CURSOR_ON) ;
        LCD_displayString("REPEAT PASSWORD : ") ;
        LCD_goToRowCol(1,0) ;
        i = 0 ;
        while(i<PASSWORD_LENGTH)
        {
            key = KeyPad_getPresssedKey() ;
            if(key>=0 && key <=9)
            {
                temp_pass[i] = key ;
                i++ ;
                LCD_displayCharacter('*') ;
            }else
            {

            }
            _delay_ms(2000) ;
        }
        /* compare */
        for(i = 0 ; i<PASSWORD_LENGTH ; i++)
        {
            if(a_ptrPassword[i] != temp_pass[i])
            {
                confirm_flag = 0 ;
                break ;
            }else{
                confirm_flag = 1 ;
            }
        }

        if(confirm_flag == 1)
        {
            LCD_sendCommand(CLEAR_DISPLAY) ;
            LCD_displayString("CONFIRMED") ;
            _delay_ms(2000) ;
        }else if(confirm_flag == 0 )
        {
            LCD_sendCommand(CLEAR_DISPLAY) ;
            LCD_displayString("NOT CONFIRMED") ;
            _delay_ms(2000) ;
        }
    }
}


void HMI_send_password(uint8 *a_ptrPass)
{
    uint8 i = 0 ;
    for(i = 0 ; i<PASSWORD_LENGTH ; i++)
    {
        UART_sendByte(a_ptrPass[i]) ;
    }
}

Second micro code

#define PASSWORD_LENGTH 5
#define PASSWORD_ADDRESS 0x0311
#define HMI_READY 0
#define CONTROL_READY 1
#define IS_PASSWORD_EXIST 6
#define PASSWORD_EXISTS 7
#define PASSWORD_NOT_EXISTS 8

#include "lcd.h"
#include "uart.h"
#include "eeprom.h"


void CONTROL_init(void) ;
uint8 CONTROL_password_exist(void) ;
void CONTROL_receive_password(uint8 *a_ptrPass) ;
void CONTROL_save_password(uint8 *a_ptrPass) ;


uint8 g_received_password[PASSWORD_LENGTH] = {0}  ;
int main(void)
{
    CONTROL_init() ;
    if(CONTROL_password_exist() == 0)
    {
        while(UART_receiveByte() != HMI_READY) ;
        UART_sendByte(PASSWORD_NOT_EXISTS) ;
    }
    else
    {
        while(UART_receiveByte() != HMI_READY) ;
        UART_sendByte(PASSWORD_EXISTS) ;
        while(1)
        {

        }
    }
}

void CONTROL_init(void)
{
    LCD_init() ;
    LCD_sendCommand(CLEAR_DISPLAY) ;
    LCD_sendCommand(CURSOR_OFF) ;
    EEPROM_init() ;
    UART_init() ;
    SREG |=(1<<7) ;
}

uint8 CONTROL_password_exist(void)
{
    uint8 i = 0 ;
    uint8 temp = 0 ;
    for(i=0 ; i<PASSWORD_LENGTH ; i++)
    {
        EEPROM_readByte((PASSWORD_ADDRESS+i) , &temp) ;
        _delay_ms(150) ;
        if(temp != 0xFF)
        {
            return 1 ;
        }
    }
    return 0 ;
}


void CONTROL_receive_password(uint8 *a_ptrPass)
{
    uint8 i = 0 ;
    for(i = 0 ; i<PASSWORD_LENGTH ; i++)
    {
        a_ptrPass[i] = UART_receiveByte() ;
    }
}

void CONTROL_save_password(uint8 *a_ptrPass)
{
    uint8 i = 0 ;
    for(i = 0 ; i<PASSWORD_LENGTH ; i++)
    {
        EEPROM_writeByte((PASSWORD_ADDRESS+i) , a_ptrPass[i]);
        _delay_ms(150) ;
    }
}

_"the code doesn't run in a right way"_, so how does it run? What actually happens is as an important a diagnostic as knowing how you intended it to run. — Clifford, Oct 17 '19 at 21:07
A two-second delay between accepting key-presses seems excessive and user-hostile; it will take >10 seconds to enter the PIN. Better to simply wait until the key is released, e.g.: `while(KeyPad_getPresssedKey() == key) ; // wait for release` — Clifford, Oct 17 '19 at 21:28
A great deal of the code in your question is never called - I appreciate that it is incomplete, but the unused code might be removed from the question to simplify it and focus attention on your actual question. The question also mentions function not yet implemented (log-in/set-password) - you might want to remove that, otherwise it looks like you are asking why that does not work, when that is not your question. — Clifford, Oct 17 '19 at 22:21
Why not pick a MCU with built-in EEPROM/data flash and save you all this trouble? — Lundin, Oct 18 '19 at 06:25
@Clifford the HMI LCD displayes the string " Stuck here " and non of the two conditions block is beeing executed . i didn't finish the code yet : ) i'll improve the method of handling the pressed key as you mentiond :) . the problem now is the pre-password checking : ( — mrMatrix, Oct 18 '19 at 07:27
You are misinterpreting the evidence - clearly it is not "stuck there" - that is merely the last observable behaviour, then the byte received is neither of the values you test, the function terminates - that function being `main()`! Also you have two separate UART reads, if the first is "EXISTS", the test will fail, and it will try to receive "NOT_EXISTS". You need to read the byte into a variable and perform the tests on the single byte. — Clifford, Oct 18 '19 at 08:31

Clifford · Answer 1 · 2019-10-17T22:25:46.723

0

The synchronisation is poorly implemented - you have a start-up race condition. You have no guarantee that CONTROL is ready when HMI sends its single HMI_READY byte.

The HMI should wait for a response, and if none is received in a reasonable time (a matter of milliseconds), resend the HMI_READY:

Pseudo-code - this is just to illustrate a possible structure and logic:

 // Wait for response
 response = false ;
 while( !response )
 {
     // Send or resend synchronisation byte
     send( HMI_READY )

     // Wait for response or timeout
     start_time = now() ;
     while( !response && (now() - start_time) < TIMEOUT )
     {
         ch = receive() ;
         response = ( ch == PASSWORD_NOT_EXISTS || ch == PASSWORD_EXISTS )
     }
}

if( ch == PASSWORD_NOT_EXISTS)
{
    ...
}
else // password exists
{
    ...
}

Your CONTROL may be fine, since it does wait indefinitely for HMI_READY, but might better be structured thus:

// Wait for synch byte
while( receive() == HMI_READY )
{
    // wait
}

if( CONTROL_password_exist() )
{
    send( PASSWORD_EXISTS )
}
else 
{
    send( PASSWORD_NOT_EXISTS )
}

...

In this way, it does not matter when or in what order each micro is started, each will wait for the other - synchronisation.

edited Oct 17 '19 at 22:25

answered Oct 17 '19 at 22:17

Clifford

88,407
13
85
165

i appretiate your help Clifford : ) i tried this method to synhronise the data transfer . it works for me but what is you opinion about it ? : ) ```c /* HMI CODE */ int main(void) { HMI_init() ; UART_sendByte(HMI_READY) ; while(UART_receiveByte() != CONTROL_READY) ; g_received_flag = UART_receiveByte() ; if(g_received_flag == 0 ) { LCD_sendCommand(CLEAR_DISPLAY) ; LCD_displayString("NOT EXIST") ; } else if(g_received_flag == 1 ) { LCD_sendCommand(CLEAR_DISPLAY) ; LCD_displayString("EXIST") ; } while(1) { } }``` – mrMatrix Oct 18 '19 at 07:34
```c /*CONTROL */ int main(void) { CONTROL_init() ; g_check_flag = CONTROL_password_exist() ; while(UART_receiveByte() != HMI_READY) ; UART_sendByte(CONTROL_READY); UART_sendByte(g_check_flag) ; while(1) { } } ``` – mrMatrix Oct 18 '19 at 07:35
SO is not a discussion forum, and comments are unsuited to posting large amounts of code. If you have a question or problem with the new code, post a new question. Without looking too hard, I can see that both devices wait for each-other - sounds like a recipe for a dead-lock. In my suggestion the HMI simply polls the CONTROL until it responds. – Clifford Oct 18 '19 at 08:36

Is that the right way to check if an existing password is stored?

1 Answers1