Timer Incremement Issue

[ed_reardon]

Hi all,


Go easy on me, I'm very fresh to this, it's taken me all day to write this and the damn thing isn't working.

Let's start with the code:

Code: [Select]

#define _XTAL_FREQ 1000000
#define PULSE LATAbits.LATA0  //define pin RA0 as PULSE pin
#define DIR LATAbits.LATA1    //define pin RA1 as DIRECTION pin
#define SPEED PORTBbits.RB0 //Define SPEED H/L Pin
#define PROGDIR PORTBbits.RB2 //Define Direction Programming Pin





//Global Variables
int T0COUNT = 0;  //Variable for Timer0 Counter
int LPFLG = 0;  //Flag for MEDIUM PUMP speed timer
int MEDTIME = 5; //Loops for TIMER0 to run


//Set Timer Interrupt Procedure
void interrupt high_isr (void){
if(INTCONbits.TMR0IF == 1) {  //Check for status of TMR0 Overflow
    INTCONbits.TMR0IF == 0; //if OVERFLOW occurs reset TMR0 overflow flag.
    TMR0L == 0; //Set TMR0 counter to 0, adjust this for time offsets
    T0COUNT++;  //Increment T0COUNT by 1 for every time TMR0 overflows.
    if(T0COUNT==MEDTIME) {
        LPFLG = 1;  //When counter total matches time required, set flag to 1
    }
   
   
}
if(INTCON3bits.INT3IF == 1){ //if programming pin is HIGH
    INTCON3bits.INT3IF == 0; //Clear interupt flag
    TRISAbits.RA0 == 1; //switch off motor
    TRISBbits.RB2 == 1; //enable RB2 PROGDIR pin
    if(PROGDIR == 1){ //check for HI PROGDIR pin
        __delay_ms (50);  //wait 50ms for bounce
        if(PROGDIR == 1){
            DIR= ~DIR;
        }
                 
       
    }
}
}
int main (){
    OSCCONbits.IRCF2 = 1; //Set clock speed to 1MHz
    OSCCONbits.IRCF1 = 0; //Set clock speed to 1MHz
    OSCCONbits.IRCF0 = 0; //Set clock speed to 1MHz
    RCONbits.IPEN = 1; //enable interrupt priority
    INTCONbits.GIE = 1; //enable all interupts
   
    T0CONbits.TMR0ON = 0; //switch timer OFF in initial state
    T0CONbits.T0PS0 = 1;  //set TIMER0 prescaler to 1:256 on TOP0-2
    T0CONbits.T0PS1 = 1;
    T0CONbits.T0PS2 = 1;
    T0CONbits.PSA = 1; //Enable pre-scaler
    T0CONbits.T0CS = 0; //TIMER0 uses Internal Osccilator
    T0CONbits.T016BIT = 0; //TIMER0 uses 16 bit counter
    ADCON1 = 0xFF; //switch OFF Port-A ADC
    CMCON = 0xFF; //switch OFF comparitors
    PWMEN1 = 0xFF; //switch OFF PWM module
    INTCON3bits.INT3E = 1; //enable INT3 External Interrupt
    INTCON2bits.INTEDG3 = 1; //enable INT3 RIDING EDGE interrupt
   
    TRISAbits.RA0 = 0; //define A0 as OUTPUT
    TRISAbits.RA1 = 0; //define A1 as OUTPUT
    TRISBbits.RB0 = 1; //define B0 as INPUT
     
       DIR = 1; //Change value to '0' for reversed motor
    while (1)  //Do this forever
       
           if(SPEED == 1 ) { //if SPPED pin HI
               T0CONbits.TMR0ON = 1; //switch timer ON
               if(LPFLG == 0){ //If Loop Counter FLAG LO
        PULSE= ~PULSE; //Set PULSE toggle
        __delay_us(25); //Toggle pulse MEDIUM
       }
               
    if(LPFLG == 1 ) {  //if Loop Counter FLAG HI
        T0CONbits.TMR0ON = 0;  //Switch TIMER0 OFF!
       PULSE= ~PULSE; //Set PULSE toggle
        __delay_us(50); //Toggle pulse SLOW
    }
                                 }
   
   else {
        T0CONbits.TMR0ON =  0;  //Switch TIMER0 OFF!     
        LPFLG == 0;       //Reset loop counter
        PULSE= ~PULSE;
        __delay_us(5); //Toggle pulse FAST
                           
                       
           
   }
         
    return 0;
}

         
Now,  this is as best I can explain what I'd hope it would have done!

