Jump to content

Recommended Posts

Hi I had some old code which turned out to be faulty when used with BoostC.

void interrupt(void)
 {
// clear_wdt();
// Timer0 Interrupt
if (T0IE && T0IF) //always true
{
tmr0=tmr0_offset;
clear_bit( intcon, T0IF );		// Clear T0IF every time
}

//Button press
if (INTE && INTF) //always true
clear_bit(intcon,INTF);
}

// Timer2 Interrupt
// ...
if (TMR2IE && TMR2IF) //always true
{
clear_bit( intcon, TMR2IF );	// Clear T2IF every time
}
}

 

Change to this:

void interrupt(void)
 {
// clear_wdt();
// Timer0 Interrupt
if (T0IE && intcon.T0IF)
{
tmr0=tmr0_offset;
clear_bit( intcon, T0IF );		// Clear T0IF every time
}

//Button press
if (INTE && intcon.INTF)
clear_bit(intcon,INTF);
}

// Timer2 Interrupt
// ...
if (TMR2IE && intcon.TMR2IF)
{
clear_bit( intcon, TMR2IF );	// Clear T2IF every time
}
}

 

Note the intcon.

Link to post
Share on other sites

Alistair,

 

Were you asking a question or simply making a statement?

 

Anyway, your code is still faulty for the same reason you showed and therefore will not work as you think!

If you are not sure why then look at the code generated.

 

I always use a standard template for every interrupt for each device I use and simply change the order of the interrupts or comment them out as required.

That way I don't have to re-invent the wheel for each project. The comments help me to remember what else I need to check/set.

The code can be cleaned up later once working if required but I don't normally bother.

 

The following is a small example from the template I use for an 18F4520. I haven't bothered to show interrupt_low() but this is usually a repeat of interupt().

void interrupt(void)
{
// External Interrupt on RB0 - Always High Priority.  Check Pull-ups intcon2,RBPU & Edge intcon2,INTEDG0
if(test_bit(intcon,INT0IE) &&	   // Interrupt enabled check (Comment out if not required)
test_bit(intcon,INT0IF))
{
	// Place code or function call here for External Interrrupt 0 Handler
	clear_bit(intcon,INT0IF);		// Clear Interrupt Flag
}

// External Interrupt on RB2 - Check Priority intcon3,INT2IP.  Check Pull-ups intcon2,RBPU & Edge intcon2,INTEDG2
if(test_bit(intcon3,INT2IE) &&	// Interrupt enabled check (Comment out if not required)
test_bit(intcon3,INT2IF))
{
	// Place code or function call here for External Interrupt 2 Handler
	clear_bit(intcon3,INT2IF);		 // Clear Interrupt Flag
}

// Timer 0
if(test_bit(intcon,TMR0IE) &&	// Interrupt enabled check (Rem out if not required)
test_bit(intcon,TMR0IF))		   // Timer 0 Interrupt?
{
	// Place code or function call here for Timer 0 Interrupt Handler
	clear_bit(intcon,T0IF);		 // Clear interrupt flag	
}

// Compare Capture Module 2	
if(test_bit(pie2,CCP2IE) &&	  // Interrupt enabled check (Rem out if not required)
test_bit(pir2,CCP2IF))			 // Compare Module 1 Interrupt?
{
	// Place code or function call here for Compare/Capture Module 2 Interrupt Handler
	clear_bit(pir2,CCP2IF);	   // Clear interrupt flag	
}

// Repeat format for all other interrupts...

}

Hope that helps

 

Regards

 

davidb

Link to post
Share on other sites

OK good on you. I dont mind being picked up.

I was hoping my effort would have been of some use to someone who came unstuck like I had. It has, because you have improved the statement to make it accurate!

Thanks and kind regards,

Alistair+

PS any chance of telling me whats wrong with my code here:

http://forum.sourceboost.com/index.php?showtopic=4397

Link to post
Share on other sites

Just because you own a pair of snow shoes doesn't mean you have to wear them to work every day of the year.

 

In the same respect, just because the PIC has interrupts does not mean they have to be coded or used.

 

The sample code corrected by davidb could be cleaned up to:

 

void interrupt(void)
 {
// Timer0 Interrupt
tmr0=tmr0_offset;
clear_bit( intcon, T0IF );		// Clear T0IF every time
}

 

The other interrupts can be turned off in INTCON or whatever, as they are not being used.

 

Also, seeing as I'm off on one here, why check to see if an interrupt is enabled as well as checking if it's interrupt flag is set? In addition, an if...else in priority order is better in an interrupt routine, as only one interrupt is processed at a time.

 

Interrupts are useful things, but easily overused for no good reason.

 

