Understanding Key Debounce

[Vaughn]

Can anyone please help me to understand this key debounce method, I've implemented this from http://www.ganssle.com/debouncing-pt2.htm with small changes in my code here is the code:

---

Code C - [expand]
1
2
3
4
ISR(TIMER0_OVF_vect)            //2 ms interval
{
    keyState = (keyState << 1) | !KeyPressed() | 0xE000;
}

---

---

Code C - [expand]
1
2
3
4
5
while(1)                                 //inside main()
{
    if(keyState == 0xF000);
        LED_PORT ^= (1<<LED);
}

---

I am using the above code and it is working well, toggling the LED on each key press (one toggle on one key press). Here what I understood is as soon the keyState value reaches to 0x1000 ORing with 0xE0000 produces 0xF000 and the key command is executed inside main(). Now what I didn't understand is why the key command is getting executed only once on each key press while the keyState value is changing on each 2 ms (millisecond range) and on the other side loop is running in micro-second range. Why don't it toggles the LED continuously on a single key press? or it is by luck?

 

[Aussie Susan]

Given only the code you have shown us, I would expect that you would actually have the LED flashing at a very high rate 25mSec after the last key press bounce. You certainly would not notice the flashing with your eye but you should see the toggling on a scope.
2mSec after that, no matter whether the key was still pressed or released, keyState would not be 0xf000 and so the toggling would stop until the next debounced key press. Whether it would stop toggling with the LED lit or not would be approximately 50/50 chance.
However, looking at the original code that you link to (I assume you have used the code in "Listing 2" on that page), the test that you have put into your 'main' function is actually in the function that they call from the ISR. Therefore that version is only tested every 2mSec and so would only return TRUE once per press.
I have implemented a software debounce like this (actually more like the version in Listing 3) but I also have a "previousState' value that mirrors the 'debouncedState' variable (along the lines of the code that Trent Cleghorn wrote in the link from the page you referenced) so that I can detect the transitions of any switch in either direction using an XOR of the two variables in my main function. As soon as a difference is detected, I copy the 'debouncedState' into the 'previousState' variable which stops the multiple detection problem.
If I were you, I'd put the test for equality to 0xf000 into the ISR and set another flag on the match. You can then test that flag in your main and reset it when you have toggled the led.
Susan

 

[Vaughn]

Thank you so much, Aussie Susan and AAP.
Dear, Susan you explained very nicely, you solved all my problem. The code I wrote before had one more bug in it, I was needed to utilise that keyState before the 2ms else the keyState would be lost in next interrupt but keeping a flag can be used anytime and I did the same as you suggested and now it is working fine too, but in a previous code you said there are 50/50 chances of LED ON or OFF but it was working 100%, I badly hit the key for more than 50 times and all the time it responded correctly(I'm sure it's not by luck since I do not have oscilloscope I cant see what is happening there), Your suggestion is good to go here is the my new code:

#define F_CPU 1E6
#include <avr/io.h>
#include <avr/interrupt.h>
#define PIN(P)				(*(&P - 2))
#define DDR(P)				(*(&P - 1))

#define PORT(P)				(P)

#define KEY					PB1
#define KEY_PORT			PORTB
#define LED					PB2
#define LED_PORT			PORTB

uint8_t KeyPressed();
volatile uint16_t keyState = 0, keyFlag = 0;

int main(void)
{
	TIMSK	|= (1<<TOIE0);
	TCNT0 = 0x06; 
	TCCR0	= (1<<CS01);
	sei();	
	
	DDR(LED_PORT) |= (1<<LED);
	DDR(KEY_PORT) &= ~(1<<KEY);
    while(1)
    {
		if(keyFlag == 1)
		{
			LED_PORT ^= (1<<LED);
			keyFlag = 0;
		}		
    }
}
ISR(TIMER0_OVF_vect)			//2 ms interrupt
{
	keyState = (keyState << 1) | !KeyPressed() | 0xE000;
	if(keyState == 0xF000) keyFlag = 1;
	TCNT0 = 0x06; 
}
uint8_t KeyPressed()
{
	if(!(PIN(KEY_PORT) & (1<<KEY)))
		return 1;
	return 0;
}

Susan can tell me that this new code is good to go or any more modification is required?

 

[Aussie Susan]

It look OK to me.
I don't know the device that you are using but I have seen (and used) that style of code before with some of the Cypress PSoC devices.
Regardless, I would suggest that you look to move the code for the keyPress function into the ISR itself, for 2 reasons.
Firstly, based on my experience with other environments, if you call a function within an ISR then the compiler must save and restore all of the devices working registers as it cannot tell what is happening within the called function and therefore which registers might be altered. If no functions are called then it can do a bit of optimisation and only save the registers that it will alter with the code that it generates for the ISR. The overall effect might be small but it can be important in other situations when ISR timing is important. Also this does not take into account the time required to call land return from the function.
Secondly, you can replace the "!keyPressed()' call with '((PIN(KEY_PORT) & (1<<KEY))?1:0)'. I think I have the logic correct such that if the switch is down then I take it that PIN(KEY_PORT) will have the bit corresponding to the pin set low and therefore "and"ing it with appropriate bit will mean the result will be true if the key is not pressed. The "!" before that will negate the logic which means that the function will return 1 when the key is pressed, and 0 otherwise.
My suggestion is to use the same basic piece of code which will result in a non-zero value when the key is not pressed. However, this relies on the C convention that "0" is taken as false but any non-zero value is taken as true - the result of the AND operation will set a bit within the result but not necessarily the LSB. The use of the ternary "?:" operator will ensure that the LSB will be set on true so that the overall logic remains the same.
Susan

 

[Vaughn]

You are a genius the way you catch the zero and non-zero issue was superb. It was exactly why I was not putting the keyPressed() function inside the interrupt but your suggestion to use the ternary operator solved my this issue too. Now I am using this code inside ISR and all are working fine.

---

Code C - [expand]
1
2
3
4
5
6
ISR(TIMER0_OVF_vect)                                    //2 ms interrupt
{
    keyState = (keyState << 1) | ((PIN(KEY_PORT) & (1<<KEY)) ? 1:0) | 0xE000;
    if(keyState == 0xF000) keyFlag = 1;
    TCNT0 = 0x06; 
}

---

Now one more thing I wanted to ask you that won't this code be tough for ISR, somewhere I read that the code inside ISR should be short and light? and if not then how long code can we put inside any ISR?

Sir, if you have any link or blog on multifunctional single key, Please let you know me. Thanks

 

[Aussie Susan]

You are correct in that an ISR should be quick to execute. Dependent on the device, when an ISR is running, it can block all other activity on the processor. (This is why you can get multiple interrupt priorities etc on some devices.)
That does not always translate into "short". For example, I have implemented quite a complex state machine within an ISR that had many states. However for each pass through the state machine, it only used one path which meant that the execution time was short.
The amount of code you have in your ISR is not what I would consider "tough" by any means.
Another consideration is how often the ISR is called. If you have an ISR that runs every mSec, then you might be able to add a bit more code than one that is called every uSec.
The key test is, does the execution of the ISR interfere with some other time-critical operation.
Susan