• Initially the output on RB0 should be 'medium' for five minutes (roughly)
• Timer0 (16bit) should overflow  5 times,  I want this to increment T0COUNT by 1 every time it goes around!
• When the number of times around the loop matches the MEDTIME it should set LPFLG to 1
• The output should then sit at 'SLOW' where it could theoretically sit for weeks in the application!

[li]If RB0 goes LOW the FAST cycle should be started, then the whole damn she-bang should reset and do the whole 5-minute nonsense again.
[/li][/list]

All other functions work fine, but in my emulator I'm not getting a loop increment,  the thing just sits in 'MEDIUM' with RB0-HIGH and runs as expected with RB0 is low.

I only learned about interupts at 10AM this morning, so please go easy on me,  I'm very new to all of this!


Any input most welcome!

Ed

[ed_reardon]

If  LPFLG flag is high you do not reset it to zero.  If it is already zero then you set it to zero.


   if(LPFLG == 1 ) {  //if Loop Counter FLAG HI
        T0CONbits.TMR0ON = 0;  //Switch TIMER0 OFF!
       PULSE= ~PULSE; //Set PULSE toggle
        __delay_us(50); //Toggle pulse SLOW
    }
                                 }
   
   else {
        T0CONbits.TMR0ON =  0;  //Switch TIMER0 OFF!     
        LPFLG == 0;       //Reset loop counter
        PULSE= ~PULSE;
        __delay_us(5); //Toggle pulse FAST
                           
                       
           
   }

Thanks skarkysparky, sorry I'm still a bit lost here,  I can't say I get what you mean in your reply.

It's me being an idiot, you might need to explain to to be as a child!

Cheers,
Ed

[Ian.M]

That 'smells' like Microchip XC8 C code for a PIC18, but the devil's in the details.  What PIC and compiler version are you actually using?

You are missing the essential 'volatile' qualifier on the definitions of the globals shared by the main program and the ISR.  See: http://www.xargs.com/pic/c-faq.html#volatile

Unless its commercially confidential, in addition to posting the specific chunk(s) of code you are asking about inline in CODE tags, always attach your code in buildable form - both MPLAB X and 8 have an option to package (zip) the current project on the project menu.  Only essential files in and below the project directory are included, so its usually fairly small.  That lets us see *EVERYTHING* you've coded or customised, with line numbers so we can discuss them without confusion.

[Brutte]

Code: [Select]

    TMR0L == 0; //Set TMR0 counter to 0, adjust this for time offsets


[ed_reardon]

Thank you very much Ian.M.

To clarify it's (mildly) sensitive,  and it's written in MPLAB with XC8 for a PIC18F1330.

I shall do some reading!

Cheers,
Ed

[ed_reardon]

Hi,

I've attached what I hope is the right file

Cheers,
Ed

[snarkysparky]

I was wrong in that post. 

T0COUNT  is tested but not reset to zero.  so it is counting to 65536.

[ed_reardon]

I was wrong in that post. 

T0COUNT  is tested but not reset to zero.  so it is counting to 65536.

Thank you sir,

I've modifed it as:

