This thread has been locked.

If you have a related question, please click the "Ask a related question" button in the top right corner. The newly created question will be automatically linked to this question.

MSP430G2553: inconsistent behavior re setting up push button interrupt with software debounce

Part Number: MSP430G2553

Hi. I have a MSP430G2553 that I am using to develop a system to select from one of four options using leds as indicators of which option is currently selectable and a push button to make the selection. The way I have the code configured, it works as expected something close to 80% of the time. My question is can anyone see how I have incorrectly coded this given that the trace on an oscilloscope shows observed button bounce is shorter than software debounce time?

In the code, I use Timer1_A1 in continuous mode to cycle between 4 leds with a delay. I have used a software debounce (from bennthomsen.wordpress.com/engineering-toolbox/ti-msp430-launchpad/using-the-switches/) that uses the watchdog timer set to create a 32ms delay (WDT_MDLY_32) before re enabling the interrupt on the push button pin. The interrupt is set to trigger on a low-to-high transition and the pin is pulled up so that the button triggers the interrupt on release. I have only observed bounce times on the order of < 200 us on this breadboarded circuit so either I didn't set the button up correctly or I have failed to implement the watchdog software delay correctly.

Thanks for any help that you can provide!

Nick

#include <msp430.h>

#define LED1 BIT4
#define LED2 BIT5
#define LED3 BIT0
#define LED4 BIT1
#define PB1 BIT2

volatile int timerOverflow = 0;
volatile int pushButtonPressed = 0;

void turnOffLeds(void);
/*
 * main.c
 */
