4
\$\begingroup\$

Im looking for a little help and advice on my code.

Im using C18 in the MPLab environment, PIC18f4520 with Fosc @ 4MHz, and usnig timer0 in 16 bit mode to count to overflow, set the overflow bit and interrupt flag, then jump to ISR and increment a variable 'count'. This is outputted to a port with LEDs attached so that I can get visualy confirmation of the program working.

However, the outputted count is always '1' (i.e. 0x01) and I believe that the ISR is only happening once, if at all.

Any help that you can offer would be most appreciated.

Here is my code:

void main (void)        /*                                                                                      */
{                   
TRISA = 0;          /*                                                                                      */
TRISC = 0;          /*                                                                                      */
TRISB = 0;
TRISD = 0x00;
RTOS();
}
void low_interrupt (void)
    {
    _asm GOTO timer_isr _endasm
    }
    #pragma code
    #pragma interruptlow timer_isr 

void timer_isr (void)
    {
    INTCONbits.TMR0IF = 0;
    count = count++;
    LATD = count;
    RTOS();
    }
void RTOS (void)
    {
    T0CONbits.T08BIT = 0;   // 16-bit timer
    T0CONbits.T0CS = 0;     // increment on instruction cycle input
    T0CONbits.T0SE = 0;     // increment on low--> high transition of clock
    T0CONbits.PSA = 1;      // T0 prescaler not assigned i.e. 1:1 prescaler.
    RCONbits.IPEN       = 1;    //Enable Interrupt Priorities
    INTCONbits.GIEL     = 1;    //Enable Low Priority Interrupt
    INTCONbits.GIE      = 1;    //Enable Global Interrupts            
    INTCONbits.TMR0IE   = 1;    //Enable Timer0 Interrupt
    INTCON2bits.TMR0IP  = 0;    //TMR0 set to Low Priority Interrupt
    INTCONbits.TMR0IF = 0;  // T0 int flag bit cleared before starting
    T0CONbits.TMR0ON = 1;   // timer0 START
    while (1);
    }

Thanks in advance for any guidance you can offer.

\$\endgroup\$
  • \$\begingroup\$ Where is count defined? Have you marked it volatile? \$\endgroup\$ – Roger Rowland Mar 3 '15 at 10:59
  • 1
    \$\begingroup\$ You should do count++; instead of count = count++;. The ++ operator saves the new value for you. \$\endgroup\$ – user17592 Mar 3 '15 at 13:40
4
\$\begingroup\$

You are calling RTOS() in your interrupt and the RTOS() function has "while (1);" in it (infinite loop?)

I'm not sure why you would reset the complete interrupt registers within your interrupt.

Having an infinite loop in your interrupt will most likely cause your program to malfunction.

Also, check the comment of Roger Rowland: "Where is count defined? Have you marked it volatile? – Roger Rowland 15 mins ago". This is a pretty common mistake and might also be the point here.

\$\endgroup\$
  • \$\begingroup\$ Yes I have an additional call to RTOS() which I should remove. \$\endgroup\$ – Mondogenerator Mar 3 '15 at 11:52
  • \$\begingroup\$ Within the ISR all I am resetting is the interrupt flag, so that if interrupt flag is set shortly after exit of ISR it will call ISR again. \$\endgroup\$ – Mondogenerator Mar 3 '15 at 11:53
  • \$\begingroup\$ count is defined in the header and is defined as int. As i am relatively inexperienced in C I have never yet had to use "volatile" for anything \$\endgroup\$ – Mondogenerator Mar 3 '15 at 11:54
  • \$\begingroup\$ @user32851 Then just add the keyword volatile to your int, so volatile int count; See if that makes a difference and in any case, get reading about why it's sometimes necessary. \$\endgroup\$ – Roger Rowland Mar 3 '15 at 12:11
  • \$\begingroup\$ @user32851 You have a while (1); in the RTOS() which is in your interrupt. Even if you reset the interrupt flag, the interrupt won't interrupt on your current interrupt routine! So it will be stuck in the while. I suggest you put the while(1); in your main, or completely erase it. `while(1){ //some code }' is often done in main to keep the program running/looping that code. But you shouldn't make this loop in an interrupt service routine. \$\endgroup\$ – Paul Mar 3 '15 at 12:52
