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.

MSP430F5219 - Problem with the ISR code?

Expert 1570 points

I'm trying to set up two pin interrupts on the same port, but when I enable the interrupts my other code doesn't run properly.  I think it's possibly getting stuck in the ISR.  Am I not clearing the flags properly? 

#define TRIGGER_LEFT                (BIT3)  //of port 2
#define TRIGGER_RIGHT               (BIT6)	//of port 2

void main ()
{	
	//enable interrupt
	P2IE |= TRIGGER_LEFT | TRIGGER_RIGHT;
	//interrupt on falling edge
	P2IES |= TRIGGER_LEFT | TRIGGER_RIGHT;
	//clear interrupt flag
	P2IFG &= ~(TRIGGER_LEFT | TRIGGER_RIGHT);

	//other code
}

#pragma vector=PORT2_VECTOR
__interrupt void Port2_ISR(void) 
{ 
	switch (P2IFG & (TRIGGER_LEFT | TRIGGER_RIGHT)) {
	case TRIGGER_LEFT:
		triggered = TRUE;
		//clear flag
		P2IFG &= ~TRIGGER_LEFT;
		break;
	case TRIGGER_RIGHT:
		triggered = TRUE;
		//clear flag
		P2IFG &= ~TRIGGER_RIGHT;
		break;
	}
}

  • Wild guess: what if both pin IF's are set? In addition to your existing case statements I would add following code and check if it changes anything:

    case (TRIGGER_RIGHT | TRIGGER_LEFT):
    triggered = TRUE;
    default:
    P2IFG = 0;
    break;

  • Thank you for your reply.

    Yes I had considered that also, but I assumed that if they were both set it would just come back in a second time and clear it then.  I'll try the third case like you suggested.  Or I was also thinking to change the case statements to if statements, handling each bit separately.  Or maybe I should just set the IFG to zero after the switch (for both cases?.

  • AQ said:
    Or maybe I should just set the IFG to zero after the switch (for both cases?.

    No. Then you shall do it after while() loop which process all pending IFG's. Otherwise you can miss some.

  • In this particular case I only need to know if one or the other occurs, and don't really care which one, so it may be OK.

  • AQ said:
    Yes I had considered that also, but I assumed that if they were both set it would just come back in a second time and clear it then.

    If both are set, none of the two cases will be executed, no IFG bit will be cleared and the ISR will be called infinitely without ever doing something.

    However, on 5x family, there is the P2IV register which reports the highest pending interrupt and at the same time clears its IFG bit. 

    while(1)
      switch (__even_in_range(P2IV, 16)){
        case 0: return;  // no more interrupts pending
        case (TRIGGER_LEFT<<1):
          break;
        case (TRIGGER_RIGHT<<1):
          break;
      }
    }

    The while(1) ensures that the switch will be looped until no more interrupts are pending. Then the ISR will exit (case 0)

  • Wow, you're right, thanks!  Not sure how I missed that. Thanks also for that tip.

  • Jens-Micheal, can you confirm that I would want to be using 64 as the upper limit for the __even_in_range function, since the two bits are 0x08 and 0x40?  Thanks

  • Actually, I don't think that's right either.  I noticed the usage with P2IV (not P2IFG), but my constants are defined as bitmasks, not bit number, e.g. BIT3 = 0x08 and BIT6 = 0x40.  So I guess I want to change those to just 3 and 6, but then wouldn't I want +2 instead of *2 (or <<1) in the case conditions?  Also in this case the upper limit should be 0x0E I believe which corresponds to the P2IFG.6 bit.  Maybe I'll just set TRIGGER_LEFT and TRIGGER_RIGHT to 0x08 and 0x0E, respectively, and use them directly?

  • Could it be that you need pull-ups on your inputs? Or need to set the direction in?

    P2OUT |= (TRIGGER_LEFT | TRIGGER_RIGHT);
        P2DIR &= ~(TRIGGER_LEFT | TRIGGER_RIGHT);
        P2REN |= (TRIGGER_LEFT | TRIGGER_RIGHT);

  • Hi Joseph, the direction bits are set OK, and I get the interrupts OK now (currently I'm using the code shown below).  I was just a little unclear with the __even_in_range usage with P2IV.

    	if (P2IFG & (TRIGGER_LEFT | TRIGGER_RIGHT)) {
    		triggered = TRUE;
    		//clear both flags if either flag is set (for now we don't care which side triggered)
    		P2IFG &= ~(TRIGGER_LEFT | TRIGGER_RIGHT);
    	}
    

  • AQ said:
    I noticed the usage with P2IV (not P2IFG), but my constants are defined as bitmasks, not bit number, e.g. BIT3 = 0x08 and BIT6 = 0x40.

    The return value of P2IV (as with all IV registers) is an enumeration. Where '0' always means 'no interrupt pending' and all other interrupt reasons are enumerated with even(!) values.

    The reason for this is that you can easily use this with the __even_in_range intrinsic in switch statements.  Instead a lengthy list of if - else if - else if instructions (the normal switch implementation) the compiler will generate a jumptable (each entry 2 bytes long) and simply add the content of the IV register to the current program counter. No comparisons, no conditional jumps. You can't decide faster which part of your code shall be executed. And from three cases on, it is smaller too.

    switch (__even_in_range(P2IV, 16))
    {
      case 0: return; // nothing to do
      case 8: // code for BIT3
        break;
      case 14: // code for BIT6
        break;
      default:
        // just for completeness, so 2,4,6,10,12,16 have a place to go.
    }

    Case 0 is handled with a return, so you can wrap the whole switch in a while(1) loop. This way, multiple interrupts will be handled without leaving and re-entering the ISR (faster). As soon as no more are pending the case 0 will exit the ISR.

**Attention** This is a public forum