 |
 |
| View previous topic :: View next topic |
| Author |
Message |
5440
Joined: 08 Feb 2009
Posts: 11
|
| Issue with <= statement |
 Posted: Wed Dec 22, 2010 11:59 pm |
 |
|
Well I am trying to create a general timer array using TMR1 with a PIC18F2585. I config'd TMR1 with a 1ms interrupt and inc's a counter. So far so good as I scoped the TMR1 int at 1ms.
I toggled a LED every 500ms and made a cheap PWM routine to use this general timer routine, when I noticed every now and then the LED and PWM would have small timing clitches ( a pulse is missed or very short). I debug using MPLAB 8.60 and seems to have an issue with a <= in my the Check Timer routine.
The idea is to set a done bit once the TMR1 over flow counter reaches the calculated T2 value. I use <= in case my scan exceeds 1ms so I can still catch it as this is a cheap, general timer routine for debouncing, blinking LED's, a cheap PWM for dimming bulbs etc.
It works well if coded as ==. I set a break point once Check Timer thinks the DN=1, but with <=, it "randomly" evaluates as true even though TMR1_OF_Cnt is less than .T2. This occurs before TMR1_OF_Cnt overflows. Note I tried to buffer .T2 and the counter in the code presented to see if that was an issue.
I roll my own, even the header file. I stripped out code so that only relevant , compilable code is present, but it may still be a bit messy.
| Code: |
/*****************************************************************************
Cruise Control
CCS Compiler ver 4.106
PIC18F2585
Dec 10-2010
Project started
******************************************************************************/
//#include <PIC18F2585.h>
#device PIC18F2585
#device *=16
#device ICD=TRUE
//#device HIGH_INTS=TRUE
#ifdef defined (__PCH__)
#fuses EC,NOPROTECT,NOLVP,NOWDT,NOMCLR,NODEBUG
#endif
#ignore_warnings ALL
#fuses DEBUG
#Define OSC_20MHZ 0x1
#Define SysClk OSC_20MHZ //sets the oscillator speed
#if(SysClk==OSC_20MHZ)
#use delay(clock=20000000)
// Clk_Mult (for Over flow)=desired ms/Tosc*prescaler**ENSURE less than 16bit (65535)
#Define TMR0_Clk_Mult_ms 7500 // cyc for 2ms with x2 prescale
#Define TMR0_msCnt 65535-TMR0_Clk_Mult_ms+15 //50 to account for some ISR overhead
#Define TMR1_Clk_Mult_ms 5000 // cyc for 1ms with x1 prescale
#Define TMR1_msCnt 65535-TMR1_Clk_Mult_ms+15 //50 to account for some ISR overhead
#endif
//General Pupose timer config
#Define MAXTIMERS 1
#Define LED_Tim 0 //GP Timer[3] array
////// TIMER///////
#Define BITS_8 255
#Define BITS_16 65535
#Define BITS_32 4294967296
#Define ClkSize Bits_16
#Define TMRX_Prescale 1
#if(ClkSize==Bits_8)
#Define Tim_Size 255
#endif
#if(ClkSize==Bits_16)
#Define Tim_Size 65535
#endif
#if(ClkSize==Bits_32)
#Define Tim_Size 4294967296
#endif
struct Ports{
unsigned char B0:1;
unsigned char B1:1;
unsigned char B2:1;
unsigned char B3:1;
unsigned char B4:1;
unsigned char B5:1;
unsigned char B6:1;
unsigned char B7:1;
}PORTA_Bits;
struct Timer{
INT16 T2;
unsigned char DN:1; //done
unsigned char EN:1; //enabled
unsigned char OF:1; //OverFlow
unsigned char TT:1; //timer timing
unsigned char SPARE1:1;
unsigned char SPARE2:1;
unsigned char SPARE3:1;
unsigned char SPARE4:1;
}Timer_Array[MAXTimers];
///// REGISTERS //////Place after struct
#byte PORTA=0XF80
#byte TRISA=0xF92
#byte LATA=0xF89
#byte PORTB=0XF81
#byte TRISB=0XF93
#byte CMCON=0xFB4
#byte PORTA_Bits=0xF80
#byte INTCON=0XFF2
#byte INTCON2=0XFF1
#byte INTCON3=0XFF0
#byte TMR0H=0XFD7
#byte TMR0L=0XFD6
#byte TMR1H=0XFCF
#byte TMR1L=0XFCE
#byte T1CON=0XFCD
#byte ADCON0=0XFC2
#byte ADCON1=0XFC1
#byte ADCON2=0XFC0
#byte PIR3=0XFA4
#byte PIE3=0XFA3
#byte IPR2=0XFA2
#byte PIR2=0XFA1
#byte PIE2=0XFA0
#byte T1CON=0XFCD
#byte T0CON=0XFD5
#byte IPR1=0XF9F
#byte PIR1=0XF9E
#byte PIE1=0XF9D
#byte LATA=0XF89
#byte RCON=0XFD0
#byte BSR=0XFE0
////bits////////
#bit GIE=INTCON.7
#bit PEIE=INTCON.6
#bit INT0IF=INTCON.1
#bit INTEDG0=INTCON2.6
#bit INTEDG1=INTCON2.5
#bit INTEDG2=INTCON2.4
#bit INT2IP=INTCON3.7
#bit INT1IP=INTCON3.6
#bit INT2IE=INTCON3.4
#bit INT1IE=INTCON3.3
#bit INT2IF=INTCON3.1
#bit INT1IF=INTCON3.0
#bit TMR1IF=PIR1.0
#bit TMR1IE=PIE1.0
#bit TMR1IP=IPR1.0
#bit IPEN=RCON.7
#bit TMR1ON=T1CON.0
#bit INT0IE=INTCON.4
#bit TMR0IF=INTCON.2
#bit TMR0IE=INTCON.5
#bit TMR0ON=T0CON.7
#bit T08BIT=T0CON.6
#bit TMR0IP=INTCON2.2
#Define PWM_Pin PORTA_Bits.B0 //
#Define LED PORTA_Bits.B1 //
///////////////////////// Prototypes ///////////////////////////
int16 TimSetup_ms(int16 Desired_ms,struct Timer *Timer_ms);
void CheckTimer(struct Timer *TA);
void TMR1_ISR();
////////////////// Defines /////////////////////////////////////
#define BYTE int
#define BOOLEAN short int
#define FALSE 0
union PICReg{
unsigned char Ch[2];
int16 I;
}T1,T2,TMR1_msLoad,TMR0_msLoad;
int16 TMR1_OF_Cnt=0,TMR0_OF_Cnt=0;
/////////////// INTERUPTS //////////////////////////
/* High Priority (HP) INTS vector to 0x000008 if IPEN set and Each INT is set a HP
Low Priority INTS vector to 0x000018 */
void TMR0_ISR();
void TMR1_ISR();
void INT_0_ISR();
//#Define HIGH 1
#DEFINE TMR0_PRIORITY 0 //LOW
#DEFINE TMR1_PRIORITY 1// HIGH
int save_w;
#locate save_w=0x80
int save_status;
#locate save_status=0x7F
int bsr_temp;
#locate save_bsr=0x7E
int save_temp1;
#locate save_temp1=0x7D
int save_temp2;
#locate save_temp2=0x7C
int save_temp3;
#locate save_temp3=0x7B
int save_temp4;
#locate save_temp4=0x7A
#byte status = 0xFD8
#bit zero_flag = status.2
#INT_GLOBAL
void isr() { //VECTOR ADX 0X08
#asm
//store current state of processor
goto High; //0x08 Vector
nop
nop
nop
nop
nop
nop
nop
#endasm
goto Low; //0x18 Vector (low priority vector when high is enabled)
#asm
High: /// HIGH PRIORITY ///
#endasm
if(TMR1IE){//TMR1 INT enabled?
if(TMR1IF)//TMR1 OF?
TMR1IF=0;
TMR1_ISR();
goto Restore_Hi;
} //end TMR1 check
if(INT0IE){//INT0 INT enabled?
if(INT0IF)//INT0?
INT0IF=0;
INT_0_ISR();
goto Restore_Hi;
} //end TMR1 check
goto Restore_Hi;//default if not Hi priority INT detected
Low: /// LOW PRIORITY ///
#asm
MOVWF save_w
SWAPF status,W
BCF status,5
BCF status,6
MOVWF save_status
movff BSR, save_bsr;
#endasm
if(TMR0IE){//TMR0 INT enabled?
if(TMR0IF)//TMR0 OF?
TMR0IF=0;
TMR0_ISR();
} //end TMR1 check
Restore_Low:
#asm
movff save_bsr,BSR
SWAPF save_status,W
MOVWF status
SWAPF save_w,F
SWAPF save_w,W
#endasm
Restore_Hi:
#asm
nop
#endasm
}
/// HIGH INTS ///
void INT_0_ISR(){ //deafualts as high priority
TMR0_OF_Cnt++; //test
}//end TMR1 ISR
void TMR1_ISR(){ //set as high priority
TMR1H=TMR1_msLoad.ch[1]; //1ms INT
TMR1L=TMR1_msLoad.ch[0];
TMR1_OF_Cnt++;
// Test_2^=1;
}//end TMR1 ISR
/// LOW INTS ///
void TMR0_ISR(){ //set as low priority
TMR0H=TMR0_msLoad.ch[1]; //1ms INT
TMR0L=TMR0_msLoad.ch[0];;
TMR0_OF_Cnt++;
}//end TMR0 ISR
/******************************************************************************
MAIN
******************************************************************************/
void main(void){
int16 Counter=0;
PORTA=0x00;
TRISA=0x00;
PORTB=0x00;
TRISB=0xFF;
TMR1_msLoad.I=TMR1_msCnt;
TMR0_msLoad.I=TMR0_msCnt;
T0CON=0b00000000;
TMR0ON=1; //TURN ON TMR0
T08BIT=0; // 0 is 16bit mode for TMRO (1 is 16 bit read/write TMR1 & TMR3)
T1CON=0b10000000; //ENABLE 16 BIT MODE TMR1, NO PRESCALE
TMR1ON=1; //TURN ON TMR1
PIR1=0b00000000;
TMR0IE=1; //enable TMR0 INT
TMR1IE=1; //enable TMR1 INT
INT0IE=0; //ENABLE INT0
TMR0IP=TMR0_PRIORITY;
TMR1IP=TMR1_PRIORITY;
IPEN=1; // high priority ints enabled
IPEN=1; //ENABLE HIGH PRIORITY INTS
PEIE=1; //ENABLE ALL UNMASKED INTS
GIE=1;//ENABLE GLOBAL INTS
ADCON0=0x00;
ADCON1=0x0F; //digital I/O
while(1){
Timer_Array[LED_Tim].EN=1;
if(Timer_Array[LED_Tim].DN){
if(!LED)
LED=1;
else
LED=0;
}
if(!Timer_Array[LED_Tim].TT && Timer_Array[LED_Tim].EN)
TimSetup_ms(500,&Timer_Array[LED_Tim]);
CheckTimer(&Timer_Array);
}// end while loop
}// end Main
//////////////////////////////////////////////
int16 TimSetup_ms(int16 Desired_ms,struct Timer *Timer_ms){
int16 TempA;
TempA=Bits_16-TMR1_OF_Cnt;
if(Desired_ms>TempA){ //OF will occur
Timer_ms->T2=Desired_ms-TempA-1;// 1 to account for zero on OF
Timer_ms->OF=1;
}
else{//no OF
Timer_ms->T2=Desired_ms+TMR1_OF_Cnt;
}
Timer_ms->DN=0; //ensire DN cleared
Timer_ms->TT=1;
// Timer_ms->EN=1;
}
//////////////////////////////////
// CheckTimer
// Timer array passed where if enabled
//
// DN set when timer times out
// Expect test (done elsewhere) to clear EN & DV
////////////////////////////////
void CheckTimer(struct Timer *TA){
int Count;
int16 Temp_Cnt_OF,Temp_T2;// buffer counter
for(Count=0;Count<MAXTIMERS;Count++){
Temp_Cnt_OF=TMR1_OF_Cnt;///TEST
Temp_T2=TA[Count].T2;
if(TA[Count].OF){
if(TMR1_OF_Cnt<=TA[Count].T2)//wait until OFCounter OF's
TA[Count].OF=0;
}
if( TA[Count].DN && !TA[Count].TT) //.TT=0 from previous scan in CheckTimer
TA[Count].DN=0; //clear .DN after one DN=1 scan in main
if( Temp_T2<=Temp_Cnt_OF && !TA[Count].OF && !TA[Count].DN && TA[Count].EN && TA[Count].TT){
TA[Count].DN=1; //allow main logic one scan to test .DN
TA[Count].TT=0; //timer done, allows for next setup
if(TMR1_OF_Cnt!=TA[Count].T2)//TEST
Temp_T2=1;
}//end if
if(!TA[Count].EN){ //clear bits if not enabled
TA[Count].DN=0;
TA[Count].TT=0;
}//end if
}//end for
}//end fn
|
|
|
 |
