reading two push buttons simultaneously with interrupts

You are not going to enter the ISR where both buttons are triggering an interrupt at the same time so your branch condition will never happen during the ISR. This means that even when you press the buttons "at the same time" one always occurs before the other.

Basically you will enter the interrupt handler for one button and clear the P1IFG for that button and then reenter the interrupt handler for the second button but at this time P1IFG has been cleared for the previous button push. This means that (P1IFG & BUTTON1) is mutually exclusive with (P1IFG & BUTTON) and the branch is never taken.

P1IFG & BUTTON1, or the other condition, tells you when you have entered the ISR for a BUTTON1, or BUTTON, change but for your purposes you only want to tell the state of both buttons in order to update your truth values and LEDs.

Thank you for your response. 

So it means that I can't read the status of 2 buttons using interrupts?

Is there a way to do it or I just should stick with the polling solution?

Shahar Levi said:

... So it means that I can't read the status of 2 buttons using interrupts? ...

Yes, you can read the status of 2 (or more) buttons using interrupts. Your problem is your fingers cannot press 2 (or more) buttons at the same instance in time. The MSP430 is much faster than your fingers. Within micro seconds of the instance one of the two buttons is pressed, your ISR code will read the status. At that time, the other button is not pressed yet despite of the fact that you think you are pressing both of them "at the same time". In reality, one can be milliseconds ahead of the other.

You may inset a delay of a few milliseconds in your ISR before attempt to read the status. This way, you have a change to fine that they are both pressed. But this is a silly idea.

Thanks for your comment Matthew ! 

I got it working with reading the pins state (PxIN) withing the ISR instead of reading the interrupt flags (PxIFG), as you suggested. 

in this manner: 