int main(void) {
    int selectingLED = 1;
    int ledPosit = 0;
    int myLeds[4] = {LED1, LED2, LED3, LED4};

    WDTCTL = WDTPW | WDTHOLD;	// Stop watchdog timer
    //set ports as i/o
    P1SEL = 0;
    P1SEL2 = 0;
    P1DIR |= (LED1 + LED2);
	P1OUT &= ~(LED1 + LED2); //turn these off

	P2SEL = 0;
    P2SEL2 = 0;
    P2DIR |= (LED3 + LED4);
	P2OUT &= ~(LED3 + LED4); //turn these off

    //start with this LED ON
    P1OUT = LED1;

    //set up pushbutton
    P2DIR &= ~PB1; // set up as input pin
    P2REN |= PB1; // enable pull up/down resistor
    P2OUT |= PB1; // select pull up
    P2IES &= ~PB1; // enable interrupt to trigger on rising edge
    P2IFG &= ~PB1; // clear interrupt flag
    P2IE |= PB1; // enable interrupts on this pin
	/*
	 * MC_2 continuous up, ID_3 input divide by 8, TASSEL_2 SMCLK, enable interrupt
	 */
	TA1CTL = MC_2 + TASSEL_2 + ID_3 + TAIE;
    __enable_interrupt();
	for(;;)
	{
	    int lpbPressed = pushButtonPressed;
	    if(lpbPressed == 1)
	    {
	        if (selectingLED == 1)
	        {
	            // disable timer interrupt
	            TA1CTL &= ~TAIE;
	            selectingLED = 0;
	        }else{
	            //enable timer interrupt and back to displaying LEDs
	            TA1CTL |= TAIE;
	            selectingLED = 1;
	            //turnOffLeds();
	        }
	        pushButtonPressed = 0;
	        //enable push button interrupt
	        P2IE |= PB1;
	    }
	    if(timerOverflow == 1)
	    {
	        timerOverflow = 0;
	        //turn off all leds
	        turnOffLeds();
	        if (ledPosit < 2) //check for PORT1
	        {
	            P1OUT |= myLeds[ledPosit];
	        }else{ // must be PORT2
	            P2OUT |= myLeds[ledPosit];
	        }
	        if (ledPosit > 2)
	        {
	            ledPosit = 0;
	        }else{
	            ledPosit = ledPosit + 1;
	        }
	    }
	}
	return 0;
}
void turnOffLeds(void)
{
    P1OUT &= ~(LED1 + LED2);
    P2OUT &= ~(LED3 + LED4);
}
#pragma vector = PORT2_VECTOR
__interrupt void PORT2_ISR (void)
{
        P2IFG &= ~PB1; // clear flag
        P2IE &= ~PB1; // disable interrupt
        /*
        WDTCTL = WDT_MDLY_32; // start and set WD timer for btn debounce
        IFG1 &= ~WDTIFG; // clear flag
        IE1 |= WDTIE; // enable WDT interrupt
        */
        pushButtonPressed = 1;
}
#pragma vector = WDT_VECTOR
__interrupt void WDT_ISR(void)
{
    IE1 &= ~WDTIE; // disable WDT
    IFG1 &= ~WDTIFG; // clear flag
    WDTCTL =  WDTPW | WDTHOLD; // stop WD
    //enable push button interrupt
    P2IE |= PB1;
}
#pragma vector = TIMER1_A1_VECTOR
__interrupt void Timer1_A1_ISR (void)
{
    static int timerCounter = 0;
    switch(TA1IV){
    case 10://overflow TA1IFG
    {
        if (timerCounter == 3)
        {
            timerOverflow = 1;
            timerCounter = 0;
        }else{
            timerCounter = timerCounter + 1;
        }
        break;
    }
    default:
        //oops
        break;
    }
}

  • Woops, that code example had one issue in my question above. The PORT2_ISR does not have the activation of the watchdog timer commented out in my working copy. It should read...

    #pragma vector = PORT2_VECTOR
    __interrupt void PORT2_ISR (void)
    {
            P2IFG &= ~PB1; // clear flag
            P2IE &= ~PB1; // disable interrupt
    
            WDTCTL = WDT_MDLY_32; // start and set WD timer for btn debounce
            IFG1 &= ~WDTIFG; // clear flag
            IE1 |= WDTIE; // enable WDT interrupt
    
            pushButtonPressed = 1;
    }
    

  • And what exactly is the problem? Doesn't the interrupt get called?
  • Good question. The PORT2_ISR is called every time the push button is released. The variable pushButtonPressed is assigned a value of 1 and the code to clear it at the end of the conditional if(lpbPressed == 1) is reached. When the behavior is not consistent with the design,  it appears that the P2IFG has been set again (or never cleared in the WDT_ISR) and the code inside the conditional if(lpbPressed == 1) is immediately run a second time thereby resetting the system to the state existing prior to when the push button was released.

    I am including the code (in its current configuration) in this reply to avoid confusion. Thanks for looking into this!

    #include <msp430.h>
    
    #define LED1 BIT4
    #define LED2 BIT5
    #define LED3 BIT0
    #define LED4 BIT1
    #define PB1 BIT2
    
    volatile int timerOverflow = 0;
    volatile int pushButtonPressed = 0;
    
    void turnOffLeds(void);
    /*
     * main.c
     */
    int main(void) {
        int selectingLED = 1;
        int ledPosit = 0;
        int myLeds[4] = {LED1, LED2, LED3, LED4};
    
        WDTCTL = WDTPW | WDTHOLD;	// Stop watchdog timer
        //set ports as i/o
        P1SEL = 0;
        P1SEL2 = 0;
        P1DIR |= (LED1 + LED2);
    	P1OUT &= ~(LED1 + LED2); //turn these off
    
    	P2SEL = 0;
        P2SEL2 = 0;
        P2DIR |= (LED3 + LED4);
    	P2OUT &= ~(LED3 + LED4); //turn these off
    
        //start with this LED ON
        P1OUT = LED1;
    
        //set up pushbutton
        P2DIR &= ~PB1; // set up as input pin
        P2REN |= PB1; // enable pull up/down resistor
        P2OUT |= PB1; // select pull up
        P2IES &= ~PB1; // enable interrupt to trigger on rising edge
        P2IFG &= ~PB1; // clear interrupt flag
        P2IE |= PB1; // enable interrupts on this pin
    	/*
    	 * MC_2 continuous up, ID_3 input divide by 8, TASSEL_2 SMCLK, enable interrupt
    	 */
    	TA1CTL = MC_2 + TASSEL_2 + ID_3 + TAIE;
        __enable_interrupt();
    	for(;;)
    	{
    	    int lpbPressed = pushButtonPressed;
    	    if(lpbPressed == 1)
    	    {
    	        if (selectingLED == 1)
    	        {
    	            // disable timer interrupt
    	            TA1CTL &= ~TAIE;
    	            selectingLED = 0;
    	        }else{
    	            //enable timer interrupt and back to displaying LEDs
    	            TA1CTL |= TAIE;
    	            selectingLED = 1;
    	        }
    	        pushButtonPressed = 0;
    	    }
    	    if(timerOverflow == 1)
    	    {
    	        timerOverflow = 0;
    	        turnOffLeds();
    	        if (ledPosit < 2) //check for PORT1
    	        {
    	            P1OUT |= myLeds[ledPosit];
    	        }else{ // must be PORT2
    	            P2OUT |= myLeds[ledPosit];
    	        }
    	        if (ledPosit > 2)
    	        {
    	            ledPosit = 0;
    	        }else{
    	            ledPosit = ledPosit + 1;
    	        }
    	    }
    	}
    	return 0;
    }
    void turnOffLeds(void)
    {
        P1OUT &= ~(LED1 + LED2);
        P2OUT &= ~(LED3 + LED4);
    }
    #pragma vector = PORT2_VECTOR
    __interrupt void PORT2_ISR (void)
    {
        P2IFG &= ~PB1; // clear flag
        P2IE &= ~PB1; // disable interrupt
    
        WDTCTL = WDT_MDLY_32; // start and set WD timer for btn debounce
        IFG1 &= ~WDTIFG; // clear flag
        IE1 |= WDTIE; // enable WDT interrupt
    
        pushButtonPressed = 1;
    }
    #pragma vector = WDT_VECTOR
    __interrupt void WDT_ISR(void)
    {
        IE1 &= ~WDTIE; // disable WDT
        IFG1 &= ~WDTIFG; // clear flag
        WDTCTL =  WDTPW | WDTHOLD; // stop WD
        //enable push button interrupt
        P2IE |= PB1;
    }
    #pragma vector = TIMER1_A1_VECTOR
    __interrupt void Timer1_A1_ISR (void)
    {
        static int timerCounter = 0;
        switch(TA1IV){
        case 10://overflow TA1IFG
        {
            if (timerCounter == 3)
            {
                timerOverflow = 1;
                timerCounter = 0;
            }else{
                timerCounter = timerCounter + 1;
            }
            break;
        }
        default:
            //oops
            break;
        }
    }

  • I should add that originally, as in the most recent code example, I am clearing the P2IFG in the PORT2_ISR (not the WDT_ISR as I indicated in the text of my reply). Even when I clear the P2IFG in both the PORT2_ISR and the WDT_ISR, the behavior is still indicative of the pushButtoPressed variable either not being assigned 0 at the end of the if(lpbPressed == 0) conditional or that the P2IFG is somehow reset without a second button press/release. Thus, with a single button press/release, the pushButtonPressed variable is set in the PORT2_ISR, cleared in the if(lpbPressed == 0) conditional and set again somewhere so that the state returns to that prior to the push button being released.
  • Okay, I fixed this by disabling the timerA1 in the PORT2_ISR and not in the if (selectingLED == 1) conditional. I'll leave this thread open for a while to see if someone can point out an error in design. Thank you.
  • it appears that the P2IFG has been set again

    Bouncing results in lots of edges at random times. A correct debouncing algorithm must be able to handle P2IFG changing at any time.

  • The WDT is nominally producing a 32 ms wait before re-enabling the PB1 interrupt in P2IE and as I indicated, the oscilloscope trace only shows bouncing for < 200 us. Is it possible that I set up my WDT incorrectly. Maybe I should validate the time delay before re-enabling the PB1 interrupt by toggling a pin I can monitor?

**Attention** This is a public forum