Appraise my clock code please :-)

[Angry Badger]

Hello,

Still quite new to PICs and programming in general. This 'project' (will probably never be built) has grown from just driving a single 7 seg display so as to learn lookup tables. Still have to add buttons for setting the actual time and perhaps an alarm function.

Before I go any further could you have a quick look at my code and suggest any improvements. I'd be grateful for any constructive comments!

Thanks in advance.

;***************************** 8th September 2008 ***************
; My notes:
; 24 hour format clock.
; Drive six seven seg displays from one port.
; T1 ext clk = 262.144KHz derived from 4.194304MHz xtal via HEF4060
; 16 bit T1 with 1:4 prescaler is clocked at: 262144/4 = 65536Hz i.e. it rolls over every 1s
; Internal RC clock = 4MHz = 1uS per instruction
; T0 with 1:8 prescaler is clocked at (Fosc/4)/8 = 125KHz, it rolls over every [1/(125000)]x256  = 2.048mS
; 2.048mS = 488.28Hz There are 6 display digits, 6/488.28 = 81.38 i.e. display refresh rate is 81.3Hz
; (need to reduce mark to space ratio to increase display brightness)
;**************************************************************
	list      p=16F690			;does this need to be here? Prog works ok without it.
	#include <p16F690.inc>
	errorlevel  -302            ; suppress message 302 from list file
	errorlevel  -305

		__config (_INTRC_OSC_NOCLKOUT &_WDT_OFF &_PWRTE_OFF &_CP_OFF &_BOR_OFF &_IESO_OFF &_FCMEN_OFF &_MCLRE_OFF)

INT_VAR     		UDATA_SHR			;this is here to allow following symbols in watch window when using ICD
secondsvalue		RES 1
tensecondsvalue		RES 1
minutesvalue		RES 1
tenminutesvalue		RES 1
hoursvalue			RES 1
tenhoursvalue		RES 1
reset_at_twentyfour	RES 1
temp				RES 1

#define		dig1		PORTA,1			;outputs: 2N7000 mosfets connected to 7 seg cathodes...
#define		dig2		PORTA,2
#define		dig3		PORTB,4
#define		dig4		PORTB,5
#define		dig5		PORTB,6
#define		dig6		PORTB,7
#define		timer0done	INTCON,T0IF		;timer0 timed out bit
#define		timer1done	PIR1,0			;when the 16 bit timer1 timer/counter register overflows, bit0 in PIR1 register is set.
#define		display		PORTC			;portC drives 6 digit 7 seg common cathode display

	org 0

Start:
	bcf		STATUS,RP0
	bcf		STATUS,RP1
	movlw	b'10100111'					;bit7 = 1 means timer1 gate active high
										;bit6 = 0 means timer1 is always counting (not gated)
										;bits 5/4 = prescaler (10 = 1:4)
										;bit3 = 0 means LP oscillator off
										;bit2 = 1 don't synchronise external clk
										;bit1 = 1 means timer1 uses external clock
										;bit0 = 1 means enable timer1
	movwf	T1CON						;move the configuration data to the T1CON register

	bsf		STATUS,RP1					;select bank 2
	clrf	ANSEL						;no analogue function required on any pins
	clrf	ANSELH
	bcf		STATUS,RP1					;back to bank 0

	bsf		STATUS,RP0					;select bank 1
	movlw	b'00101000'
	movwf	TRISA						;make portA all output except RA5 & RA3 - RA3 is input only anyway
	clrf	TRISB						;make portB all output - drives 4 of the mosfets
	clrf	TRISC						;make portC all output - drives the digits
	movlw	b'00000010'					;sets the timer prescalar value
	movwf	OPTION_REG					;bits 0,1,2 are the prescalar bits
	bcf		STATUS,RP0					;back to bank 0

	clrf	temp
	
;############################### just for debugging - start with 13:20:00 on the display #################
	movlw	0x0
	movwf	secondsvalue
	movlw	0x0
	movwf	tensecondsvalue
	movlw	0x3
	movwf	minutesvalue
	movlw	0x2
	movwf	tenminutesvalue
	movlw	0x2
	movwf	hoursvalue
	movlw	0x2
	movwf	tenhoursvalue
	movlw	0x18
	movwf	reset_at_twentyfour
;#################################################################

Main:
	btfsc	timer1done					;test to see if timer overflow bit is set (timer timed out), 1Hz
	goto	Countup						;if it is......
	goto	Display						;if not then....
		
