Coding problems on 16f88

[IMSlo]

I have put together a simple timer from code I have downloaded. It runs on a timer0 interrupt and displays elapsed time on a 16 X 2 LCD. I am trying to run the code on a 16f88. It will run for a few seconds, zero up, run for a few seconds, zero up and maybe even run for a few minutes and zero up. Eventually it quits some times leaving numbers in the display other time leaving the display blank. This code with the minor necessary changes will run flawlessly for days on a 16f627a. I am setup on a breadboard using only the PIC, a voltage regulator and a LCD. I am using the internal clock on the PIC. I am inclosing the code. Maybe someone will look at the code and tell me what the problem is.

;******************************************************************************
; ZERO-ERROR ONE SECOND TIMER
; (Roman Black 2001, public domain, use it as you like)
;

;
;******************************************************************************


;==============================================================================
; processor defined ;
LIST p=16F88 ;tell assembler what chip we are using
include "P16F88.inc" ;include the defaults for the chip
ERRORLEVEL 0, -302 ;suppress bank selection messages


;==============================================================================
; MPLAB stuff here

LIST b=5, n=97, t=ON, st=OFF
; absolute listing tabs=5, lines=97, trim long lines=ON, symbol table=OFF

__CONFIG _CONFIG1, _CP_OFF & _CCP1_RB0 & _DEBUG_OFF & _WRT_PROTECT_OFF & _CPD_OFF & _LVP_OFF & _BODEN_OFF & _MCLR_OFF & _PWRTE_ON & _WDT_OFF & _INTRC_IO

;Program Configuration Register 2
__CONFIG _CONFIG2, _IESO_OFF & _FCMEN_OFF

LCD_PORT Equ PORTB
LCD_TRIS Equ TRISB
LCD_RS Equ 0x04 ;LCD handshake lines
LCD_RW Equ 0x06
LCD_E Equ 0x07


;==============================================================================
; Variables here

CBLOCK 0x20
sec
sec10
min
min10
hr
hr10
bres_hi ; hi byte of our 24bit variable
bres_mid ; mid byte
bres_lo ; lo byte
; (we only need 3 bytes for this system)

status_temp ; used for interrupt servicing
w_temp ; used for interrupt servicing
templcd ;temp store for 4 bit mode
templcd2
count ;used in looping routines
count1 ;used in delay routine
counta ;used in delay routine
countb ;used in delay routine
oldtime
ENDC


;==============================================================================
; Code here

org 0x000 ; Set program memory base at reset vector 0x000
reset
goto setup ; set up ints and port stuff

org 0x004 ; Interrupt vector, int handler code comes next.
;==============================================================================

;******************************************************************************
; INTERRUPT HANDLER (runs this code each timer0 interrupt)
;******************************************************************************
;
;------------------
int_handler
;
;------------------

;-------------------------------------------------
; first we preserve w and status register

movwf w_temp ; save off current W register contents
movf STATUS,w ; move status register into W register
movwf status_temp ; save off contents of STATUS register



tstf bres_mid ; first test for mid==0
skpnz ; nz = no underflow needed
decf bres_hi,f ; z, so is underflow, so dec the msb

decfsz bres_mid,f ; dec the mid byte (subtract 256)


goto int_exit ; nz, so definitely not one second yet.

tstf bres_hi ; test hi for zero too
skpz ; z = both hi and mid are zero, is one second!
goto int_exit ; nz, so not one second yet.



movlw 0x0F ; get msb value
movwf bres_hi ; load in msb

movlw 0x42 ; get mid value
movwf bres_mid ; load in mid

movlw 0x40 ; lsb value to add
addwf bres_lo,f ; add it to the remainder already in lsb
skpnc ; nc = no overflow, so mid is still ok

incf bres_mid,f ; c, so lsb overflowed, so inc mid

; movlw b'00001000' ; mask for bit 3
; xorwf PORTB,f ; toggle PORTA,bit3 (toggle the led)

movlw sec ; point at sec register
movwf FSR
newdigit: incf INDF, f ; current digit up one
movlw sec ; get difference between sec and FSR
subwf FSR, W
call sethi ; use to get high limit + 1
subwf INDF, W ; reached that number yet?
btfss STATUS, Z ; skip over if yes
goto int_exit ; else exit isr
clrf INDF ; set current digit to 0
incf FSR, f ; point at next digit
goto newdigit ; no, increment the next digit



