LED wont light up right

[Sigma150]

I am making a simple led binary clock. The first four LED are the hours in binary (read left to right). LEDS 6 through 10 are the minutes(read left to right) . I run an upload the code but minute LEDs seem to not work correctly. I am posting code and attaching schmatics. What am I doing wrong?
Thanks in advance

;
;  The "Dlay" macro in this routine will provide delays ranging 
;   from 1 to 1,000,000 cycles.  
;
;  Hardware Notes:
;   PIC16F684 running at 4 MHz in Simulator
;   Reset is tied directly to Vcc via Pullup/Programming Hardware
;
;
;  Myke Predko
;  04.09.22
;
  LIST R=DEC
 INCLUDE "p16f630.inc"

 __config (_INTRC_OSC_NOCLKOUT & _WDT_OFF & _PWRTE_OFF & _MCLRE_OFF & _CP_OFF ) 

 CBLOCK 0x020                ;  Variable Declaration
DlayValue:2                  ;  Requires 24 Bit Counter (w/ WREG)
minutes
hours
 ENDC

Dlay Macro Cycles
 variable CyclesLeft            ;  Keep Track of Remaining Cycles
 variable LargeNum
CyclesLeft = Cycles
 local LongLoop
 if Cycles > 0x04FFFF00         ;  Can't Handle the Anything > 83 Seconds (@ 4 MHz)
 error "Required Delay is longer than 'Dlay' Macro can support"
 endif
 if Cycles > 327681             ;  Need Large Loop?  
LargeNum = CyclesLeft / 327681
  movlw   LargeNum
  movwf   DlayValue + 2         ;  Calculate Number of Loops
LongLoop:                       ;  Repeat for Each Loop
  clrf    DlayValue + 1         ;  Do Maximum Possible Loop Count
  clrf    DlayValue
  decf    DlayValue, f
  btfsc   STATUS, Z
   decfsz DlayValue + 1, f
    goto  $ - 3
  decfsz  DlayValue + 2, f      ;  Repeat Loop
   goto   LongLoop
CyclesLeft = CyclesLeft - ((LargeNum * 327681) + 1 + (LargeNum * 3))
 endif  ;  Need Large Loop
 if Cycles > 14                 ;  Will a Loop be required?  
  movlw   high (((CyclesLeft - 3) / 5) + 256)
  movwf   DlayValue + 1
  movlw   low (((CyclesLeft - 3)/ 5) + 256)
  movwf   DlayValue
  decf    DlayValue, f          ;  5 Cycle Constant Delay Loop
  btfsc   STATUS, Z
   decfsz DlayValue + 1, f
    goto  $ - 3
CyclesLeft = CyclesLeft - (3 + (5 * ((CyclesLeft - 3)/ 5)))
 endif                          ;  Finished with Loop Code
 while CyclesLeft >= 2          ;  Put in 2 Instruction Cycle Delays
  goto    $ + 1
CyclesLeft = CyclesLeft - 2
 endw
 if CyclesLeft == 1             ;  Put in the Last Required Cycle
  nop
 endif
 endm

  PAGE

 org      0
  nop                        ;  Required for MPLAB ICD2

;////////////////////////////////////////////////////////////////;
;Luke BrownGold's Code

  bsf STATUS,5 ;Go to Bank 1  
 	 TRIS PORTA ;Move 00000 onto TRISA ? all pins set to output 
 	 TRIS PORTC ;Move 00000 onto TRISC ? all pins set to output 
  bcf STATUS,5 ;Come back to Bank 0 
	
	clrf PORTA
	clrf PORTC
	
	;set time
	movlw d'12' ;hours cant start at zero
	movwf hours
	clrf minutes ;start minutes at 0


  Start 	
			;Checks Hours and Minutes so the do not exceed time
			checkResetTime  
			
			movf    minutes, w      ;  if minutes == numb60 then resetmin
			subwf   d'60', w
			btfsc   STATUS, Z
			goto   resetmin

			movf    hours, w      ;  if hours == numb13 then resethour
			subwf   d'13', w
			btfsc   STATUS, Z
			goto   resethour

			;clear display
 			clrf PORTA
			clrf PORTC

			


