Hi
I have written below program to read push buttons. I have doubt in Button pressed routine about De-bounce time for push button. Does this routine check each button and wait for De-bounce time ? also if do you find any errors let me know ?

#include <REG51.h>      /* Header file for 8051 */

sbit  Push_Button1 = P2^0;
sbit  Push_Button2 = P2^1;
sbit  Push_Button3 = P3^2;
sbit  Push_Button4 = P3^3;

#define Push_Button_Not_Pressed (0)
#define Push_Button_Pressed  (1)

/* Initialization System */
  void Push_Button_Initialization()
    {
        Push_Button1 = Push_Button_Not_Pressed ;
        Push_Button2 = Push_Button_Not_Pressed ;
        Push_Button3 = Push_Button_Not_Pressed ;
        Push_Button4 = Push_Button_Not_Pressed ;
   }

/* De-bounce time period*/
void Delay (unsigned int i)
{
     for (i = 0; i < 40000; i++);
       {

       }
}

/* check any button pressed and return key number */
  unsigned char Button_pressed(void)
  {
       unsigned char Key_Number;

      if(!Push_Button1)          Key_Number = '1';
      if(!Push_Button2)          Key_Number = '2';
      if(!Push_Button3)          Key_Number = '3';
      if(!Push_Button4)          Key_Number = '4';
 
       Delay (20);                                                   /* De-bounce time */
       return Key_Number;
  }

/* check any button not pressed */
unsigned int Button_not_prssed(void)
{
       unsigned int Key_Number = 0;
       return Key_Number;
}

 

You can effectively debounce push buttons by simply reading them less often than the bounce time. So mostly the bounce time is less than 50mS so read the buttons every 100mS and any effect of bounce is eliminated. Your program reads the buttons and then waits 'Delay'. As long as the delay time is greater than the bounce time then this works.

The Delay routine has a parameter (it is called as 'Delay(20)') but that parameter is not used. Delay(0) gives the same delay as Delay(100).

If more than 1 button is pressed the Button_pressed routine will return the higher numbered pressed button and ignore the lower numbered button. That may be what you want or it may not - that's up to you.

 

Not sure about the rest of the code, but the delay function is definitely incorrect. It should just count down, something like this:

void Delay (unsigned int count)
{
    while(count != 0)
        --count;
}

Also, be careful about stray semicolons at the end of loop expressions:

for (i = 0; i < 40000; i++); // <- You don't want that there!

 

.

The Delay routine has a parameter (it is called as 'Delay(20)') but that parameter is not used. Delay(0) gives the same delay as Delay(100).
.

your statement confusing me
when I need different time. Than I use this routine

void Delay (unsigned int i)
{
    for (i = 0; i < 40000; i++)
    {

    }
}
int main ()
{
   while (1)
   {
       Statment 1;
       Delay(20);
       Statment 2;
       Delay(30);
   }
}

 

for (i = 0; i < 40000; i++)

This line sets i to zero (regardless of whatever value was passed to the routine) and then counts up to 40000 and then returns to the calling routine. Whatever number you pass to this routine you will always get the same delay time.

 

This line sets i to zero (regardless of whatever value was passed to the routine) and then counts up to 40000 and then returns to the calling routine. Whatever number you pass to this routine you will always get the same delay time.

I need delay time 2ms for LCD and debouncing time 20 ms for push button
My general understanding
1 count Takes = 1.085uS time (AT89c51)
19000 count takes = 20615 microsecond = approximately 20 millisecond time
2000 count take = 2010 microsecond = approximately 2 millisecond time

void Delay(unsigned int n)
{
   unsigned i;
   for (i=0; i<n; i++)
   for (i=0; i<19000; i++)
   for (i=0; i<2000; i++);
}

Does it make any sense ?

 

Your code as it stands is not correct.
Assuming your timing details are correct, then removing the second and third 'for' statements in your code and adding a semicolon after the first, then Delay(2000) will give a 2mS delay and Delay(19000) will give a 20mS delay.

 

Your code as it stands is not correct.
Assuming your timing details are correct, then removing the second and third 'for' statements in your code and adding a semicolon after the first, then Delay(2000) will give a 2mS delay and Delay(19000) will give a 20mS delay.

void Delay(unsigned int n)
{
  unsigned i;
  for (i=0; i<n; i++);
  (i=0; i<19000; i++)
  (i=0; i<2000; i++);
}

my compiler (keil) show errors syntax error

 

I meant to remove the whole line not just the 'for' part.

1. void Delay(unsigned int n)
2. {
3. unsigned i;
4. for (i=0; i<n; i++);
5. }

 

.
If more than 1 button is pressed the Button_pressed routine will return the higher numbered pressed button and ignore the lower numbered button. That may be what you want or it may not - that's up to you.

whats wrong in routine? why it will return only one number ?

/* check any button pressed and return key number */
  unsigned char Button_prssed(void)
  {
  unsigned char Key_Number;

  if(!Push_Button1) Delay (19000); Key_Number = '1';
  if(!Push_Button2) Delay (19000); Key_Number = '2';
  if(!Push_Button3) Delay (19000); Key_Number = '3';
  if(!Push_Button4) Delay (19000); Key_Number = '4';
  
    return Key_Number;
  }

 

