PIC16F877A Assembly, Why does this code never go to the ISR when Timer1 Overflows?

Question

Very new to assembly programing and im trying to make a blinking LED, here is my code:

#include p16f877a.inc

counter equ 0x20
org 0x00
    goto main
org 0x04
    goto ISR
main
    
    bsf INTCON, GIE
    bcf PIR1, TMR1IF
    bsf STATUS, RP0
    bcf STATUS,RP1
    bsf PIE1, TMR1IE
   
    clrf TRISB
    bcf STATUS,RP0
;1 Second Hardware Delay.
Delay0.5SecondInitialization
    movlw b'00110000'
    movwf T1CON ; Set prescaler to 11(1:8), dont set TMR1ON (Bit 0) until you want timer to start.
DelayLoop
    bsf T1CON,TMR1ON
    movlw 0x0B
    movwf TMR1H
    movlw 0x0DC
    movwf TMR1L; We initialized both registers of TMR1LOW,TMR1HIGH with 0x0BDC

;0.5 HW Delay is ready, we have to turn it on now.
finish goto finish


ISR
   
    incf counter
    movf counter,W
    sublw 2
    btfss STATUS,Z
    goto DelayLoop
    comf PORTB,F
    bcf PIR1,TMR1IF
    retfie

end

I notice that it runs and stays in goto finish, never going to the ISR.

Nothing really came to mind, I expect it to do a 1 second delay then complementing PORTB. What happens is it stays in the goto finish instruction and doesnt go to the ISR.

Answer 1

The Original Poster (OP) seems lost in the complexity of the tools used to create assembly language code for the PIC16F677A controller.

The OP's question is most likely based on being lost in this issue with tools along with trying to learn how to create code without the aide of a higher level language compiler.

These questions most often will involve simulation tools like LabCenter's Proteus simulator, as many schools use Proteus to teach embedded programming concepts with Microchip PIC controllers.

In my view the OP needs to learn more about the specific tools, and how to create a less complicated sample code to validate their understanding of how their development environment actually works.

For any controller but especially the PIC16F877A it is key to specify the system oscillator frequency when posting code in a forum like this. Also always provide the state of ALL of the configuration word bits. How the configuration words are initialized will affect how the simulation runs.

To the OP: Add a comment requesting a complete application example and I will edit this answer to add that code.

Answer 2

As far as I see, I've detected 2 major problems in your code.

Major Problem 1

You did not setup the Timer1 interrupt properly. If you code in assembly for a specific device, you must see its datasheet even when you practice on it. See the whole interrupt logic of 877A below:

Find the TMR1IE and track the logical connection until you reach the rightmost AND gate which triggers the CPU for interrupt events. You will see that the output of the AND gate of the TMR1IE is connected to the input of big OR gate, the output of that big OR gate is connected to the inpt of an another AND gate which also has an input called PEIE. Now this bit masks all peripheral interrupts including the Timer1's interrupt. So you did not set the PEIE bit in your code, as a result the Timer1 interrupt will be masked even you set the TMR1IE bit and an interrupt occurs. That's why your program don't get to the ISR when Timer1 overflows.

So the first thing you have to fix is this; set the PEIE bit to unmask the peripheral interrupts. I would restructure the main code like following:

main:
    clrf    PORTB ; Clear any garbage on PORTB
    clrf    PIR1  ; Clear all peripheral interrupt flags in PIR1
    clrf    PIR2  ; Clear all peripheral interrupt flags in PIR2
    bsf     STATUS, RP0
    bcf     STATUS, RP1
    ; Setup Timer1 interrupt
    bsf     PIE1, TMR1IE
    ; Set PEIE bit to allow peripheral interrupts
    bsf     INTCON, PEIE ; Your code misses this instruction
    bsf     INTCON, GIE
   
    clrf    TRISB
    bcf STATUS,RP0
    ; Setup timer 1 for 1 Second Hardware Delay.
    movlw   b'00110000'
    movwf   T1CON ; Set prescaler to 11(1:8), dont set TMR1ON (Bit 0) until you want timer to start.
    call    reloadTimer1 ; we will implement this in the next section
    bsf     T1CON,TMR1ON ; You can actually set timer on here once in init code
    
; In this line we will just wait the timer1 interrupt to occur
finish goto finish

Major Problem 2

Even you fix the Major Problem 1 your code would got stuck in your ISR where you branch to the DelayLoop using the goto DelayLoop instruction. The flow then would end up at finish goto finish line which evetually put the CPU in an infinite loop. This is not what we want.

What we want is; when the timer1 overflows, clear the TMR1IF flag first and then reload the timer1's counter registers (TMR1H:TMR1L) with the predefined value. Your current approach for DelayLoop is actually good since you don't want code duplication. But the implementation is not in how it should be. Therefore instead of using unconditional branching (goto) you should've used the subroutine instruction (call) so that program can return to the program memory address where the subroutine got called. See the suggested implementation of the subroutine and the ISR below.

reloadTimer1: ; Formerly was DelayLoop
    movlw   0x0B
    BANKSEL T1CON ; Make sure you are in TMR1H's bank
    movwf   TMR1H
    movlw   0x0DC
    movwf   TMR1L; We initialized both registers of TMR1LOW,TMR1HIGH with 0x0BDC
    return

ISR
    bcf     PIR1,TMR1IF ; With 8 bit PIC devices, always clear the relevant 
                        ; interrupt flag first to avoid false interrupts
    incf    counter
    movf    counter,W
    sublw   2
    btfss   STATUS,Z
    call    reloadTimer1 ; We use call instruction instead of goto so that the
                         ; program flow automatically continue from where it left
    comf    PORTB,F
    retfie

Go ahead and try to restructure your code. Also let me know if there is something unclear for you.