FvM
Joined: 27 Aug 2008
Posts: 2337
Location: Germany
|
|
| Posted: Thu Dec 23, 2010 12:22 am |
|
|
There may be other issues with your code, but one is this:
Your updating 16-Bit variables in the timer ISR and reading them in the main loop. Accesses to 16 Bit variables are splitted
into multiple machine instructions on a 8-Bit processor, they can't be expected to be consistent across ISR calls.
For a save variable access, you have to disable interrupts temporarily. |
|
|
RayJones
Joined: 20 Aug 2008
Posts: 30
Location: Melbourne, Australia
|
|
| Posted: Sun Dec 26, 2010 2:05 pm |
|
|
I think more importantly you should only be reading the timer value once, and work from the stored value in your timer test routine.
The way I am reading the code in the source as published is you are reading the timer counter several times to make logical decisions upon its value.
The problem is the counter is continuing to count irrespective of disabling interrupts so this value is very volatile.
Read it once at the start, then only work with the "shadow" copy and see how you go... |
|
|
5440
Joined: 08 Feb 2009
Posts: 11
|
|
| Posted: Sun Dec 26, 2010 8:03 pm |
|
|
| RayJones wrote: | I think more importantly you should only be reading the timer value once, and work from the stored value in your timer test routine.
The way I am reading the code in the source as published is you are reading the timer counter several times to make logical decisions upon its value.
The problem is the counter is continuing to count irrespective of disabling interrupts so this value is very volatile.
Read it once at the start, then only work with the "shadow" copy and see how you go... |
I changed code to read it once, which is a good point, but still same issues with it randomly thinking the <= statement is true when the value is less than. |
|
|
FvM
Joined: 27 Aug 2008
Posts: 2337
Location: Germany
|
|
| Posted: Mon Dec 27, 2010 2:39 am |
|
|
| Only caring for the so called "more important" issue means getting occasional code failure for sure. So you better think about the mentioned 16 bit variable consistency problem. |
|
|
|
|
You cannot post new topics in this forum
You cannot reply to topics in this forum
You cannot edit your posts in this forum
You cannot delete your posts in this forum
You cannot vote in polls in this forum
|
Powered by phpBB © 2001, 2005 phpBB Group
|
FAQ 
Search
Register
Profile 
Log in to check your private messages 
Log in
