Maker Pro
Maker Pro

PIC16F84A - Confusing output

Hi

There's something wrong with my code here, I'm attempting toggle the outputs of RA0-3 between high and low in a certain sequence by looping through a data table and retrieving the corresponding values. Heres my code:

Code:
;====================================================================
; Main.asm file generated by New Project wizard
;
; Created:   Sat Feb 15 2014
; Processor: PIC16F84A
; Compiler:  MPASM (MPLAB)
;====================================================================

;====================================================================
; DEFINITIONS
;====================================================================

#include p16f84a.inc                ; Include register definition file

;====================================================================
; VARIABLES
;====================================================================

;====================================================================
; RESET and INTERRUPT VECTORS
;====================================================================

      ; Reset Vector
RST   code  0x0 
      goto  Start

;====================================================================
; CODE SEGMENT
;====================================================================

PGM   code
Start
  
STATUS          equ       03h                 ;Address of the STATUS register
TRISA             equ       85h                 ;Address of the tristate register for port A
PORTA           equ       05h                 ;Address of Port A
COUNT1        equ       08h                 ;First counter for our delay loops
COUNT2        equ       09h ; for looping
COUNT3 equ 10h ; for looping
PC equ 02h ; program counter



                         bsf                  STATUS,5       ;Switch to Bank 1
                         movlw              00h                ;Set the Port A pins
                         movwf              TRISA             ;to output.
                         bcf                   STATUS,5       ;Switch back to Bank 0
call GOCW

TABLE addwf PC
retlw 01h ;return binary 0001 RA0
retlw 04h ;0100 RA2
retlw 02h ;0010 RA1
retlw 08h ;1000 RA3
return

GOCW
movlw 04h ; put 4 in W 
movwf COUNT3 ; move W to COUNT3, 4.
loop3
movf COUNT3, 0 ;move count3 to W
CALL TABLE
movwf PORTA ; set an output to high
call DELAY ; wait a while
decfsz COUNT3,1 ;sub 1 from count3 or end loop if count3 is 0
goto loop3 

return

DELAY ;

;****Start of the delay loop 1****
movlw 80h
movwf COUNT2


                 Loop1          decfsz             COUNT1,1       ;Subtract 1 from 255
                   goto                Loop1            ;If COUNT is zero, carry on.
                   decfsz             COUNT2,1       ;Subtract 1 from 255
                   goto                Loop1            ;Go back to the start of our loop.                         ;This delay counts down from                                                                           ;255 to zero, 255 times
return ;end delay subroutine

;====================================================================
      END

What I expect to see is this:

RA3 HIGH
RA1 HIGH
RA2 HIGH
RA0 HIGH

However RA2 is taking twice as long on high (being called twice?) and RA0 is never turning on.
Attached an oscilloscope output to help visualize what's going on.

Whats wrong with my code?
 

Attachments

  • trace.jpg
    trace.jpg
    69.7 KB · Views: 141
I fixed a few minor things, but mostly try this:
Capture.PNG

Basically, the table counts from 0, not 1, so when you increment the PC by 4, it goes directly to the return with 4 still in w (i.e., RA2), later when the PC is incremented by 1, it goes to the same spot (i.e., RA2). However, on Count3 =0, you go back to the start. Thus, you are missing RA0 and wrapping RA2 around so its width is double.

A small detail... Since you are adding the p16f84a.inc file, you do not need to define your registers, i.e., STATUS,TRISA, and PORTA.

If it doesn't compile for you (your original did not compile for me), I will attach the whole, modified assembly file.

Hope that helps.

John
 

KrisBlueNZ

Sadly passed away in 2015
The errors I can see are:

1. The value in W when you call TABLE must be 0~3, not 1~4, because when the addwf PCL operation is executed, the program counter has already advanced to the first retlw instruction in the table. If you want to execute that instruction, you need to add 0 to the program counter.

2. Your code calls GOCW then falls though to TABLE. When GOCW returns, after the four bit patterns have been written to PORTA, the MCU will execute code starting from TABLE, which will execute a retlw with no return address on the stack, and the MCU will jump off into never-neverland. You should have something like goto $ after the call to GOCW, before TABLE. Or you can loop back to the call to GOCW so the code outputs the bit pattern continuously.

Other issues and suggestions:

Labels, in assembly language code generally, normally have a colon at the end. MPASM allows you to omit the colon, but your code will look more conventional and be slightly clearer if you use colons. This applies where the label is defined, not where it's referred to using goto or call.

You have redefined the PCL register in your code, calling it PC; it is already defined by P16F84A.INC as PCL. There's no need, or reason, to call it PC when its proper name is PCL and it's already defined under that name. You have also defined STATUS, TRISA, and PORTA, which are also defined in the include file.

There's no need for the final return instruction at the end of the table. It will never be executed.

Timing using loops works fine if your code doesn't need to do anything else, but if you want to add other functionality, you should use a more general timing method. A timer interrupt is one option, although it may be workable to poll the timer register to detect wraparound and avoid using interrupts, if you don't need precise or immediate timing. It depends on what the program needs to do.
 
Labels, in assembly language code generally, normally have a colon at the end. MPASM allows you to omit the colon, but your code will look more conventional and be slightly clearer if you use colons. This applies where the label is defined, not where it's referred to using goto or call.

I respectfully disagree. The popularity of Microchip PIC's and the non-transportability of Assembly programs in general support using formats common for the specific product and language used. In this case, it is MPASM.

While it is acceptable in MPASM, the use of a colon to identify a label is certainly not the most common practice. I could not find one instance in the MPASM User's Guide in which a colon instead of a white space is used. Nor is it common on PicList and other sources of Microchip Assembly programs. For someone just beginning to use MPASM, using a colon could be just an added hiccup, like redefining PCL to be PC. It doesn't stop it from working, but it is out of the norm.

John
 
Im here. Was able to get it working once I realised that it was counting from 0-3 thanks to the great feedback here. I'm sure to run into a few more problems down the line on this one so I'll yell out when I do.
 
Labels, in assembly language code generally, normally have a colon at the end. MPASM allows you to omit the colon, but your code will look more conventional and be slightly clearer if you use colons. This applies where the label is defined, not where it's referred to using goto or call.

I totally agree on using a colon to end the label definition, it makes the layout more readable.
I normally have labels on separate lines, making it easy to read, even if the label is longer than the normal indentation of the mnemonics. Another extra using colons, is that it makes it easier to search for the definition, at least if the label is called from many files.

Reading the code window above I'm surprised that no indentation is displayed for the code, since MPASM need mnemonics to start at least on position 2 on the line. I normally use an indentation of 8, but 4 should also make good readability.
 
Last edited:
Top