What am I doing wrong?

Do you have a question? Post it now! No Registration Necessary

Translate This Thread From English to

Threaded View
--nextPart2912043.rat7JAi7FX
Content-Type: text/plain; charset=iso-8859-1
Content-Transfer-Encoding: 8Bit

I'm trying to code a very simple LED flasher as a starter app, and I
can't get the damn thing to work. (This is on PIC16F627 and simulated on
MPLAB.) The timer1 rollover never fires. Code follows as attachment.

Can someone tell me what I'm doing wrong?
--nextPart2912043.rat7JAi7FX
Content-Type: text/plain; name="FlashLed.asm"
Content-Transfer-Encoding: 8Bit
Content-Disposition: attachment; filename="FlashLed.asm"

;----
; Purpose:                Flash LED once per second
; Written by:            Julian Morrison
; Date:                    6/Sep/2005
; Version:                0.1
; File saved as:        FlashLed.asm
; For PIC:                16F627
; For board:            Velleman K8048
; Clock frequency:        4 megahertz
;----
 
;                processor p16f627 ; for gpasm
                processor 16f627
        
;----
; Declarations
                
        ; reset vector
                org        0x00
                goto    Start
                
        ; interrupt vector
                org        0x04
                goto    ISR
        
        ; SFRs for p16f627
        
        ; ...shared
indirect        equ        0x00 ; address to indirect via FSR
pcl                equ        0x02 ; program counter low byte
status            equ        0x03
fsr                equ        0x04 ; indirection pointer
pclath            equ        0x0A ; program counter high 5 bits
intcon            equ        0x0B
                
        ; ...bank 0
tmr0            equ        0x01
porta            equ        0x05
portb            equ        0x06
pir1            equ        0x0C
tmr1_lo            equ        0x0E
tmr1_hi            equ        0x0F
t1con            equ        0x10
tmr2            equ        0x11
t2con            equ        0x12
ccpr1_lo        equ        0x15 ; capture/compare low byte
ccpr1_hi        equ        0x16 ; capture/compare high byte
ccp1con            equ        0x17
txreg            equ        0x19 ; USART transmit
rcreg            equ        0x1A ; USART recieve
cmcon            equ        0x1F
user_regs_0        equ        0x20
        
        ; ...bank 1
option_reg        equ        0x01
trisa            equ        0x05
trisb            equ        0x06
pie1            equ        0x0C
pcon            equ        0x0E
pr2                equ        0x12
txsta            equ        0x18
spbrg            equ        0x19
eedata            equ        0x1A
eeadr            equ        0x1B
eecon1            equ        0x1C
eecon2            equ        0x1D
vrcon            equ        0x1F
user_regs_1        equ        0x20
        
        ; ...bank 2
portb            equ        0x06
user_regs_2        equ        0x20
        
        ; ...bank 3
trisb            equ        0x06
        
        ; custom registers
rollover_count    equ        user_regs_0 + 0x00
        
        ; constants
ticks_per_10ms    equ        d'10000' ; 4 megahertz, 4 cycles per tick
t1rollover        equ        d'65536' - ticks_per_10ms
        
;----
; Subroutines
                org        0x05
Init            clrf    porta
                clrf    portb
                bsf        status,5    ; bank 1 to access tris
                movlw    b'00001111'
                movwf    trisa       ; porta is connected to switches
                movlw    b'00000000'
                movwf    trisb       ; portb is connected to LEDs
                movlw    b'00001000' ; prescale 1:1 for WDT, none for tmr0
                movwf    option_reg
                bcf        status,5    ; bank 0
                movlw    b'00000000'
                movwf    intcon
                return
        
ISR                btfss    pir1, 0        ; only interested in timer 1 overflow
                retfie
                bcf        pir1, 0        ; reset timer 1 interrupt
                decfsz    rollover_count, f
                retfie
                movlw    b'00000001'
                xorwf    portb, f       ; flip the LED
                call    ResetClock
                retfie
                