int_exit
BCF INTCON,2 ; reset the tmr0 interrupt flag
bcf STATUS,Z
movf status_temp,w ; retrieve copy of STATUS register
movwf STATUS ; restore pre-isr STATUS register contents
swapf w_temp,f
swapf w_temp,w ; restore pre-isr W register contents

retfie ; return from interrupt

;------------------------------------------------------------------------------
;-------------------------------------------------------------------------;
; High limit + 1 of digits at position W ;
;-------------------------------------------------------------------------;
sethi:
addwf PCL, f
dt H'A',H'6',H'A',H'6',H'A',H'A'



;******************************************************************************
; SETUP (runs this only once at startup)
;******************************************************************************


;
;------------------
setup ; goto label
;------------------

movlw B'01100000' ; 4Mhz = B'01100000'
banksel OSCCON
movwf OSCCON
banksel 0
;Wait until osc stabilizes
lp:
btfss OSCCON, 2
goto lp



; OPTION setup
movlw b'00001000' ;

; -------x ;
; Note! We set the prescaler to the wdt, so timer0


; has NO prescaler and will overflow every 256
; instructions and make an interrupt.
;


banksel OPTION_REG ; go proper reg bank
movwf OPTION_REG ; load data into OPTION_REG
banksel 0 ; back to normal bank 0
;-------------------------------------------------
; PORTB pins direction setup
; 1=input, 0=output
clrf PORTB ;
;
movlw b'00000000' ; all 8 portb are outputs
;
banksel TRISB ; go proper reg bank
movwf TRISB ; send mask to portb
banksel 0 ; back to normal reg bank
;-------------------------------------------------
; PORTA pins direction setup
; 1=input, 0=output
clrf PORTA ;
;
movlw b'00000000' ; all 5 porta are outputs,
; (with 16F84 porta only has lower 5 bits)
;
banksel TRISA ; go proper reg bank
movwf TRISA ; send mask to porta
banksel 0 ; back to normal reg bank
;-------------------------------------------------

movlw b'11100000' ; GIE=on TOIE=on (timer0 overflow int)
; bsf INTCON,7
; bsf INTCON,6
; bsf INTCON,5
banksel INTCON
movwf INTCON
banksel 0
clrf sec
clrf sec10
clrf min
clrf min10
clrf hr
clrf hr10


movlw 0x0F ; get msb value
movwf bres_hi ; put in hi

movlw 0x42 +1 ; get mid value (note we added 1 to it)
movwf bres_mid ; put in mid

movlw 0x40 ; get lsb value
movwf bres_lo ; put in mid




; now setup is complete, we can start execution.
;-------------------------------------------------
goto main ; start main program

;------------------------------------------------------------------------------
;Initialise LCD
LCD_Init call Delay100 ;wait for LCD to settle

movlw 0x20 ;Set 4 bit mode
call LCD_Cmd

movlw 0x28 ;Set display shift
call LCD_Cmd

movlw 0x06 ;Set display character mode
call LCD_Cmd

movlw 0x0c ;Set display on/off and cursor command
call LCD_Cmd ;Set cursor off

call LCD_Clr ;clear display

retlw 0x00
; command set routine
LCD_Cmd movwf templcd
swapf templcd, w ;send upper nibble
andlw 0x0f ;clear upper 4 bits of W
movwf LCD_PORT
bcf LCD_PORT, LCD_RS ;RS line to 1
call Pulse_e ;Pulse the E line high

movf templcd, w ;send lower nibble
andlw 0x0f ;clear upper 4 bits of W
movwf LCD_PORT
bcf LCD_PORT, LCD_RS ;RS line to 1
call Pulse_e ;Pulse the E line high
call Delay5
retlw 0x00

LCD_CharD addlw 0x30 ;add 0x30 to convert to ASCII
LCD_Char movwf templcd
swapf templcd, w ;send upper nibble
andlw 0x0f ;clear upper 4 bits of W
movwf LCD_PORT
bsf LCD_PORT, LCD_RS ;RS line to 1
call Pulse_e ;Pulse the E line high

movf templcd, w ;send lower nibble
andlw 0x0f ;clear upper 4 bits of W
movwf LCD_PORT
bsf LCD_PORT, LCD_RS ;RS line to 1
call Pulse_e ;Pulse the E line high
call Delay5
retlw 0x00

