View previous topic :: View next topic |
Author |
Message |
mfeinstein
Joined: 05 Jul 2012 Posts: 35
|
PIC18F2550 - Timer1 Interrupt causing error in Timer0 |
Posted: Mon Sep 03, 2012 5:28 pm |
|
|
Hi everyone!
I am making a project using the PIC18F2550 and PCW 4.120. As you can see in the code below I have a Timer0 interrupt and a Timer1 Interrupt. The Timer1 Interrupt is currently disabled and the Timer0 Interrupt is expected to be called every 500 microseconds. Instead, the Timer0 Interrupt is being called every 250 milliseconds...after hours looking at the code, searching for a flaw, I finally found out that actually the delay_ms(75) in the Timer1 is triggering this flaw. I don't have any idea why it is happening, but it is! I commented all the delays in the Timer1 and it work as expected...but leaving them in the code makes the Timer0 go nuts....the weirdest thing is that Timer1 is disabled and every piece of code I place inside Timer1 is never executed....
Any ideas how to fix it? I don't know what else to do :/
Thank you very much!!
EDIT: Since many people were complaining, I posted a simpler code, just to show the real problem...so dont try to look at the code, figuring what it does functionally...just look at the Timers problem (Timer1 is disabled but its delay function makes the Timer0 interrupt be called late)
Code: |
#include <18F2550.h>
#device adc=10
#FUSES PLL5 //Divide By 5(20MHz oscillator input)
#FUSES CPUDIV1 //Configurado errado pelo wizard... CPUDIV depende da entrada selecionada...olhe o datasheet
#FUSES HSPLL //High Speed Crystal/Resonator with PLL enabled
#use delay(clock=48000000)
#define LED PIN_B3
unsigned int16 time = 0;
int8 counter = 0;
#int_TIMER1
void TIMER1_isr(void)
{
if(counter == 0)
{
counter++;
}
else if(counter == 1)
{
counter = 0;
delay_ms(75);
}
}
#int_TIMER0
void TIMER0_isr(void) // Saida PWM
{
output_toggle(LED);
set_timer0(time);
}
void main(void)
{
delay_us(10);
port_b_pullups(FALSE);
disable_interrupts(GLOBAL);
setup_timer_1(T1_INTERNAL|T1_DIV_BY_2);
setup_timer_0(RTCC_INTERNAL|RTCC_DIV_32|RTCC_8_BIT);
time = 69;
set_timer0(time);
enable_interrupts(INT_TIMER0);
disable_interrupts(INT_TIMER1); // Timer1 is always DISABLED!!
enable_interrupts(GLOBAL);
while(TRUE)
{
delay_ms(1000);
}
}
|
Last edited by mfeinstein on Tue Sep 04, 2012 5:35 pm; edited 1 time in total |
|
|
PCM programmer
Joined: 06 Sep 2003 Posts: 21708
|
|
Posted: Mon Sep 03, 2012 5:47 pm |
|
|
Quote: | I finally found out that actually the delay_ms(75) in the Timer1 is triggering this flaw.
|
Those delays make the PIC stay in the #int_timer1 routine for 150 ms.
The PIC can't do anything else during that time. It can't execute any
other interrupt until it finishes those delays and exits from #int_timer1.
Solution: Get rid of all delay_ms() inside your interrupt routines. Do
your delays by some other method. |
|
|
asmboy
Joined: 20 Nov 2007 Posts: 2128 Location: albany ny
|
|
Posted: Mon Sep 03, 2012 5:55 pm |
|
|
1-learn to use the CODE button when posting( ah fixed -good)
2- where do you enable timer1 ints? ( answer=nowhere in this post)
the delays in the int won't hurt you if the int handler is not enabled
3-a more organized approach to SETUP of TRIS and timers would benefit you
just some EZ observations' |
|
|
mfeinstein
Joined: 05 Jul 2012 Posts: 35
|
|
Posted: Mon Sep 03, 2012 6:32 pm |
|
|
PCM programmer:
As you can see in the code, the Timer1 Interrupt is Disabled, so even if I had a delay_ms(1000) inside of it, this delay will never be executed, so this isnt the reason why I have this problem (as I already explained in my previous post)....as a matter of fact, even if I have just a delay_ms(75) and nothing else inside of the Timer1 Interruptor I still get this error...
again: Timer1 is disabled...no code is being run...but it still causes this error, if you have any doubts, you can try this code and simulate it yourself...
asmboy:
1 - Sorry, I was in a hurry
2 - That's the whole thing...I dont! It is disabled and it is hurting me! if I comment it, the Timer0 work as expected, but if the delay line is in the Timer1 code (even if that's and unreachable code) the Timer0 works flawlessly
3 - What do you mean? This is actually not my complete code, I made it simpler (since posting 870 lines didnt sound as a good idea :P )
4 - Do you have any ideas how to fix it?
Thank you all!
PS: Ttelmah this is the moment where you show up and save the day ; ) hahaha |
|
|
asmboy
Joined: 20 Nov 2007 Posts: 2128 Location: albany ny
|
|
Posted: Mon Sep 03, 2012 9:22 pm |
|
|
configuraSlewRate() is a functional mess.....
setup the ISR INT handler before setting up the timer hardware ???
you should 1- setup hardware 2- clear the int flag , then 3- THEN enable the ISR
AND
no return is required from an ISR or any callable function, unless you want an early exit
also
you might have a close look at ALL the fuses for that pic and make sure you are setting the part up, correctly for your intended use.
also PCM is 1000% correct in this advice:
never put a fixed delay of more than a few cycles or usecs in ANY ISR.
ONLY trouble flows from ignoring that |
|
|
mfeinstein
Joined: 05 Jul 2012 Posts: 35
|
|
Posted: Mon Sep 03, 2012 10:22 pm |
|
|
asmboy,
Thank you for your reply... as I said before, this is a 870 lines code, so I erased most of it to post in here, isolating just the minimum parts to show you guys the problem, and yes, it became a mess after the "edition"... (the return is indeed an early exit to debug the interruption...and if you look closer ALL the interrupts are disabled until EVERYTHING is already setup as it should...I just didnt clear the interrupt flags as you said)...
BUT, this has nothing to do with the actual problem...I just posted this code in here so people with more knowledge than me might figure out what is going on...so please dont focus on the code organization, just try to figure out why the Timer1 is messing with Timer0 even if it is inoperative...so far your advices were very productive in concern with "code organization" and "good programming techniques" but this is not the subject matter of this post...
Best Regards |
|
|
Ttelmah
Joined: 11 Mar 2010 Posts: 19499
|
|
Posted: Tue Sep 04, 2012 1:58 am |
|
|
Key point is re-entrancy.
The PIC does not support re-entrant code. Hence if _any_ complex function, is used inside an interrupt, and also outside the interrupt, then interrupts have to be disabled in the external copy. This will affect everything else that uses interrupts. Since the compiler _must_ ensure this is done, it will disable interrupts this way, even if the interrupt is never called.
Hence, as soon as you have a delay function inside an ISR, and the same function in the main code, interrupts _will_ be disabled in the external copy, _even if the interrupt is not enabled_. So if you have a routine for timer0, using interrupts, this will have to wait till the delay_ms(1000) in the external code completes, before it'll be called.
Seriously, your code, is a 'bad example' of how to debug a problem. 90% of it is not needed to show what you are worrying about, and what is posted, won't actually display the problem, since it lacks the timer0 initialisation...
As another comment, is ADC_CLOCK_INTERNAL, legal for a chip running at this speed?.
Best Wishes |
|
|
Mike Walne
Joined: 19 Feb 2004 Posts: 1785 Location: Boston Spa UK
|
|
Posted: Tue Sep 04, 2012 8:10 am |
|
|
Quote: | BUT, this has nothing to do with the actual problem...I just posted this code in here so people with more knowledge than me might figure out what is going on...so please dont focus on the code organization, just try to figure out why the Timer1 is messing with Timer0 even if it is inoperative...so far your advices were very productive in concern with "code organization" and "good programming techniques" but this is not the subject matter of this post... |
The point is, if your code were better organised YOU would be able to see the problem, and/or it would be easier for the rest of us to figure out what you're trying to do.
Like Ttelmah says you've posted far more code than necessary to highlight your problem.
How do you KNOW which bits of code are executing? It appears you're using the LED of pin B3 as an indicator. Code to change the state of LED appears in at least three places. Why not use other pins to make it easy to tell which bit of code is which.
Have you tried looking at the assembly code which the compiler generates? When you do, you may very well see HOW one section of code can interfere with the operation of others.
I hope this is helpful.
Mike |
|
|
mfeinstein
Joined: 05 Jul 2012 Posts: 35
|
|
Posted: Tue Sep 04, 2012 10:35 am |
|
|
Mike Walne,
1 - I am not familiar with PIC assembly code, and the code generated will be huge in any case (its more than 870 lines of C).
2 - I cant use other pins, the board is already full and functional.
3 - The code is well organized...but it is in Portuguese, so the function names dont make much sense for you guys, and they look confuse, but there is a reason for it...I am sorry :P I tried to delete most of the parts, but I can see the example was still a bad one, sorry again!
Ttelmah,
As usual, thank you my friend! Could you give me some links to study more about reentrancy in the PIC?
I do have the Timer0 initialization...I dont understand why you said I dont... its the
Code: | setup_timer_0(RTCC_INTERNAL|RTCC_DIV_32|RTCC_8_BIT); |
line and every Interrupt I do
Code: | set_timer0(timerParaSlewRate); |
to make 500 microsecods...
The ADC code is just there, I didnt use the ADC for nothing yet, I just copied all the configurations in case there was any problems...The ADC will be used in the future, but until now, its just resting...
Ttelmah... I thought that maybe things were happening as you said, but instead of getting an interrupt at every 1000ms, I am getting at every 250ms or 252ms....so how may it be possible? And shouldnt the compiler give me some warning about reentrant code? |
|
|
FvM
Joined: 27 Aug 2008 Posts: 2337 Location: Germany
|
|
Posted: Tue Sep 04, 2012 11:35 am |
|
|
I can't see the point where Timer1 get's enabled in the present code. I even don't see PIE1 referenced in the compiled code of V4.120. And there are no known silicon erratas related to a similar problem.
So for the time being, I'll assume that the problem can't be reproduced with the posted code.
To find out how timer1 ISR can be called though, hardware debugging is the most promising way. |
|
|
Douglas Kennedy
Joined: 07 Sep 2003 Posts: 755 Location: Florida
|
|
Posted: Tue Sep 04, 2012 11:48 am |
|
|
Errors are often related to probability. The more code the higher the probability. This board would ideally like all issues reduced to one line of code. Most accept this as impracticable so a few lines is next best. Dozens of lines does violence to the concept of paring down the issue. Statistically 99% of issues are of the coders own making. Breaking the code down into simple blocks and testing them one by one is the analytical approach. This board offers the opportunity of learning from the mistakes of others but more importantly learning from your own mistakes by the process of simplifying your code so you can spot your own mistake.When TV's had tubes sometimes they could be made to work by just thumping them. No electrical knowledge was needed. Posting dozens of lines of code culled from a larger body of code without even bothering to test them as exhibiting the issue might be considered by some as the thumping the TV approach to resolving issues.
Last edited by Douglas Kennedy on Tue Sep 04, 2012 11:55 am; edited 1 time in total |
|
|
mfeinstein
Joined: 05 Jul 2012 Posts: 35
|
|
Posted: Tue Sep 04, 2012 11:48 am |
|
|
FvM...please read through the post...I already said it before, many times indeed, that Timer1 ISNT enabled at all AND it's causing an error (and that's the whole question.."how may it be possible?") |
|
|
asmboy
Joined: 20 Nov 2007 Posts: 2128 Location: albany ny
|
|
Posted: Tue Sep 04, 2012 12:34 pm |
|
|
here is the way to converge on a solution then:
keep on reducing the code to:
the FEWEST lines that WILL compile - AND still .
show the problem.
get rid of EVERYTHING you can - and still see the problem.
!! START WITH the RETURN in your TIMER0 ISR !!
I C 2 much forest in your code to let ME see the rotten tree i guess.
besides - i have my own project to debug this afternoon |
|
|
mfeinstein
Joined: 05 Jul 2012 Posts: 35
|
|
Posted: Tue Sep 04, 2012 5:37 pm |
|
|
asmboy,
I edited the code so you all could see it in a more simple way.
Ttelmah,
I agree with you that this is a reentrancy problem, but still I dont undestand why the PIC interrupts at 250ms instead of 1000ms, as the delay_ms should be executing and then the Timer0 will be called...
Thank you! |
|
|
jeremiah
Joined: 20 Jul 2010 Posts: 1345
|
|
Posted: Tue Sep 04, 2012 6:29 pm |
|
|
You're not understanding what they are saying. It absolutely does NOT matter that you disabled timer1. The fact you have the delay statement in the timer code at all (regardless of disabled/enabled) tells the compiler that you could enable it at some point, so it automatically disables all interrupts while any delay statement, which will mess up timer0 isr timing. That's why the behavior changes when you comment the line out. Even if you never ever ever enable the timer1 interrupt, the code there will cause the compiler to disable all interrupts while in delay_ms(). |
|
|
|