ResetClock        movlw    HIGH t1rollover
                movwf    tmr1_hi
                movlw    LOW t1rollover
                movwf    tmr1_lo
                movlw    d'100'
                movwf    rollover_count
                return
                
SetClock        call    ResetClock
                bcf        pir1, 0        ; reset timer 1 interrupt
                bsf        status, 5
                bsf        pie1, 0   ; enable tmr1 rollover interrupt
                bcf        status, 5
                bsf        intcon, 7 ; global interrupt enable
                movlw    b'00000001'
                movwf    t1con     ; start clock with no prescaling
                return
        
;----
; Main program
        
Start            call    Init
                call    SetClock
Main            clrwdt
                goto    Main ; loop for interrupt
        
                end
        

--nextPart2912043.rat7JAi7FX--

Re: What am I doing wrong?
Quoted text here. Click to load it

I don't know that CPU, but I saw this:
Quoted text here. Click to load it
  ; interrupt vector
    org  0x04
    goto ISR
...
; Subroutines
    org  0x05
Init   clrf porta
<<

You sure you're leaving enough room for the "goto ISR"? Also, are there any
other vectors that map to 0x05 etc?

Steve
http://www.fivetrees.com



Re: What am I doing wrong?

Quoted text here. Click to load it

AFAIK (could be wrong, but I think not) the program memory addresses are
counted in whole-instruction increments, not in bytes. So 0x05 is the
instruction after. Still, I commented the "org 0x05" out, rebuilt and
reran, unfortunately with no more success. The same result: the rollover
bit in PIR1 gets set as the timer runs, but the ISR is never called.

Re: What am I doing wrong?
On Sat, 17 Sep 2005 00:31:00 +0100, the renowned Julian Morrison

Quoted text here. Click to load it

You need to set PEIE, as well as GIE for the TMR1 interrupts to work.


Best regards,
Spehro Pefhany
--
"it's the network..."                          "The Journey is the reward"
snipped-for-privacy@interlog.com             Info for manufacturers: http://www.trexon.com
We've slightly trimmed the long signature. Click to see the full one.
Re: What am I doing wrong?

Quoted text here. Click to load it

Thanks! It works now.

Damn, who writes these chip manuals anyhow? They buried that info in a
boolean-gate diagram on page 103. No thought to mention "actually,
timer1 needs PEIE" in the section actually about timer1!

At least it wasn't any worse mistake.

Again, thanks :-)

Re: What am I doing wrong?
On Sat, 17 Sep 2005 02:39:07 +0100, the renowned Julian Morrison

Quoted text here. Click to load it

No problem.

Another hint you'll find in Microchip's datasheets is that they often
summarize the various SFR bits and registers required for a given
function, in the section related to the function (in this case TMR1).
You'd probably have to search to find the function of that bit if you
hadn't run into it before, but at least you'd know to look. I have a
datasheet here with that info in Table 7-2. The relevant stuff to TMR1
is spread over 6 bytes in 2 banks. Also, the midrange reference manual
provides more detail organized differently (although it may not always
match newer chips exactly, it can be helpful).


Best regards,
Spehro Pefhany
--
"it's the network..."                          "The Journey is the reward"
snipped-for-privacy@interlog.com             Info for manufacturers: http://www.trexon.com
We've slightly trimmed the long signature. Click to see the full one.
Re: What am I doing wrong?
On Sat, 17 Sep 2005 02:39:07 +0100, the renowned Julian Morrison

Quoted text here. Click to load it

P.S.

You probably know this already, but just in case, you are not saving
and restoring context in the ISR. That means that most anything beyond
your simple loop at Main: will fail because things will be corrupted
by the ISR. Don't bother trying to figure out the save/restore stuff
yourself, just look it up. It requires the use of the swapf (swap
nibbles) instruction... and there are some banking issues (code and
data) <ugh>. Let's not even get into nested ISRs. You have to admire
an architecture that requires an application note to explain how to do
a table lookup.