2
\$\begingroup\$

I am not familiar with C18, but I do use its successor XC8.

Regarding your ISR, does the C18 compiler know to execute void low_interrupt (void) when a low-priority interrupt is generated? I have seen designated functions for interrupts with some compilers but XC8 works as follows:

// HP int declared with 'high_priority' identifier, name function as you see fit
interrupt high_priority void isr_high(void)

// LP int declared with 'low_priority' identifier, name function as you see fit
interrupt low_priority void isr_low(void)

If C18 does use designated function names instead, make sure you've got them correct.

Secondly, here's how I would have handled your timer overflow:

interrupt high_priority void isr_high(void)
{  
    // TIMER0 Overflow
    if (INTCONbits.TMR0IF)
    {
        LATEbits.LE0 ^= 1;               // Toggle RE0
        INTCONbits.TMR0IF = 0;           // Clear the interrupt flag
    }
}

Hope this helps.

\$\endgroup\$
  • \$\begingroup\$ I have to pragma code for each ISR, then use the predefined interrupt funbction names. One thing I havent done is give an action for the High ISR, or even include the function; so it is possible that the High ISR is doing something. thanks for the code snippet, I am actually intending to make the ISR jump to a new routine on overflow, but it helps some \$\endgroup\$ – Mondogenerator Mar 3 '15 at 12:09
  • \$\begingroup\$ If the routine you are jumping to is likely to do significant work, it's better to get your ISR to set a flag which is polled within your main program. Then do the routine work in your main program. But as long as your routine is short and sweet then it should be okay to jump to it from the ISR. The code snippet I gave you is what I use to confirm my micro is still running in many of my projects. It simply flashes an LED continually provided the ISR is responding to TIMER0 overflows :) Enjoy :) \$\endgroup\$ – Brian J Hoskins Mar 3 '15 at 13:49
0
\$\begingroup\$

Thanks for all the help.

With a bit of work in MPLabSim I have found my mistake and the corrected code below functions correctly, at least in MPLabSim:

EDIT: now also works on real hardware!!!!

 void main (void)       /*                                                                                      */
    {                   
    TRISA = 0;          /*                                                                                      */
    TRISC = 0;          /*                                                                                      */
    TRISB = 0;
    TRISD = 0x00;
    RTOS();
    }
void low_interrupt ()
    {
    _asm GOTO timer_isr _endasm
    }
    #pragma code
    #pragma interrupt low_interrupt //save =PROD
void timer_isr ()
    {
    if(INTCONbits.TMR0IF==1)
    {
    count++;
    INTCONbits.TMR0IF = 0;
    }
    }
void RTOS ()
    {
    T0CONbits.T08BIT = 0;   // 16-bit timer
    T0CONbits.T0CS = 0;     // increment on instruction cycle input
    T0CONbits.T0SE = 0;     // increment on low--> high transition of clock
    T0CONbits.PSA = 1;      // T0 prescaler not assigned i.e. 1:1 prescaler.
    RCONbits.IPEN       = 1;    //Enable Interrupt Priorities
    INTCONbits.GIEL     = 1;    //Enable Low Priority Interrupt
    INTCONbits.GIEH     = 0;    // disable high priority interrupts
    INTCONbits.GIE      = 1;    //Enable Global Interrupts            
    INTCONbits.TMR0IE   = 1;    //Enable Timer0 Interrupt
    INTCON2bits.TMR0IP  = 0;    //TMR0 set to low Priority Interrupt

    INTCONbits.TMR0IF = 0;  // T0 int flag bit cleared before starting
    T0CONbits.TMR0ON = 1;   // timer0 START
    counter(count);
    }   
void counter ()
{
LATD = count;
}   
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.