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.

asked Mar 3, 2015 at 10:50
\$\endgroup\$
2
  • \$\begingroup\$ Where is count defined? Have you marked it volatile? \$\endgroup\$ Commented Mar 3, 2015 at 10:59
  • 1
    \$\begingroup\$ You should do count++; instead of count = count++;. The ++ operator saves the new value for you. \$\endgroup\$ Commented Mar 3, 2015 at 13:40

3 Answers 3

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.

answered Mar 3, 2015 at 11:16
\$\endgroup\$
8
  • \$\begingroup\$ Yes I have an additional call to RTOS() which I should remove. \$\endgroup\$ Commented Mar 3, 2015 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\$ Commented Mar 3, 2015 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\$ Commented Mar 3, 2015 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\$ Commented Mar 3, 2015 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\$ Commented Mar 3, 2015 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.

answered Mar 3, 2015 at 12:04
\$\endgroup\$
2
  • \$\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\$ Commented Mar 3, 2015 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\$ Commented Mar 3, 2015 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;
} 
answered Mar 5, 2015 at 8:58
\$\endgroup\$
0

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.