void interrupt high_isr (void){
if(INTCONbits.TMR0IF == 1) {  //Check for status of TMR0 Overflow
    INTCONbits.TMR0IF == 0; //if OVERFLOW occurs reset TMR0 overflow flag.
      T0COUNT++;  //Increment T0COUNT by 1 for every time TMR0 overflows.
    if(T0COUNT==MEDTIME) {
        LPFLG = 1;  //When counter total matches time required, set flag to 1
        T0COUNT == 0;
    }

Thanks again,
Ed

[snarkysparky]

Make it

T0COUNT = 0;

[ed_reardon]

Okay so I've changed the code and I've changed to 'volatile' ints.

Still no further forward, something I have noticed is that if I watch the emulator TMR0L increases but TMR0H stays firmly at zero.

Surely if it's a 16bit counter both registers should be flying up?

Have a ballsed this right up?

[ed_reardon]

My suspicion here is that it's completely ignoring the interrupt,  like I haven't enabled something, or I've set it up wrong 

What should be an external interrupt on RB3 still shows as a normal I/O pin in the simulator rather than a INT3 pin.

 

[Ian.M]

TMR0H is latched, and the latch is triggered by access to TMR0L.  To write it successfully use the macro WRITETIMER0(x) (included by xc.h, defined in pic18.h) with the value to be written as x.  Evaluating x must not have any side-effects.  You should also use the READTIMER0() when reading the timer to future-proof your code although current versions of the compiler have the byte accesses in the correct order for direct use of TMR0 to work for reads.  The debugger UI doesn't 'know' about the TMR0H latch so the value displayed will only update if TMR0L is read.  The interaction of the latch and hardware emulators/debuggers is problematic, and its best not to watch or otherwise display TMR0H and TMR0L directly as it can cause unexpected values to be 'seen' by your code.  The simulator is safe as it can directly read the simulated timer without having to go through the latch, while still simulating the latch for program accesses.

If you suspect the interrupt isn't triggering, put a breakpoint at the beginning of the ISR.  You shouldn't enable interrupt priorities because you only have a single ISR. 

You never enable the timer 0 interrupt so in effect your timer 0 handling code in the ISR only polls the interrupt flag when INT3 triggers the ISR.  If this is by intention O.K, but for %DEITY%s sake comment that its polled!  If its not by intetion, you need to enable the interrupt.  If you are going to modify individual interrupt enables after GIE has been enabled, its good practice to change the tests in the ISR to:

if(whateverIE && whateverIF){ //ISR for whatever

so the wrong part of the ISR doesn't run in an interrupt flag is set but its enable flag is currently disabled.

[ed_reardon]

TMR0H is latched, and the latch is triggered by access to TMR0L.  To write it successfully use the macro WRITETIMER0(x) (included by xc.h, defined in pic18.h) with the value to be written as x.  Evaluating x must not have any side-effects.  You should also use the READTIMER0() when reading the timer to future-proof your code although current versions of the compiler have the byte accesses in the correct order for direct use of TMR0 to work for reads.  The debugger UI doesn't 'know' about the TMR0H latch so the value displayed will only update if TMR0L is read.  The interaction of the latch and hardware emulators/debuggers is problematic, and its best not to watch or otherwise display TMR0H and TMR0L directly as it can cause unexpected values to be 'seen' by your code.  The simulator is safe as it can directly read the simulated timer without having to go through the latch, while still simulating the latch for program accesses.

If you suspect the interrupt isn't triggering, put a breakpoint at the beginning of the ISR.

Thanks for the explaination Ian, it seems as if it is triggering.

It must be the counter,  to confirm (remember I'm very new to this):

T0COUNT++; in the interupt will simply increment the counter by 1?

[Ian.M]

T0COUNT++; in the interupt will simply increment the counter by 1?

Yes, but as T0COUNT is a 16 bit variable, reads from it aren't atomic so if the interrupt hits while the main program is reading it you will get a mish-mash of one byte from before the increment and the other from after it.  This is a low probability event but is likely to cause your code to glitch occasionally so you shoud temporarily disable interrupts while reading it.

[ed_reardon]

T0COUNT++; in the interupt will simply increment the counter by 1?

Yes, but as T0COUNT is a 16 bit variable, reads from it aren't atomic so if the interrupt hits while the main program is reading it you will get a mish-mash of one byte from before the increment and the other from after it.  This is a low probability event but is likely to cause your code to glitch occasionally so you shoud temporarily disable interrupts while reading it.

Ah that's useful to know, I shall adjust that once I've got functionality!

This is really causing me to tear-my-hair out, I've not moved from this desk since 0800 this morning staring at it!

[IanB]

I have not seen a direct and clear acknowledgement about the few comments concerning the difference between "=" and "==".

If you write this:

Code: [Select]

  LPFLG = 0; // sets LPFLG to 0it is an assignment, it assigns the value on the right to the variable on the left.

But if you write this:

Code: [Select]

  LPFLG == 0; // DOES NOT set LPFLG to 0it is an equality test. The expression returns true or false, but LPFLG is not modified.

From the code fragments posted there are several places where "==" has been used when "=" was intended.

[ed_reardon]

I have not seen a direct and clear acknowledgement about the few comments concerning the difference between "=" and "==".

If you write this:

Code: [Select]

  LPFLG = 0; // sets LPFLG to 0it is an assignment, it assigns the value on the right to the variable on the left.

But if you write this:

Code: [Select]

  LPFLG == 0; // DOES NOT set LPFLG to 0it is an equality test. The expression returns true or false, but LPFLG is not modified.

From the code fragments posted there are several places where "==" has been used when "=" was intended.

Thankyou for that helpful post,  yes,  you're entirely right there!  I have since amended (what I think) they should be!

That was a bit of silliness on my part!

Cheers,
Ed

[snarkysparky]

Ed

If you are not tightly wed to the pic18f  I suggest a pic24.  They are 16 bit with a lot more power.

That way all that high/low byte business is gone.  And they are really nice chips with atomic 16 bit timer operations.

[ed_reardon]

Ed

If you are not tightly wed to the pic18f  I suggest a pic24.  They are 16 bit with a lot more power.

That way all that high/low byte business is gone.  And they are really nice chips with atomic 16 bit timer operations.

UNfortunately this is part of an 'upgrade' on a good few thousands units so I'm stuck with the chip in question

Code: [Select]

#define _XTAL_FREQ 1000000
#define PULSE LATAbits.LATA0  //define pin RA0 as PULSE pin
#define DIR LATAbits.LATA1    //define pin RA1 as DIRECTION pin
#define SPEED PORTBbits.RB0 //Define SPEED H/L Pin
#define PROGDIR PORTBbits.RB2 //Define Direction Programming Pin





//Global Variables
volatile int T0COUNT;  //Variable for Timer0 Counter
volatile int LPFLG ;  //Flag for MEDIUM PUMP speed timer
volatile int MEDTIME = 5; //Loops for TIMER0 to run


//Set Timer Interrupt Procedure

void interrupt high_isr (void){
if(INTCONbits.TMR0IF == 1) {  //Check for status of TMR0 Overflow
INTCONbits.TMR0IF = 0; //if OVERFLOW occurs reset TMR0 overflow flag.
   
      T0COUNT++;  //Increment T0COUNT by 1 for every time TMR0 overflows.
    if(T0COUNT==MEDTIME) {
        LPFLG = 1;  //When counter total matches time required, set flag to 1
        T0COUNT = 0;
    }

   
}
if(INTCON3bits.INT3IF == 1){ //if programming pin is HIGH
    INTCON3bits.INT3IF = 0; //Clear interrupt flag
    TRISAbits.RA0 = 1; //switch off motor
    TRISBbits.RB2 = 1; //enable RB2 PROGDIR pin
    if(PROGDIR == 1){ //check for HI PROGDIR pin
        __delay_ms (50);  //wait 50ms for bounce
        if(PROGDIR == 1){
            DIR= ~DIR;
        }
                 
       
    }
}
}
int main (){
    OSCCONbits.IRCF2 = 1; //Set clock speed to 1MHz
    OSCCONbits.IRCF1 = 0; //Set clock speed to 1MHz
    OSCCONbits.IRCF0 = 0; //Set clock speed to 1MHz
    RCONbits.IPEN = 0; //enable interrupt priority
    INTCONbits.GIE = 1; //enable all interrupts
    T0CONbits.TMR0ON = 0; //switch timer OFF in initial state
    T0CONbits.T0PS0 = 1;  //set TIMER0 prescaler to 1:256 on TOP0-2
    T0CONbits.T0PS1 = 1;
    T0CONbits.T0PS2 = 1;
    T0CONbits.PSA = 1; //Enable pre-scaler
    T0CONbits.T0CS = 0; //TIMER0 uses Internal Osccilator
    T0CONbits.T016BIT = 0; //TIMER0 uses 16 bit counter
    ADCON1 = 0xFF; //switch OFF Port-A ADC
    CMCON = 0xFF; //switch OFF comparitors
    PWMEN1 = 0xFF; //switch OFF PWM module
    INTCON3bits.INT3E = 1; //enable INT3 External Interrupt
    INTCON2bits.INTEDG3 = 1; //enable INT3 RIDING EDGE interrupt
   
    TRISAbits.RA0 = 0; //define A0 as OUTPUT
    TRISAbits.RA1 = 0; //define A1 as OUTPUT
    TRISBbits.RB0 = 1; //define B0 as INPUT
     
       DIR = 1; //Change value to '0' for reversed motor
    while (1)  //Do this forever
       
           if(SPEED == 1 ) { //if SPPED pin HI
               T0CONbits.TMR0ON = 1; //switch timer ON
               if(LPFLG == 0){ //If Loop Counter FLAG LO
        PULSE= ~PULSE; //Set PULSE toggle
        __delay_us(25); //Toggle pulse MEDIUM
       }
               
    if(LPFLG == 1 ) {  //if Loop Counter FLAG HI
        T0CONbits.TMR0ON = 0;  //Switch TIMER0 OFF!
       PULSE= ~PULSE; //Set PULSE toggle
        __delay_us(50); //Toggle pulse SLOW
    }
                                 }
   
   else {
        T0CONbits.TMR0ON =  0;  //Switch TIMER0 OFF!     
        LPFLG = 0;       //Reset loop counter
        PULSE= ~PULSE;
        __delay_us(5); //Toggle pulse FAST
                           
                       
           
   }
         
    return 0;
}

This is my latest developments, hopefully I've got my '==' and '=' brainfarts resolved if nothing else, it still doesn't work though, the loop counter just won't increment

[ed_reardon]

TMR0H is latched, and the latch is triggered by access to TMR0L.  To write it successfully use the macro WRITETIMER0(x) (included by xc.h, defined in pic18.h) with the value to be written as x.  Evaluating x must not have any side-effects.  You should also use the READTIMER0() when reading the timer to future-proof your code although current versions of the compiler have the byte accesses in the correct order for direct use of TMR0 to work for reads.  The debugger UI doesn't 'know' about the TMR0H latch so the value displayed will only update if TMR0L is read.  The interaction of the latch and hardware emulators/debuggers is problematic, and its best not to watch or otherwise display TMR0H and TMR0L directly as it can cause unexpected values to be 'seen' by your code.  The simulator is safe as it can directly read the simulated timer without having to go through the latch, while still simulating the latch for program accesses.

If you suspect the interrupt isn't triggering, put a breakpoint at the beginning of the ISR.  You shouldn't enable interrupt priorities because you only have a single ISR. 

You never enable the timer 0 interrupt so in effect your timer 0 handling code in the ISR only polls the interrupt flag when INT3 triggers the ISR.  If this is by intention O.K, but for %DEITY%s sake comment that its polled!  If its not by intetion, you need to enable the interrupt.  If you are going to modify individual interrupt enables after GIE has been enabled, its good practice to change the tests in the ISR to:

if(whateverIE && whateverIF){ //ISR for whatever

so the wrong part of the ISR doesn't run in an interrupt flag is set but its enable flag is currently disabled.

My apologies Ian,  I missed the edit,  yes entirely right I hadn't enabled the overflow bit!

It's now working, well it's working about a-hundred times faster than I'd like it to in the simulator but at least it is working!

Cheers,
Ed