Best regards,
Spehro Pefhany
--
"it's the network..."                          "The Journey is the reward"
snipped-for-privacy@interlog.com             Info for manufacturers: http://www.trexon.com
We've slightly trimmed the long signature. Click to see the full one.
Re: What am I doing wrong?

Quoted text here. Click to load it

D'oh! As you can see I'm still new to this.

Quoted text here. Click to load it

There's a design approach I've been using to minimize that: "either spin
and poll without using interrupts, or do everything inside the ISR".
This was mainly practise code at interrupt handling.

Quoted text here. Click to load it

Thanks, now I know what to look for, google makes finding it easy.
Banking issues? I'm guessing, switch off GIE before changing banks away
from 0, then run the ISR manually after switching back so as to avoid
dropping interrupts. Is that right?

Quoted text here. Click to load it

"brought to you by the folks who did INTERCAL and Malbolge"

Re: What am I doing wrong?
Quoted text here. Click to load it

The 16Cxxx PIC architecture requires the programmer to be aware of a lot of
things that are not very obvious. There are some pretty clunky things about
interrupts in particular.

First, the stack is used only for return address information.

You cannot push data onto it.

Second, the MOVFW instruction affects the ZERO flag.

This means that when an ISR needs to save the context it must:

Save the W register to a file register location that is allocated at the
same offset in all banks.

Save the status register into the W register using the SWAPF nibble
instruction. The SWAPF instruction must be used because it does not affect
the flags in the status register.

Microchip has application notes and code examples on how to do this.

-------------

You should not need to switch off the GIE from within an ISR as the
interrupt system is disabled until the RETFIE instruction is executed.

You need to know about this when doing nested interrupts. When using nested
interrupts you really need to understand the PIC very thoroughly.

-------------

But the single most important thing about ISR code:

NEVER USE ALL OF THE AVAILABLE REAL TIME IN THE INTERRUPT SERVICE ROUTINES!

In fact using more than 20% of the processor bandwidth in the ISRs can make
debug quite a challenge. Plus the interrupt response could be quite bad.



Re: What am I doing wrong?
To Keyser and Julian.

Quoted text here. Click to load it

In fact, it's even described in the manual, how to save the context
during interrupt (chapter 14.7 in the PIC16F62x manual).

Quoted text here. Click to load it

Nested interrupts on PIC are a very bad sugestion on a newbe.

Quoted text here. Click to load it

This applies not only to PICs, but for most other micros.

At least:
It is always a more helpful, to read also about related things (here the
basics about interrupt on PIC) in the manual.


HTH
Michael

Interrupts and stuff on PIC

Quoted text here. Click to load it

Which may actually be an advantage (for severely limited definitions of
advantage) since the stack can be ignored and let to wrap around if
you're coding in C and you want to use functions as if they were gotos,
such as for coroutines.

Quoted text here. Click to load it

Ah, that explains the swapf.

Quoted text here. Click to load it

I was meaning: switch off the GIE when executing "regular" non-interrupt
code that leaves bank 0 (to avoid being interrupted while in an
unexpected bank).

Also, if code passes through a critical section with GIE off, am I right
it's a good idea to run the ISR manually to catch any missed interrupts
(and rely on the retfie at the end to re-enable GIE)?

Quoted text here. Click to load it

When I said "do everything inside the ISR" I was meaning: sleep, wake on
a timer interrupt, do what needs to be done, and sleep again. So the
processor spends most of its time sleeping. Also, when doing this, only
turn on the one timer interrupt; use the ISR to poll the others.


Re: Interrupts and stuff on PIC
Julian Morrison schrieb:

Quoted text here. Click to load it

It is good practice, to protect critical code sections, but why not save
the context inside the ISR? If you let the PIC sleeping, the additonal
instructions should not be a problem!

Quoted text here. Click to load it

