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 - Timer Compare ISR acting running multiple times, can't stop counter or write to registers in ISR

Other Parts Discussed in Thread: MSP430G2553

Hello, I'll try to go in steps - what I want to do, what is happening, and what I know and have tried.

I've built a state machine using function pointers, and I want one of the state transitions to be when a timer counts up to a value.  Sounds simple enough right?  I then want to reset the timer, wait a little bit, then turn it on again and use it for another state transition.

I've verified that I am correctly setting up Timer_A, turning it on, and entering my interrupt when the Timer_A register counts to the Timer_A compare register.  I've meticulously stepped through the code in DEBUG mode of CCS and watched the registers change, so I know it's at least partially working.  I've also o-scoped much of the code to get more information. However, when I try to turn off the counter in my ISR or disable the interrupt capability so I don't generate a Timer_A interrupt while still in my ISR, the register I write to either doesn't change, or i get strange errors.  I'll post the (relevant) code below with some comments and hopefully someone can help me solve this.  I can post full source code if someone wants to see everything in detail.  Hopefully I've described the operation well enough.

//---- Timer A Setup (16 Bit Timer) ----//
void set_TimerA(void){
    TACCR0 = 20;                 //num of cycles to count to
    TACTL |= TASSEL_2;    //set Timer_A clock source to SMCLK
}

// A button is pressed, and then goes to this state
void goto_PULSE1(void){
    state = PULSE1;                                     //Not important - set the state
    P1IE &= ~BIT3;                                       //Not important - disable button interrupt in case it gets pressed again
    TACCTL0 |= CCIE;                                //Enabled the compare interrupt
    P2OUT = PULSE1_OUTPUT_BITS;  //Not important - set Port2 outputs
    TACTL |= MC_1;                                   //Set counter to up mode and start counting
}

//Timer A interrupt, gets to here correctly
#pragma vector=TIMER0_A0_VECTOR
__interrupt void Timer_A_ISR(void){

     // TACTL &= ~MC_0;
    // TACCR0 = 0x00;
    // TACTL |= TACLR;
    TACCTL0 &= ~CCIE;
    TACTL |= TACLR;
    // TACTL &= ~TAIFG;
   ^^^^^^^^^^^^^^^^^^^^^^
   I've tried different combinations of the above, but it doesn't do what I want it to.  All i want to do is stop and clear the timer so that I can enable it again in another state.  The above "works" in that it runs, but it runs the ISR multiple times because the timer isn't stopping and causes erratic behavior.

    if (state == PULSE1){
      event = TIME1;
    }
    else if (state == PULSE2_POSV){
      event = TIME2;
     }
    else if (state == PULSE2_NEGV){
       CACTL1 &= ~CAIE;
       state = PULSE2_POSV;
       event = TIME2;
    }
}

I've meticulously combed through the User Guide and can't seem to get a solution, no matter how hard or how many combinations I try.  Any suggestions are welcome, thanks for your time.

  • (1) Be aware that the compiler might ignore parts of your source code without warning.

    For example, you declared a global variable state. Your main() or some of its subordinates only writes state and never reads state. Your ISR, on the other hand, only reads state and never writes state. Your compiler might ignore everything about state. It is not assigned a place in RAM, it is never read, it is never written.

    (2) You need to be careful in using symbols that involves multiple bits in peripheral registers.

    For example, in the TACTL register, TASSEL0 is a symbol for a single bit and is the same as BIT8, or 0x0100. TASSEL1 is a symbol for a single bit and is the same as BIT9, or 0x0200. However, TASSEL_0, TASSEL_1, TASSEL_2, and TASSEL_3 all involve both bits in a different way. When you write:

    TACTL |= TASSEL_2;    //set Timer_A clock source to SMCLK

    It might work the way your comment says. But it might not always work that way.

    Similarly,

    TACTL &= ~MC_0;

    will read TACLT and write the same thing back to TACLT. (Is that what you want?)

  • I suppose I should just post the entire source code to clarify and address some of the issues you've brought up.  It's 186 lines though and I thought that may be a bit lengthy.

    (1)

    My main() loop does read the global variable state and some of my functions also write to stateSo I believe this is a non-issue. Again, I suppose a source code post would have worked around this.

    (2)

    I want to set Timer_A to count using SMCLK, and in the header file <msp430g2553.h> it states

    #define TASSEL_2 (2*0x100u)

    Which should give me the bits 0000 0010 0000 0000, comparing to the TACTLx register in the datasheet, looks like it matches up with setting the clock source to SMCLK.  Also, looking at the TA0CTL register in DEBUG mode shows that it is written correctly.

    (3)

    You're correct, that was an unfortunate error.  I've since tried:

    TACTL &= ~MC_3

    to halt the timer, and it appears to actually stop the clock now, which is an improvement.

    You brought up some good points, but I think I got it to work for the time being actually.  Or I hacked it together enough to do so.

  • While OCY is right about global variables, his example was a bit misleading. The problem is not the missing read. The problem is that an ISR accesses global vars outside the normal (predictable) program flow. The compiler doesn’t know this. So the compiler assumes that multiple writes to a variable in main are redundant and will only do the last one, and only if there is a chance that anyone would be able to access it (besides ISRs). That means, any write is delayed until the next point where the program flow leaves the current function. Also, the compiler may hold a local copy in a register and use this rather than reading the ram each time, if there is no funciton called that could change the variable. A code like this:

    int wait = 1;

    while(wait);

    will wait forever, even if an ISR changes wait to 0. Because the compiler will put wait into a register and during the while will compare the register, not the memory location. And the register copy isn’t changed by the ISR.

    To tell the compiler that a variable doesn’t act as expected and reading and writing has to take place immediately and exactly as often as the code says, it needs the volatile keyword. However, this negatively affects code optimization and should be used only if necessary. Also, the compiler may ignore the volatile modifier if the variable is local (and therefore auto-storage) and the code doesn’t pass its reference to anyone.
    For this reason, delay loops with “for” and a local variable may fail.

**Attention** This is a public forum