LCD_Line1 movlw 0x80 ;move to 1st row, first column
call LCD_Cmd
retlw 0x00

LCD_Line2 movlw 0xc0 ;move to 2nd row, first column
call LCD_Cmd
retlw 0x00

LCD_Line1W addlw 0x80 ;move to 1st row, column W
call LCD_Cmd
retlw 0x00

LCD_Line2W addlw 0xc0 ;move to 2nd row, column W
call LCD_Cmd
retlw 0x00

LCD_CurOn movlw 0x0d ;Set display on/off and cursor command
call LCD_Cmd
retlw 0x00

LCD_CurOff movlw 0x0c ;Set display on/off and cursor command
call LCD_Cmd
retlw 0x00

LCD_Clr movlw 0x01 ;Clear display
call LCD_Cmd
retlw 0x00

;LCD_HEX movwf tmp1
; swapf tmp1, w
; andlw 0x0f
; call HEX_Table
; call LCD_Char
; movf tmp1, w
; andlw 0x0f
; call HEX_Table
; call LCD_Char
; retlw 0x00

Delay255 movlw 0xff ;delay 255 mS
goto d0
Delay100 movlw d'100' ;delay 100mS
goto d0
Delay50 movlw d'50' ;delay 50mS
goto d0
Delay20 movlw d'20' ;delay 20mS
goto d0
Delay5 movlw 0x05 ;delay 5.000 ms (4 MHz clock)
d0 movwf count1
d1 movlw 0xC7 ;delay 1mS
movwf counta
movlw 0x01
movwf countb
Delay_0
decfsz counta, f
goto $+2
decfsz countb, f
goto Delay_0

decfsz count1 ,f
goto d1
retlw 0x00

Pulse_e bsf LCD_PORT, LCD_E
nop
bcf LCD_PORT, LCD_E
retlw 0x00

;end of LCD routines

DispTime
MOVLW 0x04
CALL LCD_Line1W
movf hr10,w
CALL LCD_CharD
movf hr,w
CALL LCD_CharD
MOVLW ":"
CALL LCD_Char
movf min10,w
CALL LCD_CharD
movf min,w
CALL LCD_CharD
MOVLW ":"
CALL LCD_Char
movf sec10,10
CALL LCD_CharD
movf sec,w
CALL LCD_CharD
RETURN



;*****************************************************************************


;******************************************************************************
; MAIN (main program loop)
;******************************************************************************
;
;------------------
main ; goto label
;------------------
call LCD_Init

main_loop ;
movf oldtime, W ; is oldtime the same as sec?
subwf sec, W
btfsc STATUS, Z ; if not, skip over next instruction
goto main_loop ; else continue checking
call DispTime ; sec has changed, display the time
movf sec, W ; make sec and oldsec the same
movwf oldtime

; or calls to the main program pieces.
; The interrupt does all the one second timer stuff.
;
goto main_loop ; keep running the main code.

;------------------------------------------------------------------------------


;==============================================================================
end ; no code after this point.
;==============================================================================

 

[Pommie]

Can you edit your post and put code tags around your code. You'll find the code tags by clicking the go advanced button.

From your description, it sounds like your circuit is getting randomly reset. Try adding decoupling capacitors.

Mike.

 

[IMSlo]

Thanks for your reply Pommie.
The code I posted will compile and run (a little while). I would like for someone to run the code. I will edit and repost if it must be done. I got advise from another forum to add caps at VSS and VDD. I added these caps with no results. I might have installed them wrong. I am attaching a sketch to show their insstallation. Any help will be appreciated.

 

[philba]

"simple" code, eh?

your diagram is completely meaningless, by the way. you don't apply -5V to a PIC. maybe you meant the - ternminal of a battery?

sounds like the watchdog timer though you do disable it. do you have a pull-up on /mclr?

 

[IMSlo]

philba,

I am sorry my diagram was completely meaningless to you. I don't know much about electronics. I was only trying to show a cap connected in parallel to both the positive and negative sides of the power supply. If I am using the wrong terms again please excuse my political incorrectness I don't know any better.

I would like to get back to my problem now. Something is resetting the PIC. The Watchdog timer is disable. I don't think it has to do with the power supply because the transition to zero's on the display is too smooth and I have run other applications without problems. I think there is a timer or some setting that I don't know about causing the problem. This device seems to have many more registers that can be set than the 84 or the 628 as partly evidenced by the 2 Config statements.

