View previous topic :: View next topic |
Author |
Message |
tonkostz
Joined: 07 May 2011 Posts: 42 Location: Bulgaria
|
Issue with time delays |
Posted: Tue Sep 04, 2012 11:03 pm |
|
|
I have an issue with the following code.
Compiler version is 4.120, MCU is PIC12F683 and clock is the internal 4MHz oscillator.
Problem is that every time the time delays are different. For example when i push button on ADC pin AN3) and the voltage is above a certain value, a pwm is started after a 5 seconds delay. This delay is different every time. When i release the button and the voltage disappears, a delay of 5 seconds should be present. But this delay is not the same every time. Sometimes is 3 seconds, sometimes more.
Code: | #include <12f683.h>
#device adc=10
#fuses INTRC_IO,NOWDT,NOMCLR,NOBROWNOUT
#fuses PROTECT // code protect
#fuses CPD // data eeprom code protect
#use delay(clock=4000000)
//time delay constants
#define delay_on 76 //5sec on delay
#define delay_off_5 76 //5sec off delay
#define delay_off_5_gab 15 //1sec delay
int8 ss=0,hs=0,status=0;
int16 on_counter,off_counter,off_gab_counter;
int16 kl15,mode,duty=0,set;
void soft_start()
{
if(ss==1)
{
duty++;
if(duty>set)
duty=set;
}
else
{
duty=0;
}
}
void hid_start()
{
if(hs==1)
{
duty=set;
}
else
{
duty=0;
}
}
#int_RTCC
void RTCC_isr()
{
if(kl15>1017) //overvoltage protection (over 4.98V)
{
setup_ccp1(CCP_OFF);
output_low(PIN_A2);
output_low(PIN_A4);
duty=0;
}
else
{
if(input(PIN_A3)==1) //if gab is on
{
off_gab_counter--;
if(!off_gab_counter )
{
off_gab_counter = delay_off_5_gab;
setup_ccp1(CCP_OFF);
output_low(PIN_A2);
output_low(PIN_A4);
duty=0;
ss=0;
hs=0;
status=0;
}
}
else
{
if(kl15>=700 && input(PIN_A5)==0 && status==0)
{
if(kl15>=820)
{
setup_ccp1(CCP_PWM);
on_counter--;
if(!on_counter )
{
on_counter = delay_on;
ss=1;
hs=1;
output_high(PIN_A4);
}
}
}
else if(kl15>=700 && status==1)
{
if(kl15>=820)
{
setup_ccp1(CCP_PWM);
on_counter--;
if(!on_counter )
{
on_counter = delay_on;
ss=1;
hs=1;
output_high(PIN_A4);
}
}
}
else if(kl15<700)
{
off_counter--;
if(!off_counter )
{
off_counter = delay_off_5;
setup_ccp1(CCP_OFF);
output_low(PIN_A2);
output_low(PIN_A4);
duty=0;
ss=0;
hs=0;
status=0;
}
}
else
{
ss=0;
hs=0;
duty=0;
status=0;
setup_ccp1(CCP_OFF);
output_low(PIN_A4);
}
}
}
}
void main()
{
on_counter = delay_on;
off_counter = delay_off_5;
off_gab_counter = delay_off_5_gab;
output_low(PIN_A2);
output_low(PIN_A4);
setup_timer_0(RTCC_INTERNAL|RTCC_DIV_256|RTCC_8_BIT);
enable_interrupts(INT_RTCC);
setup_adc_ports(sAN0 || sAN1);
setup_adc(ADC_CLOCK_DIV_8);
setup_ccp1(CCP_PWM);
set_pwm1_duty(0);
setup_timer_2(T2_DIV_BY_16, 249, 1); //250Hz
enable_interrupts(GLOBAL);
while(true)
{
set_pwm1_duty(duty);
set_adc_channel(0);
delay_us(25);
kl15=read_adc();
set_adc_channel(1);
delay_us(25);
mode=read_adc();
if(mode>260 && mode<325)
{
set=250;
status=0;
soft_start();
}
if(mode>430 && mode<495)
{
set=300;
status=0;
soft_start();
}
if(mode>820 && mode<870)
{
set=700;
status=0;
soft_start();
}
if(mode>890 && mode<955)
{
set=1023;
hid_start();
status=1;
}
delay_ms(10);
}
}
|
|
|
|
temtronic
Joined: 01 Jul 2010 Posts: 9246 Location: Greensville,Ontario
|
|
Posted: Wed Sep 05, 2012 5:04 am |
|
|
You've got WAY too much code in the ISR !!
ISRs must be small and simple. Just reset/set a flag or two,set a few(<5) variables and get out fast!
In 'main', check the 'flag' variables and at accordingly..
Trying to do a lot of code inside the ISR will 'stall' or 'delay' the actual overall program or as you've found out , give random timings.
hth
jay |
|
|
Mike Walne
Joined: 19 Feb 2004 Posts: 1785 Location: Boston Spa UK
|
|
Posted: Wed Sep 05, 2012 5:17 am |
|
|
Please explain how you think the 5s delays are created.
Mike |
|
|
ckielstra
Joined: 18 Mar 2004 Posts: 3680 Location: The Netherlands
|
|
Posted: Wed Sep 05, 2012 5:32 am |
|
|
I do agree with Jay that your ISR should be made smaller and faster, but I doubt this is the source of your problem. Your ISR is only executed every 65536 instruction cycles, that should be long enough to allow some processing.
More likely the problem is in your decision tree inside the ISR. This spaghetti code of if-the-else constructions is very difficult to follow and easy to overlook one execution path. A better design is to use the technique known as 'State machine'. Basically it is a method to first draw on paper all your allowed 'stages' and allowed 'transitions'. Then explicitly convert this drawing to code. Search the internet for more information and code examples.
Problem with your approach is that all allowed transitions are _implicit_ in the code and this is for us humans hard to follow in all details. |
|
|
tonkostz
Joined: 07 May 2011 Posts: 42 Location: Bulgaria
|
|
Posted: Wed Sep 05, 2012 11:20 am |
|
|
Mike Walne wrote: | Please explain how you think the 5s delays are created.
Mike |
Timer0 will overflow after 65.5ms. So 76x65.5ms=4978ms. Am i wrong? |
|
|
PCM programmer
Joined: 06 Sep 2003 Posts: 21708
|
|
Posted: Wed Sep 05, 2012 11:43 am |
|
|
Quote: |
setup_adc_ports(sAN0 || sAN1);
|
The || operatior is not the same as | and will have undesired results,
such as one of the pins above not being configured for analog input.
Quote: |
//time delay constants
#define delay_on 76 //5sec on delay
#define delay_off_5 76 //5sec off delay
#define delay_off_5_gab 15 //1sec delay
on_counter = delay_on;
off_counter = delay_off_5;
off_gab_counter = delay_off_5_gab;
|
The C style convention is to put constants in all caps. In the code above,
this is not done and the visual cue is missing. It appears to be loading
variables from other variables.
See section 11 - Naming Conventions on page 13:
http://www.literateprogramming.com/indhill-cstyle.pdf |
|
|
Mike Walne
Joined: 19 Feb 2004 Posts: 1785 Location: Boston Spa UK
|
|
Posted: Thu Sep 06, 2012 9:29 am |
|
|
Quote: | Timer0 will overflow after 65.5ms. So 76x65.5ms=4978ms. Am i wrong? |
Yes, I get the 65.5ms part.
BUT, somehow you are wanting to wait for 76 of these 65.5ms intervals.
You are complaining you get ~3s delays rather than 5s.
So, I presume that something goes wrong in the way you count off 76 of your 65.5ms intervals.
What I was asking for, was a lucid explantion of HOW the counting is achieved.
This problem is your's not mine. I don't want to decipher all of your code to arrive at the answer. I want you to either work it out for yourself, or give me enough guidance to be able to help you.
Mike |
|
|
|