Odd situation on a PIC 16F872: Still coding the pet feeder and things
were going along quite nicely, until I added a piece of code and everything
freaked out. When it normally starts up, the (4) 7-seg digits show '_'
and then goes in to clock mode, counting from 00:01 upwards. With
the new piece of code it shows the start-up underscores, then freezes.
However, the code is still running. I know this cause I can press the
buttons and get some actions out of them (though some behaviors
an incorrect now). Also added a blinking LED within the ISR and
that shows up even when the code is broken. No WDT.
So I commented out the new code (subroutine) and it works again, BUT
if I comment out the code that calls the new subroutine (but leave the
subroutine in place), it still does NOT work. I've narrowed it down to the
existence of the code that causes a problem. I've even changed the new
subroutine to 26 nop's (instead of the 26 instructions that were there) with
absolutely no calls to it, and it still fails.
So now I look at page boundaries, but my total code size is under 600
bytes, and I should not have to worry about paging on a 2K-prog-memory
'F872. Another check for tables crossing the 256-byte boundary and no
violations there. Also using indirect addressing in various places, but
all addresses pointed to are in bank 0 of data mem so that should not
be a problem.
I skimmed thru the memory-organization chapter (ch 6) of the mid-range
mcu doc again and haven't come up with anything new that could be
causing this.
Some memory map info:
Main code (init stuff and main loop): 0x0005 thru 0x00E2.
Subroutines: 0x00E3 thru 0x0020E
ISR: 0x020F thru 0x0244
What else could I be neglecting to check here?
My next test will be slowly reducing the 26 nops to see when the code
works again and see if that leads to anything. After that I may reinstall
the MPLAB simulator and try it there.
Thoughts appreciated,
-Neil.
-- http://www.piclist.com hint: PICList Posts must start with ONE topic:
[PIC]:,[SX]:,[AVR]: ->uP ONLY! [EE]:,[OT]: ->Other [BUY]:,[AD]: ->Ads
>So I commented out the new code (subroutine) and it works again, BUT
>if I comment out the code that calls the new subroutine (but leave the
>subroutine in place), it still does NOT work. I've narrowed it down to the
>existence of the code that causes a problem. I've even changed the new
>subroutine to 26 nop's (instead of the 26 instructions that were there) with
>absolutely no calls to it, and it still fails.
I *really* would take a look at any and all jump tables. You have
described a classic page boundary problem (the nop's tell me that). Do a
search for PCL in your source and see if you have missed a computed goto or
jump somewhere.
Celebrating 18 years of Engineering Innovation (1984 - 2002)
.-. .-. .-. .-. .-. .-. .-. .-. .-. .-
`-' `-' `-' `-' `-' `-' `-' `-' `-'
Do NOT send unsolicited commercial email to this email address.
This message neither grants consent to receive unsolicited
commercial email nor is intended to solicit commercial email.
-- http://www.piclist.com hint: PICList Posts must start with ONE topic:
[PIC]:,[SX]:,[AVR]: ->uP ONLY! [EE]:,[OT]: ->Other [BUY]:,[AD]: ->Ads
Also went looking for FSR/INDF, but can't seem understand
how that would cause a problem by being shifted in program
memory -- it still accesses low data memory.
Page 1 to page 2? Why? Page sizes are 2048 bytes each (11 bits),
and my ISR goes from byt 527 to byt 580, so I should not have to
worry about PCLATH. Or so I interpreted from the datasheet.
Can you explain, por favor?
BTW, no, I don't have any references to PCLATH in the ISR code,
for the reason above.
Bingo! Ran this thru the simulator and came up with this
too -- the addwf PCL,F instruction kept jumping back to the
Init section of my code (near the beginning).
Page 6-2 in DS31006A (Mid-Range MCU Family doc) told me
"..... 2K-word devices (or less) do not require paging".
Liars! :-) Unless I'm interpreting that too generally.
A little more delving found me figure 6-2 which showed
that instructions modifying PCL are 8-bit and I need to
set PCLATH etc.
So since I did not want to have to be on the lookout for
this as I continually grow the code, I thought I'd move
the table read subroutine to a far off spot in upper mem.
I tried this in the sim...
org 0x0700 ; Start in really high memory
;-----------------------------------------------------
; Convert 4-digit data to 7-segment data
;-----------------------------------------------------
It worked! BUT it didn't work on the actual chip. On the
actual chip, I moved the subroutine to the beginning of
code (before even the initialization stuff), and it works.
That's still an unexplained mystery, but based on this
256-byte block issue, your #2 below makes sense now. I'll
go add the PCLATH protection code in the ISR.
Actually, now that I look into this carefully, the
same doc tells me that calls and retfie's will
save/retrieve at least all 11 bits on a 2048-byte
device, so for the 16F872, I won't actually need
to save/restore PCLATH in the ISR.
>I won't actually need
>to save/restore PCLATH in the ISR.
Not a good idea, as you will surely alter the ISR at some stage and break
your whole project, not just the ISR :)
For the four instructions and one data ram location it takes, it is not
worth the trouble of dealing with later problems that seem unrelated to a
code change you make.
> Actually, now that I look into this carefully, the
> same doc tells me that calls and retfie's will
> save/retrieve at least all 11 bits on a 2048-byte
> device, so for the 16F872, I won't actually need
> to save/restore PCLATH in the ISR.
>
> Yes, that was all one sentence.
>
> Cheers,
> -Neil.
>
This is true, only if the ISR doesn't use any lookup tables or otherwise
have to manipulate PCLATH. Also, you can get stung in the future if you move
up to a larger device, so be sure to well comment the situation, at least.
> So I commented out the new code (subroutine) and it works again, BUT
> if I comment out the code that calls the new subroutine (but leave the
> subroutine in place), it still does NOT work. I've narrowed it down to
the
> existence of the code that causes a problem. I've even changed the new
> subroutine to 26 nop's (instead of the 26 instructions that were there)
with
> absolutely no calls to it, and it still fails.
>
> So now I look at page boundaries, but my total code size is under 600
> bytes, and I should not have to worry about paging on a 2K-prog-memory
> 'F872. Another check for tables crossing the 256-byte boundary and no
> violations there. Also using indirect addressing in various places, but
> all addresses pointed to are in bank 0 of data mem so that should not
> be a problem.
Obviously this bug has something to do with code moving around in memory.
Here are a few possible causes of the bug:
1 - PCLATH is incorrect when you jump into a table. PCLATH can be ignored
for the purpose of paging because all your code is on a single page. This
is because the GOTO and CALL instruction supply 11 bits of the address, and
PCLATH is only used for the remaining upper two bits - which aren't used on
the '872 anyway. However, when PCL is modified directly, then PCLATH
supplies all but the low 8 bits of the target address. If a table doesn't
cross a 256 word boundary, then the same PCLATH value can be used for all
table entries, and it can therefore be explicilty loaded with the
appropriate constant. It does NOT mean that PCLATH can be simply ignored.
2 - You have a bug in either case that causes a jump to an invalid
location. The additional code moves things around so that a different
section of your code gets jumped to. The previous one happened to cover up
the bug, the new one doesn't.
Either way this should be easy to find with the emulator, ICD, or perhaps
even the simulator. The PIClist is not a substitute for either of these.
You should do all reasonable homework (debugging with ICE, ICD, or SIM is
certainly part of this) before asking 2000 people to take time out to do you
a favor.
*****************************************************************
Embed Inc, embedded system specialists in Littleton Massachusetts
(978) 742-9014, http://www.embedinc.com
> > Some memory map info:
> > Main code (init stuff and main loop): 0x0005 thru 0x00E2.
> > Subroutines: 0x00E3 thru 0x0020E
> > ISR: 0x020F thru 0x0244
>
> You just pushed your interrupt handler from page 1 to page 2. Are you
> properly handling PCLATH in the handler save/restore code?
All his code is on page 0, which extends from 0 to 7FF. The 16F872 only has
this single page anyway.
*****************************************************************
Embed Inc, embedded system specialists in Littleton Massachusetts
(978) 742-9014, http://www.embedinc.com
Once again bitten by the stupid addage "Tables shalt not cross a 256 word
boundary". Please, everyone forget this and more importantly, stop passing
on this bit of "wisdom".
First, it is blantantly false. I have tables regularly crossing 256 word
boundaries, or take no care to insure they don't. Second, it there are many
important contstaints that should qualify this statement that seem to get
lost. Third, it give the impression that you need to do nothing other than
ADDWF as long as your table doesn't cross a 256 word boundary. That is also
blantantly false, of course. Fourth, these addages are harmful in that they
prevent or delay the only correct course of action, which is to actually
UNDERSTAND what is going on. Yes, this means actually reading the manual it
you understand what it says.
So, Pic Dude, your routine above has a bug, and the symptoms would be
exactly what you describe. I'm not going to just tell you what is wrong,
because I want you to actually do a little homework for a change. Read up
on PCLATH, PCL, and related topics and then report back here to explain what
is wrong with this routine and how you fixed it.
*****************************************************************
Embed Inc, embedded system specialists in Littleton Massachusetts
(978) 742-9014, http://www.embedinc.com
> Page 1 to page 2? Why? Page sizes are 2048 bytes each (11 bits),
> and my ISR goes from byt 527 to byt 580, so I should not have to
> worry about PCLATH. Or so I interpreted from the datasheet.
>
> Can you explain, por favor?
>
> BTW, no, I don't have any references to PCLATH in the ISR code,
> for the reason above.
In this case you were right. Bob screwed up, which is only noteworthy
because it happens so rarely.
*****************************************************************
Embed Inc, embedded system specialists in Littleton Massachusetts
(978) 742-9014, http://www.embedinc.com
> org 0x0700 ; Start in really high memory
> ;-----------------------------------------------------
> ; Convert 4-digit data to 7-segment data
> ;-----------------------------------------------------
>
> Convert7Seg:
> movwf SAVE_ADDR ; Save offset
> movlw 0x07
> movwf PCLATH
> movf SAVE_ADDR,W ; Retrieve offset
Saving W around setting PCLATH is pointless because you are about to trash
the old W value with the table entry value.
I also don't like the MOVLW 7 to get the constant to load into PCLATH. If
you change the ORG value, you have to remember to change this value. Worse,
you didn't even document this dependecy. A far better answer is to have the
assembler load the right value for you automatically. I've added a label to
the start of the table for that purpose:
movlw high table1 ;set the high bits of the table entry adr
movwf pclath
> the
> same doc tells me that calls and retfie's will
> save/retrieve at least all 11 bits on a 2048-byte
> device, so for the 16F872, I won't actually need
> to save/restore PCLATH in the ISR.
Correct.
*****************************************************************
Embed Inc, embedded system specialists in Littleton Massachusetts
(978) 742-9014, http://www.embedinc.com
> >I won't actually need
> >to save/restore PCLATH in the ISR.
>
> Not a good idea, as you will surely alter the ISR at some stage and break
> your whole project, not just the ISR :)
>
> For the four instructions and one data ram location it takes, it is not
> worth the trouble of dealing with later problems that seem unrelated to a
> code change you make.
I agree with both statements. Pic Dude is right in that if you're running
on a 2K machine, you do not need to save/restore PCLATH. On the other hand,
Allan also has a valid point.
But there is an ever better way. See my QQQ_INTR.ASPIC template interrupt
module at http://www.embedinc.com/pic/. The STD_DEF.INS.ASPIC include file
checks which processor the code will run on, and sets the NCODEPAGES
assembler constant. The interrupt routine code checks the NCODEPAGES
assembler value and creates the code to save/restore PCLATH only if the
machine has more than one code page. Pretty cool, eh?
*****************************************************************
Embed Inc, embedded system specialists in Littleton Massachusetts
(978) 742-9014, http://www.embedinc.com
True .... only other reason I thought of not adding it was
to reduce display flicker as I'm running on a 131.072khz
chip, so I've been in a "trim the fat" sort of mentality
lately.
>True .... only other reason I thought of not adding it was
>to reduce display flicker as I'm running on a 131.072khz
>chip, so I've been in a "trim the fat" sort of mentality
>lately.
Well fair enough, but it may pay to put a comment in the code at the
beginning of the interrupt routine to point out the possible "gotcha".
You may also want to put the instructions in at an appropriate point in the
code, but comment them out if you are really on time and space constraints.
This "high" keyword is excellent. Much nicer, I agree.
However, I don't understand why you say "Saving W around setting
PCLATH is pointless because you are about to trash the old W value
with the table entry value."
The value in W is the digit value that needs to be converted
to a 7-segment pattern. For example...
...
movf DIGIT0,W
call Convert7Seg
...
In the code sample you added, that value is thrown out.
I agree. Use a macro or a #defined build switch.
I often add code for debug purposes but under an assembler build switch (compile switch) so if it suddenly stops I don't have to go trawling through adding debug again, I literally rebuild with my debug macro or #define switch on.
Also anything I have written in C and then later change into assembler for speed or whatever is also still left in the project, but again switched out. Far simpler to reject areas of code this way when a fault turns up. Sort of adding configuration management within my code to go back to older working versions. (obviously there's a limit)
Maybe put a WARNING preprocessor directive in to always remind you at build time that you've done this. As even a comment buried in the code can be missed. Doesn't cost anything.
Pete
> -----Original Message-----
> You may also want to put the instructions in at an
> appropriate point in the
> code, but comment them out if you are really on time and
> space constraints.
> Actually, now that I look into this carefully, the
> same doc tells me that calls and retfie's will
> save/retrieve at least all 11 bits on a 2048-byte
> device, so for the 16F872, I won't actually need
> to save/restore PCLATH in the ISR.
That is a good strategy, unless of course you use (change) PCLATH inside
your interrupt routine. So to be totally safe just save & restore.
Wouter van Ooijen
--
Van Ooijen Technische Informatica: http://www.voti.nl
Jal compiler, Wisp programmer, WLoader bootloader, PICs kopen
> A far better answer is to have the
>assembler load the right value for you automatically. I've added a label to
>the start of the table for that purpose:
2 things here: you *need* to save W before using it to set PCLATH - W
contains the jump offset upon entry to this routine.
2nd - what Olin posted still does not deal with a wrap at the 256 byte
boundaries. I've made the appropriate changes below:
movwf SAVE_ADDR ; Save offset
movlw high table1 ;set the high bits of the table entry adr
movwf pclath
movlw low (table1)
addwf SAVE_ADDR,W ; find appropriate offset
skpnc ;wrap past end of current page?
incf pclath,F ;yep - point to next page
movwf PCL ;now go to correct entry in table
table1 ;start address of the table
retlw B'01111110' ; '0'
retlw B'00110000' ; '1'
.
.
.
nop ; Error - '15'
retlw B'00000001' ; Return dash
Celebrating 18 years of Engineering Innovation (1984 - 2002)
.-. .-. .-. .-. .-. .-. .-. .-. .-. .-
`-' `-' `-' `-' `-' `-' `-' `-' `-'
Do NOT send unsolicited commercial email to this email address.
This message neither grants consent to receive unsolicited
commercial email nor is intended to solicit commercial email.
> This "high" keyword is excellent. Much nicer, I agree.
>
> However, I don't understand why you say "Saving W around setting
> PCLATH is pointless because you are about to trash the old W value
> with the table entry value."
>
> The value in W is the digit value that needs to be converted
> to a 7-segment pattern. For example...
> ...
> movf DIGIT0,W
> call Convert7Seg
> ...
>
> In the code sample you added, that value is thrown out.
Yeah, you're right. Sorry.
*****************************************************************
Embed Inc, embedded system specialists in Littleton Massachusetts
(978) 742-9014, http://www.embedinc.com
> > > Some memory map info:
> > > Main code (init stuff and main loop): 0x0005 thru 0x00E2.
> > > Subroutines: 0x00E3 thru 0x0020E
> > > ISR: 0x020F thru 0x0244
> >
> > You just pushed your interrupt handler from page 1 to page 2. Are you
> > properly handling PCLATH in the handler save/restore code?
>
> All his code is on page 0, which extends from 0 to 7FF. The 16F872 only
has
> this single page anyway.
Ok, I'll buy that...
So we call each 2K chunk of program memory a page, and each 128 byte piece
of data area addressability a bank (on some PICS anyway).
What do we call a 256 byte piece of program memory? (which is what I was
calling a 'page' above)
> > > Some memory map info:
> > > Main code (init stuff and main loop): 0x0005 thru 0x00E2.
> > > Subroutines: 0x00E3 thru 0x0020E
> > > ISR: 0x020F thru 0x0244
> >
> > You just pushed your interrupt handler from page 1 to page 2. Are you
> > properly handling PCLATH in the handler save/restore code?
>
> All his code is on page 0, which extends from 0 to 7FF. The 16F872 only
has
> this single page anyway.
Ok, I'll buy that...
So we call each 2K chunk of program memory a page, and each 128 byte piece
of data area addressability a bank (on some PICS anyway).
What do we call a 256 byte piece of program memory? (which is what I was
calling a 'page' above)
> So we call each 2K chunk of program memory a page, and each 128 byte piece
> of data area addressability a bank (on some PICS anyway).
>
> What do we call a 256 byte piece of program memory? (which is what I was
> calling a 'page' above)
Microchip uses the term "page" for a 2K chunk of program memory, and "bank"
for a 128 byte chunk of data memory, on the 16 family at least. I don't
know of any special name for a 256 word chunk of program memory, but it
shouldn't be called page or bank to avoid confusion.
*****************************************************************
Embed Inc, embedded system specialists in Littleton Massachusetts
(978) 742-9014, http://www.embedinc.com
>So we call each 2K chunk of program memory a page, and each 128 byte piece
>of data area addressability a bank (on some PICS anyway).
>
>What do we call a 256 byte piece of program memory? (which is what I was
>calling a 'page' above)
I've been calling each 256 chunk of ROM by the name of 'segment' in my code
- its different from all the other names that Microchip currently uses and
kinda sorta fits in with the x86 naming conventions. I'm certainly open to
a better name but until one comes along, segment is what I use.
Celebrating 18 years of Engineering Innovation (1984 - 2002)
.-. .-. .-. .-. .-. .-. .-. .-. .-. .-
`-' `-' `-' `-' `-' `-' `-' `-' `-'
Do NOT send unsolicited commercial email to this email address.
This message neither grants consent to receive unsolicited
commercial email nor is intended to solicit commercial email.
> > So we call each 2K chunk of program memory a page, and each 128 byte piece
> > of data area addressability a bank (on some PICS anyway).
> >
> > What do we call a 256 byte piece of program memory? (which is what I was
> > calling a 'page' above)
>
> Microchip uses the term "page" for a 2K chunk of program memory, and "bank"
> for a 128 byte chunk of data memory, on the 16 family at least. I don't
> know of any special name for a 256 word chunk of program memory, but it
> shouldn't be called page or bank to avoid confusion.
Microchip also uses the term of "page" for an 512 word piece of program
memory in the context of 16C5x (12-bit processors). For the sake of
"clarity"...
> Microchip also uses the term of "page" for an 512 word piece of program
> memory in the context of 16C5x (12-bit processors). For the sake of
> "clarity"...
>
This is consistent with the 14 bit core's 2048 word 'page'. In both cases it
is how far you can GOTO without having to play with PCLATH/page bits.
planet.eon.net>