;/////////////////-------------HOURS 
			BTFSS hours,d'3' 
			BSF PORTA,d'4' 

			BTFSS hours,d'2' 
			BSF PORTC,d'5' 
			
			BTFSS hours,d'1' 
			BSF PORTC,d'4' 
			
			BTFSS hours,d'0' 
			BSF PORTC,d'3' 			


;/////////////////-------------Minutes
			BTFSS minutes,d'5' 
			BSF PORTC,d'2' 

			BTFSS minutes,d'4' 
			BSF PORTC,d'1'
			
			BTFSS minutes,d'3'
			BSF PORTC,d'0'
			
			BTFSS minutes,d'2' ;
			BSF PORTA,d'2' ;

			BTFSS minutes,d'1' 
			BSF PORTA,d'1' 

			BTFSS minutes,0 
			BSF PORTA,d'0' 
			

			call oneSec ;will increase delay later (shorter for testing purposes)
			
			incf minutes, f  ;increments minutes



  goto Start 

 resetmin   movlw d'0'
			movwf minutes
			incf hours, f  ;increments hours
			goto checkResetTime

 resethour  movlw d'1'
			movwf hours
			goto checkResetTime
oneSec:
			Dlay 100000                 ;  Delay 0.1s
			Dlay 100000                 ;  Delay 0.1s
			Dlay 100000                 ;  Delay 0.1s
			Dlay 100000                 ;  Delay 0.1s
			Dlay 100000                 ;  Delay 0.1s
			Dlay 100000                 ;  Delay 0.1s
			Dlay 100000                 ;  Delay 0.1s
			Dlay 100000                 ;  Delay 0.1s
			Dlay 100000                 ;  Delay 0.1s
			Dlay 100000                 ;  Delay 0.1s
  			nop
			nop
return 

tenSec:
		Call oneSec
		Call oneSec
		Call oneSec
		Call oneSec
		Call oneSec
		Call oneSec
		Call oneSec
		Call oneSec
		Call oneSec
		Call oneSec
return
  end

 

[Pommie]

In your circuit diagram, you don't show any current limiting resisters. If you don't have any then this is probably your problem. Try placing some resistors (220R to 1K) between the output pins and the LEDs. Also, your most significant hours LED is on PortA,3 not PortA,4.

Mike.

 

[Sigma150]

I changed that port number on the hour. I do have limiting resistors I just excluded them in the schematic. It still doesnt work.

 

[Pommie]

I think you have your logic inverted.

As your LEDs are connected to 5V, then in order to illuminate them, you have to put a zero on your output pin. Your instruction clrf PORTA will turn all LEDs ON. I assume you want a zero to be OFF.

What you should have is,

         ;clear display 
         movlw 0xff
         movwf PORTA 
         movwf PORTC 

;/////////////////-------------HOURS 
         BTFSC hours,d'3' ;    is bit set
         BCF PORTA,d'3' ;      yes, so illuminate LED

         BTFSC hours,d'2' 
         BCF PORTC,d'5'

HTH

Mike.

 

[Oznog]

That's not a very good way to code a clock at all. A hardware timer should be used.

 

[Sigma150]

I know but i want it as small as possible.

 

[Nigel Goodwin]

I know but i want it as small as possible.

Hardware timers are internal to the PIC.

 

[Pommie]

That's not a very good way to code a clock at all. A hardware timer should be used.

There are better ways to implement a clock. You can add 7 segment displays, LCD displays, crystal controlled oscillators etc. I think that the way he is going about it is good. He's using assembler, seems to understand the circuitry and the software. When he gets up to speed with the instruction set and can tell the time in binary then I think he will be very well prepared to tackle the hardware timers.

Mike.

 

[mstechca]

You can even make a clock without using a microcontroller.

Can you show us a picture of how the LED's are to be arranged?

 

[Sigma150]

Im new to electronics and pic programming. I choose to use the pic because it seemed the easiest way to make a very small binary clock. If there is an easier way please enlighten me.
For the display the leds are just arranged using a 10 segment led bar graph like the on here https://www.radioshack.com/product/...h&cp=&productId=2062560&kw=led+10&tab=summary