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.

freezing

Other Parts Discussed in Thread: MSP430F2013

Hello

I developed the code for turning on/off the LED and i used the interrupt but after clicking 5 times to go to ISR, it freezes up.I attaceh my and as you see i don't need to return from interrup becaus after finishing the ISR , I put my cpu in Low Power mode. Is it for over flowing stack? if yes how can i clear that ?

Thanks

Behnam



#include  <msp430f2013.h>
 volatile unsigned int ic;
 volatile unsigned int mode;
void led(void)
{
    volatile unsigned int i,j;           

  P1DIR |= 0xC1;                            // P1.0, P1.6 and P1.7 output
  P1SEL |= 0xC0;                            // P1.6 and P1.7 TA1/2 options
 CCR0 = 512-1;                             // PWM Period
  CCTL1 = OUTMOD_7;                         // CCR1 reset/set
  ic=0;
  mode=0;
   while(1)
  {
   
    if (mode==0)
    {
    ic=ic+5;
  CCR1 = ic;                               // CCR1 PWM duty cycle
  TACTL = TASSEL_2 + MC_1;                  // SMCLK, up mode

    i = 8000;                          // SW Delay
    do i--;
    while (i != 0);
    if (ic>500)
    { mode=1;
      ic=512;
      P1SEL &= 0x3F;
      P1OUT |= 0xc0;
      j=26;
      do { j--;
       i = 50000;                          // SW Delay
    do i--;
    while (i != 0);}
    while (j != 0);
     P1DIR |= 0xC1;                            // P1.0, P1.6 and P1.7 output
  P1SEL |= 0xC0;                            // P1.6 and P1.7 TA1/2 options
  CCR0 = 512-1;                             // PWM Period
  CCTL1 = OUTMOD_7;                         // CCR1 reset/set 
    }
    }
    if (mode==1)
    {
        ic=ic-10;
  CCR1 = ic;                               // CCR1 PWM duty cycle
  TACTL = TASSEL_2 + MC_1;                  // SMCLK, up mode

     i = 5000;                          // SW Delay
    do i--;
    while (i != 0);
    if (ic<10)
    {
      mode=2;
      P1SEL &= 0x3F;
      P1OUT &= 0x3f;
      _BIS_SR(LPM4_bits + GIE);                 // Enter LPM4 w/interrupt
    }
    }
  }
}
int main(void)
{
 volatile unsigned int i,j;         
   _EINT();
  ic=0;
  mode=0;
 // WDTCTL = WDT_MRST_32;
  WDTCTL = WDTPW + WDTHOLD;                 // Stop WDT
  P1DIR |= 0xC1;                            // P1.0, P1.6 and P1.7 output
  P1SEL |= 0xC0;                            // P1.6 and P1.7 TA1/2 options
  P1IE |= 0x20;                             // P1.5 interrupt enabled
  P1IES |= 0x20;                            // P1.5 Hi/lo edge
  P1IFG &= ~0x20;                           // P1.5 IFG cleared
  CCR0 = 512-1;                             // PWM Period
  CCTL1 = OUTMOD_7;                         // CCR1 reset/set
  while(1)
  {
   
    if (mode==0)
    {
    ic=ic+5;
  CCR1 = ic;                               // CCR1 PWM duty cycle
  TACTL = TASSEL_2 + MC_1;                  // SMCLK, up mode

    i = 8000;                          // SW Delay
    do i--;
    while (i != 0);
    if (ic>500)
    { mode=1;
      ic=512;
      P1SEL &= 0x3F;
      P1OUT |= 0xc0;
      j=26;
      do { j--;
       i = 50000;                          // SW Delay
    do i--;
    while (i != 0);}
    while (j != 0);
     P1DIR |= 0xC1;                            // P1.0, P1.6 and P1.7 output
  P1SEL |= 0xC0;                            // P1.6 and P1.7 TA1/2 options
  CCR0 = 512-1;                             // PWM Period
  CCTL1 = OUTMOD_7;                         // CCR1 reset/set 
    }
    }
    if (mode==1)
    {
        ic=ic-10;
  CCR1 = ic;                               // CCR1 PWM duty cycle
  TACTL = TASSEL_2 + MC_1;                  // SMCLK, up mode

    i = 5000;                          // SW Delay
    do i--;
    while (i != 0);
    if (ic<10)
    {
      mode=2;
        P1SEL &= 0x3F;
      P1OUT &= 0x3f;
      _BIS_SR(LPM4_bits + GIE);                 // Enter LPM4 w/interrupt
     
    }
    }
  }
 
}
#pragma vector=PORT1_VECTOR
__interrupt void Port_1(void)
{
 volatile unsigned int q;          

 P1IFG &= ~0x20;                           // P1.5 IFG cleared
 P1OUT ^= 0x01;
 
    q = P1IN & 0x20;                         
     while (q == 0)
     {
        q = P1IN & 0x20;
     }
   ic=0;
 if (mode<2)
 {mode=2;
 P1SEL &= 0x3F;
      P1OUT &= 0x3f;
      _BIS_SR(LPM4_bits + GIE);                 // Enter LPM4 w/interrupt
 }
 else
 {
  mode=0;
  _EINT();
  led();
 }
}

  • First some general remarks:

    behnam azimi said:
        i = 8000;                          // SW Delay
        do i--;
        while (i != 0);
     


    This is no delay. It is an empty loop with no other effect than setting i=0, so the compiler can detect it as dead code and eliminate it. Also, the execution time depends on teh current MCLK and is therefore subject to undetected change. The best thing for delays is setting up a timer (this is what timers are made for). If you insist on using a non-deterministic time-wasting loop, then use the __delay_cycles() intrinsic. It will not be subject to optimization, yet it is still subject to MCLK speed changes. Also, the number of cycles is defined, while your implementation may greatly vary depending on the way the compiler translates it (if it is not optimized away totally).

    Also, doing a busy, wait inside a function is a bad thing on MCUs. Here again the use of timers has to be preferred. You can enter LPM0 until the timer expires, saving a great deal of power in the meantime. It's not a critical thing in your current code, but you better start to get used to this as soon as possible :)

    Now to the program itself.
    What you do is from main() you go into LPM And tehre is no code anywhere that will wake your main() up again. Well, that's not a necessity if there's nothign to be done in main() ever after.
    THen your Port1 ISR is entered. Fine. This wakes the MCU up for LPM, but only temporary. Since the LPM bits reside in the status register and the status register is saved on stack before entering the ISR, the LPM will be immediately in effect again once the ISR exits (unless oyu use the special ..._on_exit() intrinsic to alter the saved status register on the stack).

    But now you do some bad  things:
    1) you do some busy-waiting loops inside the ISR. Which is one of the least things you should ever do. An ISR should be in and out as fast as possible, doing only the time-critical things that must be done immediately. Everything time-consuming shouldn't be inside an ISR. Eitehr set a global flag and return to main, so main will handle the lenghty job, or split the ISR into parts with a state-machine, or let a timer do the wait.
    2) you use idealized components. But you sa 'clicking' which seems to indicate a manually pressed button. This kind of data source is usually subject to bouncing. That means that every press triggers a whole bunch of interrupts. Yet in you code you only check for the line coming high again and do not check whether it is bouncing back to low immediately. Here again, a timer for a timeout is the tool of choice (there is another thread only a few days old in this forum about debouncing. With code)
    3) you set GIE inside an ISR. While this is some (very few) cases might be necessary, it definitely isn't in your case. It allows other ISRs (including itself) interrupting. This can (and in your case will) lead to a plethora of ISR calls stacked into each other, each requiring much space on the stack. So yes, I definitely think you're sufferign a stack overflow.
    4) you call this sloooow led() function inside your ISR, which promotes the long busy-waiting loops inside this function from bad to 666. And since you enabled interrupts before doing so, chances are that your ISR is called again and again while still in this function, piling more and more data on your stack. And to make things worse, inside this funciton you enter LPM again (still inside your ISR). And since you never wakeup from LPM (except when anothe rinstance of the ISR is called), the code ends here once more, never returning. Yet the next call of the ISR will reset your status variables and begin the cycle again, leaving all previous efforts buried on the stack.

    I'm sure this code will bring down even BlueGene with its 74Gb of ram  as soon as someone dares to touch the button.

    And on the bottom line, for what this code does (or rather is intended to do), it is really a looong piece of code. There's much space for optimization and increased readability. For example:

    behnam azimi said:
        q = P1IN & 0x20;                         
         while (q == 0)
         {
            q = P1IN & 0x20;
         }

    q is only used here. You don't use it anywhere else. So why do you use it at all?
    while(!(p1IN&0x20));
    does exactly the same. It is likely that the compiler will optimize it to the same result, but my line is way more understandable, as it exactly expresses what is being done: it waits until bit 5 is set. Nothing else, no hidden side-effects or further use. Same for the while waiting loops. While being dead code anyway, the use of a "for" construct would take only one line of code and express exactly the same.

  • Hello Jens-Michael

                    Thank you for your answer and useful details. I have to use MSP430F2013 and it is my first experience to work with Ti Microcontrollers . As you said my main function don't do any things just initialize some parameters and go to LPM , and it wakes up by pushing button connected to P1.5  . It will be used 4 or 5 times in a day ,So I put it in LPM . The LED function turn ON my LEDs for 17 sec and I use PWM for smooth blow up in turning ON/OFF moments .When user pushes the button it goes to LED function and turn LEDs ON but if they push it again before finishing the LED routine, it turns OFF the LEDs . I need to enable interrupt in my ISR again. As you see I don't need to return to the previous procedure after ISR, So is there any way to ask the compiler to NOT save anything on the stack?

  • behnam azimi said:
    and it wakes up by pushing button connected to P1.5

    No. Your main() never wakes up from LPM, only the ISRs do (by hardware). Your ISRs are never returning to main (they don't ever return to anyone) and never finally exit LPM at all.

     

    behnam azimi said:
    I need to enable interrupt in my ISR again.
    No, you need to restructure your program. You try to do things in a plain way, entering an endless recursion.
    Basically, you abuse the program counter as state machine, meaning that the position where you are in the program determines what to do. The 'normal' way is to use state variables (flags) which hold the information about what happened and what has to be done next.

    Flow example:

    Main: init everything and go into LPM.
    port interrupt: if 'led on' flag is clear, set LED on, set 'led on' flag and start timer. If 'led on' flag is set, set LED off and stop timer. IN BOTH CASES RETURN!
    timer interrupt: clear 'led on' flag and set LED off AND RETURN.

    Only two or three lines each.
    well, the smooth turn-on is a bit more complex but can be done with the same timer function. You only need to increment a global counter variable let the timer run several times (several interrupts) and depending on the counter turn on or off the LED for the PWM effect. It is also possible to use a hardware PWM and just increment/decrement the duty-cycle on every timer interrupt.

    After the ISR returns, it will restore teh pprevious LPM state, so main remains stopped and the processor will be in LPM if not currently executing an ISR.

    behnam azimi said:
    So is there any way to ask the compiler to NOT save anything on the stack?

    This is a hardware function. If an interrupt comes, the processor will (without any code involved) put the current program counter and status register on the stack.

    You can write special code that removes these bytes from the stack again (in your program structure, you don't need a stack at all), bu tthis requires usage of the assembly language. It cannot be done in C as C is a procedural language and your program structure is not.
    C is NOT suited to do it the way you want to do it.

    So either change your program structure or change the programming language.

    Since many people have already done similar things in C, I'd suggest the first of the two options.

**Attention** This is a public forum