Sorry, but it looks like you are missunderstanding the term ISR! An ISR
is a pice of code, which is not called directly from the ohter code
stuff. The entry to the ISR is placed to a special adress, and the
processor itself run this code, if an event is detected. This event can
be each enabled interrupt condition.
If the processor jumps to the ISR, GIE will automaticly disabled, and
inside the ISR you should never(!) re-enable interrupts (GIE).
The RETFIE instruction is needed to leave the ISR and also re-enables
interrupts. This is made to prevent nested runs of the ISR.

You can also manualy check the interrupt conditions in your main
programm, without(!) implementing an ISR. Then do not enable any
interrupt, and only check the xxxF bits in INTCON and PIR1.

Quoted text here. Click to load it

If the PIC should sleep most time, and do some things from time to time,
then you don't need implicitly a timer with interrupt. Often the the
Watchdog is the better way, and with postscaler the PIC can sleep up to
2.3 sec. But please read the manual sections about all related things
and conditions.


HTH
Michael

Sneaky tricks with ISR on PIC


Quoted text here. Click to load it

Nope, not misunderstanding, being sneaky by doubling up the use of the
ISR code. Based on the assumption: "retfie" is exactly equivalent to
"bsf INTCON, 7" followed by "return". Therefore it doesn't matter who
calls the ISR so long as this behaviour is intended.

The code sequence I was thinking about:

1. normal code
2. GIE off
3. critical section
(at this stage we need to check interrupts and switch on GIE)
4. jump to ISR via a normal "call"
5. ISR has been written to check interrupt bits
6. ISR conditionally does what needs doing
7. ISR finishes with "retfie"
8. retfie re-enables GIE and returns

Result: code saved and simplified by checking interrupts in one place.

Re: Sneaky tricks with ISR on PIC

Quoted text here. Click to load it

Why not just re-enable GIE - the interrupt will just happen if one is pending.

Re: Sneaky tricks with ISR on PIC
Quoted text here. Click to load it

It will? What counts as "pending"? Will the interrupt fire if it happened
at any time while GIE was off? Or, only if the cause is ongoing?

Re: Sneaky tricks with ISR on PIC

Quoted text here. Click to load it

Yes, so you don't have to do anything.

Quoted text here. Click to load it

The interrupt flags are sticky and are maintained regardless of the
interrupt enables. (This also means it's usually disastrous to enable
interrupts in the ISR.)


Re: Sneaky tricks with ISR on PIC
Quoted text here. Click to load it
We have not told you one of the really sneaky bit of the interrupt handling
on the PIC processors.

There are two global interrupt enable bits.

One bit is the GIE bit in the INTCON register, bit 7.

The other is buried inside the PIC with no direct access from the program.

This bit is "set" when the RETFIE is executed and "cleared" when any
interrupt is processed.

For an interrupt to occur at least one interrupt source must be enabled, the
GIE bit in INTCON must be set to one and the internal global interrupt
enable must be "set". If the interrupt source is one of the peripheral
device then the PEIE bit in INTCON must also be set to one.

-----

An a second note as to "what else was wrong" in your code.

There is a bug with the way you reload the TIMER ONE count register from the
ISR.

The bug is that the "t1rollover" is used to update TIMER ONE only when
"rollover_count" reaches zero.

This means the TIMER ONE will request an interrupt 10 milliseconds after the
call to "SetClock" then count 65536 cycles 99 more times until
"rollover_count" reaches zero. This results in a 6,498,064 cycles between
changes of the LED. Or about 1.6 seconds using a 4MHz clock.

But then you have already found this one. :)




Re: Sneaky tricks with ISR on PIC
Quoted text here. Click to load it
           this should be ---> 6.5 seconds <---

Forgot the divide by 4.




Re: Sneaky tricks with ISR on PIC
Quoted text here. Click to load it

There is no "internal global interrupt enable bit."

--
John W. Temples, III

Re: Sneaky tricks with ISR on PIC
Quoted text here. Click to load it
Oh my, you are of course correct.

Although I seem to remember an errata for one of the first 16Cxx
parts to implement interrupt may have had some such quirk.

Sorry for the missinformation.



Site Timeline