#pragma vector=PORT1_VECTOR
__interrupt void Port_1(void)
{

switch (random){

case 1:   // logic gate "AND" 
if((P1IN & 0x88) == 0x88){ 
      P1OUT |= 0x10; // set "TRUE" LED at P1.4 ON
      P1OUT &= ~0x20; // set "FALSE" LED at P1.5 OFF
            }

else{

      P1OUT |= 0x20; // set "FALSE" LED at P1.5 ON

      P1OUT &= ~0x10; // set "TRUE" LED at P1.4 OFF

             }

break;

case 2:  //logic gate "OR"

(...)

next stage will be to Implement it with timer interrupts!

Just to make sure you are still clearing the interrupt flag and toggling the edge interrupt condition within the handler, correct? If you don't clear the flag you will constantly enter the ISR which will give you the same performance as polling. Just wanted to make sure as I didn't see it in your code above.

I was actually wondering if I'm doing it right. I am clearing the interrupt flag and toggling the edge interrupt in the if statements that check the P1IFG, but not in the if statements which read the P1IN.

Please have a look in the full code. the first 2 if branches in the ISR are handling the LEDs which indicate the each of the buttons is pressed. there I used interrupt flag clearing and edge toggling. The other branches are in switch cases and each represents different gate logic. There I am reading the P1IN. Is it correct interrupt handling? if not, where should I add the flag clearing? thanks!

#include <msp430x20x2.h>
#include <stdlib.h>
#include <time.h>

#define LED0 BIT0
#define LED1 BIT6
#define LED2 BIT4
#define LED3 BIT5
#define BUTTON BIT3
#define BUTTON1 BIT7

long random;

int main(void)
{
WDTCTL = WDTPW + WDTHOLD; // Stop watchdog timer
P1DIR |= (LED0 + LED1 + LED2 + LED3);
P1OUT &= ~(LED0 + LED1 + LED2 + LED3);
P1IE |= (BUTTON + BUTTON1); // P1.3 and P1.7 interrupt enabled
P1IFG &= ~(BUTTON + BUTTON1); // P1.3 and P1.7 IFG cleared
__enable_interrupt(); // enable all interrupts

srand (time(NULL));				// Initialise random seed

 random= rand() % 6 + 1;	// generate random number from 1 to 6

for(;;)
{}
}

// Port 1 interrupt service routine
#pragma vector=PORT1_VECTOR
__interrupt void Port_1(void)
{

	if(P1IFG & BUTTON){  //handle the LED which indicates the right button
	P1OUT ^= LED1; // P1.6 = toggle
	P1IFG &= ~BUTTON; // P1.3 IFG cleared
	P1IES ^= BUTTON; // toggle the interrupt edge,
	// the interrupt vector will be called
	// when P1.3 goes from HitoLow as well as
	// LowtoHigh
		}
	if (P1IFG & BUTTON1){//handle the LED which indicates the left button
	P1OUT ^= LED0; // P1.0 = toggle
	P1IFG &= ~BUTTON1; // P1.7 IFG cleared
	P1IES ^= BUTTON1; // toggle the interrupt edge,
	// the interrupt vector will be called
	// when P1.7 goes from HitoLow as well as
	// LowtoHigh
		}

    switch(random){

case 1: 							//logic "AND"
     if ((P1IN & 0x88) == 0x88)   //if both push buttons are pressed
              {
              P1OUT |= 0x10;		// Set "TRUE" LED at 1.4 ON
              P1OUT &= ~0x20;		// Set "FALSE" LED at 1.5 OFF
                  }
             else{
              P1OUT |= 0x20;      // Set "FALSE" LED at 1.5 ON
              P1OUT &= ~0x10;		// Set "TRUE" LED at 1.4 OFF
                 }
              	   break;

case 2:							 //logic "OR"
	  if (((P1IN & 0x88) == 0x88) || ((P1IN & 0x80) == 0x80) || ((P1IN & 0x08) == 0x08) )
	  {
		  	  	 P1OUT |= 0x10;
		  	  	 P1OUT &= ~0x20;

              }
             else{
          	 P1OUT |= 0x20;
          	 P1OUT &= ~0x10;
                 }
	  	  	  	  	  	break;
case 3:							 //logic "XNOR"
	  if (((P1IN & 0x88) == 0x88) || ((P1IN & 0x80) == 0x80) || ((P1IN & 0x08) == 0x08) )                {
		  	  	 P1OUT |= 0x20;
		         P1OUT &= ~0x10;
               }
              else{
	             P1OUT |= 0x10;
	             P1OUT &= ~0x20;
              	}
	  	  	  	  	  	break;
case 4:						     //logic "XOR"
	  if  ((P1IN & 0x88) == 0x88)
	  {
		  	     P1OUT |= 0x20;
		  	     P1OUT &= ~0x10;
	  }
	  else  if  ((P1IN & 0x80) == 0x80){

		  	  	 P1OUT |= 0x10;
		         P1OUT &= ~0x20;
               }
	  else if  ((P1IN & 0x08) == 0x08){

		 	 	 P1OUT |= 0x10;
		 		 P1OUT &= ~0x20;

	   }
              else{
	             P1OUT |= 0x20;
	             P1OUT &= ~0x10;
              }
           	         break;
case 5: 							   //logic "NAND" {
                   if ((P1IN & 0x88) == 0x88)
                   {
               P1OUT |= 0x20;
               P1OUT &= ~0x10;
                   }
                  else{
               P1OUT |= 0x10;        // Set green LED off when button up
               P1OUT &= ~0x20;
                  }
               	      break;
case 6: 							   //logic "XNOR"
	  if  ((P1IN & 0x88) == 0x88)
	  {
		  	  	 P1OUT |= 0x10;
		  	     P1OUT &= ~0x20;
	  }
	  else  if  ((P1IN & 0x80) == 0x80){

		  	  	 P1OUT |= 0x20;
		         P1OUT &= ~0x10;
               }
	  else if  ((P1IN & 0x08) == 0x08)
	  {
		  	  	 P1OUT |= 0x20;
		 	     P1OUT &= ~0x10;
	  }
              else{
	             P1OUT |= 0x10;
	             P1OUT &= ~0x20;
              }
           	   	      break;

default:
	  P1OUT &= ~0x71;
  }

}

#include <msp430x20x2.h>
#include <stdlib.h>
#include <time.h>

#define LED0 BIT0
#define LED1 BIT6
#define LED2 BIT4
#define LED3 BIT5
#define BUTTON BIT3
#define BUTTON1 BIT7

long random;

int main(void)
{
WDTCTL = WDTPW + WDTHOLD; // Stop watchdog timer
P1DIR |= (LED0 + LED1 + LED2 + LED3);
P1OUT &= ~(LED0 + LED1 + LED2 + LED3);
P1IE |= (BUTTON + BUTTON1); // P1.3 and P1.7 interrupt enabled
P1IFG &= ~(BUTTON + BUTTON1); // P1.3 and P1.7 IFG cleared
__enable_interrupt(); // enable all interrupts

srand (time(NULL));				// Initialise random seed

 random= rand() % 6 + 1;	// generate random number from 1 to 6

for(;;)
{}
}

// Port 1 interrupt service routine
#pragma vector=PORT1_VECTOR
__interrupt void Port_1(void)
{

	if(P1IFG & BUTTON){  //handle the LED which indicates the right button
	P1OUT ^= LED1; // P1.6 = toggle
	P1IFG &= ~BUTTON; // P1.3 IFG cleared
	P1IES ^= BUTTON; // toggle the interrupt edge,
	// the interrupt vector will be called
	// when P1.3 goes from HitoLow as well as
	// LowtoHigh
		}
	if (P1IFG & BUTTON1){//handle the LED which indicates the left button
	P1OUT ^= LED0; // P1.0 = toggle
	P1IFG &= ~BUTTON1; // P1.7 IFG cleared
	P1IES ^= BUTTON1; // toggle the interrupt edge,
	// the interrupt vector will be called
	// when P1.7 goes from HitoLow as well as
	// LowtoHigh
		}

    switch(random){

case 1: 							//logic "AND"
     if ((P1IN & 0x88) == 0x88)   //if both push buttons are pressed
              {
              P1OUT |= 0x10;		// Set "TRUE" LED at 1.4 ON
              P1OUT &= ~0x20;		// Set "FALSE" LED at 1.5 OFF
                  }
             else{
              P1OUT |= 0x20;      // Set "FALSE" LED at 1.5 ON
              P1OUT &= ~0x10;		// Set "TRUE" LED at 1.4 OFF
                 }
              	   break;

case 2:							 //logic "OR"
	  if (((P1IN & 0x88) == 0x88) || ((P1IN & 0x80) == 0x80) || ((P1IN & 0x08) == 0x08) )
	  {
		  	  	 P1OUT |= 0x10;
		  	  	 P1OUT &= ~0x20;

              }
             else{
          	 P1OUT |= 0x20;
          	 P1OUT &= ~0x10;
                 }
	  	  	  	  	  	break;
case 3:							 //logic "XNOR"
	  if (((P1IN & 0x88) == 0x88) || ((P1IN & 0x80) == 0x80) || ((P1IN & 0x08) == 0x08) )                {
		  	  	 P1OUT |= 0x20;
		         P1OUT &= ~0x10;
               }
              else{
	             P1OUT |= 0x10;
	             P1OUT &= ~0x20;
              	}
	  	  	  	  	  	break;
case 4:						     //logic "XOR"
	  if  ((P1IN & 0x88) == 0x88)
	  {
		  	     P1OUT |= 0x20;
		  	     P1OUT &= ~0x10;
	  }
	  else  if  ((P1IN & 0x80) == 0x80){

		  	  	 P1OUT |= 0x10;
		         P1OUT &= ~0x20;
               }
	  else if  ((P1IN & 0x08) == 0x08){

		 	 	 P1OUT |= 0x10;
		 		 P1OUT &= ~0x20;

	   }
              else{
	             P1OUT |= 0x20;
	             P1OUT &= ~0x10;
              }
           	         break;
case 5: 							   //logic "NAND" {
                   if ((P1IN & 0x88) == 0x88)
                   {
               P1OUT |= 0x20;
               P1OUT &= ~0x10;
                   }
                  else{
               P1OUT |= 0x10;        // Set green LED off when button up
               P1OUT &= ~0x20;
                  }
               	      break;
case 6: 							   //logic "XNOR"
	  if  ((P1IN & 0x88) == 0x88)
	  {
		  	  	 P1OUT |= 0x10;
		  	     P1OUT &= ~0x20;
	  }
	  else  if  ((P1IN & 0x80) == 0x80){

		  	  	 P1OUT |= 0x20;
		         P1OUT &= ~0x10;
               }
	  else if  ((P1IN & 0x08) == 0x08)
	  {
		  	  	 P1OUT |= 0x20;
		 	     P1OUT &= ~0x10;
	  }
              else{
	             P1OUT |= 0x10;
	             P1OUT &= ~0x20;
              }
           	   	      break;

default:
	  P1OUT &= ~0x71;
  }

}

This looks fine as far as clearing the flags. You should only be generating an interrupt per each button press or release with this code.

You only need to clear the flags and switch triggering condition once within the ISR so you don't need to worry about it within your switch statement.

Thanks. I appreciate it

Matthew Kracht said:

This looks fine as far as clearing the flags. You should only be generating an interrupt per each button press or release with this code.

A good expansion to the posted code is to move it into a timer interrupt:

On a port pin edge interrupt, port pin interrupts are blocked and a timer is started. When the timer interrupt comes, after any bouncing has settled, the port pin status is read and port interrupts are enabled again. Well, for detecting 'simultaneous' pressing of two buttons, things get even more complex, as between the first button's press and the second button's press, many to several hundreds of milliseconds may pass. Yet a single button press shall be detected quickly.
UI systems usually have a long delay to detect a button press (or double-button press) but react immediately when the button is released.
So B1low, B1 high is detected as a single-button press, while B1 low, B2 low, B1 (or B2) high is detected as a double-press. Also, B1 low and hold is detected as single button press, but only after some time (to ensure there won't be B2 pressed anymore).
The required state machine is rather complex and can't be done with a simple if-chain. But the only way to ensure you're not misinterpreting user input. Except if you want to give instructions like 'press and hold button 1, then press button 2' (which could be handled with a simple if chain)