Hi Rex, Firstly, as a first attempt, you seem to have grasped a lot of concepts. Your code is readable, but could be improved by using the include file definitions with your project, seeing as how you have included the 16F84a.inc file with your code. Thus... bcf INTCON,2 becomes... bcf INTCON,T0IF This is much more understandable. Also, the __CONFIG directive is not easy to decypher... __CONFIG H'3FFA' This is better... __CONFIG CP_OFF & PWRTE_OFF & WDT_OFF & HS_OSC Notice that you have the WDT disabled. Therefore, you don't need CLRWDT instructions in your code. The interrupt routine will work in your case, because the main loop doesn't access the W reg or STATUS. However, if you need these registers in the main loop the interrupt routine will have to be changed. If your interrupt routine alters the W and STATUS registers then you should preserve their values before executing the interrupt code. At the end of the interrupt routine, restore them back to their original condition. Intsvc movwf W_Hold ; store w reg contents swapf STATUS,W clrf STATUS ; ram page 0 movwf S_Hold ; store STATUS contents ExIRQ swapf S_Hold,W ; restore STATUS movwf STATUS swapf W_Hold ; W reg restored from rp0 or rp1 swapf W_Hold,W retfie Now, all of your RETFIE instructions, except for the last one, will have to be changed to... goto ExIRQ ... so that the W and STATUS are restored on exit. You have a RLF PORTB,F instruction after the seconds register is incremented. I don't see how this flashes the LED every 7 seconds. This may work better... incf Seconds,f ;increment seconds finally decfsz Sec_7 ; decrement 7 sec counter goto NoFlash ; not 0 yet movlw 7h ; = 0 movwf Sec_7 ; restore 7 sec counter movlw b'00010000' ; toggle LED xorwf PORTB NoFlash clrf timer_lo ;clear LSB .... Sec_7 is initialised to 7h at start up. Try enabling the interrupt after you have set the OPTION_REG bsf STATUS,RP0 movlw b'11011000' ; no pullups, internal clock movwf OPTION_REG ; prescaler to WDT, rate = 1:2 bcf STATUS,RP0 clrf TMR0 bcf INTCON,T0IF ; clear TMR0 interrupt flag bsf INTCON,T0IE ; enable TMR0 interrupt bsf INTCON,GIE ; enable global interrupt I didn't see where you initialised PORTA. Even though you don't seem to use it, it is wise to set the unused pins as outputs to stop them floating and cause trouble. Port_Init clrf PORTA ; IO pins are all output, zero clrf PORTB bsf STATUS,RP0 clrf TRISA clrf TRISB bcf STATUS,RP0 return Notice the RETURN instruction. You can use this at the end of subroutines in the 16F84A instead of RETLW 0h, if you like. I hope this helps. -- Best regards Tony mICros http://www.bubblesoftonline.com mailto:sales@bubblesoftonline.com Rex Byrns wrote: > > Hi everyone. The following is my first attempt at a program. I built the > NOPPP and everything looks great. I am using a 20mghz ceramic resonator and > my breadboard circuit is crude. All IO are open except for the RB4 to which > I have connected an LED. It flashes every 16 seconds for 2 seconds. The > MPLAB simulator runs as planned - Flash every 8 seconds for one. > > I would welcome any comments on code style, technique etc. as well. > > Thanks for your time... > > LIST P=PIC16F84A, R=d > __config H'3FFA' > > #include "p16f84a.inc" > > ScratchPadRam EQU 0x20 > TMR0 EQU H'01' > STATUS EQU H'03' > #DEFINE RB4 4 > ;-------------------------------------------------------------------------- > ; Variables > ;-------------------------------------------------------------------------- > cblock 0x0C > timer_lo ;LSB for period counter > timer_hi ;MSB for period counter > Seconds ;seconds > Minutes ;minutes 0-59 > Hours ;hours > Quant1 ;number of counts for feeder 1 > Quant2 ;number of counts for feeder 2 > Quant3 ;number of counts for feeder 3 > Quant4 ;number of counts for feeder 4 > DispenCnt ;int? input from counter > endc > > ;-------------------------------------------------------------------------- > ;Start of ROM > ;-------------------------------------------------------------------------- > > ORG 0 > GOTO Start > > ;---------------------------------------- > ;Interrupt Service Routine > ;---------------------------------------- > ORG 0x04 > Intsvc > clrwdt ;clear watchdog timer to ensure startup > bcf INTCON,2 ;clear interrupt flag before return > incf timer_lo,f ;increment LSByte > btfsc STATUS,Z ;zero set means overflow > incf timer_hi,f ;increment MSByte > movlw H'4B' ;LSB tweak factor 4996891 > xorwf timer_lo,w ;compare > btfss STATUS,Z ;are they the same? > retfie ;nope go back > movlw H'4C' ;MSB tweak factor > xorwf timer_hi,w ;compare > btfss STATUS,Z ;are they the same? > retfie ;nope go back > incf Seconds,f ;increment seconds finally > rlf PORTB,f ;every 7 seconds portb4 should flash > clrf timer_lo ;clear LSB > clrf timer_hi ;clear MSB > movlw H'3C' ;get ready to check for a minute > xorwf Seconds,w ;compare > btfss STATUS,Z ;are they the same? > retfie ;nope go back > incf Minutes,f ;yes-add a minute > clrf Seconds ;clear Seconds - at 60 > movlw H'3C' ;get ready to check for an hour > xorwf Minutes,w ;compare > btfss STATUS,Z ;are they the same? > retfie ;nope go back > incf Hours,f ;yes - another hour > clrf Minutes ;clear Minutes - at 60 > movlw H'18' ;check for hour roll > xorwf Hours,w ;compare > btfss STATUS,Z ;are they the same? > retfie ;not yet midnight > clrf Hours ;it is midnight > retfie ;return from interrupt > > Timer_init > bcf INTCON, 2 > bsf INTCON, 7 > bsf INTCON, 5 > clrf TMR0 > clrwdt > movlw b'11011000' > option > clrf TMR0 > retlw 0 > > Port_Init > BCF STATUS, RP0 ; > CLRF PORTB ; Initialize PORTB > BSF STATUS, RP0 ; Select Bank 1 > MOVLW H'00' ; Value used to > MOVWF TRISB ; Set RB<3:0> as inputs RB<5:4> as outputs RB<7:6> as inputs > > Start > call Timer_init > Loop: nop > nop > goto Loop > end > > -- > http://www.piclist.com hint: The list server can filter out subtopics > (like ads or off topics) for you. See http://www.piclist.com/#topics -- http://www.piclist.com hint: The list server can filter out subtopics (like ads or off topics) for you. See http://www.piclist.com/#topics