There isn't necessarily anything wrong. It depends what you want to happen if more than one button is pressed.
As it stands it works on a priority system. If buttons 3 and 1 are pressed at the same time, your routine will return 3. If that is not what you want then let us know how you want it to work. For instance it could return a number with bit 1 set if button 1 is pressed and bit 2 set if button 2 is pressed. That way the return value contains the state of all 4 buttons but, of course, you then need to check that return value to determine what actions your program should take which adds a little bit of processing code (but not much).

 

For instance it could return a number with bit 1 set if button 1 is pressed and bit 2 set if button 2 is pressed. That way the return value contains the state of all 4 buttons but, of course, you then need to check that return value to determine what actions your program should take which adds a little bit of processing code (but not much).

I did that in my other thread but I want to make single routine can read each switch and if it has been pressed return the value of that switch.

/* check any button pressed and return key number */
  unsigned char Button_pressed(void)
  {
  unsigned char Key_Number;

  if(!Push_Button1) Delay (19000); Key_Number = '1';
  if(!Push_Button2) Delay (19000); Key_Number = '2';
  if(!Push_Button3) Delay (19000); Key_Number = '3';
  if(!Push_Button4) Delay (19000); Key_Number = '4';
 
    return Key_Number;
  }

does this routine read each switch and if it has been pressed return the value of that switch?

 

Yes it does but you only need one delay like your original program.
What do you want the routine to do if multiple buttons are pressed at the same time?

 

.
What do you want the routine to do if multiple buttons are pressed at the same time?

It should be like mobile keypad work. For Example dial number 2134. if I press any numbers, That should be show on screen

/* check any button pressed and return key number */
  unsigned char Button_pressed(void)
  {
       unsigned char Key_Number;

       if(!Push_Button1)    Key_Number = '1';
       if(!Push_Button2)    Key_Number = '2';
       if(!Push_Button3)    Key_Number = '3';
       if(!Push_Button4) Key_Number = '4';
       Delay (19000);
       return Key_Number;
  }

 

@Parth786 You know, you have been over this very problem in at least 3 other threads most recently here -
https://forum.allaboutcircuits.com/...51-timer-interrupt.139793/page-4#post-1177838
for pages and pages. You said it was working and now apparently that was not true. There are several things wrong here
Leaving a thread before you finish something.
Starting another thread on the same topic.
And, most distressingly, reverting back to code that caused problems that others solved for you in those other threads.

/* check any button pressed and return key number */
  unsigned char Button_pressed(void)
  {
  unsigned char Key_Number;

  if(!Push_Button1) Delay (19000); Key_Number = '1';
  if(!Push_Button2) Delay (19000); Key_Number = '2';
  if(!Push_Button3) Delay (19000); Key_Number = '3';
  if(!Push_Button4) Delay (19000); Key_Number = '4';

    return Key_Number;
  }

You were told what the problems in this code were but as my last effort I will tell you once again:
1) You still haven't initialized Key_Number so if no buttons are pressed, you would get an unexpected value but only if your C syntax was correct - and it is not.
2) This version will always return '4' because you don't have brackets around the compound statements after the button tests. If the button is pressed, the only thing that will happen is the delay. After that, it will ALWAYS assign the Key_Number following the test.
3) The delay really doesn't do anything useful. The debouncing and wait for button release was also covered in detail in the noted thread. Again, why are you reverting to code that didn't and doesn't work??
4) No design effort shown and no flow charts - again.
5) At the end of that thread, you indicated that you would and were learning the excellent debugger in your programming environment. You are back to slapping some junk together and then hitting the boards when it doesn't work.

/* check any button pressed and return key number - checks ONCE */
  unsigned char Button_pressed(void)
  {
  unsigned char Key_Number;

  Key_Number = 0;  // initialize the key number in case none are pressed!!!

  if(!Push_Button1) {
   Delay (19000);   // this really doesn't do anything useful.
   Key_Number = '1';
  }

  if(!Push_Button2) {
    Delay (19000);
    Key_Number = '2';
}

  if(!Push_Button3){
   Delay (19000);
   Key_Number = '3';
}
  if(!Push_Button4){
   Delay (19000);
   Key_Number = '4';
  }

    return Key_Number;  // 0 if no key pressed
  }

Since you have multiple threads including keypads, switch entry, delays and debouncing, LCD, password entry and a host of others, this thread is a duplicate and will be closed. It is unfair to ask yet another member to invest time in something that has already been addressed - frequently in some detail - only to show up in a new thread in some vain hope that starting over will help you get lucky.
https://forum.allaboutcircuits.com/threads/get-the-input-from-user-and-print-on-screen.139613/
https://forum.allaboutcircuits.com/threads/reading-data-from-device-in-programming.138302/
https://forum.allaboutcircuits.com/threads/keypad-interfacing-using-8051.137828/
https://forum.allaboutcircuits.com/threads/how-to-display-number-on-lcd-through-user-input.138089/
https://forum.allaboutcircuits.com/...51-timer-interrupt.139793/page-5#post-1178726