The interrupt flags generally get set without the need to also use the interrupt routine, so it is often practical to check for an interrupt flag in the main code loop, rather than have to juggle interrupts. If the timing of interrupts are not crucial, and all you do is set a status in the interrupt routine anyway, why bother?

 

For example, if timing for timer0 is not crucial, the above code could be handled as follows, without even bothering the PIC with interruptions. This results in better overall performance:

 

while(1) {
...
... doing processing
...
// Timer0 Interrupt
if(test_bit(intcon,INT0IF)){ // just as good as setting a flag in the interrupt routine for say switches etc.
	tmr0=tmr0_offset;
	clear_bit( intcon, T0IF );		// Clear T0IF 
}
...
.... doing processing
...
}

 

 

I know this all seems picky, so please accept this post as helpful criticism, but my experience has always been that writing good, clean, code even for apparently simple apps pays dividends when things get more complex and performance becomes an issue.

Link to post
Share on other sites

Hi Tim,

 

The interrupt template I gave was just an example for Al. I didn't expect him or anybody else to copy it literally, whether they used the particular interrupts shown or not. I just find it useful when starting a new project to remind me of the various interrupt enables, control bits, flags etc. available for a particular micro without having to look them up.

 

Normally the individual interrupt checks are commented out until I need to use them. You will also notice that the interrupt enable checks do not have to be used and can be easily commented out as required. In addition the interrupts can be moved around to change the priority.

 

The reason that the enable check is sometimes required is that the interrupt might be temporarily disabled within the code for operational reasons. Since the interrupt flag can still be set the check is there to prevent the interrupt being processed incorrectly when other interrupts are processed. If the interrupt is never disabled then the check can be removed.

 

Obviously in your Timer 0 example where only one interrupt is used you could get away with not checking the enable or the flag. I never have that situation but I think I would still at least check the flag!

 

As far as using inline interrupts or if...else structure, it entirely depends on what you are trying to do. If you have many interrupts that could occur at a similar time then the if...else structure would probably slow things down since you might exit the isr then immediately re-enter it to process the next interrupt with the consequent extra overhead. On the other hand it does save time by not having to check any lower priority enables and flags. I really do not know which is the best method to use and I think some discussion on this backed up by a few tests might be interesting.

 

Regards

 

davidb

Link to post
Share on other sites

Hi davidb

 

I agree that depending on the requirement, there are different ways of doing things, no problem there.

 

I see so many code snippets posted which seem unnecessarily complex, or bloated, that from time to time I feel compelled to put in my 10p worth.

 

In all cases writing robust, easy to comprehend, code provides the basis for reliable operation.

 

Cheers

 

Tim

Link to post
Share on other sites
Hi davidb

 

I agree that depending on the requirement, there are different ways of doing things, no problem there.

 

I see so many code snippets posted which seem unnecessarily complex, or bloated, that from time to time I feel compelled to put in my 10p worth.

 

In all cases writing robust, easy to comprehend, code provides the basis for reliable operation.

 

Cheers

 

Tim

I like it; thinking outside the square. Yep, I have a main loop which is forever shooting off to IRQ then returning with the IRQ flagging relevant IRQ proc. It would make sense to enable desired interrupts, and check some in effected subroutines instead. EG why go to IRQ for buttonpress, as you suggest, when it can be tested anywhere else?

Cheers,

Al.

Link to post
Share on other sites
...It would make sense to enable desired interrupts, and check some in effected subroutines instead. EG why go to IRQ for buttonpress, as you suggest, when it can be tested anywhere else?

Cheers,

Al.

Al.

 

If you are going to check an interrupt flag in a main() function rather than in interrupt() then DO NOT enable the interrupt. The interrupt flag gets set regardless of the state of its enable. Also don't forget to clear the flag when read.

 

BTW Using an interrupt to wake from sleep with a button press might be your only option.

 

Regards

 

davidb

Link to post
Share on other sites
Al.

 

If you are going to check an interrupt flag in a main() function rather than in interrupt() then DO NOT enable the interrupt. The interrupt flag gets set regardless of the state of its enable. Also don't forget to clear the flag when read.

BTW Using an interrupt to wake from sleep with a button press might be your only option.

 

What I'd done was Timer0 only in the interrupt. Keypress IRQ was enabled, bit checked and cleared in main. So I'll disable the keypress interrupt and just check and clear the flag in main thanks vm Al.

Link to post
Share on other sites

Join the conversation

You are posting as a guest. If you have an account, sign in now to post with your account.
Note: Your post will require moderator approval before it will be visible.

Guest
Reply to this topic...

×   Pasted as rich text.   Paste as plain text instead

  Only 75 emoji are allowed.

×   Your link has been automatically embedded.   Display as a link instead

×   Your previous content has been restored.   Clear editor

×   You cannot paste images directly. Upload or insert images from URL.

Loading...
×
×
  • Create New...