Again, any help will be appreciated.

 

[philba]

no political correctness involved - schematics should be clear and show connections. I'd spend a bit of time studying how other schematics are done and try to follow what they do.

It is possible it is your code but frankly, there was so much and it is unformated, it's not worth dredging through.

I would suggest you verify your hardware first. Get an LED with a 470 ohm resistor. Connect it to PortB.0 (resistor to LED, LED to gnd) and try blinking it with a 1 second period. If the blinking is steady, then your hardware is probably OK.

 

[eblc1388]

Subtle bug in interrupt routine, on where the content of w_temp is saved.

At the end of interrupt handling, near the end of restoring context, the STATUS register is restored. The physical effect of this is to switch the current BANK back to the bank where the interrupt occurs.

If interrupt occurs when PIC is running codes in BANK1,2 or 3, then the value of w_temp, which is saved originally in BANK0, will not be accessible and will be wrong after restore.

Therefore, the w_temp must be saved to register with address starting at 0x70 on 16F88.

Try this:

CBLOCK 0x20
sec
sec10
min
min10
hr
hr10
bres_hi ; hi byte of our 24bit variable
bres_mid ; mid byte
bres_lo ; lo byte
; (we only need 3 bytes for this system)

status_temp ; used for interrupt servicing
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;w_temp ; used for interrupt servicing
templcd ;temp store for 4 bit mode
templcd2
count ;used in looping routines
count1 ;used in delay routine
counta ;used in delay routine
countb ;used in delay routine
oldtime
ENDC

CBLOCK 0x70
w_temp          ;saving W content during interrupt
ENDC

 

[IMSlo]

Thanks LC

I made the changes you suggested. It did not fix the problem. But it did run a little past 7 mins once. I also installed the cap right that might have helped. I don't know.

 

[tunedwolf]

If your display update code attempts to update the display and the registers for it have their contents changed by your interrupt routine, you will have no end of junk on the display...
The temporary registers used for context saving during your interrupts, as LC suggested, must reside in an area of the register file that is accessable to all banks, otherwise unexpected problems may creep in as your program grows in size and function.
My head hurts trying to read through the code you have posted, perhaps re-post it as an attached but formatted asm file might help.

 

[IMSlo]

Cleaned up code

I have cleaned up the code and I will attemp to upload it as a zipped .asm file. Maybe it will be a little easier to follow.

Concerning the interrupt during the display procedure

I have never really worried about that because I knew I had 250+- instruction cycles to get it displayed. Maybe these are getting eaten up during the interrupt service and the display routine. I guess I will have to try to count how many instructions are used.

Thanks again

 

[Pommie]

Now I can read it a little clearer, I have spotted what maybe the problem. In your context save code you have,

	movwf w_temp      	
	movf	STATUS,w          	
	movwf status_temp

This may cause a problem because the movf STATUS,w will clear the zero flag. You should change it to,

	movwf w_temp      	
	swapf  STATUS,w          	
	movwf status_temp

and change the corresponding restore to,

int_exit
		BCF INTCON,2
		bcf	STATUS,Z	; this isn't needed	
		swapf status_temp,w     	;<--changed
		movwf STATUS            	
		swapf w_temp,f
		swapf w_temp,w          	
	retfie

HTH

Mike.

 

[IMSlo]

Mike,

I made the suggested changes. It ran 00:03:44 and zeroed the display.

Thanks anyway

Steve

 

[tunedwolf]

The movf isn't traditional, but will still work, the status register is set based on the result of the opcode execution, so the status register contents are copied into W prior to being altered, although I would use a clrf after storing it in any case to ensure I knew which bank etc I'm starting from within the interrupt routine.

like this..

movwf w_temp
movf status,w
movwf status_temp
clrf status

Be careful that you are not violating the correct set-up and hold times for the data and strobe lines for the display module you are using.
Also your code is able to respond to interrupts before you have even finished initialising your display module and variables. Set up everything before you allow your clock to run. Your code still doesn't put your vital register contents in an area of the register file that is accessible across all banks, you are only storing the W register in an area accessible from all banks.
PCLath, Status, FSR & W plus what ever else you regard as vital should be in your context saving in the interrupt service routine. I'm not saying that you need to store anything more than W at the moment, but it's one of those things that you will forget about later as your program grows, and will lead you into trouble.
You are using all those banksels to initialise everything, why not set the bank bits once, set up all your ports and all other options etc, then clr the bank bits to get you back to bank 0 and save yourself some code space for future use. One bit doesn't make sense to me, why split your initialsation code...and use a goto to get to your interrupt service routine ?