Countup:
	bcf		timer1done
	incf	secondsvalue				;increase by 1
	movlw	0xa							;10 to W reg
	subwf	secondsvalue,w				;subtract it from secondsvalue
	btfsc	STATUS,Z					;when = 0 (10 from 10)
	goto	Reset						;need to reset back to zero
	goto	Display						;if still not zero....

Reset:									;###### GO TO THIS ROUTINE WHEN UNIT SECONDS VALUE = 10

seconds:
	clrf	secondsvalue
	incf	tensecondsvalue	

tenseconds:								;reset when = 6
	movlw	0x6	
	subwf	tensecondsvalue,w
	btfss	STATUS,Z
	goto	Display
	clrf	tensecondsvalue
	incf	minutesvalue

minutes:								;reset when = 10
	movlw	0xa
	subwf	minutesvalue,w
	btfss	STATUS,Z
	goto	Display
	clrf	minutesvalue
	incf	tenminutesvalue

tenminutes:								;reset when = 6
	movlw	0x6
	subwf	tenminutesvalue	,w
	btfss	STATUS,Z
	goto	Display
	clrf	tenminutesvalue
	incf	hoursvalue
	incf	reset_at_twentyfour
	movlw	0x18
	subwf	reset_at_twentyfour,w
	btfsc	STATUS,Z
	goto	Resetall

unithours:								;reset when = 10
	movlw	0xa
	subwf	hoursvalue,w
	btfss	STATUS,Z
	goto	Display
	clrf	hoursvalue
	incf	tenhoursvalue
	goto	Display

Resetall:	
	clrf	hoursvalue
	clrf	tenhoursvalue
	clrf	reset_at_twentyfour
	goto	Main

Display:
	call	Timer0						;******unit seconds******
	movf	secondsvalue,w
 	call	Binaryto7seg
 	bcf		dig6						
 	movwf	display
	bsf		dig1						

	call	Timer0						;******decade seconds******
	movf	tensecondsvalue,w
	call	Binaryto7seg
	bcf		dig1
	movwf	display
	bsf		dig2

	call	Timer0						;******unit minutes******
	movf	minutesvalue,w
	call	Binaryto7seg
	bcf		dig2
	movwf	display
	bsf		dig3

	call	Timer0						;******decade minutes******
	movf	tenminutesvalue,w
	call	Binaryto7seg
	bcf		dig3
	movwf	display
	bsf		dig4

	call	Timer0						;******unit hours******
	movf	hoursvalue,w
	call	Binaryto7seg
	bcf		dig4
	movwf	display
	bsf		dig5

	call	Timer0						;******decade hours******
	movf	tenhoursvalue,w
	call	Binaryto7seg
	bcf		dig5
	movwf	display
	bsf		dig6
	goto	Main

Timer0:
	btfss	timer0done					;test to see if timer overflow bit is set (timer timed out), 81Hz
	goto	$-1							;keep checking until it is
	bcf		timer0done					;it has so clear the overflow bit
	return

	org	0xf7							;the start address of the next part of the program (movwf temp)
	
Binaryto7seg:
	movwf	temp						;temporary store for '*value'
	movlw	high Lookuptable
	movwf	PCLATH
	movlw	low Lookuptable				;start address of lookuptable
	addwf	temp,w
	btfsc	STATUS,C
	incf	PCLATH,F					;!!! don't understand what this is for, yet !!!
	movwf	PCL

Lookuptable:
	retlw	b'00111111'					; 0
	retlw	b'00000110'					; 1
	retlw	b'01011011'					; 2
	retlw	b'01001111'					; 3
	retlw	b'01100110'					; 4
	retlw	b'01101101'					; 5
	retlw	b'01111100'					; 6
	retlw	b'00000111'					; 7
	retlw	b'01111111'					; 8
	retlw	b'01100111'					; 9

       end

**broken link removed**

**broken link removed**


****** Thanks Mike. ******

 

[Pommie]

Your code is good. I like the use of timer 0 for controlling the refresh rate and timer 1 for the actual clock, good use of the available resources and it avoids the use of interrupts.

The only thing I could see that is wrong (more confusing than wrong) is you have mixed up absolute and relocatable code. Using udata_shr should give you an error but you appear to have it working.

For absolute code you would normally declare your variables using cblock. In your case it would be,

		cblock	0x20
INT_VAR				;this is here to allow following symbols in watch window when using ICD
secondsvalue	
tensecondsvalue	
minutesvalue	
tenminutesvalue	
hoursvalue	
tenhoursvalue	
reset_at_twentyfour 
temp		
		endc

You also don't need the second org (0xf7) as you are calculating the full 16 bit address of the table. PCLATH is the high byte of the program counter but only comes into effect when you do the write to PCL.

Mike.