Hello Olin,

> First, DOCUMENT YOUR CODE!!!  If you had forced yourself to explain what
> each instruction was doing, you might have found the bug yourself.

    :) I agree with you. But this is documented on the starting of the file.

> Second, why are you hard setting PCLATH to 1?  This is very bad
programming
> practise and a bug waiting to happen (maybe it just did).  Your code will
> only work if all the GOTO instructions are located from 100h - 1FFh.  This
> should be derived symbolically, or at least the same symbolic constant
used
> to force the address of the table as derive the value to stick into
PCLATH.

    This is the reason why this code is documented on the starting of the
file with a big WARNING! I'm using PCH (by CCS) and haven't found a way to
derive the PCLATH value simbolically.

    CCS have a label_address() function but I can't find a way to make
something like "PCLATH = hi(label_address(dkstart));" to work.

> Note that the PIC16 addresses program memory as whole words, with each
> instruction taking up one word.  The PIC18 addresses program memory in
> bytes, with instructions taking up either 2 or 4 bytes.  The PIC18 GOTO
> instruction takes up 4 bytes, so the DKSTATE needs to be multiplied be 4,
> not 2, before used to index into the table.  Also, this type of GOTO table
> can only support up to 64 entries because that are all that fit into a 256
> byte region.  Larger tables require computation of the PCLATH value, since
a
> single PCLATH value addresses a 256 byte region.

    Yes, this is the problem. I've changed the code to:
        MOVLW  0x01
        MOVWF  PCLATH
        MOVF   dkstate,W_
        MULLW  4
        MOVF   PRODL,W_
        ADDWF  PCL
        GOTO   case0
        GOTO   case1
        GOTO   case2

    and now it seens to be working fine. The 64 "cases" limitation isn't a
issue in this application. I'm taking the precautions to not having PCL
"pages" crossing.

> Note also that the PIC18 has a PCLATU register in addition to PCLATH.  You
> can get away with ignoring PCLATU on the 18F452 because it only has
32Kbytes
> of program memory, but this is another bug waiting to happen.  I would use
> assembly conditionals to insert the proper code if this is ever ported to
a
> machine with more than 64Kbytes.  Or, at least generate an assembly error
in
> that case so that someone is forced to inspect the code.  It's easy to do
> now, a real pain 4 years from now when you've forgotten all about this
> little section of code.  While you're at it, you can generate an assembly
> error automatically is someone tries to grow the table beyond 64 entries.

    This is a good idea. I don't think this code will ever be implemented on
more powerfull machines, but better be safe than sorry. :)

    Thank you very much,

    Brusque

-----------------------------------------------------------------
Edson Brusque                 C.I.Tronics Lighting Designers Ltda
Research and Development               Blumenau  -  SC  -  Brazil
Say NO to HTML mail                          www.citronics.com.br
-----------------------------------------------------------------

--
http://www.piclist.com#nomail Going offline? Don't AutoReply us!
email listserv@mitvma.mit.edu with SET PICList DIGEST in the body