Interrupt on Timer0 Not working properly for PIC18F4550

[Micro9900]

Hey guys, I'm trying to do software PWM at 100Hz for a RGB LED. I am using the PIC18F4550 and c18 LITE compiler. The main issue I am having is getting the interrupt to work. I have looked over AN1074 and how to calulate a 100Hz signal with 32 steps (1/100Hz = 10ms/32 => 312uS).

According to the application note => 256 - ( (312uS) / (Prescale x (1/(48MHz/4)) ) ) => 256 - ( 312uS / (16 * (83.33nS)) ) => 22

So I am trying to set timer0 to interrupt every 312uS. I have it set to low priority for now, but I can always change it to high priority. So, my question is, why isn't the below code working properly? Do I need to use high priority? Is the TMR0L the wrong value? Am I using the wrong prescaler? Any help is appreciated. Thank you.

Global area interrupt

// Global area
// Interrupt (see pg. 99 of PIC18F4550 datasheet)
//______________________________________

//Declare interrupt function
void timer_isr(void);

#pragma code low_vector=0x18
	void low_PWM_interrupt (void)
	{
		_asm 
			GOTO timer_isr 
		_endasm 

	}
#pragma code 

//code for defining interrupt function
#pragma interruptlow timer_isr
void timer_isr(void)
{
	//what you want the interrupt to do goes here
	
	//Check timer0 iunterupt flag 
	if (INTCONbits.TMR0IF==1 )
	{
		//Code that tests interrupt
		int x;
		for(x =0;x<=100;x++)
		{
		LEDPin1=1;LEDPin2=0;LEDPin3=1;

		Delay10KTCYx(100);  // must wait at least 212.5mS (120*100000*4/48e6) for 48Mhz
		Delay10KTCYx(100);  // must wait at least 212.5mS (120*100000*4/48e6) for 48Mhz
		Delay10KTCYx(100);  // must wait at least 212.5mS (120*100000*4/48e6) for 48Mhz
		Delay10KTCYx(100);  // must wait at least 212.5mS (120*100000*4/48e6) for 48Mhz
		Delay10KTCYx(100);  // must wait at least 212.5mS (120*100000*4/48e6) for 48Mhz

		LEDPin1=1;LEDPin2=1;LEDPin3=0;
		}
		

		//TMR0L=22; 			//see calculation in main	
		INTCONbits.TMR0IF=0;	//clear timer0 interrupt flag	
	}	
}
//_______________________________________________

Timer and interrupt configurations in main

	// Set up interrrupt control register pg. 101 of pic18f4550 datasheet
	INTCONbits.GIE = 1; 	// Enable global and high priority interrupts
	INTCONbits.GIEL = 1; 	// Enable low priority interrupts
	RCONbits.IPEN = 1; 		// Enables proirity levels on interrupts


	
	//Added
	T0CONbits.TMR0ON=1;		// Turn timer on = 1
	T0CONbits.T08BIT=1;		// 8bit = 1, 16 bit = 0
	T0CONbits.T0CS=0;		// Internal instuction clock cycle = 0
	T0CONbits.T0SE=0;		// transistion on high CLK
	T0CONbits.PSA=0;		// prescaler is assigned = 0
	T0CONbits.T0PS2=0;
	T0CONbits.T0PS1=1;
	T0CONbits.T0PS0=1;	

	TMR0H=0; 				// Ensure high bytes of timer register are 0 since using 8bit timer and not 16bits
		
	TMR0L=22;				// Using 100Hz for LEDs => 1/100Hz => 10mS, if using 32 steps => 10mS/32 => 312uS interrupt period
							// Resets Timer0 counter based on calculation for PWM (256 - (InterruptPeriod / Prescale x Tcy)
							// Prescaler is set to ensure resultant value of the below equation is 255 or less
							// 256 - ( (312uS) / (Prescale x (1/(48MHz/4)) ) ) => 256 - ( 312uS / (16 * (83.33nS)) ) => 22
							// 48Mhz is divided by 4 since each instruction takes 4 clock cycles
	//___

	INTCONbits.TMR0IE=0; 	// Enables Timer0 interrupt
	INTCONbits.TMR0IF=0; 	// Clear timer interupt flag

	INTCON2bits.TMR0IP=0;	 // Overflow interrupt priority bit (0=low priority, 1=high priority)

 

[nsaspook]

The first thing is to NEVER use delays in an ISR. You should clear the interrupt flag and exit as soon as possible so the next interrupt is not missed. If work needs to be done in a ISR a STATE machine should be used so all work in done multiple passes until completed.

INTCONbits.TMR0IE=0; // Enables Timer0 interrupt

Should be 1.
https://www.microchip.com/forums/m163362.aspx

INTCONbits.GIEL = 1; // Enable low priority interrupts

Should be enabled after all interrupt configuration is done.

Take a look at some of my source code here. https://github.com/nsaspook/RGBLED.git

 

[Micro9900]

Thank you so much it now works. But, now this bring up a different issue. I have a character display and the microcontroller also is receiving data through USART module to be displayed. Now that the interrupt is working the LCD fails to initialize unless I call the LCD initialize function within the interrupt. Why is this? Is my interrupt being called too often for the character LCD to initialize and for it to have data written to it?

So, it appears that nothing within main that is time critical is working. The LCD doesn't get initialized, there is no data being displayed. How should I go about this? Create separate interrupts for the LCD and UART?

EDIT:
_____________

Never mind. The issue was that since I only want the LED to cycle through colors when a button is pressed, I forgot to reset the timer interrupt back to 0 outside of the if statement.

	//Check timer0 iunterupt flag 
	if (INTCONbits.TMR0IF==1 & LightShowEnable==1)
	{
		//Code that tests interrupt
		int x;

		LEDPin1=1;LEDPin2=0;LEDPin3=1;
		LEDPin1=1;LEDPin2=1;LEDPin3=0;
		

		//TMR0L=22; 			//see calculation in main	
			
	}	
	INTCONbits.TMR0IF=0;	//clear timer0 interrupt flag
}

Notice INTCONbits.TMR0IF=0; clearing the timer0 interrupt flag is now outside of the if statement. Before, since it was within the if statement the interrupt flag wasn't being reset, so it locked up the microcontroller since it was interrupting indefinitely.

Thank you so much nsaspook. You really helped me out a lot ^^.

EDIT2:
_________________

Here is what the final interrupt code looks like:

//code for defining interrupt function
#pragma interruptlow timer_isr
void timer_isr(void)
{
	//what you want the interrupt to do goes here
	
	//Check timer0 iunterupt flag 
	if (INTCONbits.TMR0IF==1 )
	{
		//Code that tests interrupt
		if(LightShowEnable==1)
		{		
				//PWM code goes here
				LEDPin1=1;LEDPin2=1;LEDPin3=0;
		}		

		TMR0L=22; 			//see calculation in main	
		INTCONbits.TMR0IF=0;	//clear timer0 interrupt flag
	}	
	
}

I now have INTCONbits.TMR0IF=0; back in the if statement, but I now have a nested if statement, which is most likely a better solution.