part 0 44 bytes his is a multi-part message in MIME format. part 1 4203 bytes content-type:text/plain; charset=ISO-8859-1; format=flowed (decoded 7bit)
Hi all.
Attached is the schematic for my current project. It is a wireless
garage door monitor. It uses two magnetic switches to monitor two garage
doors, and wirelessly transmits the door state and battery state to a
receiver.
The basic concept is simple. The PIC puts all pins as input except AN0
which is used to monitor the battery health. It then sleeps for about
2.5 seconds, wakes up, checks the battery state, checks the door states,
and then activates the transmitter, and transmits a 24 bit data stream
(ID, state, checksum).
The problem is that, after a number of hours (about 5) of continuously
correct operation, it starts transmitting that the one door is 'open'.
First though, some details.
The batteries are 2xAA Lithium 1.5V
IC1 is a PIC12F675 with MCLR off (pin is used as digital input). The PIC
is the DIP version and is socketed, pulled for programming, and then
re-socketed. No need for ICSP.
U1 is a 1.25V reference - REF3312 from Texas Instruments - it has a 5uA
quiescent current.
U2 is a TXM-418-LR from Linx which has 5nA drain when /PDN is pulled low.
The momentary switch is only used to generate a new ID for the
transmitter. If the switch is closed then the unit powers up (resets),
the PIC will simply run a timer until the button is released. Whatever
the value in the (8-bit) timer is when the button is released will
become the devices new ID. The button will do nothing else while the
system operates.
The door that is indicated to be open is always the one that is sensed
through GP1 (pin 6). The door sensed through GP3 is never sensed wrong
(yet).
When the system runs correctly, I measure the following values at the
various pins (using a DMM all measurements on 2000mV range except pin 1
on 20V range, all measurements to ground):
1 3.29
2 0.000
3 0.000
4 0.000
5 0.002
6 0.010 and 3.28 with the magnetic switch closed.
7 1.249
8 0.000
When the system is 'broken', I have the following measurements:
1 3.31
2 0.000
3 0.000
4 0.000
5 0.000
6 1.250 and 1.318 with the magnetic switch 'closed'
7 1.249
8 0.000
The actual values for the 'broken' situation are suspiciously similar
between pin 6 and 7... but, it is weird that it rises to 1.318V when the
switch is closed.....
I am at a loss as to what to check, what may be broken, or where the
design may be wrong.
I have tried the circuit numerous times, and have 5 different
PIC12F675's, and at least two of them have shown this 'broken' behaviour.
The trace from R9 to GP1 (the door sense trace) runs underneath both C1
and R1 which are 1206 footprint parts. I initially suspected that there
was a solder bridge from one of these pads to the trace, but, that
appears not to be the case, and, it would be a consistent problem,
rather than the delayed/intermittent one.
About the only thing I can believe to be happening is that there is a
short of some sort between pin AN0 and GP1 causing the 1.25V reference
voltage to appear on the GP1 pin. The REF3312 is able to sink current to
maintain 1.25V, so, with the effective 2K impedance 3.3V source when the
sensor circuit is closed, and with the 1K between pin AN0 and the
REF3312, I do some math, and determine....
IF the pins have a resistance bridge between them somewhere, the 1M pull
down resistor R3 is not enough to drop the voltage at pin GP1, but, when
the door is open (the switch is closed), the 3.3V source with 2K
impedance is enough to pull the 1.25V to 1.32, or to raise it 0.7V ....
but, it makes no sense that AN0 does not change.......
Finally, before I got so stuck in to the problem, I decided that the 1M
pull-down was not 'strong' enough, so, I decided that it may be a
'capacitive' build up on pin GP1 that was causing the problem. To
'resolve' this perceived issue, I decided to set the GP1 as an output,
and output low for 50us before setting the pin back to an input for
another 200us or so before reading the pin's state through GPIO. I
figured this would be enough to drain any 'spurrious' charges on the pins.
But, it was not successful....
Anyone have any ideas?
Anyone suggest another avenue to explore?
Thanks in advance
Rolf
0.0
part 2 15372 bytes content-type:image/png; (decode) part 3 35 bytes content-type:text/plain; charset="us-ascii" (decoded 7bit)
part 0 44 bytes his is a multi-part message in MIME format. part 1 5132 bytes content-type:text/plain; charset=ISO-8859-1; format=flowed (decoded 7bit)
Two additional items of information:
To 'fix' the circuit, I disconnect the battery and wait a while before
re-connecting it.
I can unplug the magnetic switch entirely leaving the circuit open and
it is still 'broken'. Also, one test showed that when working fine, with
the switch disconnected, it still later 'broke'.
Thanks again
I can supply the source code too... actually, it is attached.
> Hi all.
>
> Attached is the schematic for my current project. It is a wireless
> garage door monitor. It uses two magnetic switches to monitor two
> garage doors, and wirelessly transmits the door state and battery
> state to a receiver.
>
> The basic concept is simple. The PIC puts all pins as input except AN0
> which is used to monitor the battery health. It then sleeps for about
> 2.5 seconds, wakes up, checks the battery state, checks the door
> states, and then activates the transmitter, and transmits a 24 bit
> data stream (ID, state, checksum).
>
> The problem is that, after a number of hours (about 5) of continuously
> correct operation, it starts transmitting that the one door is 'open'.
>
> First though, some details.
>
> The batteries are 2xAA Lithium 1.5V
> IC1 is a PIC12F675 with MCLR off (pin is used as digital input). The
> PIC is the DIP version and is socketed, pulled for programming, and
> then re-socketed. No need for ICSP.
> U1 is a 1.25V reference - REF3312 from Texas Instruments - it has a
> 5uA quiescent current.
> U2 is a TXM-418-LR from Linx which has 5nA drain when /PDN is pulled low.
>
> The momentary switch is only used to generate a new ID for the
> transmitter. If the switch is closed then the unit powers up (resets),
> the PIC will simply run a timer until the button is released. Whatever
> the value in the (8-bit) timer is when the button is released will
> become the devices new ID. The button will do nothing else while the
> system operates.
>
> The door that is indicated to be open is always the one that is sensed
> through GP1 (pin 6). The door sensed through GP3 is never sensed wrong
> (yet).
>
> When the system runs correctly, I measure the following values at the
> various pins (using a DMM all measurements on 2000mV range except pin
> 1 on 20V range, all measurements to ground):
> 1 3.29
> 2 0.000
> 3 0.000
> 4 0.000
> 5 0.002
> 6 0.010 and 3.28 with the magnetic switch closed.
> 7 1.249
> 8 0.000
>
>
> When the system is 'broken', I have the following measurements:
> 1 3.31
> 2 0.000
> 3 0.000
> 4 0.000
> 5 0.000
> 6 1.250 and 1.318 with the magnetic switch 'closed'
> 7 1.249
> 8 0.000
>
> The actual values for the 'broken' situation are suspiciously similar
> between pin 6 and 7... but, it is weird that it rises to 1.318V when
> the switch is closed.....
>
> I am at a loss as to what to check, what may be broken, or where the
> design may be wrong.
>
> I have tried the circuit numerous times, and have 5 different
> PIC12F675's, and at least two of them have shown this 'broken' behaviour.
>
> The trace from R9 to GP1 (the door sense trace) runs underneath both
> C1 and R1 which are 1206 footprint parts. I initially suspected that
> there was a solder bridge from one of these pads to the trace, but,
> that appears not to be the case, and, it would be a consistent
> problem, rather than the delayed/intermittent one.
>
> About the only thing I can believe to be happening is that there is a
> short of some sort between pin AN0 and GP1 causing the 1.25V reference
> voltage to appear on the GP1 pin. The REF3312 is able to sink current
> to maintain 1.25V, so, with the effective 2K impedance 3.3V source
> when the sensor circuit is closed, and with the 1K between pin AN0 and
> the REF3312, I do some math, and determine....
>
> IF the pins have a resistance bridge between them somewhere, the 1M
> pull down resistor R3 is not enough to drop the voltage at pin GP1,
> but, when the door is open (the switch is closed), the 3.3V source
> with 2K impedance is enough to pull the 1.25V to 1.32, or to raise it
> 0.7V .... but, it makes no sense that AN0 does not change.......
>
> Finally, before I got so stuck in to the problem, I decided that the
> 1M pull-down was not 'strong' enough, so, I decided that it may be a
> 'capacitive' build up on pin GP1 that was causing the problem. To
> 'resolve' this perceived issue, I decided to set the GP1 as an output,
> and output low for 50us before setting the pin back to an input for
> another 200us or so before reading the pin's state through GPIO. I
> figured this would be enough to drain any 'spurrious' charges on the
> pins.
>
> But, it was not successful....
>
> Anyone have any ideas?
>
> Anyone suggest another avenue to explore?
>
> Thanks in advance
>
> Rolf
part 2 27458 bytes content-type:text/plain; (decoded 7bit)
;******************************************************************************
; This file is a basic code template for object module code *
; generation on the PIC12F675. This file contains the *
; basic code building blocks to build upon. *
; *
; Refer to the MPASM User's Guide for additional information on *
; features of the assembler and linker (Document DS33014). *
; *
; Refer to the respective PIC data sheet for additional *
; information on the instruction set. *
; *
;******************************************************************************
; *
; Filename: tx.asm *
; Date: 22 May 2008 *
; File Version: 0.9 *
; *
; Author: Rolf Lear *
; Company: tuis.net *
; *
; *
;******************************************************************************
; *
; Files required: P12F675.INC *
; 12f675.lkr *
; *
; *
;******************************************************************************
; *
; Notes: *
; *
; This program will broadcast the A/D voltage based on a 1.25V reference *
; as well as the state of 2 digital inputs over a data link to an RF TX *
; it is designed to work with the Linx TXM series... *
; *
; *
;******************************************************************************
LIST P=12F675 ; list directive to define processor
#INCLUDE <P12F675.INC> ; processor specific variable definitions
radix dec
errorlevel -302
;errorlevel +302
; The button is used to set an ID for the chip.
; connect it to the MCLR pin (which is set as digital input).
; Don't press the button while programming!
#define BUTTONIO 2
; garage A io pin.
#define GAIO 1
; garage B io pin
#define GBIO 3
; output pin for the LED. It is shared with the data pin of the TX
#define LEDIO 4
; The data pin on the TX. The LED goes on when transmitting data too.
#define DATAIO 4
; the power down pin for the TX module. Uses 5nA when powered down.
#define PDNIO 5
; where in EEPROM to store the ID for the device.
#define IDADDRESS 0x00
; which bit in the flags is used for the next data state.
#define BCTOGGLE 0
; whether the bit has been set in the ISR.
#define BCISR 1
; the set happens 18 instructions after the timer resets... and,
; with 2 idle instructions after setting TMR0, plus the set instruction itself
; we need to set the timer for: 256 - (us - 21) where 'us' is the time we want in microseconds
#define TMRSYNC 256 - (160 - 21)
#define TMRSHRT 256 - (120 - 21)
#define TMRLONG 256 - (200 - 21)
;------------------------------------------------------------------------------
;
; CONFIGURATION WORD SETUP
;
; The 'CONFIG' directive is used to embed the configuration word within the
; .asm file. The lables following the directive are located in the respective
; .inc file. See the data sheet for additional information on configuration
; word settings.
;
;------------------------------------------------------------------------------
; read the specified EEPROM address, and put the value in W.
read_eeprom macro ee_address
bsf STATUS,RP0
movlw ee_address
movwf EEADR
bsf EECON1,RD
movf EEDATA,w
bcf STATUS,RP0
endm
; write_eeprom will write the value in W to the ee-prom address 'ee_address'
write_eeprom macro ee_address
clrwdt
bsf STATUS,RP0
movwf EEDATA
movlw ee_address
movwf EEADR
bcf PIR1,EEIF ; ensure the ee-prom interrupt is not set.
; example of using Shared Uninitialized Data Section
INT_VAR UDATA_SHR 0x20
W_TEMP RES 1 ; variable used for context saving
STATUS_TEMP RES 1 ; variable used for context saving
latio res 1 ; latch for GPIO
id res 1 ; our ID
state res 1 ; our state - battery and door...
txc res 1 ; the tx count.
tock res 1 ; used for a timer.....
tick res 1 ; used for a timer.....
bcnext res 1 ; the value to load in to the timer next.
bcflags res 1 ; a bitmask for the t0 state..
bcxmit res 1 ; the byte value to transmit
adtmp res 1 ; used for A/D calculations
bctmp res 1 ; used for broadcast things
;------------------------------------------------------------------------------
; EEPROM INITIALIZATION
;
; The 12F675 has 128 bytes of non-volatile EEPROM, starting at address 0x2100
;
;------------------------------------------------------------------------------
DATAEE CODE 0x2100
DE " Garage V1.0" ; Place Text string in EE-PROM
DATAWWW CODE 0x2140
DE "(c) Rolf Lear. spam_OUTrolfTakeThisOuTtuis.net"
;------------------------------------------------------------------------------
; OSCILLATOR CALIBRATION VALUE
;------------------------------------------------------------------------------
OSC CODE 0x03FF
; Internal RC calibration value is placed at location 0x3FF by Microchip as
; a 0xADDLW K instruction, where the K is a literal value to be loaded into
; the OSCCAL register.
RESET_VECTOR CODE 0x0000 ; processor reset vector
GOTO START ; go to beginning of program
;------------------------------------------------------------------------------
; INTERRUPT SERVICE ROUTINE
;------------------------------------------------------------------------------
INT_VECTOR CODE 0x0004 ; interrupt vector location
MOVWF W_TEMP ; save off current W register contents
MOVF STATUS,w ; move status register into W register
MOVWF STATUS_TEMP ; save off contents of STATUS register
; isr code can go here or be located as a call subroutine elsewhere
clrwdt
; Deal with TMR0
i_tmr0
btfss INTCON,T0IE
goto i_ad
btfss INTCON,T0IF
goto i_ad
bcf INTCON,T0IF
; right, the timer 0 interrupt is set.
; this is used to toggle the DATA line to the radio.
; So, toggle it to the state in bcflags,BCTOGGLE
; this sequence is guaranteed to take 6 instructions
; regardless of whether it is to be toggled on or off.
bcf latio,DATAIO ; 1
btfss bcflags,BCTOGGLE ; 2 or 2/3
goto $+2 ; 3/4
bsf latio,DATAIO ; 4
movf latio,w ; 5
movwf GPIO ; 6
movf bcnext,w ; now, move the next value to the timer...
movwf TMR0 ; this will cost some ticks on the timer too.
bsf bcflags,BCISR ; record the fact that we have been to the ISR...
i_ad
; in the event of a A/D interrupt, the only time we call A/D is
; followed by a sleep. We can ignore the situation in the ISR
; because the handler will be the instruction after the sleep.
; really, the only reason we have the A/D interrupt is to wake
; from sleep.....
btfss PIR1,ADIF
goto i_end
bcf PIR1,ADIF
i_end
MOVF STATUS_TEMP,w ; retrieve copy of STATUS register
MOVWF STATUS ; restore pre-isr STATUS register contents
SWAPF W_TEMP,f
SWAPF W_TEMP,w ; restore pre-isr W register contents
RETFIE ; return from interrupt
;------------------------------------------------------------------------------
; MAIN PROGRAM
;------------------------------------------------------------------------------
MAIN_PROG CODE
START
;------------------------------------------------------------------------------
; OSCCAL RESTORE (not required if internal OSC is not used)
;------------------------------------------------------------------------------
BSF STATUS,RP0 ; set file register bank to 1
CALL 0x3FF ; retrieve factory calibration value
MOVWF OSCCAL ; update register with factory cal value
; BCF STATUS,RP0 ; set file register bank to 0
;------------------------------------------------------------------------------
; PLACE USER PROGRAM HERE
; First, do some initialization.
;------------------------------------------------------------------------------
movlw b'10001111' ; set up the option register.
movwf OPTION_REG ; pull-ups off, T0 from clock, and 1:64 prescalar on WDT (about 1152ms).
movlw b'01000000' ; set up peripheral interupts.
movwf PIE1 ; so that only ADIE is enabled.
movlw b'00110001' ; Set up AN0 as analog, other pins as digital.
movwf ANSEL ; and the A/D unit to use internal dediated 500kHz RC clock...
bcf STATUS,RP0 ; set file register bank to 0
clrf latio ; make latio and
clrf GPIO ; make all outputs low.
clrf PIR1 ; clear all peripheral Interrupt flags...
movlw b'11000000' ; set up the INTCON for enable GIE and PIE.
movwf INTCON
movlw b'00000001' ; Set up the A/D unit. Start with it on, set to AN0 as input.
movwf ADCON0 ; The AD module takes almost nothing to have on....
; numbers are ambiguous on Datasheet, but probably less than 1uA
movlw b'00000111' ; turn off comparator. This is a power hog if on.
movwf CMCON
; Check status of 'data' input line. It will be high if we need to randomize our ID.
btfsc GPIO,BUTTONIO;
call fn_new_id
clrwdt
; Right, let's read our id.... from EEPROM...
read_eeprom IDADDRESS
movwf id
; OK, to the business of monitoring the garage doors.
; The WDT is enabled, and will timeout every 1.1s or so.
; the routine is to sleep until the WDT expires, then to
; broadcast our state. then to sleep all over again.
main_loop
; sleep will go for a second while the WDT expires.
nop
sleep
;clrf tock
;clrwdt
;decfsz tock
;goto $-2
nop
nop
call fn_get_state
clrf latio
clrf GPIO
; set the output pins back to input (saves power).
; first, pull pins hard to ground.
setio_low DATAIO
setio_low PDNIO
; then set the IO's as input....
bsf STATUS,RP0
bsf TRISIO,PDNIO
bsf TRISIO,DATAIO
bcf STATUS,RP0
goto main_loop
; -----------------------------------------
; Function that broadcasts our ID and State
; to anyone who cares to listen
; -----------------------------------------
fn_broadcast
; right, the broadcast is a predefined sequence.
; 0. Neaten up the broadcast period to a nice round number....
; 1. send a series of sync bits (31).
; 2. send a low then high bit to help lock on.
; 3. send the ID
; 4. send the state
; 5. send the XOR of the ID and state for validation....
; 6. send a transmit ID, short for the first cycle, long for the second.
; 7. send a single bit to clock out the receiver.
; 0. now, set the next timer interrupt for something reasonable
; and neat... exactly 20ms from the start of one broadcast
; to the start of the next.....
clrf bcnext ; set up the next interval...
bsf bcflags,BCTOGGLE
clrwdt
btfss bcflags,BCISR ; wait for the ISR to run.
goto $-2
bcf bcflags,BCISR ; indicate that we have come out of the wait for ISR
; then a low pulse.
clrf bcnext ; set up the next interval...
bcf bcflags,BCTOGGLE
clrwdt
btfss bcflags,BCISR ; wait for the ISR to run.
goto $-2
bcf bcflags,BCISR ; indicate that we have come out of the wait for ISR
movlw 33 ; this is the tweak constant to make the cycle 20ms.
movwf bcnext;
btfss bcflags,BCISR ; wait for the ISR to run.
goto $-2
bcf bcflags,BCISR ; indicate that we have come out of the wait for ISR
; 1. send 32 sync bits.
; each bit is 50/50 duty cycle, with rate of about 7.5kBaud.
; this makes each part about 150us.
; the trick with this code is to use the instruction time to
; keep constant bit-rate.
movlw 32
movwf bctmp
; 2. send a short bit for reference....
; followed by a long bit.
call fn_send_data_low
call fn_send_data_high
; 3. now, send the ID
movf id,w
call fn_send_w
; 4. now, send the state
movf state,w
call fn_send_w
; 5. now send the XOR
movf state,w
xorwf id,w
call fn_send_w
; 6. now send an ID pulse.
; send a short pulse if we are the first transmission,
; send a long pulse if we are the second.
btfss txc,0
call fn_send_data_low
btfsc txc,0
call fn_send_data_high
; 7. now send a single pulse to clock out the receiver.
call fn_send_data_low
retlw 0
; -----------------------------------------
; Function that reads W, and sends the bits
; over the air....
; -----------------------------------------
fn_send_w
movwf bcxmit
movlw 8
movwf bctmp
; function that schedules a high value
; to be broadcast next for the time period in W
fn_send_bit_high
movwf bcnext ; set up the next interval...
bsf bcflags,BCTOGGLE
clrwdt
btfss bcflags,BCISR ; wait for the ISR to run.
goto $-2
bcf bcflags,BCISR ; indicate that we have come out of the wait for ISR
retlw 0
; function that schedules a high value
; to be broadcast next for the time period in W
fn_send_bit_low
movwf bcnext ; set up the next interval...
bcf bcflags,BCTOGGLE
clrwdt
btfss bcflags,BCISR ; wait for the ISR to run.
goto $-2
bcf bcflags,BCISR ; indicate that we have come out of the wait for ISR
retlw 0
; ---------------------------------------
; Function that reads the Battery voltage
; and converts it in to an 0.05V offset
; from 2.20V with 32 battery levels.
; ---------------------------------------
fn_get_state
; for debug purposes let's make the ADRESH = 150
; movlw 150
; movwf ADRESH
bcf latio,GAIO
bcf latio,GBIO
; set the door sense pins as output
; we set the door pins to ground to clear any capacitive charges.
bsf STATUS,RP0
bcf TRISIO,GAIO
bcf TRISIO,GBIO
bcf STATUS,RP0
; Set the input pins as 0V to clear capacitance...
movf latio,W
movwf GPIO
; wait 30 or so us to ensure the pin is grounded...
movlw 10
movwf tick
decfsz tick,f
goto $-1
; set the door sense pins back as input
; the door pins were connected to ground to clear any capacitive charges.
; now we set them back to input.
; and give it a short while to settle.
bsf STATUS,RP0
bsf TRISIO,GAIO
bsf TRISIO,GBIO
bcf STATUS,RP0
; Right, let's do some math....
; the objective is to get a scaled value of the voltage to 0.05V
; increments on a scale of 0-32 where 0 indicates 2.0V and 40 indicates
; 4.0 v. We do this with table lookups....
; we set up the A/D to be left justified. We expect values
; between 319 and 640 (correspond to 4.0 and 2.0V)
; with the high-byte left-formatted in ADRESH, this is about 79 and 160
; respectively.
; do some sanity checking....
movf ADRESH,w
sublw 79 ; W is subtracted from 79. We expect a negative result.
btfsc STATUS,C ; which would make C = 0.
goto _fn_get_state_error
; good, now check for values more than 160...
movf ADRESH,w
sublw 160 ; W is subtracted from 160. We expect a value <= 160,
btfss STATUS,C ; so, the result should be >= 0
goto _fn_get_state_error
; OK, we are in the batt-voltage ball-park.
; we need to subtract 256 from the 10-bit A/D value, which means subtracting
; 64 from the high byte....
movlw 64
subwf ADRESH,w
movwf adtmp
; now, we need to halve the 10bit value, but, we do this by shifting the Most-
; significant bit of the ADRESL value in to the high byte. We are certain that the high-byte will
; be less than 128 because, at worst, the high byte started at 160 (640/4), and we subtracted 64
bsf STATUS,RP0
rlf ADRESL,w ; put the high-bit in to STATUS,C
bcf STATUS,RP0
rlf adtmp,f ; move STATUS,C in to adtmp after shifting it....
; our value in adtmp will be between 31 and 191 for voltages between 4.0 and 2.0V respectively.
; lets make it easier to digest... subtract the value from 191 giving a value from 0 to 160 for 2.0V to 4.0V
movf adtmp,w
sublw 191
movwf adtmp
; now, we have an 8-bit value which is relatively scaled according to the Vbatt.
; we need to check our lookup table to get an idea of where the actual voltage is...
; the lookup function will return the voltage as a number of 0.05V offsets from 2.0V
call fn_vbatt_lookup
movwf state
btfsc GPIO,GAIO
bsf state,7
btfsc GPIO, GBIO
bsf state,6
clrwdt
retlw 0;
_fn_get_state_error
movlw 0x01
movwf state
btfsc GPIO,GAIO
bsf state,7
btfsc GPIO, GBIO
bsf state,6
retlw 0
fn_vbatt_lookup
; this is a series of 'sieve' fall throughs.
; we keep subtracting a pre-calculated value from adtmp
; until it breaks through zero. at which point we return
; the 0.05V offset from 2.0V
; the ad_calc macro will subtract the first argument, and,
; if the result is less than zero, it will return the second argument.
; i.e. if adtmp starts at 5, it will subtract 8, which results
; in a negative value, so we will return the constant 0
; --------------------------------
; Function to create new ID for TX
; --------------------------------
fn_new_id
; we come here if the button is pressed when the chip resets.
; we start the timer, and let it run until the button is no longer pressed.
; then we use the value in the timer to produce a new ID, and store the ID in EEPROM.
; TMR0 runs continuously. We don't really care what the value is.
; make the LED output
bsf STATUS,RP0
bcf TRISIO,LEDIO
bcf STATUS,RP0
setio_high LEDIO
btfsc GPIO, BUTTONIO ; do a tight loop to get release-point
goto $-1 ; for when user lets go of button.
; right, the user has released the button.
; because the loop is 3 instructions long, it could cause funny gaps in possible numbers.
; but, when it wraps around it will be a different sequence.
movfw TMR0
write_eeprom IDADDRESS
; OK, the value is written to EEPROM.
setio_low LEDIO
; turn the LED back to input.
bsf STATUS,RP0
bsf TRISIO,LEDIO
bcf STATUS,RP0
retlw 0
END ; directive 'end of program'
part 3 35 bytes content-type:text/plain; charset="us-ascii" (decoded 7bit)
I found a bug in the code. I was not ensuring I was in memory bank 0 in
the ISR. This could have caused all sorts of problems, I guess, but,
could it have caused this one? The only SFR's that the ISR modifies are
GPIO, and TMR0 which in turn, if I was in the wrong bank, would have
modified TRISIO, and the OPTION register.
I guess the combination of those two registers could do some pretty
weird things to the code, but, changes to the prescalar or TMR0 clock
source would have been very obvious, and changes to the TRISIO would
also have been noticed.... I think.
Anyway, I have fixed the code, but now have to wait (forever) to see if
it breaks again.....
> Two additional items of information:
>
> To 'fix' the circuit, I disconnect the battery and wait a while before
> re-connecting it.
>
> I can unplug the magnetic switch entirely leaving the circuit open and
> it is still 'broken'. Also, one test showed that when working fine,
> with the switch disconnected, it still later 'broke'.
>
> Thanks again
>
> I can supply the source code too... actually, it is attached.
>
> Rolf
>
> Rolf wrote:
>> Hi all.
>>
>> Attached is the schematic for my current project. It is a wireless
>> garage door monitor. It uses two magnetic switches to monitor two
>> garage doors, and wirelessly transmits the door state and battery
>> state to a receiver.
[snip]
>> Anyone have any ideas?
>>
>> Anyone suggest another avenue to explore?
>>
>> Thanks in advance
>>
>> Rolf
If you have long wires going to your door sensor magnets, I would suggest
lowering the 1M to ground to 10K and bypassing them with caps. You have
quite an "antenna system" there. In fact I would use a dual opto-isolator
on those inputs. I did that on quite a few projects in a factory
environment.
I know you are on batteries, but isolating those wires from rf interference
is necessary. Besides radio RF there are other things in the garage, tools,
washer, dryer, and the garage door motors themselves. All could cause
problems.
Maybe you could use a counter hooked to your sleep interrupt to extend data
transmission to a few minutes to save power too?
My Ham radio here caused the neighbors solar circulation system to start
when I went on 40 and 20 M!
They were really mad at me when they figured it out.
The wires were just right, about 33ft long! Several 0.1uf caps saved the
day.
What do you have figured out for a receiver? Is it done yet?
Now if I could just shut the doors from inside the house with this system?
Oh...another project!
The device has two door channels. Building it for myself, and then
another for a friend.
The wires on the magnetic switches are about a foot long. Pretty short I
thought. The device will sit between the two doors on the centre column.
I debated the 1M pull downs. I pull all pins down except the AN0 pin. I
figured 1M was at the high end, but I did not want to go as low as 10K.
I could have gone 100K or there abouts, but I was really pushing for
long battery life. In the end, I guess I get 4 years of continuous use
out of two 3000mAh Lithium cells, and because of the nice discharge
curve, the power will be there till the end.
I could change out the 1M resistors easily, but, they seem to be
working, unless they are not, of course.
My problem is that the circuit board is home-made. I believe it to be of
high quality and I did a full board test before I soldered anything on.
There were no bridged traces, and, after soldering on everything, I had
one bridged trace which I was able to resolve...
It is possible that there is another bridged trace, but I can't find it,
and I have looked hard. Also, because I don't have a solder mask, I have
'brushed' the board with epoxy to protect the exposed copper. Changing
things now will be a little messy, but possible.
I was sort of thinking that I could enter the project in to Jason's PCB
contest to get free PCB's.... but I may just go through sparkfun too.
But, first it has to work.
As for the antenna, I am using an RP-SMA mounted antenna, and it has 4
ground 'pins'. That's why it has the 4 legs.
The receiver is already designed, built, soldered too. It all works
quite well, when it works. I have been doing this one for a few months
now, it is really frustrating to only find the problem now...
> Nice project Rolf!
>
> If you have long wires going to your door sensor magnets, I would suggest
> lowering the 1M to ground to 10K and bypassing them with caps. You have
> quite an "antenna system" there. In fact I would use a dual opto-isolator
> on those inputs. I did that on quite a few projects in a factory
> environment.
> I know you are on batteries, but isolating those wires from rf interference
> is necessary. Besides radio RF there are other things in the garage, tools,
> washer, dryer, and the garage door motors themselves. All could cause
> problems.
> Maybe you could use a counter hooked to your sleep interrupt to extend data
> transmission to a few minutes to save power too?
>
> My Ham radio here caused the neighbors solar circulation system to start
> when I went on 40 and 20 M!
> They were really mad at me when they figured it out.
> The wires were just right, about 33ft long! Several 0.1uf caps saved the
> day.
>
> What do you have figured out for a receiver? Is it done yet?
>
> Now if I could just shut the doors from inside the house with this system?
> Oh...another project!
>
> Good Luck,
>
> Mike
>
> Hmmm...
>
> I found a bug in the code. I was not ensuring I was in memory bank 0 in
> the ISR. This could have caused all sorts of problems, I guess, but,
> could it have caused this one? The only SFR's that the ISR modifies are
> GPIO, and TMR0 which in turn, if I was in the wrong bank, would have
> modified TRISIO, and the OPTION register.
>
> I guess the combination of those two registers could do some pretty
> weird things to the code, but, changes to the prescalar or TMR0 clock
> source would have been very obvious, and changes to the TRISIO would
> also have been noticed.... I think.
>
> Anyway, I have fixed the code, but now have to wait (forever) to see if
> it breaks again.....
>
> Still looking for advice.
>
> Rolf
>
> Rolf wrote:
>
>> Two additional items of information:
>>
>> To 'fix' the circuit, I disconnect the battery and wait a while before
>> re-connecting it.
>>
>> I can unplug the magnetic switch entirely leaving the circuit open and
>> it is still 'broken'. Also, one test showed that when working fine,
>> with the switch disconnected, it still later 'broke'.
>>
>> Thanks again
>>
>> I can supply the source code too... actually, it is attached.
>>
>> Rolf
>>
>> Rolf wrote:
>>
>>> Hi all.
>>>
>>> Attached is the schematic for my current project. It is a wireless
>>> garage door monitor. It uses two magnetic switches to monitor two
>>> garage doors, and wirelessly transmits the door state and battery
>>> state to a receiver.
>>>
> [snip]
>
>
>>> Anyone have any ideas?
>>>
>>> Anyone suggest another avenue to explore?
>>>
>>> Thanks in advance
>>>
>>> Rolf
>>>
>
>
I'd also suggest stronger pulldowns than 1Meg on the door switches
(and capacitors), To minimise power consumption, maybe retain the 1Meg
resistors, but add 10ks to a "spare" pin and activate these only when
taking a reading. You should be able to figure out a way to use pin 5
as the pulldown pin as it's only used once for code setting. (I
think).
I was going to suggest swapping the connections from the "good" &
"bad" doors, but since the problem remains with neither attached, it
seems more a software or pcb problem than external hardware.
Since it always (?) works OK for the first few hours, I'd suspect a
software, rather than hardware issue.
Getting the memory banking wrong can lead to all sorts of strange
problems and I wouldn't have been surprised if your problem is now
fixed (But obviously not however, based on your latest email).
Possibly another, similar error somewhere?
> Thanks...
>
> The device has two door channels. Building it for myself, and then
> another for a friend.
>
> The wires on the magnetic switches are about a foot long. Pretty short I
> thought. The device will sit between the two doors on the centre column.
>
> I debated the 1M pull downs. I pull all pins down except the AN0 pin. I
> figured 1M was at the high end, but I did not want to go as low as 10K.
> I could have gone 100K or there abouts, but I was really pushing for
> long battery life. In the end, I guess I get 4 years of continuous use
> out of two 3000mAh Lithium cells, and because of the nice discharge
> curve, the power will be there till the end.
>
> I could change out the 1M resistors easily, but, they seem to be
> working, unless they are not, of course.
>
> My problem is that the circuit board is home-made. I believe it to be of
> high quality and I did a full board test before I soldered anything on.
> There were no bridged traces, and, after soldering on everything, I had
> one bridged trace which I was able to resolve...
>
> It is possible that there is another bridged trace, but I can't find it,
> and I have looked hard. Also, because I don't have a solder mask, I have
> 'brushed' the board with epoxy to protect the exposed copper. Changing
> things now will be a little messy, but possible.
>
> I was sort of thinking that I could enter the project in to Jason's PCB
> contest to get free PCB's.... but I may just go through sparkfun too.
> But, first it has to work.
>
> As for the antenna, I am using an RP-SMA mounted antenna, and it has 4
> ground 'pins'. That's why it has the 4 legs.
>
> The receiver is already designed, built, soldered too. It all works
> quite well, when it works. I have been doing this one for a few months
> now, it is really frustrating to only find the problem now...
>
> Rolf
>
>
>
> Michael Hagen wrote:
>> Nice project Rolf!
>>
>> If you have long wires going to your door sensor magnets, I would suggest
>> lowering the 1M to ground to 10K and bypassing them with caps. You have
>> quite an "antenna system" there. In fact I would use a dual opto-isolator
>> on those inputs. I did that on quite a few projects in a factory
>> environment.
>> I know you are on batteries, but isolating those wires from rf interference
>> is necessary. Besides radio RF there are other things in the garage, tools,
>> washer, dryer, and the garage door motors themselves. All could cause
>> problems.
>> Maybe you could use a counter hooked to your sleep interrupt to extend data
>> transmission to a few minutes to save power too?
>>
>> My Ham radio here caused the neighbors solar circulation system to start
>> when I went on 40 and 20 M!
>> They were really mad at me when they figured it out.
>> The wires were just right, about 33ft long! Several 0.1uf caps saved the
>> day.
>>
>> What do you have figured out for a receiver? Is it done yet?
>>
>> Now if I could just shut the doors from inside the house with this system?
>> Oh...another project!
>>
>> Good Luck,
>>
>> Mike
>>
On Sunday, June 29, 2008 5:30 PM [GMT-3=CET],
Rolf wrote:
> Hi all.
>
> Nope, problem still there. Somewhere between 2.5 and 3 hours to manifest
> this time.
>
> Anyone? ;-)
>
> Rolf
Rolf,
I know is somewhat anoying, but Do you have the same problem when you test
it over your working table?
I don't think you have noise.
Since this is the unique pin that makes you crazy, why you don't use another
pin? or switch the function pin to another?, or try with another pic?,...
just to be sure is not the damn pic.
I have replaced the 1M pull downs on the doors with 100K pull downs. I
also pulled up the R1 1K resistor that feeds the 1.25V to the A/D unit.
The trace for the door runs onder this 1206 resistor, so I thought there
may be a bridge under it. Well, there *may* have been. When I ripped it
up I found some un-melted solder paste there...
Well, I put another 1K resistor back there and I'm hoping this resolves it.
I followed the discussion not so long ago about solder paste, and, since
then I have not use ot for hand-soldering (I did a 'toaster-oven'
project a while back that I used the paste for, and it makes life quite
easy (or so I thought) even when hand-soldering.
Well, if the problem goes away I will attribute it to the solder paste.
If not, well, back to square one.
I'll run it overnight and see in the morning.
Attached is a small pic of the circuit (because I am quite happy with
the etching process, etc.
> Rolf,
>
> I'd also suggest stronger pulldowns than 1Meg on the door switches
> (and capacitors), To minimise power consumption, maybe retain the 1Meg
> resistors, but add 10ks to a "spare" pin and activate these only when
> taking a reading. You should be able to figure out a way to use pin 5
> as the pulldown pin as it's only used once for code setting. (I
> think).
>
> I was going to suggest swapping the connections from the "good" &
> "bad" doors, but since the problem remains with neither attached, it
> seems more a software or pcb problem than external hardware.
>
> Since it always (?) works OK for the first few hours, I'd suspect a
> software, rather than hardware issue.
>
> Getting the memory banking wrong can lead to all sorts of strange
> problems and I wouldn't have been surprised if your problem is now
> fixed (But obviously not however, based on your latest email).
> Possibly another, similar error somewhere?
>
> Richard P
>
>
>
>
> 2008/6/30 Rolf <learrKILLspamrogers.com>:
>
>> Thanks...
>>
Tried multiple PICs, I don't believe I had the problem when the TX unit
was in a solderless breadboard, but that could mean anything. The
circuit is now 'established' in a hand-made PCB, so I am somewhat
reluctant to cut traces and rewire it. I also 'potted' the board with
epoxy to stop the copper from oxidising. Access is a little problmatic.
Still, I have changed the 1M pull downs to 100K, and found a potential
bridge under a 1206 resistor.
The only change in the code since I posted it was to add 'bcf
STATUS,RP0' to the ISR after saving away the STATUS register. (updated,
I also bcf INTCON,T0IE after the second fn_broadcast).
Further analysis of mine indicates that the PIC is not likely to be in
the ISR with RP0 set (high memory bank). The interrupts will never be
enabled at any point when the main loop is in the high bank. Hmm,
actually, they may be... which is a bug too. Fixed now, although the
timing of the TMR0 is such that I don't believe that the ISR is entered
at when in Bank 1. I offer as evidence, that, if that were to happen,
the check for the PIR1,ADIF flag would actually hit the PIE1 register,
and swithc off the A/D Interrupt enable, causing the A/D sleep operation
to never exit till the WDT reset a full 2.3 seconds later...
This would be very obvious to me since the receiver would complain lots
and lots about a missed transmission. Further, the A/D setting would
never be reset to on, so, there would always be 5 seconds between
transmisions instead of 2.5. No, that's wrong. If the ADIE were set to
0, the A/D sleep instruction would reset the PIC and the ADIE will be
reset in the initialization routine to 1. Which will mean just one
glitch which I may miss...
All very confusing for a simple project.
Well, it is bed time for me. Let's see what has happened in the morning.
Thanks again
P.S. hope you don't waste time on this. I am thinking that the most
logical answer to my riddle was the solder paste under R1. But, it is
very weird how it manifest with a 'delayed' activation. There were times
in the past few days where I have felt that physically manipulating the
board has 'fixed' the problem, but I always deemed that to be a result
of my fingers changing the capacitive load somewhere.... but, it may
have been me 'jiggling' the bridge under R1.
Anyway, the code is relatively presentable, and I have been racking my
brain for a few days, so I would greatly appreciate another pair of eyes
to see anything else I may have missed.
Regradless, until it manifests again I am calling it 'case closed'.
> On Sunday, June 29, 2008 5:30 PM [GMT-3=CET],
> Rolf wrote:
>
>
>> Hi all.
>>
>> Nope, problem still there. Somewhere between 2.5 and 3 hours to manifest
>> this time.
>>
>> Anyone? ;-)
>>
>> Rolf
>>
>
> Rolf,
> I know is somewhat anoying, but Do you have the same problem when you test
> it over your working table?
> I don't think you have noise.
>
> Since this is the unique pin that makes you crazy, why you don't use another
> pin? or switch the function pin to another?, or try with another pic?,...
> just to be sure is not the damn pic.
>
> I'll dig into the code now.
> bye
> Dennis
>
>
>
>
>
>The only change in the code since I posted it was to add 'bcf
>STATUS,RP0' to the ISR after saving away the STATUS register.
>(updated, I also bcf INTCON,T0IE after the second fn_broadcast).
I assume you are also restoring the STATUS register at the end of the
interrupt (haven't looked at the code you attached earlier) ???
> All very confusing for a simple project.
>
> Well, it is bed time for me. Let's see what has happened in the morning.
>
> Thanks again
>
>
> P.S. hope you don't waste time on this. I am thinking that the most
> logical answer to my riddle was the solder paste under R1. But, it is
> very weird how it manifest with a 'delayed' activation. There were times
> in the past few days where I have felt that physically manipulating the
> board has 'fixed' the problem, but I always deemed that to be a result
> of my fingers changing the capacitive load somewhere.... but, it may
> have been me 'jiggling' the bridge under R1.
>
> Anyway, the code is relatively presentable, and I have been racking my
> brain for a few days, so I would greatly appreciate another pair of eyes
> to see anything else I may have missed.
>
> Regradless, until it manifests again I am calling it 'case closed'.
>
> Rolf
>
>
> Dennis Crawley wrote:
>
>> On Sunday, June 29, 2008 5:30 PM [GMT-3=CET],
>> Rolf wrote:
>>
>>
>>
>>> Hi all.
>>>
>>> Nope, problem still there. Somewhere between 2.5 and 3 hours to manifest
>>> this time.
>>>
>>> Anyone? ;-)
>>>
>>> Rolf
>>>
>>>
>> Rolf,
>> I know is somewhat anoying, but Do you have the same problem when you test
>> it over your working table?
>> I don't think you have noise.
>>
>> Since this is the unique pin that makes you crazy, why you don't use another
>> pin? or switch the function pin to another?, or try with another pic?,...
>> just to be sure is not the damn pic.
>>
>> I'll dig into the code now.
>> bye
>> Dennis
>>
>>
>>
>>
>>
>>
>
>
Alan B. Pearce wrote:
>> The only change in the code since I posted it was to add 'bcf
>> STATUS,RP0' to the ISR after saving away the STATUS register.
>> (updated, I also bcf INTCON,T0IE after the second fn_broadcast).
>>
>
> I assume you are also restoring the STATUS register at the end of the
> interrupt (haven't looked at the code you attached earlier) ???
>
>
Thanks Alan.
The code is base don the 'default' Microchip .asm template for the
PIC12F675. It does a full save and restore of the W and STATUS
registers. So, yes, I restore STATUS.
The receiver part of the project also uses the PIC12F675 and the same
code template. I *did* remember to switch to bank 0 in the receiver code
though.
In reference to my project in particular though, I don't believe it is
possible for the interrupt to be called when the main loop is in bank 1.
The symptoms of such an event would be very obvious if it were to
happen. Even I would have found that first time... ;-)
Olin Lathrop wrote:
>> The only change in the code since I posted it was to add 'bcf
>> STATUS,RP0' to the ISR after saving away the STATUS register.
>>
>
> And why would you do that? What about RP1? Doing a CLRF STATUS takes care
> of all three bank bits in one instruction.
>
>
> ********************************************************************
> Embed Inc, Littleton Massachusetts, http://www.embedinc.com/products
> (978) 742-9014. Gold level PIC consultants since 2000.
>
Well, there are only two banks on the PIC12F675, and thus only RP0 (RP1
is reserved and should always be maintained as 0).
So, since all the banking I have in my program is done with bcf and bsf
STATUS,RP0 it seemed to be consistent to do it the same way in the ISR.
I have it 'mentally embedded' in my head that bcf STATUS,RP0 and bsf
STATUS,RP0 are to do with banking, and I never otherwise manipulate the
STATUS register with non-bitwise operations.
In other words, to keep things straight in my head I try to only change
what I intend to change. Changing things 'for the sake of it' leads me
in to deep dark places of despair. Also, the bcf/bsf is what the
assembler would do with a banksel instruction...
I may not be doing the best thing in your eyes on this one, but, it
keeps things somewhat straighter in my head that all banking is done the
same way.
Rolf wrote:
> Well, there are only two banks on the PIC12F675, and thus only RP0
> (RP1 is reserved and should always be maintained as 0).
On the 12F675, but not so on many other PICs. You're going to have trouble
when you copy and paste this code for use with another PIC. It's the same
number of instructions to do it right.
> So, since all the banking I have in my program is done with bcf and
> bsf STATUS,RP0
That's another problem. You're screwed if you ever try to copy the code for
use on a 4 bank PIC. At the very least use BANKSEL. Once you are
comfortable with the PIC banking, you can use my DBANKIF and related macros
that track the bank setting at assembly time.
> I have it 'mentally embedded' in my head that bcf STATUS,RP0 and bsf
> STATUS,RP0 are to do with banking, and I never otherwise manipulate
> the STATUS register with non-bitwise operations.
Then you should un-embed it. That is a bad way to handle banking.
> In other words, to keep things straight in my head I try to only
> change what I intend to change.
But this changes between PICs, and also invites you to make a mistake by
selecting the wrong bank for a particular register. BANKSEL is much better
in that it sets the bank to whatever is required to access the address you
specify. On a PIC with only two banks it will only generate a single BSF or
BCF instruction on RP0. BANKSEL eliminates the error of chosing the wrong
bank, hides the PIC-specific mechanics of switching the bank, and works
accross PICs. There is less to keep straight in your head.
I inderstand banksel, and what it does... and I know the assembler will
reduce it to a single instruction for the PIC12F675... but...
the code was written with bank 0 being the default. The thinking is that
the device is always in bank 0 except when accessing bank1 registers.
Thus, the code is simplified to a large extent, with a small scattering
of bsf and bcf STATUS,RP0.
Using this 'philosophy, I would have to do a banksel before *and after*
accessing a Bank 1 register. So, what do you choose as a banksel
'target' after accessing a bank 1 variable...
banksel TRISIO
bcf TRISIO,0
banksel ??????
Well, I could choose something I know in the PIC12F675 is in bank 1,
like GPIO, but, banksel is supposed to be called *before* accessing a
register, not after it, and it should be called for the register being
accessed, etc. Having banksel ??? after accessing a bank 1 address is
more problematic than a specific bcf STATUS,RP0.
Anyway, there are other reasons too, just like your copy paste example.
If I ued banksels and then used the code in another device with more
banks (and RP1), then the timing of some loops would be different, etc.
Bottom line is that if I ever were to copy/paste the code it would have
a different set of issues depending on whether I used banksel of not.
As for using your development environment, well, no. I am very happy
using MPLab to do all the code, and in-place debugging. I have no
intention of introducing additional non-compatible steps to the build
environment. I have in the past downloaded and evaluated your system,
and I can see that it has many strengths. On the other hand, My
assembler programs are few, simple, and easy to manage with out the
administrative costs of your system, as well as losing the integration
with MPLab and the ICD2 and PicKit2 I already have. Also, for the larger
devices I have been using C18 with The student edition compiler, I still
keep all the debug features, as well as the integration, etc, but would
be completely incompatible with your toolset.
'Doing it right' is not necessarily clrf STATUS though, is it. It will
work fine in the ISR, but, not elsewhere. Regarless, it is an opinion
statement, and there are different times when different methods are
better. I am happy with the bcf in my context because it makes the
entire program internally consistent.
A consitent program is far more intelligible than an inconsistent one.
The strategy I have chosen is incompatible with your way of thinking,
but, from my perspective, that's OK. It makes sense to me. There may
come a time when I am ready to onvest more in the infrastructure
surrounding the code, but now is not the time. If I use copy/paste for
this code more than twice in my life I will be more than surprised.
Further, I would much rather have 'obvious' incompatibilities like
mising RP1 on a status register, than to have 'obscure' problems like
changes to program timings, and changes to relative GOTO statements that
happen when using banksel.
> Rolf wrote:
>
>> Well, there are only two banks on the PIC12F675, and thus only RP0
>> (RP1 is reserved and should always be maintained as 0).
>>
>
> On the 12F675, but not so on many other PICs. You're going to have trouble
> when you copy and paste this code for use with another PIC. It's the same
> number of instructions to do it right.
>
>
>> So, since all the banking I have in my program is done with bcf and
>> bsf STATUS,RP0
>>
>
> That's another problem. You're screwed if you ever try to copy the code for
> use on a 4 bank PIC. At the very least use BANKSEL. Once you are
> comfortable with the PIC banking, you can use my DBANKIF and related macros
> that track the bank setting at assembly time.
>
>
>> I have it 'mentally embedded' in my head that bcf STATUS,RP0 and bsf
>> STATUS,RP0 are to do with banking, and I never otherwise manipulate
>> the STATUS register with non-bitwise operations.
>>
>
> Then you should un-embed it. That is a bad way to handle banking.
>
>
>> In other words, to keep things straight in my head I try to only
>> change what I intend to change.
>>
>
> But this changes between PICs, and also invites you to make a mistake by
> selecting the wrong bank for a particular register. BANKSEL is much better
> in that it sets the bank to whatever is required to access the address you
> specify. On a PIC with only two banks it will only generate a single BSF or
> BCF instruction on RP0. BANKSEL eliminates the error of chosing the wrong
> bank, hides the PIC-specific mechanics of switching the bank, and works
> accross PICs. There is less to keep straight in your head.
>
>
> ********************************************************************
> Embed Inc, Littleton Massachusetts, http://www.embedinc.com/products
> (978) 742-9014. Gold level PIC consultants since 2000.
>
I believe that the context saving is not right in your code. You likely to
overwrite Z flag, however, I can't see any section of your code that would
use the zero flag so it might not be the case.
Tamas
;------------------------------------------------------------------------------
> ; INTERRUPT SERVICE ROUTINE
>
> ;------------------------------------------------------------------------------
>
> INT_VECTOR CODE 0x0004 ; interrupt vector location
> MOVWF W_TEMP ; save off current W register contents
> MOVF STATUS,w ; move status register into W register
> MOVWF STATUS_TEMP ; save off contents of STATUS register
>
>
Should be:
INT_VECTOR CODE 0x0004 ; interrupt vector location
MOVWF W_TEMP ; save off current W register contents
SWAPF STATUS,w ; move status register into W register
MOVWF STATUS_TEMP ; save off contents of STATUS register
i_end
>
> MOVF STATUS_TEMP,w ; retrieve copy of STATUS register
> MOVWF STATUS ; restore pre-isr STATUS register contents
> SWAPF W_TEMP,f
> SWAPF W_TEMP,w ; restore pre-isr W register contents
> RETFIE ; return from interrupt
>
Should be:
i_end
SWAPF STATUS_TEMP,w ; retrieve copy of STATUS register
MOVWF STATUS ; restore pre-isr STATUS register contents
SWAPF W_TEMP,f
SWAPF W_TEMP,w ; restore pre-isr W register contents
RETFIE ; return from interrupt
The code itself is unmodified from the 'template' as supplied by
Microchip, so I would be surprised... but, two observations....
1. If the MOVF,w changes Z before actually moving the value, then you
may be right, otherwise I think it is 'bulletproof'.
2. in your alternative suggestion, you have an issue with the 'restore'
section. you use swapf to get STATUS in to W, so the nybbles will be
swapped, so you only need one swapf to restore them in the return.
So, when exactly does the STATUS value get moved from the register to W?
According to mid-range-manual.... well, it appears that the register
(status) is read before the values are processed (thus re-affecting status).
Hmm. It all looks OK to me, though, using swapf may save an instruction
in the ISR....
> Hi Rolf,
>
> I believe that the context saving is not right in your code. You likely to
> overwrite Z flag, however, I can't see any section of your code that would
> use the zero flag so it might not be the case.
>
> Tamas
>
>
>
> ;------------------------------------------------------------------------------
>
>> ; INTERRUPT SERVICE ROUTINE
>>
>> ;------------------------------------------------------------------------------
>>
>> INT_VECTOR CODE 0x0004 ; interrupt vector location
>> MOVWF W_TEMP ; save off current W register contents
>> MOVF STATUS,w ; move status register into W register
>> MOVWF STATUS_TEMP ; save off contents of STATUS register
>>
>>
>>
> Should be:
>
> INT_VECTOR CODE 0x0004 ; interrupt vector location
> MOVWF W_TEMP ; save off current W register contents
> SWAPF STATUS,w ; move status register into W register
> MOVWF STATUS_TEMP ; save off contents of STATUS register
>
>
> i_end
>
>> MOVF STATUS_TEMP,w ; retrieve copy of STATUS register
>> MOVWF STATUS ; restore pre-isr STATUS register contents
>> SWAPF W_TEMP,f
>> SWAPF W_TEMP,w ; restore pre-isr W register contents
>> RETFIE ; return from interrupt
>>
>>
>
> Should be:
>
> i_end
>
> SWAPF STATUS_TEMP,w ; retrieve copy of STATUS register
> MOVWF STATUS ; restore pre-isr STATUS register contents
> SWAPF W_TEMP,f
> SWAPF W_TEMP,w ; restore pre-isr W register contents
> RETFIE ; return from interrupt
>
> Tamas
>
>
>
Rolf wrote:
> Using this 'philosophy, I would have to do a banksel before *and after*
> accessing a Bank 1 register. So, what do you choose as a banksel
> 'target' after accessing a bank 1 variable...
I wouldn't. I don't think that philosophy makes sense. I would set the
bank on demand as needed.
But, if you really want to abuse BANKSEL to set access to a particular bank,
you can use the first address in the bank. That would be the bank number
times 128.
> Also, for the larger
> devices I have been using C18 with The student edition compiler, I still
> keep all the debug features, as well as the integration, etc, but would
> be completely incompatible with your toolset.
I don't care if you use my PIC development environment or not, since it's
available for free. However, it does work with C18. C18 uses really stupid
stack management, but I've got a assembly time switch that causes my stuff
to use the same brain dead stack management for compatibility. I have done
several mixed MPASM/C18 projects.
> 'Doing it right' is not necessarily clrf STATUS though, is it. It will
> work fine in the ISR, but, not elsewhere.
Right, but that's where this came up. It's one instruction that sets the
banks to a predictable state on entry to interrupt, and it works on a 4 bank
machine too.
> Further, I would much rather have 'obvious' incompatibilities like
> mising RP1 on a status register, than to have 'obscure' problems like
> changes to program timings, and changes to relative GOTO statements that
> happen when using banksel.
Huh? On your machine BANKSEL will always emit the exact same instruction as
you are hard coding. The difference is that it won't screw up and select
the wrong bank.
If you're talking about moving the code to a 4 bank PIC, then BANKSEL will
take one extra instruction, but at least the bank will be correctly
selected. Worrying about single instruction timing when the bank is wrong
is silly. Unless you are doing something very unusual, you wouldn't be
setting the bank in the middle of some code where single instruction timing
matters. With that kind of code, you would usually arrange to not need to
switch banks in the first place. If your timing is really that tight, you
should probably use assembly time math to make sure the timing is met and
generate a error if it's not. Of if you know you have extra cycles,
something like my WAITNS macro is much better than hand-counting NOPs.
Hmmm... I see yu got your routine from the datasheet.
I guess this is a place where the datasheet and the example code differ.
Also, disregard my second 'observation' with your example code, I mixed
up the W and STATUS restore.
I am looking in to the code more carefully now too. I think the existing
code works fine, but, I will use the swapf anyway because it comes from
the datasheet which I trust more than the .asm template.
I am also going to run it through the MPSIM to see if it is ever a factor...
> That's fascinating observation...
>
> The code itself is unmodified from the 'template' as supplied by
> Microchip, so I would be surprised... but, two observations....
> 1. If the MOVF,w changes Z before actually moving the value, then you
> may be right, otherwise I think it is 'bulletproof'.
> 2. in your alternative suggestion, you have an issue with the 'restore'
> section. you use swapf to get STATUS in to W, so the nybbles will be
> swapped, so you only need one swapf to restore them in the return.
>
> So, when exactly does the STATUS value get moved from the register to W?
> According to mid-range-manual.... well, it appears that the register
> (status) is read before the values are processed (thus re-affecting status).
>
> Hmm. It all looks OK to me, though, using swapf may save an instruction
> in the ISR....
>
> Rolf
>
>
>
> Tamas Rudnai wrote:
>
>> Hi Rolf,
>>
>> I believe that the context saving is not right in your code. You likely to
>> overwrite Z flag, however, I can't see any section of your code that would
>> use the zero flag so it might not be the case.
>>
>> Tamas
>>
>>
>>
>> ;------------------------------------------------------------------------------
>>
>>
>>> ; INTERRUPT SERVICE ROUTINE
>>>
>>> ;------------------------------------------------------------------------------
>>>
>>> INT_VECTOR CODE 0x0004 ; interrupt vector location
>>> MOVWF W_TEMP ; save off current W register contents
>>> MOVF STATUS,w ; move status register into W register
>>> MOVWF STATUS_TEMP ; save off contents of STATUS register
>>>
>>>
>>>
>>>
>> Should be:
>>
>> INT_VECTOR CODE 0x0004 ; interrupt vector location
>> MOVWF W_TEMP ; save off current W register contents
>> SWAPF STATUS,w ; move status register into W register
>> MOVWF STATUS_TEMP ; save off contents of STATUS register
>>
>>
>> i_end
>>
>>
>>> MOVF STATUS_TEMP,w ; retrieve copy of STATUS register
>>> MOVWF STATUS ; restore pre-isr STATUS register contents
>>> SWAPF W_TEMP,f
>>> SWAPF W_TEMP,w ; restore pre-isr W register contents
>>> RETFIE ; return from interrupt
>>>
>>>
>>>
>> Should be:
>>
>> i_end
>>
>> SWAPF STATUS_TEMP,w ; retrieve copy of STATUS register
>> MOVWF STATUS ; restore pre-isr STATUS register contents
>> SWAPF W_TEMP,f
>> SWAPF W_TEMP,w ; restore pre-isr W register contents
>> RETFIE ; return from interrupt
>>
>> Tamas
>>
>>
>>
>>
>
>
Just as a 'by the way', the Microchip datasheet and .asm templates
suggest different ways of doing things, but, the following is from the
PIC12F675 datasheet:
MOVWF W_TEMP ;copy W to temp register,
could be in either bank
SWAPF STATUS,W ;swap status to be saved into W
BCF STATUS,RP0 ;change to bank 0 regardless of
current bank
MOVWF STATUS_TEMP ;save status to bank 0 register
:
:(ISR)
:
SWAPF STATUS_TEMP,W;swap STATUS_TEMP register into
W, sets bank to original state
MOVWF STATUS ;move W into STATUS register
SWAPF W_TEMP,F ;swap W_TEMP
SWAPF W_TEMP,W ;swap W_TEMP into W
It is very happy to use the bcf STATUS,RP0. I am not the only person who
uses bcf STATUS,RP0 and the datasheet is fairly authoritative, not? Oh,
and it does not change RP1 either! Nor does it banksel.
In my mind it makes fine sense, it has clear (or at least clearer)
meaning of what the intent of the instruction is. the clrf STATUS could
have all sorts of intended, and unintended consequences, especially
since it never works (clrf STATUS will always set STATUS,Z).
Assembler appears to be somewhat like perl, TIMTOWTDI ... ;-)
Anyway, using hybrid/your logic, it would be better not to have a 'clrf
STATUS', but to have banksel's before each SFR. Just in case you
copy/paste the code.
Regardless, this all comes down to how the program is arranged in your
mind. This is my program and it makes sense to me. I would rather use
less efficient mechanisms that work for me and are clear in my head and
that I understand, than use 'better' mechanisms that don't. At least I
know who to blame when they don't work.
> Rolf wrote:
>
>> Using this 'philosophy, I would have to do a banksel before *and after*
>> accessing a Bank 1 register. So, what do you choose as a banksel
>> 'target' after accessing a bank 1 variable...
>>
>
> I wouldn't. I don't think that philosophy makes sense. I would set the
> bank on demand as needed.
>
> But, if you really want to abuse BANKSEL to set access to a particular bank,
> you can use the first address in the bank. That would be the bank number
> times 128.
>
>
>> Also, for the larger
>> devices I have been using C18 with The student edition compiler, I still
>> keep all the debug features, as well as the integration, etc, but would
>> be completely incompatible with your toolset.
>>
>
> I don't care if you use my PIC development environment or not, since it's
> available for free. However, it does work with C18. C18 uses really stupid
> stack management, but I've got a assembly time switch that causes my stuff
> to use the same brain dead stack management for compatibility. I have done
> several mixed MPASM/C18 projects.
>
>
>> 'Doing it right' is not necessarily clrf STATUS though, is it. It will
>> work fine in the ISR, but, not elsewhere.
>>
>
> Right, but that's where this came up. It's one instruction that sets the
> banks to a predictable state on entry to interrupt, and it works on a 4 bank
> machine too.
>
>
>> Further, I would much rather have 'obvious' incompatibilities like
>> mising RP1 on a status register, than to have 'obscure' problems like
>> changes to program timings, and changes to relative GOTO statements that
>> happen when using banksel.
>>
>
> Huh? On your machine BANKSEL will always emit the exact same instruction as
> you are hard coding. The difference is that it won't screw up and select
> the wrong bank.
>
> If you're talking about moving the code to a 4 bank PIC, then BANKSEL will
> take one extra instruction, but at least the bank will be correctly
> selected. Worrying about single instruction timing when the bank is wrong
> is silly. Unless you are doing something very unusual, you wouldn't be
> setting the bank in the middle of some code where single instruction timing
> matters. With that kind of code, you would usually arrange to not need to
> switch banks in the first place. If your timing is really that tight, you
> should probably use assembly time math to make sure the timing is met and
> generate a error if it's not. Of if you know you have extra cycles,
> something like my WAITNS macro is much better than hand-counting NOPs.
>
>
> ********************************************************************
> Embed Inc, Littleton Massachusetts, http://www.embedinc.com/products
> (978) 742-9014. Gold level PIC consultants since 2000.
>
> Hmmm... I see yu got your routine from the datasheet.
>
> I guess this is a place where the datasheet and the example code differ.
> Also, disregard my second 'observation' with your example code, I mixed
> up the W and STATUS restore.
>
> I am looking in to the code more carefully now too. I think the existing
> code works fine, but, I will use the swapf anyway because it comes from
> the datasheet which I trust more than the .asm template.
>
> I am also going to run it through the MPSIM to see if it is ever a factor...
>
> Rolf
>
> Rolf wrote:
>
>> That's fascinating observation...
>>
>> The code itself is unmodified from the 'template' as supplied by
>> Microchip, so I would be surprised... but, two observations....
>> 1. If the MOVF,w changes Z before actually moving the value, then you
>> may be right, otherwise I think it is 'bulletproof'.
>> 2. in your alternative suggestion, you have an issue with the 'restore'
>> section. you use swapf to get STATUS in to W, so the nybbles will be
>> swapped, so you only need one swapf to restore them in the return.
>>
>> So, when exactly does the STATUS value get moved from the register to W?
>> According to mid-range-manual.... well, it appears that the register
>> (status) is read before the values are processed (thus re-affecting status).
>>
>> Hmm. It all looks OK to me, though, using swapf may save an instruction
>> in the ISR....
>>
>> Rolf
>>
>>
>>
>> Tamas Rudnai wrote:
>>
>>
>>> Hi Rolf,
>>>
>>> I believe that the context saving is not right in your code. You likely to
>>> overwrite Z flag, however, I can't see any section of your code that would
>>> use the zero flag so it might not be the case.
>>>
>>> Tamas
>>>
>>>
>>>
>>> ;------------------------------------------------------------------------------
>>>
>>>
>>>
>>>> ; INTERRUPT SERVICE ROUTINE
>>>>
>>>> ;------------------------------------------------------------------------------
>>>>
>>>> INT_VECTOR CODE 0x0004 ; interrupt vector location
>>>> MOVWF W_TEMP ; save off current W register contents
>>>> MOVF STATUS,w ; move status register into W register
>>>> MOVWF STATUS_TEMP ; save off contents of STATUS register
>>>>
>>>>
>>>>
>>>>
>>>>
>>> Should be:
>>>
>>> INT_VECTOR CODE 0x0004 ; interrupt vector location
>>> MOVWF W_TEMP ; save off current W register contents
>>> SWAPF STATUS,w ; move status register into W register
>>> MOVWF STATUS_TEMP ; save off contents of STATUS register
>>>
>>>
>>> i_end
>>>
>>>
>>>
>>>> MOVF STATUS_TEMP,w ; retrieve copy of STATUS register
>>>> MOVWF STATUS ; restore pre-isr STATUS register contents
>>>> SWAPF W_TEMP,f
>>>> SWAPF W_TEMP,w ; restore pre-isr W register contents
>>>> RETFIE ; return from interrupt
>>>>
>>>>
>>>>
>>>>
>>> Should be:
>>>
>>> i_end
>>>
>>> SWAPF STATUS_TEMP,w ; retrieve copy of STATUS register
>>> MOVWF STATUS ; restore pre-isr STATUS register contents
>>> SWAPF W_TEMP,f
>>> SWAPF W_TEMP,w ; restore pre-isr W register contents
>>> RETFIE ; return from interrupt
>>>
>>> Tamas
>>>
>>>
>>>
>>>
>>>
>>
>>
>
>
> Hi Richard, all.
>
> I have replaced the 1M pull downs on the doors with 100K pull downs. I
> also pulled up the R1 1K resistor that feeds the 1.25V to the A/D unit.
> The trace for the door runs onder this 1206 resistor, so I thought there
> may be a bridge under it. Well, there *may* have been. When I ripped it
> up I found some un-melted solder paste there...
>
> Well, I put another 1K resistor back there and I'm hoping this resolves it.
>
> I followed the discussion not so long ago about solder paste, and, since
> then I have not use ot for hand-soldering (I did a 'toaster-oven'
> project a while back that I used the paste for, and it makes life quite
> easy (or so I thought) even when hand-soldering.
>
> Well, if the problem goes away I will attribute it to the solder paste.
> If not, well, back to square one.
>
> I'll run it overnight and see in the morning.
>
> Attached is a small pic of the circuit (because I am quite happy with
> the etching process, etc.
>
> Hmmm, not attached, but posted here:
> http://www.flickr.com/photos/gus_goose/2623093614
>
> Rolf
>
> Richard Prosser wrote:
>
>> Rolf,
>>
>> I'd also suggest stronger pulldowns than 1Meg on the door switches
>> (and capacitors), To minimise power consumption, maybe retain the 1Meg
>> resistors, but add 10ks to a "spare" pin and activate these only when
>> taking a reading. You should be able to figure out a way to use pin 5
>> as the pulldown pin as it's only used once for code setting. (I
>> think).
>>
>> I was going to suggest swapping the connections from the "good" &
>> "bad" doors, but since the problem remains with neither attached, it
>> seems more a software or pcb problem than external hardware.
>>
>> Since it always (?) works OK for the first few hours, I'd suspect a
>> software, rather than hardware issue.
>>
>> Getting the memory banking wrong can lead to all sorts of strange
>> problems and I wouldn't have been surprised if your problem is now
>> fixed (But obviously not however, based on your latest email).
>> Possibly another, similar error somewhere?
>>
>> Richard P
>>
>>
>>
>>
>> 2008/6/30 Rolf <.....learrKILLspam.....rogers.com>:
>>
>>
>>> Thanks...
>>>
>>>
>
>
>
If you look at the MOVF instruction, that changes the Z, while SWAPF does
not. Also SWAPF takes the same instruction cycle and code space, so there is
no reason not to use here. Anyway, if STATUS is entirely 0x00 then the Z
should be set after the MOVF, otherwise you will not be able to observe
difference in between using the two instructions.
> MPSIM suggests the code works when entering with either STATUS,Z set as
> 1 or 0. The STATUS is correctly restored.
>
> I'm still going to change it to match the datasheet.
>
> Rolf
>
> Rolf wrote:
> > Hmmm... I see yu got your routine from the datasheet.
> >
> > I guess this is a place where the datasheet and the example code differ.
> > Also, disregard my second 'observation' with your example code, I mixed
> > up the W and STATUS restore.
> >
> > I am looking in to the code more carefully now too. I think the existing
> > code works fine, but, I will use the swapf anyway because it comes from
> > the datasheet which I trust more than the .asm template.
> >
> > I am also going to run it through the MPSIM to see if it is ever a
> factor...
> >
> > Rolf
> >
> > Rolf wrote:
> >
> >> That's fascinating observation...
> >>
> >> The code itself is unmodified from the 'template' as supplied by
> >> Microchip, so I would be surprised... but, two observations....
> >> 1. If the MOVF,w changes Z before actually moving the value, then you
> >> may be right, otherwise I think it is 'bulletproof'.
> >> 2. in your alternative suggestion, you have an issue with the 'restore'
> >> section. you use swapf to get STATUS in to W, so the nybbles will be
> >> swapped, so you only need one swapf to restore them in the return.
> >>
> >> So, when exactly does the STATUS value get moved from the register to W?
> >> According to mid-range-manual.... well, it appears that the register
> >> (status) is read before the values are processed (thus re-affecting
> status).
> >>
> >> Hmm. It all looks OK to me, though, using swapf may save an instruction
> >> in the ISR....
> >>
> >> Rolf
> >>
> >>
> >>
> >> Tamas Rudnai wrote:
> >>
> >>
> >>> Hi Rolf,
> >>>
> >>> I believe that the context saving is not right in your code. You likely
> to
> >>> overwrite Z flag, however, I can't see any section of your code that
> would
> >>> use the zero flag so it might not be the case.
> >>>
> >>> Tamas
> >>>
> >>>
> >>>
> >>>
> ;------------------------------------------------------------------------------
> >>>
> >>>
> >>>
> >>>> ; INTERRUPT SERVICE ROUTINE
> >>>>
> >>>>
> ;------------------------------------------------------------------------------
> >>>>
> >>>> INT_VECTOR CODE 0x0004 ; interrupt vector location
> >>>> MOVWF W_TEMP ; save off current W register contents
> >>>> MOVF STATUS,w ; move status register into W register
> >>>> MOVWF STATUS_TEMP ; save off contents of STATUS register
> >>>>
> >>>>
> >>>>
> >>>>
> >>>>
> >>> Should be:
> >>>
> >>> INT_VECTOR CODE 0x0004 ; interrupt vector location
> >>> MOVWF W_TEMP ; save off current W register contents
> >>> SWAPF STATUS,w ; move status register into W register
> >>> MOVWF STATUS_TEMP ; save off contents of STATUS register
> >>>
> >>>
> >>> i_end
> >>>
> >>>
> >>>
> >>>> MOVF STATUS_TEMP,w ; retrieve copy of STATUS register
> >>>> MOVWF STATUS ; restore pre-isr STATUS register
> contents
> >>>> SWAPF W_TEMP,f
> >>>> SWAPF W_TEMP,w ; restore pre-isr W register contents
> >>>> RETFIE ; return from interrupt
> >>>>
> >>>>
> >>>>
> >>>>
> >>> Should be:
> >>>
> >>> i_end
> >>>
> >>> SWAPF STATUS_TEMP,w ; retrieve copy of STATUS register
> >>> MOVWF STATUS ; restore pre-isr STATUS register contents
> >>> SWAPF W_TEMP,f
> >>> SWAPF W_TEMP,w ; restore pre-isr W register contents
> >>> RETFIE ; return from interrupt
> >>>
> >>> Tamas
> >>>
> >>>
> >>>
> >>>
> >>>
> >>
> >>
> >
> >
>
>I am looking in to the code more carefully now too. I think the existing
>code works fine, but, I will use the swapf anyway because it comes from
>the datasheet which I trust more than the .asm template.
You need to look at why SWAPF is used - big clue, look at what happens to
the flags when doing the restore at the end of the interrupt routine with
your code.
Rolf wrote:
> I am not the only person who
> uses bcf STATUS,RP0
Lots of people don't comment their code, but that doesn't make it a good
idea either.
> and the datasheet is fairly authoritative, not?
Not when it comes to code. Microchip examples generally suck, and sometimes
contain outright errors. Look at the instruction set, how the machine and
the peripherals work, then write your own code.
> the clrf STATUS could
> have all sorts of intended, and unintended consequences, especially
> since it never works (clrf STATUS will always set STATUS,Z).
Then you might as well wave a dead fish over your programs in case that
makes them work better too.
> Assembler appears to be somewhat like perl, TIMTOWTDI ... ;-)
Huh?
> Anyway, using hybrid/your logic, it would be better not to have a
> 'clrf STATUS', but to have banksel's before each SFR. Just in case you
> copy/paste the code.
Newbies should use BANKSEL in front of every reference that could be in a
different bank from the previous. Once they get more comfortable with the
rest of the PIC, then they can go back and learn better ways to manage
banking. Explictly writing BSF/BCF on RP0/RP1 is bad programming and is
never encouraged.
If you look at my interrupt template module, QQQ_INTR.ASPIC, you will see I
don't use BANKSEL, but my DBANKIF and DBANKIS macros. I force the interrupt
temporary save registers to be in bank 0, and use CLRF STATUS as a single
instruction to select bank 0 regardless of what state STATUS was in or which
14 bit core PIC this is running on. This is immediately followed by DBANKIS
0 and IBANKIS 0 to tell the assembler what the bank settings are. After
that, the bank setting is tracked at assembly time and only the minimum bank
switching instructions are inserted by DBANKIF as needed. This is of course
all explained in the comments too.
> I would rather use
> less efficient mechanisms that work for me and are clear in my head
> and
> that I understand, than use 'better' mechanisms that don't.
That's your business. The discussion was only because you asked for help
here, and apparently did have some problems in this area. Given that, you
might want to revist your thinking.
Alan B. Pearce wrote:
>> I am looking in to the code more carefully now too. I think the existing
>> code works fine, but, I will use the swapf anyway because it comes from
>> the datasheet which I trust more than the .asm template.
>>
>
> You need to look at why SWAPF is used - big clue, look at what happens to
> the flags when doing the restore at the end of the interrupt routine with
> your code.
>
>
Hi Alan.
I am not sure what you are suggesting... oh, I see. You think that the
restore of the STATUS register is broken.... well, I don't believe it
is. I have inspected the code, inspected the datasheets, and run various
scenarios through MPSIM, and I can't see where the code is broken. If it
is in fact broken, I need you to do more than just give me a 'clue'.
Where is it.....
Here is the snippet from the file C:\Program Files\Microchip\MPASM
Suite\Template\Code\12F675TEMP.ASM. I have taken the liberty of
numbering the lines, and adding some comments of my own...:
ORG 0x004 ; interrupt vector location 1 -
fine, get us to the reset vector.
movwf w_temp ; save off current W register
contents 2 - movwf does not affect STATUS. Good.
movf STATUS,w ; move status register into W register
3 - movf *does* affect STATUS,Z, but, the copy of STATUS in W is from
before the Z is (potentially) changed.
movwf status_temp ; save off contents of STATUS
register 4 - movwf does not affect STATUS, Good.
; isr code can go here or be located as a call subroutine elsewhere
movf status_temp,w ; retrieve copy of STATUS register 5
- movf *does* affect status, but, that's OK because we are putting the
original STATUS in to W.
movwf STATUS ; restore pre-isr STATUS register
contents 6 - movwf does not affect STATUS, which is good, because we
are overwriting it's contents with W.
swapf w_temp,f ; 7 - swap the W nybbles because we
want to use SWAPF again later because SWAPF does not affect STATUS.
swapf w_temp,w ; restore pre-isr W register contents
8 - the final SWAPF to W with the correct W register restore.
retfie ; return from interrupt
So, the way I see it is that the template code *does the right thing*.
It may not do it the same way as the datasheet, but, it works just fine.
The lines of potential confusion are lines 2, 3, 5, and 6. Which line
has it wrong?
Also, just by the way, if I randomly select PIC 10, 12 and 16 files from
C:\Program Files\Microchip\MPASM Suite\Template\Code\ I find that either
they do not have code for the ISR, or they use the system I pasted
above. The templates for common PICS like the 12F675, 16F84A 16F628
16F648 16F877 16F88 16F887 and more *all* have the system above. Are
*all* the templates wrong for the non PIC18F parts?
> am not sure what you are suggesting... oh, I see. You think that the
> restore of the STATUS register is broken.... well, I don't believe it is.
It is actually, that context saving does not care about the Z flag...
> I have inspected the code, inspected the datasheets, and run various
> scenarios through MPSIM, and I can't see where the code is broken. If it
> is in fact broken, I need you to do more than just give me a 'clue'.
> Where is it.....
I have already mentioned, even with the code example...
> Alan B. Pearce wrote:
> >> I am looking in to the code more carefully now too. I think the existing
> >> code works fine, but, I will use the swapf anyway because it comes from
> >> the datasheet which I trust more than the .asm template.
> >>
> >
> > You need to look at why SWAPF is used - big clue, look at what happens to
> > the flags when doing the restore at the end of the interrupt routine with
> > your code.
> >
> >
> Hi Alan.
>
> I am not sure what you are suggesting... oh, I see. You think that the
> restore of the STATUS register is broken.... well, I don't believe it
> is. I have inspected the code, inspected the datasheets, and run various
> scenarios through MPSIM, and I can't see where the code is broken. If it
> is in fact broken, I need you to do more than just give me a 'clue'.
> Where is it.....
>
> Here is the snippet from the file C:\Program Files\Microchip\MPASM
> Suite\Template\Code\12F675TEMP.ASM. I have taken the liberty of
> numbering the lines, and adding some comments of my own...:
>
>
> ORG 0x004 ; interrupt vector location 1 -
> fine, get us to the reset vector.
> movwf w_temp ; save off current W register
> contents 2 - movwf does not affect STATUS. Good.
> movf STATUS,w ; move status register into W register
> 3 - movf *does* affect STATUS,Z, but, the copy of STATUS in W is from
> before the Z is (potentially) changed.
> movwf status_temp ; save off contents of STATUS
> register 4 - movwf does not affect STATUS, Good.
>
>
> ; isr code can go here or be located as a call subroutine elsewhere
>
>
> movf status_temp,w ; retrieve copy of STATUS register 5
> - movf *does* affect status, but, that's OK because we are putting the
> original STATUS in to W.
> movwf STATUS ; restore pre-isr STATUS register
> contents 6 - movwf does not affect STATUS, which is good, because we
> are overwriting it's contents with W.
> swapf w_temp,f ; 7 - swap the W nybbles because we
> want to use SWAPF again later because SWAPF does not affect STATUS.
> swapf w_temp,w ; restore pre-isr W register contents
> 8 - the final SWAPF to W with the correct W register restore.
> retfie ; return from interrupt
>
>
> So, the way I see it is that the template code *does the right thing*.
> It may not do it the same way as the datasheet, but, it works just fine.
> The lines of potential confusion are lines 2, 3, 5, and 6. Which line
> has it wrong?
>
> Also, just by the way, if I randomly select PIC 10, 12 and 16 files from
> C:\Program Files\Microchip\MPASM Suite\Template\Code\ I find that either
> they do not have code for the ISR, or they use the system I pasted
> above. The templates for common PICS like the 12F675, 16F84A 16F628
> 16F648 16F877 16F88 16F887 and more *all* have the system above. Are
> *all* the templates wrong for the non PIC18F parts?
>
> Rolf
>
> > and the datasheet is fairly authoritative, not?
>
> Not when it comes to code
Do not trust datasheet code examples. Some are too simple (bank
selection for example), others are just wrong and/or inappropriate
Copy/Pastes from other PIC datasheets
> > Assembler appears to be somewhat like perl, TIMTOWTDI ... ;-)
You can use that (whatever it means), but not BANKSEL ? !! 8-)
> > I would rather use less efficient mechanisms that work for me
> > and are clear in my head and that I understand, than use 'better'
> > mechanisms that don't
To be honest, scatterings of
BCx STATUS,RP0, with or without BCx STATUS,RP1
through code looks really untidy and is distracting and tiring to read.
Plus it's more typing and a lot easier to make a mistake with. There
are so many code faults that can be traced back to banking
>I am not sure what you are suggesting... oh, I see. You think
>that the restore of the STATUS register is broken....
No, I thought you were not using SWAPF at all in the restore section, but I
just went back and looked at the code that Tamas quoted, and found you were.
It is more the reason why SWAPF is used rather than a MOV instruction, that
I was trying to reinforce in your mind.
I'm sorry for being so apparently dense, but, I disagree with you, at
least until I can see where the problem is. You have insisted that the
microchip code is wrong somewhere, saying it is with the MOVF
instruction. I have literally spent hours trying to understand the
problem, run the code, simulated it, and it all works just fine as far
as everything I have available is concerned. What more do you expect me
to do.... have you considered for a moment that you may be wrong?
In your other mail you suggested that the STATUS,Z will be
saved/restored incorrectly. There are 8 lines of code, with 2 MOVF
instructions... I believe they both work fine. You suggest that one of
them does the wrong thing. So, please, spare me some anxiety and tell me
which is broken. I have spent too much time already on this, especially
when I appear to be missing something 'obvious'.
To repeat, here are the instructions.....
ORG 0x004
movwf w_temp
movf STATUS,w
movwf status_temp
; isr code can go here or be located as a call subroutine elsewhere
movf status_temp,w
movwf STATUS
swapf w_temp,f
swapf w_temp,w
retfi
The first MOVF instruction correctly stores the current STATUS in W
(even though after the instruction the STATUS,Z may have changed).
The second MOVF may affect STATUS too, but, it is inconsequential
because STATUS is neither the source nor target of the instruction...
and the next instruction (MOVWF) will overwrite the status anyway.
So, please, the context saving *does* care about the Z bit.... if not, I
have spent a lot of time on it (based on your assertion that it is
broken), I still don't see the problem, so please spell it out for me
with a little patience...
> Hi Rolf,
>
>
>> am not sure what you are suggesting... oh, I see. You think that the
>> restore of the STATUS register is broken.... well, I don't believe it is.
>>
>
> It is actually, that context saving does not care about the Z flag...
>
>
>> I have inspected the code, inspected the datasheets, and run various
>> scenarios through MPSIM, and I can't see where the code is broken. If it
>> is in fact broken, I need you to do more than just give me a 'clue'.
>> Where is it.....
>>
>
> I have already mentioned, even with the code example...
>
> Tamas
>
>
> On Tue, Jul 1, 2008 at 1:13 PM, Rolf <@spam@learrKILLspamrogers.com> wrote:
>
>
>> Alan B. Pearce wrote:
>>
>>>> I am looking in to the code more carefully now too. I think the existing
>>>> code works fine, but, I will use the swapf anyway because it comes from
>>>> the datasheet which I trust more than the .asm template.
>>>>
>>>>
>>> You need to look at why SWAPF is used - big clue, look at what happens to
>>> the flags when doing the restore at the end of the interrupt routine with
>>> your code.
>>>
>>>
>>>
>> Hi Alan.
>>
>> I am not sure what you are suggesting... oh, I see. You think that the
>> restore of the STATUS register is broken.... well, I don't believe it
>> is. I have inspected the code, inspected the datasheets, and run various
>> scenarios through MPSIM, and I can't see where the code is broken. If it
>> is in fact broken, I need you to do more than just give me a 'clue'.
>> Where is it.....
>>
>> Here is the snippet from the file C:\Program Files\Microchip\MPASM
>> Suite\Template\Code\12F675TEMP.ASM. I have taken the liberty of
>> numbering the lines, and adding some comments of my own...:
>>
>>
>> ORG 0x004 ; interrupt vector location 1 -
>> fine, get us to the reset vector.
>> movwf w_temp ; save off current W register
>> contents 2 - movwf does not affect STATUS. Good.
>> movf STATUS,w ; move status register into W register
>> 3 - movf *does* affect STATUS,Z, but, the copy of STATUS in W is from
>> before the Z is (potentially) changed.
>> movwf status_temp ; save off contents of STATUS
>> register 4 - movwf does not affect STATUS, Good.
>>
>>
>> ; isr code can go here or be located as a call subroutine elsewhere
>>
>>
>> movf status_temp,w ; retrieve copy of STATUS register 5
>> - movf *does* affect status, but, that's OK because we are putting the
>> original STATUS in to W.
>> movwf STATUS ; restore pre-isr STATUS register
>> contents 6 - movwf does not affect STATUS, which is good, because we
>> are overwriting it's contents with W.
>> swapf w_temp,f ; 7 - swap the W nybbles because we
>> want to use SWAPF again later because SWAPF does not affect STATUS.
>> swapf w_temp,w ; restore pre-isr W register contents
>> 8 - the final SWAPF to W with the correct W register restore.
>> retfie ; return from interrupt
>>
>>
>> So, the way I see it is that the template code *does the right thing*.
>> It may not do it the same way as the datasheet, but, it works just fine.
>> The lines of potential confusion are lines 2, 3, 5, and 6. Which line
>> has it wrong?
>>
>> Also, just by the way, if I randomly select PIC 10, 12 and 16 files from
>> C:\Program Files\Microchip\MPASM Suite\Template\Code\ I find that either
>> they do not have code for the ISR, or they use the system I pasted
>> above. The templates for common PICS like the 12F675, 16F84A 16F628
>> 16F648 16F877 16F88 16F887 and more *all* have the system above. Are
>> *all* the templates wrong for the non PIC18F parts?
>>
>> Rolf
>>
Alan B. Pearce wrote:
>> I am not sure what you are suggesting... oh, I see. You think
>> that the restore of the STATUS register is broken....
>>
>
> No, I thought you were not using SWAPF at all in the restore section, but I
> just went back and looked at the code that Tamas quoted, and found you were.
> It is more the reason why SWAPF is used rather than a MOV instruction, that
> I was trying to reinforce in your mind.
>
>
Thanks Alan.
Yes, I know why SWAPF is used. I prefer the example routine in the
datasheet to the one in the .asm template, and I have changed my code
accordingly. SwapF does make things more transparent. I would just like
to confirm though that the original (microchip .asm template) routine is
functionally accurate. I have spent hours now trying to find holes in
it, and I can't. It is important for me to understand, and if my
understanding is wrong, then I need to know why...
Right now there is a lot of debate in this thread, and I am not sure
what has been resolved or not ;-)
Olin Lathrop wrote:
> Rolf wrote:
>
>> I am not the only person who
>> uses bcf STATUS,RP0
>>
>
> Lots of people don't comment their code, but that doesn't make it a good
> idea either.
>
>
Agreed. You will find my code is commented.... indented consistently, etc.
>> and the datasheet is fairly authoritative, not?
>>
>
> Not when it comes to code. Microchip examples generally suck, and sometimes
> contain outright errors. Look at the instruction set, how the machine and
> the peripherals work, then write your own code.
>
>
Apparently, as a newbie, I should know when RTFDS is the right thing to
do, and when I should use my own initiative.
For what it's worth, the way I select banks is based on looking at the
instruction set, how the machine works, and then writing my own code...
but apparently this is not OK in that instance....?
There is a fine line in what you believe to be 'right' somewhere I have
not come to grips with yet....
>> the clrf STATUS could
>> have all sorts of intended, and unintended consequences, especially
>> since it never works (clrf STATUS will always set STATUS,Z).
>>
>
> Then you might as well wave a dead fish over your programs in case that
> makes them work better too.
>
>
If it helps, I will. Hang on, dead insects have been known to change the
bahaviour of programs, so there may be some merit.
>> Assembler appears to be somewhat like perl, TIMTOWTDI ... ;-)
>>
>
> Huh?
>
>
There Is More Than One Way To Do It - this is the perl 'mantra'. a
simple google of TIMTOWTDI reveals all... but, I don't expect everyone
to know that. It was intended for the audience who have already used
perl, and for those who were willing to invest some small amounts of
time to understand... Still, you expect me to research a bunch of things
in your 'domain' every time you make a reply, it would have been nice if
you could have returned the favour a little. But, as I say, it was just
meant to point out that there are many ways to skin a cat, and, the way
I have implemented are jsut different to yours, but accomplish the same
result (in the end). {Quote hidden}
>> Anyway, using hybrid/your logic, it would be better not to have a
>> 'clrf STATUS', but to have banksel's before each SFR. Just in case you
>> copy/paste the code.
>>
>
> Newbies should use BANKSEL in front of every reference that could be in a
> different bank from the previous. Once they get more comfortable with the
> rest of the PIC, then they can go back and learn better ways to manage
> banking. Explictly writing BSF/BCF on RP0/RP1 is bad programming and is
> never encouraged.
>
>
Two things... First, I am confused. I though newbies (like me) should
download your development environment and use things like QQQ_BANKIS, or
IBANKIS or something. That was the 'magic cure to all things PIC
related, it slices, dices, and does everything you need...'.
Secondly, Explicitly writing BSF/BCF on RP0/RP1 *IS* encouraged. It may
not be encouraged by YOU, and you may think it is bad programming...
but, that does not make it a universal truth. In fact, the datasheet
(which you disagree with) explicitly recommends it in the ISR.
> If you look at my interrupt template module, QQQ_INTR.ASPIC, you will see I
> don't use BANKSEL, but my DBANKIF and DBANKIS macros. I force the interrupt
> temporary save registers to be in bank 0, and use CLRF STATUS as a single
> instruction to select bank 0 regardless of what state STATUS was in or which
> 14 bit core PIC this is running on. This is immediately followed by DBANKIS
> 0 and IBANKIS 0 to tell the assembler what the bank settings are. After
> that, the bank setting is tracked at assembly time and only the minimum bank
> switching instructions are inserted by DBANKIF as needed. This is of course
> all explained in the comments too.
>
>
Olin, your PIC environment is a fantastic accomplishment, I am sure.
But, it has limitiations that I am not willing to compromise. I have
previously evaluated it, and found that the things I value in MPLab are
too important to sacrifice for your assembler pre-processor. When your
system allows the building, debugging, watching of variables, a
graphical interface, and preferablly run native on Linux (which MPLAB
does not do, but I am willing to compromise on that), then I will
re-avaluate it. The fact is that your system works fine for you, and
some others who are willing to compromise in different ways than I am.
But, it is not for everyone.
>> I would rather use
>> less efficient mechanisms that work for me and are clear in my head
>> and
>> that I understand, than use 'better' mechanisms that don't.
>>
>
> That's your business. The discussion was only because you asked for help
> here, and apparently did have some problems in this area. Given that, you
> might want to revist your thinking.
>
I absolutely agree. I did have some problems here. Some are still
unresolved. I must always revisit my thinking. But, my problem was that
I did not ensure my memory bank in the ISR. This could have been fixed
in four 'broad' ways, with a banksel, with a bcf STATUS,RP0, with a clrf
STATUS, or with a re-implementation using your PIC macros and systems.
In each of the solutions I have to do something specific to set the
bank. Well, I could have made the same mistake using any of those
systems. If I had left out a banksel it would be the same as missing out
a clrf, or a bcf, or whatever. I did not make the mistake because I used
a bcf STATUS,RP0, I made the mistake because I am a newbie and did
*nothing*. I have learned to avoid this mistake now (though I am sure it
will happen somewhere else again). The solution to the problem is to do
*something*.
You have now suggested three different solutions to one problem. First
you said I should do a clrf STATUS. Then you said I should always use
banksels. Finally, you are suggesting I use your macros. Frankly, I
disagree with all your suggestions, and I want the program to be
consistent with the bcf STATUS,RP0, and that's fine.
Your advice is somewhat contradictory at times. Especially when you are
so emphatic with your advice for different approaches... first, you say...
> And why would you do that? What about RP1? Doing a CLRF STATUS takes care of all three bank bits in one instruction.
Then you say:
> That's another problem. You're screwed if you ever try to copy the code for use on a 4 bank PIC. At the very least use BANKSEL. Once you are comfortable with the PIC banking, you can use my DBANKIF and related macros that track the bank setting at assembly time.
So, regardless of your own programming 'philosophy', it seems that
whatever I do would be wrong in some way according to you. I am OK with
that. I don't mind being different to you. At the same time, you are
also correct that I did come here asking for help, I did have problems
in banking. I have revisited my thinking, compared it to what you
suggest, and decided that my banking is now resolved, the solutions you
propose all have merits in their own way, but I have chosen a different
solution that makes sense to me in other ways. Debating whether to use
banksels, or whatever is not going to be much fun or use... so, I'll
move on and agree to differ with you on the 'right philosophy' for
banking. Also agree to differ with you about using your PIC environment.
Finally, also agree to differ with you on whether to consider you more
authoritative than the datasheets (I know the datasheets may be wrong,
but, so may you).
Don't do that. I understand recent versions of MPASM now allow ORG in
relocatable mode, but CODE allows you to explictly name the section, which
is a good idea.
> movwf w_temp
This is wrong without a guarantee that W_TEMP is either in unbanked memory
or reserved at the same offset in every bank.
> movf STATUS,w
> movwf status_temp
Without explicitly setting the banks between these two instructions, you
have to reserve STATUS_TEMP the same was as W_TEMP. A better way is to
insert a CLRF STATUS between these two instructions. Then STATUS_TEMP goes
in regular bank 0.
> ; isr code can go here or be located as a call subroutine elsewhere
It should be located here. Some code will end up here, so it might as well
be the one routine that benefits from being at this location. Doing a CALL
uses up a call stack slot that can't be used by the foreground code.
You also need to pay attention to PCLATH. On machines with more than one
code page you need to save PCLATH then clear it, else local GOTO's in the
interrupt routine won't work as expected. Since this is easy to forget, so
it's better to have your interrupt routine template include assembly time
conditional code for saving/restoring PCLATH based on whether the current
PIC has multiple code pages or not.
Interrupt routines are a solved problem. There is no need to reinvent the
wheel. You can use or modify my QQQ_INTR.ASPIC interrupt routine template
that is part of my PIC develpment environment at http://www.embedinc.com/pic. It contains various assembly time conditionals
to configure itself to different 14 bit core PICs.
> I'm sorry for being so apparently dense, but, I disagree with you, at
> least until I can see where the problem is. You have insisted that the
> microchip code is wrong somewhere, saying it is with the MOVF
> instruction. I have literally spent hours trying to understand the
> problem, run the code, simulated it, and it all works just fine as far
> as everything I have available is concerned. What more do you expect me
> to do.... have you considered for a moment that you may be wrong?
>
> In your other mail you suggested that the STATUS,Z will be
> saved/restored incorrectly. There are 8 lines of code, with 2 MOVF
> instructions... I believe they both work fine. You suggest that one of
> them does the wrong thing. So, please, spare me some anxiety and tell me
> which is broken. I have spent too much time already on this, especially
> when I appear to be missing something 'obvious'.
>
> To repeat, here are the instructions.....
>
> ORG 0x004
> movwf w_temp
> movf STATUS,w
> movwf status_temp
> ; isr code can go here or be located as a call subroutine elsewhere
> movf status_temp,w
> movwf STATUS
> swapf w_temp,f
> swapf w_temp,w
> retfi
>
>
> The first MOVF instruction correctly stores the current STATUS in W
> (even though after the instruction the STATUS,Z may have changed).
> The second MOVF may affect STATUS too, but, it is inconsequential
> because STATUS is neither the source nor target of the instruction...
> and the next instruction (MOVWF) will overwrite the status anyway.
>
> So, please, the context saving *does* care about the Z bit.... if not, I
> have spent a lot of time on it (based on your assertion that it is
> broken), I still don't see the problem, so please spell it out for me
> with a little patience...
>
> Thanks
>
> Rolf
>
>
>
> Tamas Rudnai wrote:
>
>> Hi Rolf,
>>
>>
>>
>>> am not sure what you are suggesting... oh, I see. You think that the
>>> restore of the STATUS register is broken.... well, I don't believe it is.
>>>
>>>
>> It is actually, that context saving does not care about the Z flag...
>>
>>
>>
>>> I have inspected the code, inspected the datasheets, and run various
>>> scenarios through MPSIM, and I can't see where the code is broken. If it
>>> is in fact broken, I need you to do more than just give me a 'clue'.
>>> Where is it.....
>>>
>>>
>> I have already mentioned, even with the code example...
>>
>> Tamas
>>
>>
>> On Tue, Jul 1, 2008 at 1:13 PM, Rolf <KILLspamlearrKILLspamrogers.com> wrote:
>>
>>
>>
>>> Alan B. Pearce wrote:
>>>
>>>
>>>>> I am looking in to the code more carefully now too. I think the existing
>>>>> code works fine, but, I will use the swapf anyway because it comes from
>>>>> the datasheet which I trust more than the .asm template.
>>>>>
>>>>>
>>>>>
>>>> You need to look at why SWAPF is used - big clue, look at what happens to
>>>> the flags when doing the restore at the end of the interrupt routine with
>>>> your code.
>>>>
>>>>
>>>>
>>>>
>>> Hi Alan.
>>>
>>> I am not sure what you are suggesting... oh, I see. You think that the
>>> restore of the STATUS register is broken.... well, I don't believe it
>>> is. I have inspected the code, inspected the datasheets, and run various
>>> scenarios through MPSIM, and I can't see where the code is broken. If it
>>> is in fact broken, I need you to do more than just give me a 'clue'.
>>> Where is it.....
>>>
>>> Here is the snippet from the file C:\Program Files\Microchip\MPASM
>>> Suite\Template\Code\12F675TEMP.ASM. I have taken the liberty of
>>> numbering the lines, and adding some comments of my own...:
>>>
>>>
>>> ORG 0x004 ; interrupt vector location 1 -
>>> fine, get us to the reset vector.
>>> movwf w_temp ; save off current W register
>>> contents 2 - movwf does not affect STATUS. Good.
>>> movf STATUS,w ; move status register into W register
>>> 3 - movf *does* affect STATUS,Z, but, the copy of STATUS in W is from
>>> before the Z is (potentially) changed.
>>> movwf status_temp ; save off contents of STATUS
>>> register 4 - movwf does not affect STATUS, Good.
>>>
>>>
>>> ; isr code can go here or be located as a call subroutine elsewhere
>>>
>>>
>>> movf status_temp,w ; retrieve copy of STATUS register 5
>>> - movf *does* affect status, but, that's OK because we are putting the
>>> original STATUS in to W.
>>> movwf STATUS ; restore pre-isr STATUS register
>>> contents 6 - movwf does not affect STATUS, which is good, because we
>>> are overwriting it's contents with W.
>>> swapf w_temp,f ; 7 - swap the W nybbles because we
>>> want to use SWAPF again later because SWAPF does not affect STATUS.
>>> swapf w_temp,w ; restore pre-isr W register contents
>>> 8 - the final SWAPF to W with the correct W register restore.
>>> retfie ; return from interrupt
>>>
>>>
>>> So, the way I see it is that the template code *does the right thing*.
>>> It may not do it the same way as the datasheet, but, it works just fine.
>>> The lines of potential confusion are lines 2, 3, 5, and 6. Which line
>>> has it wrong?
>>>
>>> Also, just by the way, if I randomly select PIC 10, 12 and 16 files from
>>> C:\Program Files\Microchip\MPASM Suite\Template\Code\ I find that either
>>> they do not have code for the ISR, or they use the system I pasted
>>> above. The templates for common PICS like the 12F675, 16F84A 16F628
>>> 16F648 16F877 16F88 16F887 and more *all* have the system above. Are
>>> *all* the templates wrong for the non PIC18F parts?
>>>
>>> Rolf
>>>
>>> --
This one sets the Z even if there are non-writeable flags in STATUS.
Question is that when can be the STATUS completely 0, as !TO and !PD for
instance likely to be 1, !TO can be zero after WDT but !PD is only when in
sleep mode. So MOVF might do, except if anyone knows when and how can be the
STATUS cooled down to absolute zero :-)
Tamas
PS: Still, there is no reason not to use SWAPF in my opinion, there is no
harm with that...
> Hi Rolf,
> try this code:
>
> clrf STATUS ;not really 0!
> clrf X
> movf X,w
> movf STATUS,w
>
> nop
>
> movwf X
> movf STATUS,W
>
> If you trace with MP SIM you see that the Z flag is changed!
> Cheers
> Nicola
>
>
> Rolf wrote:
> > I'm sorry for being so apparently dense, but, I disagree with you, at
> > least until I can see where the problem is. You have insisted that the
> > microchip code is wrong somewhere, saying it is with the MOVF
> > instruction. I have literally spent hours trying to understand the
> > problem, run the code, simulated it, and it all works just fine as far
> > as everything I have available is concerned. What more do you expect me
> > to do.... have you considered for a moment that you may be wrong?
> >
> > In your other mail you suggested that the STATUS,Z will be
> > saved/restored incorrectly. There are 8 lines of code, with 2 MOVF
> > instructions... I believe they both work fine. You suggest that one of
> > them does the wrong thing. So, please, spare me some anxiety and tell me
> > which is broken. I have spent too much time already on this, especially
> > when I appear to be missing something 'obvious'.
> >
> > To repeat, here are the instructions.....
> >
> > ORG 0x004
> > movwf w_temp
> > movf STATUS,w
> > movwf status_temp
> > ; isr code can go here or be located as a call subroutine elsewhere
> > movf status_temp,w
> > movwf STATUS
> > swapf w_temp,f
> > swapf w_temp,w
> > retfi
> >
> >
> > The first MOVF instruction correctly stores the current STATUS in W
> > (even though after the instruction the STATUS,Z may have changed).
> > The second MOVF may affect STATUS too, but, it is inconsequential
> > because STATUS is neither the source nor target of the instruction...
> > and the next instruction (MOVWF) will overwrite the status anyway.
> >
> > So, please, the context saving *does* care about the Z bit.... if not, I
> > have spent a lot of time on it (based on your assertion that it is
> > broken), I still don't see the problem, so please spell it out for me
> > with a little patience...
> >
> > Thanks
> >
> > Rolf
> >
> >
> >
> > Tamas Rudnai wrote:
> >
> >> Hi Rolf,
> >>
> >>
> >>
> >>> am not sure what you are suggesting... oh, I see. You think that the
> >>> restore of the STATUS register is broken.... well, I don't believe it
> is.
> >>>
> >>>
> >> It is actually, that context saving does not care about the Z flag...
> >>
> >>
> >>
> >>> I have inspected the code, inspected the datasheets, and run various
> >>> scenarios through MPSIM, and I can't see where the code is broken. If
> it
> >>> is in fact broken, I need you to do more than just give me a 'clue'.
> >>> Where is it.....
> >>>
> >>>
> >> I have already mentioned, even with the code example...
> >>
> >> Tamas
> >>
> >>
> >> On Tue, Jul 1, 2008 at 1:13 PM, Rolf <spamBeGonelearrspamBeGonerogers.com> wrote:
> >>
> >>
> >>
> >>> Alan B. Pearce wrote:
> >>>
> >>>
> >>>>> I am looking in to the code more carefully now too. I think the
> existing
> >>>>> code works fine, but, I will use the swapf anyway because it comes
> from
> >>>>> the datasheet which I trust more than the .asm template.
> >>>>>
> >>>>>
> >>>>>
> >>>> You need to look at why SWAPF is used - big clue, look at what happens
> to
> >>>> the flags when doing the restore at the end of the interrupt routine
> with
> >>>> your code.
> >>>>
> >>>>
> >>>>
> >>>>
> >>> Hi Alan.
> >>>
> >>> I am not sure what you are suggesting... oh, I see. You think that the
> >>> restore of the STATUS register is broken.... well, I don't believe it
> >>> is. I have inspected the code, inspected the datasheets, and run
> various
> >>> scenarios through MPSIM, and I can't see where the code is broken. If
> it
> >>> is in fact broken, I need you to do more than just give me a 'clue'.
> >>> Where is it.....
> >>>
> >>> Here is the snippet from the file C:\Program Files\Microchip\MPASM
> >>> Suite\Template\Code\12F675TEMP.ASM. I have taken the liberty of
> >>> numbering the lines, and adding some comments of my own...:
> >>>
> >>>
> >>> ORG 0x004 ; interrupt vector location 1 -
> >>> fine, get us to the reset vector.
> >>> movwf w_temp ; save off current W register
> >>> contents 2 - movwf does not affect STATUS. Good.
> >>> movf STATUS,w ; move status register into W register
> >>> 3 - movf *does* affect STATUS,Z, but, the copy of STATUS in W is from
> >>> before the Z is (potentially) changed.
> >>> movwf status_temp ; save off contents of STATUS
> >>> register 4 - movwf does not affect STATUS, Good.
> >>>
> >>>
> >>> ; isr code can go here or be located as a call subroutine elsewhere
> >>>
> >>>
> >>> movf status_temp,w ; retrieve copy of STATUS register 5
> >>> - movf *does* affect status, but, that's OK because we are putting the
> >>> original STATUS in to W.
> >>> movwf STATUS ; restore pre-isr STATUS register
> >>> contents 6 - movwf does not affect STATUS, which is good, because we
> >>> are overwriting it's contents with W.
> >>> swapf w_temp,f ; 7 - swap the W nybbles because we
> >>> want to use SWAPF again later because SWAPF does not affect STATUS.
> >>> swapf w_temp,w ; restore pre-isr W register contents
> >>> 8 - the final SWAPF to W with the correct W register restore.
> >>> retfie ; return from interrupt
> >>>
> >>>
> >>> So, the way I see it is that the template code *does the right thing*.
> >>> It may not do it the same way as the datasheet, but, it works just
> fine.
> >>> The lines of potential confusion are lines 2, 3, 5, and 6. Which line
> >>> has it wrong?
> >>>
> >>> Also, just by the way, if I randomly select PIC 10, 12 and 16 files
> from
> >>> C:\Program Files\Microchip\MPASM Suite\Template\Code\ I find that
> either
> >>> they do not have code for the ISR, or they use the system I pasted
> >>> above. The templates for common PICS like the 12F675, 16F84A 16F628
> >>> 16F648 16F877 16F88 16F887 and more *all* have the system above. Are
> >>> *all* the templates wrong for the non PIC18F parts?
> >>>
> >>> Rolf
> >>>
> >>> --
Rolf wrote:
> Two things... First, I am confused. I though newbies (like me) should
> download your development environment ...
No. I have never suggested it as a starting point. There is too much there
to learn beyond just the PIC and MPASM/MPLIB/MPLINK. Once someone is
familiar with the PIC and tools, then they can possibly use my PIC
development environment. I don't care if they do, but it's there for free
for anyone that wants to.
> Secondly, Explicitly writing BSF/BCF on RP0/RP1 *IS* encouraged.
> ...
> In fact, the datasheet
> (which you disagree with) explicitly recommends it in the ISR.
No, the data sheet doesn't recommend or encourage any particular style of
programming. Occasionally small snippets of code are shown in the data
sheet to illustrate points. These are not meant as good examples of
programming, and usually aren't. They are usually just the bare minimum
instructions to perform whatever action they are describing.
> You have now suggested three different solutions to one problem. First
> you said I should do a clrf STATUS.
Yes, in the one specific case in the interrupt routine. I couldn't fault
anyone for using BANKSEL there instead, which I even pointed out.
> Then you said I should always use banksels.
That is the absolutely safe approach everyone should start with. Some may
never get beyond that. That's OK. BANKSEL is never wrong, only less
efficient than more advanced techniques like my macros that track the bank
setting at assembly time.
> Finally, you are suggesting I use your macros.
No, only making them available and sometimes referring to how they work.
However, BSF/BCF on RP0/RP1 is bad programming practise. That's the one
thing you should definitely not do.
Look, you can do what you want. I have no reason to care. You can ignore
all the advice you get here. My programs will still run fine. However, I
don't want anyone else listening in to pick up bad habits and think that
direct manipulation of the bank bits is somehow acceptable programming
practise.
The question is whether the .asm templates from Microchip for the
PIC12F675 successfully save and restore STATUS and W registers.
I believe they do, Tamas believes they don't (I think he believes that
anyway, as well as perhaps some others).
Here is the code, again.... as copied / pasted from the file
12F675TEMP.ASM file distributed by Microchip (this time with the
trailing 'e').
ORG 0x004 ; interrupt vector location
movwf w_temp ; save off current W register contents
movf STATUS,w ; move status register into W register
movwf status_temp ; save off contents of STATUS register
; isr code can go here or be located as a call subroutine elsewhere
movf status_temp,w ; retrieve copy of STATUS register
movwf STATUS ; restore pre-isr STATUS register
contents
swapf w_temp,f
swapf w_temp,w ; restore pre-isr W register contents
retfie ; return from interrupt
As has been suggested numerous times by others, there are other ways to
do the same thing (and I have changed my code as well). The question is,
whether, on the PIC12F675, the above actually works?
Yes, No, if not, then why...?
It is only 8 instructions, and, if it does not work then there is
something I have missed in my understanding of the individual
instructions, MPSIM, and more.
> ORG 0x004 ; interrupt vector location
> movwf w_temp ; save off current W register contents
> movf STATUS,w ; move status register into W register
> movwf status_temp ; save off contents of STATUS register
>
They key point here (and also what I *think* that Tamas
has got wrong) is that MOVWF does *not* change the Z-flag.
It doesn't change any STATUS flags. Thats why the code
*does* work, even without any SWAPF's in the code above.
Below, OTOH, SWAP *must* be used, since MOVF (either to "W"
or to "F") *does* set/clear the Z-flag.
>
> ; isr code can go here or be located as a call subroutine elsewhere
>
>
> movf status_temp,w ; retrieve copy of STATUS register
> movwf STATUS ; restore pre-isr STATUS register
> contents
> swapf w_temp,f
> swapf w_temp,w ; restore pre-isr W register contents
> retfie ; return from interrupt
>
> Rolf wrote:
>
>> ORG 0x004 ; interrupt vector location
>> movwf w_temp ; save off current W register contents
>> movf STATUS,w ; move status register into W register
>> movwf status_temp ; save off contents of STATUS register
>>
>
> They key point here (and also what I *think* that Tamas
> has got wrong) is that MOVWF does *not* change the Z-flag.
> It doesn't change any STATUS flags. Thats why the code
> *does* work, even without any SWAPF's in the code above.
>
> Below, OTOH, SWAP *must* be used, since MOVF (either to "W"
> or to "F") *does* set/clear the Z-flag.
>
> Jan-Erik.
>
>
>> ; isr code can go here or be located as a call subroutine elsewhere
>>
>>
>> movf status_temp,w ; retrieve copy of STATUS register
>> movwf STATUS ; restore pre-isr STATUS register
>> contents
>> swapf w_temp,f
>> swapf w_temp,w ; restore pre-isr W register contents
>> retfie ; return from interrupt
>>
>
Jan-Erik (he reads always the f manual ;-) is right!
The point is that the first MOVF works but the second not, so you MUST
use the SWAPF in the second position.
But if you use the SWAPF in the second position you expect a swapped
data! So you need the SWAPF also in the first position.
Nic
> Rolf wrote:
>
>
>> ORG 0x004 ; interrupt vector location
>> movwf w_temp ; save off current W register contents
>> movf STATUS,w ; move status register into W register
>> movwf status_temp ; save off contents of STATUS register
>>
>>
>
> They key point here (and also what I *think* that Tamas
> has got wrong) is that MOVWF does *not* change the Z-flag.
> It doesn't change any STATUS flags. Thats why the code
> *does* work, even without any SWAPF's in the code above.
>
> Below, OTOH, SWAP *must* be used, since MOVF (either to "W"
> or to "F") *does* set/clear the Z-flag.
>
> Jan-Erik.
>
>
>
>> ; isr code can go here or be located as a call subroutine elsewhere
>>
>>
>> movf status_temp,w ; retrieve copy of STATUS register
>> movwf STATUS ; restore pre-isr STATUS register
>> contents
>> swapf w_temp,f
>> swapf w_temp,w ; restore pre-isr W register contents
>> retfie ; return from interrupt
>>
>>
>
>
> They key point here (and also what I *think* that Tamas
> has got wrong) is that MOVWF does *not* change the Z-flag.
Sorry I did not say that :-) I said MOVF changes the Z. Also said that as
there are at least two flags in STATUS that are not writable and have no
idea when and what circumstances both of them get cleared MOVF as saving the
STATUS might work (as STATUS will never be completely zero). And also said
:-) that I think it is safer to use SWAPF and there is no any reason not to
use that.
> Rolf wrote:
>
> > ORG 0x004 ; interrupt vector location
> > movwf w_temp ; save off current W register contents
> > movf STATUS,w ; move status register into W register
> > movwf status_temp ; save off contents of STATUS register
> >
>
> They key point here (and also what I *think* that Tamas
> has got wrong) is that MOVWF does *not* change the Z-flag.
> It doesn't change any STATUS flags. Thats why the code
> *does* work, even without any SWAPF's in the code above.
>
> Below, OTOH, SWAP *must* be used, since MOVF (either to "W"
> or to "F") *does* set/clear the Z-flag.
>
> Jan-Erik.
>
>
> >
> > ; isr code can go here or be located as a call subroutine elsewhere
> >
> >
> > movf status_temp,w ; retrieve copy of STATUS register
> > movwf STATUS ; restore pre-isr STATUS register
> > contents
> > swapf w_temp,f
> > swapf w_temp,w ; restore pre-isr W register contents
> > retfie ; return from interrupt
> >
>
> Also said that as
> there are at least two flags in STATUS that are not writable and have no
> idea when and what circumstances both of them get cleared MOVF as saving
> the STATUS might work...
Can you show a case when it *does not* work ?
Or are you guessing ?
> (as STATUS will never be completely zero).
Even so, Z would have been set *after* STATUS is saved,
so that doesn't matter.
> And also said
> :-) that I think it is safer to use SWAPF and there is no any reason not to
> use that.
Right, first show a case when the shown example goes wrong.
I've not looked into it fully, but doesn't SWAP's
add a few (one?) extra instruction(s) ? Since this is in the
ISR-overhead, I'd care about that.
*OR*, use a PIC18 and the automatic shadow regs... :-)
The data memory (see Figure 2-2) is partitioned into
two banks, which contain the General Purpose regis-
ters and the Special Function registers. The Special
Function registers are located in the first 32 locations of
each bank. Register locations 20h-5Fh are General
Purpose registers, implemented as static RAM and are
mapped across both banks.
This is somewhat ambiguous, but, addresses 20h through 5Fh are all in
'bank 0'. The concept of being mapped across 'both banks' is illustrated
better in the Figure 2-2 which indicates, in Bank 1 that addresses A0h
through DFh access 20h through 5Fh respectively.
Further, the mid-range reference manual has the following:
Some Mid-Range MCU devices have banked memory in the GPR area. GPRs are
not initialized
by a Power-on Reset and are unchanged on all other resets.
The register ï¬le can be accessed either directly, or using the File
Select Register FSR, indirectly.
Some devices have areas that are shared across the data memory banks, so
a read / write to
that area will appear as the same location (value) regardless of the
current bank. We refer to this
area as the Common RAM.
I presume it is safe to suggest that the 12F675 is one of the 'Common
RAM' devices, where ALL GPR memory is common, (but the 32 SFR addresses
in each bank are not).
Hence, I presume this confirms that there is no need to do banking for
GPR memory for the PIC12F675 (but I am sure that Olin will suggest that
is bad practice if I were to copy/paste the code to another device.... ;-)
> I would just like to add that, *of course*
> are w_temp and status_temp allocated from
> the "share bank" (or "unbanked memory").
>
> Jan-Erik.
>
> Jan-Erik Soderholm wrote:
>
>> Rolf wrote:
>>
>>
>>> ORG 0x004 ; interrupt vector location
>>> movwf w_temp ; save off current W register contents
>>> movf STATUS,w ; move status register into W register
>>> movwf status_temp ; save off contents of STATUS register
>>>
>>>
>> They key point here (and also what I *think* that Tamas
>> has got wrong) is that MOVWF does *not* change the Z-flag.
>> It doesn't change any STATUS flags. Thats why the code
>> *does* work, even without any SWAPF's in the code above.
>>
>> Below, OTOH, SWAP *must* be used, since MOVF (either to "W"
>> or to "F") *does* set/clear the Z-flag.
>>
>> Jan-Erik.
>>
>>
>>
>>> ; isr code can go here or be located as a call subroutine elsewhere
>>>
>>>
>>> movf status_temp,w ; retrieve copy of STATUS register
>>> movwf STATUS ; restore pre-isr STATUS register
>>> contents
>>> swapf w_temp,f
>>> swapf w_temp,w ; restore pre-isr W register contents
>>> retfie ; return from interrupt
>>>
>>>
> ORG 0x004
> movwf w_temp - could go in either bank!
> movf STATUS,w
> movwf status_temp -- could go in either bank!
> ; isr code can go here or be located as a call subroutine elsewhere
> movf status_temp,w -- you're dead if the bank has changed!
> movwf STATUS
> swapf w_temp,f
> swapf w_temp,w
> retfi
The above code requires a 'shadow' of w_temp in all banks.
It also requires a shadow of 'status_temp' in all banks, which shouldn't be
needed.
It also requires that the banking status be saved/restored separately, which
is a pain.
Code that works:
org 0x4
movwf w_temp ; either bank!
movf STATUS,w ; ok: clobbers Z, but saves STATUS first
bcf STATUS,RP0 ; ** add this **
-or-
clrf STATUS ; ** or this **
movwf status_temp ; certain to be in bank 0
<your isr code here>
<make sure you are (back) in bank 0>
movf status_temp,W ; ok: clobbers Z, but W contains original
value of status
movwf STATUS ; ok: bank is now what it was at interrupt
entry
swapf w_temp,F ; ok: no flags changed, could be in either
bank
-- Bob Ammerman
RAm Systems
swapf w_temp,W ; ok: no flags changed, could be in either
bank
retfi
> Can you show a case when it *does not* work ?
> Or are you guessing ?
Nope, I can't. I am guessing about STATUS, that STATUS never goes to zero
(as !TO and/or !PD) will always set - that's my guessing and coul dnot find
any info about a situation when it would be the case. The problem with MOVF
is that it depends on this guessing, that STATUS never goes zero
completely...
> Even so, Z would have been set *after* STATUS is saved,
> so that doesn't matter.
I see what you mean. I saw many interesting behaviours, like if you have the
FSR on a 10F202 and INCFSZ on it, FSR never goes to zero as the 3 higher
bits are always read as zero, however, INCFSZ skips the next instrution if
FSR previously was 0xFF. It means for me that actually the status checked
_before_ the incrementation would be made - in other words the micro code of
the PIC checks if the register was FF before the incrementation instead of
checking if that was 0 after it.
Because of this experience I would not be surprised if Z would set before
the move had been done. Other words I would not be surprised if microcode
checks first if STATUS is zero, then sets Z (in STATUS) then moves STATUS to
W (with the Z already set)...
> I've not looked into it fully, but doesn't SWAP's
> add a few (one?) extra instruction(s) ? Since this is in the
> ISR-overhead, I'd care about that.
All you have to do is _instead_ of MOVF use SWAPF... both of them 1 word / 1
cycle, so no harm in my opinion.
> Tamas Rudnai wrote:
>
> > Also said that as
> > there are at least two flags in STATUS that are not writable and have no
> > idea when and what circumstances both of them get cleared MOVF as saving
> > the STATUS might work...
>
> Can you show a case when it *does not* work ?
> Or are you guessing ?
>
> > (as STATUS will never be completely zero).
>
> Even so, Z would have been set *after* STATUS is saved,
> so that doesn't matter.
>
> > And also said
> > :-) that I think it is safer to use SWAPF and there is no any reason not
> to
> > use that.
>
> Right, first show a case when the shown example goes wrong.
>
> I've not looked into it fully, but doesn't SWAP's
> add a few (one?) extra instruction(s) ? Since this is in the
> ISR-overhead, I'd care about that.
>
> *OR*, use a PIC18 and the automatic shadow regs... :-)
>
> Jan-Erik.
> > Can you show a case when it *does not* work ?
> > Or are you guessing ?
>
> Nope, I can't. I am guessing about STATUS, that STATUS never goes to zero
> (as !TO and/or !PD) will always set - that's my guessing and coul dnot find
> any info about a situation when it would be the case. The problem with MOVF
> is that it depends on this guessing, that STATUS never goes zero
> completely...
>
> > Even so, Z would have been set *after* STATUS is saved,
> > so that doesn't matter.
>
> I see what you mean. I saw many interesting behaviours, like if you have
> the FSR on a 10F202 and INCFSZ on it, FSR never goes to zero as the 3 higher
> bits are always read as zero, however, INCFSZ skips the next instrution if
> FSR previously was 0xFF. It means for me that actually the status checked
> _before_ the incrementation would be made - in other words the micro code of
> the PIC checks if the register was FF before the incrementation instead of
> checking if that was 0 after it.
>
> Because of this experience I would not be surprised if Z would set before
> the move had been done. Other words I would not be surprised if microcode
> checks first if STATUS is zero, then sets Z (in STATUS) then moves STATUS to
> W (with the Z already set)...
>
> > I've not looked into it fully, but doesn't SWAP's
> > add a few (one?) extra instruction(s) ? Since this is in the
> > ISR-overhead, I'd care about that.
>
> All you have to do is _instead_ of MOVF use SWAPF... both of them 1 word /
> 1 cycle, so no harm in my opinion.
>
> Tamas
>
>
>
>
>
>
>
>
> On Tue, Jul 1, 2008 at 7:25 PM, Jan-Erik Soderholm <
> EraseMEjan-erik.soderholmtelia.com> wrote:
>
>> Tamas Rudnai wrote:
>>
>> > Also said that as
>> > there are at least two flags in STATUS that are not writable and have no
>> > idea when and what circumstances both of them get cleared MOVF as saving
>> > the STATUS might work...
>>
>> Can you show a case when it *does not* work ?
>> Or are you guessing ?
>>
>> > (as STATUS will never be completely zero).
>>
>> Even so, Z would have been set *after* STATUS is saved,
>> so that doesn't matter.
>>
>> > And also said
>> > :-) that I think it is safer to use SWAPF and there is no any reason not
>> to
>> > use that.
>>
>> Right, first show a case when the shown example goes wrong.
>>
>> I've not looked into it fully, but doesn't SWAP's
>> add a few (one?) extra instruction(s) ? Since this is in the
>> ISR-overhead, I'd care about that.
>>
>> *OR*, use a PIC18 and the automatic shadow regs... :-)
>>
>> Jan-Erik.
> Hence, I presume this confirms that there is no need to do banking for
> GPR memory for the PIC12F675
Correct. Or for the PIC12F629.
(And I was the one who discovered that Olins environment
didn't support "only-nonbanked-GPR" PICs, several years ago :-)
That is, the 12F675/629 types...)
> (but I am sure that Olin will suggest that
> is bad practice if I were to copy/paste the code
> to another device....
Or if you simply decide to use the 12F683 instead.
That one has both banked and unbanked GPR's... :-)
Rolf wrote:
> I presume it is safe to suggest that the 12F675 is one of the 'Common
> RAM' devices, where ALL GPR memory is common, (but the 32 SFR
> addresses in each bank are not).
Yes, it appears to be one of those.
> Hence, I presume this confirms that there is no need to do banking for
> GPR memory for the PIC12F675
It means all your variables only live in bank 0. They are also aliased from
bank 1, and that PIC only has banks 0 and 1, so you don't need to worry
about banking when accessing a variable in RAM.
However, don't get to used to that. Most PICs don't work that way. It's a
good idea to learn the discipline the right way even if you could get away
with being more sloppy in this case.
You can define all your variables in a UDATA_SHR section. That states that
they must go in shared memory. If the code is ever built for a PIC that
doesn't have that much shared memory, you will get build errors. Then you
can inspect the variables and decide how to move them around and add the
necessary bank switching code.
> Because of this experience I would not be surprised if Z would set before
> the move had been done. Other words I would not be surprised if microcode
> checks first if STATUS is zero, then sets Z (in STATUS) then moves STATUS to
> W (with the Z already set)...
I would be surpriced.
It would probably break a lot of code depending
on the bits beeing updated after the move.
You were right Jan, MOVF is not strange at all and works as expected. Tried
the following code on MPSIM compiled for 12F683, and I think MPSIM is good
enough simulator to believe that this is correct on real MCU as well:
start
movlw 0
movwf STATUS ; clears all flags except !TO and !PD
sleep ; clears !PD
nop ; after WDT !TO cleared as well
; now STATUS is absolute 0x00
movf STATUS,w ; sets Z but W will be 0x00
movwf STATUS ; STATUS remains 0x00
goto start
So the conclusion is that I was wrong, we can safely use MOVF to save STATUS
- sorry for my skepticism.
> Tamas Rudnai wrote:
>
> > Because of this experience I would not be surprised if Z would set before
> > the move had been done. Other words I would not be surprised if microcode
> > checks first if STATUS is zero, then sets Z (in STATUS) then moves STATUS
> to
> > W (with the Z already set)...
>
> I would be surpriced.
> It would probably break a lot of code depending
> on the bits beeing updated after the move.
>
> Jan-Erik.
>
> Assembler appears to be somewhat like perl, TIMTOWTDI ... ;-)
>
> There Is More Than One Way To Do It - this is the perl 'mantra'
The big difference of course is that you're working right down at the
chip level. I suggest that to do many things in firmware in machine
code there is only one way to do it correctly and reliably
I'm still reading the thread, but I wanted to post the ISR code that I use,
since that seems to be what the discussion is most about. This code is due
to Fr. McGee (I think!) and it has always worked with the 16F and 12F parts
I use. Here you go:
; The ISR begins here at program location 0x04.
movwf w_temp ; copy W to temp register
swapf STATUS, W ; swap status to be saved into W
movwf status_temp
movf PCLATH, W
movwf pclath_temp
movf FSR, W
movwf fsr_temp
; ISR goes here...
leave_isr
movf fsr_temp, W
movwf FSR
movf pclath_temp, W
movwf PCLATH
swapf status_temp, W ; swap STATUS_TEMP register into W, sets bank to original state
movwf STATUS ; move W into STATUS register
swapf w_temp, F ; swap w_temp
swapf w_temp, W ; swap W_TEMP into W
retfie
Best wishes Rolf.
--
"You're one of those condescending Unix computer users!"
"Here's a nickel, kid. Get yourself a better computer" -- Dilbert.
>I've not looked into it fully, but doesn't SWAP's
>add a few (one?) extra instruction(s) ? Since this is in the
>ISR-overhead, I'd care about that.
This is the trick - to use swapf STATUS,w instead of movwf STATUS at the
entry of the routine, so status gets stored with the two nybbles swapped.
Then at the end of the routine only one SWAPF is needed instead of two,
thereby saving an instruction in the ISR.
Matthew Miller wrote:
> ... but I wanted to post the ISR code that ...
> it has always worked with the 16F and 12F parts
Note that the 12 bit core parts don't have interrupts, so the 12F part you
refer to are really misnamed 14 bit core parts.
> ; The ISR begins here at program location 0x04.
> movwf w_temp ; copy W to temp register
You didn't show how the variables were defined. W_TEMP must be in unbanked
memory or multiply defined at the same offset in every bank.
> swapf STATUS, W ; swap status to be saved into W
You should insert a CRLF STATUS here. That selects a predictable bank. If
not then you either have to reserve STATUS_TEMP, PCLATH_TEMP and FSR_TEMP in
unbanked memory or at the same offset in all banks. Unbanked memory is
usually precious, and reserving the same variable in multiple banks is a
waste.
> movwf status_temp
>
> movf PCLATH, W
> movwf pclath_temp
>
> movf FSR, W
> movwf fsr_temp
I put assembly time conditionals around saving PCLATH and FSR so that they
only get saved/restored when appropriate. Also, you forgot to clear PCLATH
after saving it.
> ; ISR goes here...
>
> leave_isr
Another CLRF STATUS and FSR_TEMP, PCLATH_TEMP, and STATUS_TEMP can be in
bank 0.
> movf fsr_temp, W
> movwf FSR
> movf pclath_temp, W
> movwf PCLATH
> swapf status_temp, W ; swap STATUS_TEMP register into W,
> sets bank to original state movwf STATUS ; move W
> into STATUS register swapf w_temp, F ; swap w_temp
> swapf w_temp, W ; swap W_TEMP into W
> retfie
This got garbled so I didn't look at it closely.
Again, a 14 bit core interrupt routine is a solved problem. Even if you're
not using my PIC development environment (http://www.embedinc.com/pic), you
can mostly copy my interrupt template routine QQQ_INTR.ASPIC. It has all
the details taken care of. Too many of you are trying to reinvent the
wheel, and then getting it wrong on top of it.
> You should insert a CRLF STATUS here. That selects a predictable bank.
> If
> not then you either have to reserve STATUS_TEMP, PCLATH_TEMP and FSR_TEMP
> in
> unbanked memory or at the same offset in all banks. Unbanked memory is
> usually precious, and reserving the same variable in multiple banks is a
> waste.
Certainly reserving the same variables in multiple banks would be
wasteful, but as to how precious unbanked memory - surely it depends?
It seems to me that using shared memory for context saving is quite valid.
It's true that it only costs one instruction (CLRF STATUS) to use bank 0,
but I still think that using shared registers is a valid choice, if they
are available - especially on a MCU like the 12F675 (the subject of this
thread) where all the GPRs are shared.
On Wed, Jul 02, 2008 at 07:09:34AM -0400, Olin Lathrop wrote:
>
> Also, you forgot to clear PCLATH after saving it.
Oh, this is a big one! All the locations in the previous post are in the
global bank, but me not clearning PCLATH is a mistake.
Olin, thanks for your comments and looking over this code that I posted. I'm
just a hobbyist, but I've learned some things from your post and this
thread.
>Certainly reserving the same variables in multiple banks would be
>wasteful, but as to how precious unbanked memory - surely it depends?
>It seems to me that using shared memory for context saving is quite valid.
You need only 1 byte in shared memory for the interrupt variable save - the
one that saves W. The rest can go in bank 0 if you use Olins instruction
sequence (and I believe this instruction sequence has long been advocated by
Microchip as well).
Alan B. Pearce wrote:
>
> >Certainly reserving the same variables in multiple banks would be
> >wasteful, but as to how precious unbanked memory - surely it depends?
> >It seems to me that using shared memory for context saving is quite
> valid.
>
> You need only 1 byte in shared memory for the interrupt variable save -
> the
> one that saves W. The rest can go in bank 0 if you use Olins
> instruction
> sequence
Yep, completely understand that, don't disagree - just saying that, if
enough shared registers are available, using them to save context is a valid
choice.
> (and I believe this instruction sequence has long been
> advocated by Microchip as well).
Perhaps, but here's a quote from the 16F887 data sheet (quite recent, from
2007):
"Since the upper 16 bytes of all GPR banks are common
in the PIC16F882/883/884/886/887 (see
Figures 2-2 and 2-3), temporary holding registers,
W_TEMP and STATUS_TEMP, should be placed in
here. These 16 locations do not require banking and
therefore, make it easier to context save and restore."
Now, I'm NOT saying that just because that's what's recommended in the data
sheet, it's what you have to do. Just that Microchip seems to agree with me
that using shared registers is reasonable.
> "Since the upper 16 bytes of all GPR banks are common
> in the PIC16F882/883/884/886/887 (see
That's true, however, the only consideration is that sometimes you can use
the common area for optimized RAM usage - aka to avoid using bank selection
code outside of your isr (inside you use the bank0 with Olin's suggestion
anyway, so there is no need to waste the common area).
> Alan B. Pearce wrote:
> >
> > >Certainly reserving the same variables in multiple banks would be
> > >wasteful, but as to how precious unbanked memory - surely it depends?
> > >It seems to me that using shared memory for context saving is quite
> > valid.
> >
> > You need only 1 byte in shared memory for the interrupt variable save -
> > the
> > one that saves W. The rest can go in bank 0 if you use Olins
> > instruction
> > sequence
>
> Yep, completely understand that, don't disagree - just saying that, if
> enough shared registers are available, using them to save context is a
> valid
> choice.
>
> > (and I believe this instruction sequence has long been
> > advocated by Microchip as well).
>
> Perhaps, but here's a quote from the 16F887 data sheet (quite recent, from
> 2007):
>
> "Since the upper 16 bytes of all GPR banks are common
> in the PIC16F882/883/884/886/887 (see
> Figures 2-2 and 2-3), temporary holding registers,
> W_TEMP and STATUS_TEMP, should be placed in
> here. These 16 locations do not require banking and
> therefore, make it easier to context save and restore."
>
> Now, I'm NOT saying that just because that's what's recommended in the data
> sheet, it's what you have to do. Just that Microchip seems to agree with
> me
> that using shared registers is reasonable.
>
>
> David Meiklejohn
> http://www.gooligum.com.au
>
>
>
Tamas Rudnai wrote:
>> "Since the upper 16 bytes of all GPR banks are common
>> in the PIC16F882/883/884/886/887 (see
>
> That's true, however, the only consideration is that sometimes you can use
> the common area for optimized RAM usage - aka to avoid using bank selection
> code outside of your isr (inside you use the bank0 with Olin's suggestion
> anyway, so there is no need to waste the common area).
>
> Tamas
>
OTOH, it's often in the ISR that you'd like to save
every cycle possible. So *I* do not see the usage of
shared memory in the ISR as "waste"...
tuis.net"