Something along the lines of this makes much more sense to me personally...


org h'00'
goto init

org h'04'
context saving
interrupt routine
context restoring
return from interrupt

init:
set bank bits
initialise all ports, options, prescaler etc etc
clr bank bits
set/clr variables
clr junk on ports
initialse lcd module
set timer
set interrupts active and clr flags

loop:
test for roll-over, if so then
update display
if not goto loop and try again

Also, you mention 250 +/- instruction cycles, it's actually clock cycles because the timer is driven by the internal clocking scheme, some instructions use two cycles etc, I don't think that's where your problem lies in this case though, actually, now that I have re-read the code you posted, it looks close enough that I would be counting on my fingers just to be sure...

Anyway, I hope I'm helping you rather than confusing you further

 

[Pommie]

The movf isn't traditional, but will still work, the status register is set based on the result of the opcode execution, so the status register contents are copied into W prior to being altered, although I would use a clrf after storing it in any case to ensure I knew which bank etc I'm starting from within the interrupt routine.

Except, when you restore it, the movfw will clear the Zero flag. Hence the use of swapf.

Mike.

 

[Pommie]

IMSlo,

Can you increase the clock speed to 8 meg to see if it is timing related. I expect it to either run correctly (only double speed) or crash consistently.

Mike.

 

[tunedwolf]

Except, when you restore it, the movfw will clear the Zero flag. Hence the use of swapf.

Mike.

MOVWF doesn't affect the status bits. I think you are refering to him using a MOVF to restore his temporary status bits into the W register, which as you rightly point out, will affect the status register bits. However, as he then uses a MOVWF to actually put the W register contents (original status bits) back into the status register, no status bits are affected. He then goes on to use SWAPF to restore the W register and doesn't further affect the status bits prior to his return from interrupt.
The only use of the SWAPF in the context restore, would be to swap the nibbles back round again after using a SWAPF in the context saving to begin with.

 

[Pommie]

The movf status_temp,w will clear the zero flag.

Try,

		bsf	STATUS,Z
		movf	STATUS,w
		movwf	status_temp

		movf	status_temp,w
		movwf	STATUS

After this code, the zero flag will always be reset.

Mike.

 

[eblc1388]

The movf status_temp,w will clear the zero flag.

It doesn't matter if the STATUS flags are changed using the movf STATUS_TEMP,W , the next MOVWF STATUS will always setup up STATUS to what it should be. The Zero flag is set in your above example because you have set it deliberately using the first BSF instruction.

In fact I have asked the same question on this forum a year ago and Microchip's example codes use both methods to save/restore STATUS.

 

[Pommie]

It doesn't matter if the STATUS flags are changed using the movf STATUS_TEMP,W , the next MOVWF STATUS will always setup up STATUS to what it should be. The Zero flag is set in your above example because you have set it deliberately using the first BSF instruction.

In fact I have asked the same question on this forum a year ago and Microchip's example codes use both methods to save/restore STATUS.

Yes, my mistake, I should have thought about it a bit longer before posting.

Sorry to the OP for sidetracking their thread.

Mike.

 

[eblc1388]

Thanks LC

I made the changes you suggested. It did not fix the problem. But it did run a little past 7 mins once. I also installed the cap right that might have helped. I don't know.

Seven minutes for PIC is a very very long time and nearly billion of instructions later. At 4MHz, one million instructions get executed each second.

If a fault/bug then appears, it is likely related to a certain combination of codes which exceed the hardware capacity of the PIC or some variables have reached boundary condition where coding branches to a new section in program memory and bug in new section of code causes the fault.

The most likely cause in your case will be stack overflow, where you are deep inside several CALL routines and an interrupt occurs. If you code CALL several times more inside the interrupt routine, you are done.

To test for this condition, disable INT before calling LCD display in the main_loop. After LCD display is done, re-enable interrupt. I know this will upset the correct timing, but just do it as a test. If your code then runs without problem albeit the timing display is not correct, then stack overflow is most likely the cause.