Assembly code Help! PIC16F57

[KL27x]

I love how you encapsulated that so I can see what was changed and what to put back if it doesn't work. brilliant.

When the changes get more involved, you need to get more serious and start using IDE directives (#include, if/else/endif) to blink code in/out of existence but leave it intact. It will get tedious adding and erasing scores of semicolons.

just testing now, so far so good. will report back.

Mind that without my fully unraveling the code, this is just a hack. If this were anything important, I would encapsulate the added code rather than mucking with a variable that is used in other places. IF it works, it is because counter_hi gets redefined/reloaded before all those other places it gets used. If it ends up causing other bugs, no problem. We can fix that.

[Electrofinn]

ok this seems much, much better than before, I did have one instance where on a win it seemed like it displayed the first 8 of the correct "pattern2" table before going back to my credit led's but I can't seem to get it to do it again. being playing for ages now. but then I guess if it did it once it could do it again.   

[KL27x]

Ok, I suspected this. Because it's a slot machine it might take years to confirm a bug in w/e is called "win2" lol.

We can either
1. do a bit mask in every other instance counter_hi is used. And in this case, it would be trivial, because it's only a short code. This is what we just did, btw. The last bit of code you added is called a bit mask.
2. we can allocate 1 bit of memory only accessed by feature/pattern and add a bit of confoculated code to inject this into feature. But the current registers end at 1F, and now I have to look up the memory map to make sure there's more memory. grr.

I'll report back in a bit.

Well, this decision was made for me. Every byte of memory is already in use.
The general memory goes from 0x07 through 0x1F. And the code uses them all. (oddly, 0x07 is defined as portC, lol. Which even though it doesn't exist, it is used by the code. I don't know if is being used as more than an output, so I shall not mess with it).

So there end up more than 4 places this counter_ hi is used. So because I don't want to map out the entire code loop, this may take a bit and I just have to hope we don't run out of code space. Because this is not going to be elegant.

[KL27x]

Hah, ran out of program memory.

And time. Gotta eat dinner.

I'll take another look, later. I'm sure there is some unused program memory if I look at the list file and shuffle things around.

But one weird thing is that counter_hi is used in one area
incfsz counter_hi

And it will never roll over to 0, unless it goes beyond 16. So now it makes it much more iffy where and when to bitmask this register.

This would be sooo much easier if there was a free byte of memory. I might look for some bit that is unused and do it that way.

[Electrofinn]

Lol yeah I would imagine confirming a bug on a full sized machine would be nothing short of an absolute nightmare. Looking over the conversation I have saved from the forum seven odd years ago, their recollection is they kept adding features until they run out of program memory and ended up with just 3 bytes to spare! Would of been interesting to know how different the code is on the protected chip.

When I was talking to him back then I had all these wild ideas of expanding on the project but he quickly brought me back down to planet earth with what would be involved. I was going change things like how much gets paid out for a given win, but I am not going to bother, I'm just after the original result now.

Incidentally if you wondered what the "odds" table was for, its contains the hex codes that when converted to decimal, is the number of servo actuations made when paying out coins.

As for the PORTC output thing you mentioned, not sure if this is relevant or not but the machine has things like a servo I mentioned earlier for the payout mechanism, a piezo speaker which generates a clicky sound to imitate the reels spinning and beeps when there's a win, the hold/collect and gamble/start buttons have leds in them that flash when an option to hold arises or a when a gamble game appears if all three reels match (this doesn't happen every time and I believe is randomly generated)

When the option to hold is available the flashing buttons stay solid when pressed to indicate the reel has been held. There is also break beam sensor (Phototransistor and infrared led) for detecting inserted coins.

[KL27x]

edited for clarity Ok, so your version doesn't do a bunch of that stuff and doesn't use the non-existent portC, right?
Oh, I was looking at the wrong part of the datasheet for the 16F54. And you use the 57. So disregard this.

they kept adding features until they run out of program memory and ended up with just 3 bytes to spare!

Ok, this is good. I went over it and found TWO seemingly unused registers, 0x0E and 0x0F.

The code uses fsr and ind here and there, so I can't be 100% positive without even MORE work, but if they said it and I found two, it looks like these are in the clear.
*Also in the 16F57, there are actually many more available registers. If it were the 16F54, then yeah, there would only be these couple left.

Fix: (this will not affect anything except pattern)
1. define a new variable as follows. At the top with all the other variables, add this:
;Variables

temp_reg1   equ   08
temp_reg2   equ   09
temp_reg3   equ   0A
temp_reg4   equ   0B
mode_byte   equ   0C
function_byte   equ   0D

new_byte   equ 0E   ;bit 0 used for extending "pattern" to 5 bit range. new_byte<7-1> are available. init2 loads 0x00

loop_divider   equ   10
credit_count   equ   11

2. Good practice to initialize variables. So in init2, add this line
   init2
      clrf   fsr
      movlw   30         ;FILE TABLE START POSITION
      movwf   data_counter
      movlw   01
      movwf   digit_pointer
      movwf   mode_byte
      clrf   switch_old
      clrf   switch_new
      clrf   counter_lo
      clrf   counter_hi
      clrf   credit_count
      clrf   leds_value
      clrf   credit_display
clrf new_byte

3.This is where we are using the new bit to save information. Change sequence1  to
sequence1   
      bsf      counter_lo,7
      incf   counter_lo,f
      btfss   status,zer
   goto      sequend
      incf   counter_hi,f
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
;insert this code
movlw .1
btfsc   counter_hi,4
xorwf new_byte,f      ;this toggles bit0 of new_byte when counter_hi overflows to 16
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;

   bcf      counter_hi,4   ;this must go back to the original bcf counter_hi,4 !!

4. This is where we recall this information and utilize it. Change feature to:
feature   
   clrf   fsr
   clrw
   movwf   portA         ;SWITCH OFF DIGIT DRIVE
   movf   credit_display,W
   btfss   mode_byte,5
   goto   feature1
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
;   movf   counter_hi,W
;
   clrw
   btfsc   new_byte,0      ;if this bit is set
   movlw   0x10        ;we are going to add .16 to whatever is in the counter_hi and place the result
   addwf   counter_hi,w   ;in w, leaving counter_hi alone. So this is invisible to the rest of the code.
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
   call   pattern

5. Ok, this is where the paging thing throws error. so you have to go to "dummy" and change the position of the org instruction

org   0x600   ;put the org 600 instruction in front of "dummy"
dummy   
   clrf   fsr
   movf   data_counter,W
   movwf   fsr
   movlw   4
   addwf   fsr,f
   movlw   slowdown_time      ;CONSTANT FOR SPEED CONTROL
   movf   ind,f
   btfss   status,zer      ;HAS 'SLOW' REACHED 0 ?
   decf   ind,W
   movwf   ind         ;RESET 'SLOW' CONSTANT
   retlw   00

; org 600  ; remove the org 600 instruction from here and put it in front of this subroutine.



The change you made to feature1 doesn't make any difference, anymore. You can leave it or change it back to how it was, before.


6. Cross your fingers. 

[westfw]

KL27x - that was really excellent analysis (and write-up)!  (man, talk about write-only code, though!)

which for a win is the "pattern2" table.

Unfortunately, that's exactly what you'd expect to happen.  The program is using the same code to display pattern and pattern2, so both patterns need to be the same length. Can you just add empty or repeated code to pattern2?  It seems to still compile for the simulator for me.

[KL27x]

^yeah, that would have fixed the pattern2  bug, if you just redundantly copied those 16 lines! Good catch! (But my previous change was adding just 1 line of code by comparison, which was better!) But there seems to also be something going on with win2, as well. So by not clearing counter_hi at 16 in the "sequence1", and clearing it at 32, instead, you might adversely affect some other part of the code that is also using counter_hi before reloading the value.

As I said, it is sometimes easier to just use more memory to add stuff, so you know there is no conflict. It is one of those things that go along with writing absolute code. Keeping track of memory is a pain and should be notated, carefully, especially on device with tiny amount of memory! In this case we used just one more bit of memory and left the rest of the byte usable.

Also, if you're playing along, write "errorlevel -305" and "errorlevel -306" at the top of the page, in case you aren't aware of that. Very handy to clean up your error code dialog box.

[westfw]

, ran out of program memory.

their recollection is they kept adding features until they run out of program memory and ended up with just 3 bytes to spare!

Really?  The assembly shows lots of space left.   Though of course there's that PIC limitation where it's a lot harder to use the 2nd half of each "program" page...
(Or am I missing something else?  It's been a long time since I've looked very hard at a low-end PIC...)

Code: [Select]

                              Program Memory Usage                                Start         End     
                           ---------   ---------     
                            0x000000    0x00008b     
                            0x000200    0x0002c7     
                            0x000400    0x00050b     
                            0x000600    0x000708     
                            0x000fff    0x000fff     
            874 out of 2053 program addresses used, program memory utilization is 42%

[KL27x]

Yeah, I was wrong when I "ran out of program memory." In fact, I got a "must be in lower half of page" error, which I admit I am not even sure I know what that means, anymore. In the modern midrange, when you run across the page boundary, that usually means you have to shuffle code and might be out of space - at least on the current page... and since all the code is written with short calls and gotos, this is like running out of easily used space, even if you have blank pages! I just jumped the gun on that because I had to be somewhere's else. There is none of this half page stuff in the modern PICs.

When OP said that they ran out of program memory, I assume he means variable memory, not programming space. (In PIC Harvard architecture, program memory is separate from variable memory!) And the "3 bytes left" seems to be a match. I found 0x0E, 0x0F are undefined. And it LOOKS like 0x10 might also be unused, but I'm not positive. It is defined, but the name is not used in the program, anywhere. But fsr and ind are used to initialize it to zero, and many a time I use code space without naming it (but I will notate the used sections of memory)

In the datasheet, general purpose (this is basically RAM or variable memory) is located between 0x07 and 0x1F. So this thing has nearly zip for memory. 25 bytes.

where it's a lot harder to use the 2nd half of each "program" page...

Yeah, I have no clue what this means. It has been a decade since I used this type of chip, and I don't think I ever learned this part. But I take it that there are some people who actually get this bit. If changing the position of "dummy" subroutine borks the OP's code, I am at a loss. It wouldn't assemble until I did that.

*edit: oh, I was looking at 16F54. The 16F57 has 72 bytes! So there's plenty more variable memory, too.

[Ian.M]

The PIC baseline program memory limitations are due to the short instruction word of the 12 bit core.  It takes 11 address bits for the full address range (2K words) possible for the baseline architecture.  The top two bits come from STATUS<6:5>, and the remaining 9 bits must be provided by the instruction.    GOTO is special - it has a three bit opcode, so has 9 address bits and can jump to any address, but CALL has a 4 bit opcode so only has an 8 bit address.  This maxes the upper 256 words of every 512 word page inaccessible to CALL. 

The easy fix is to put a jump table for subroutines in the bottom half of the page so their entry point is always at a callable address.

However IMHO the pain of wrestling with the baseline architecture just isn't worth it.  See my comments earlier about replacing it with a modern PIC.

[KL27x]

^So the part that doesn't make sense to me is that the code assembles and apparently works just fine the way that the op posted it.
There are many subroutines between 0x400 and 0x600. But after adding the seven lines of code in this patch, I get error on the "call dummy" saying it has to be in the lower half of the page.

Now after moving this "dummy" subroutine to 0x600, it assembles without error. Meanwhile there are other subroutines left between 400f and 600f which (are still being called but) are still not causing that error. "slow" is located at 0x4EC, and it is called from code. And it does cause this error.

Also, what is half a page? In the midrange pics, a page is every 800f. So a half page would be 400f. What does the 600f line have to do with anything, then? 

There was no overwriting of code. I did not run out of space between any org instruction.

 

Hang on, I'll change the org 600 instruction and see exactly where "dummy" was when it caused error.
* Ok. with the error, dummy is located at 0x503. So subroutines at 0x4EC work. At 0x600 works. At 0x503 it doesn't work. What the heck is half page in this context? Is it every 0x200 = a page? (With midrange, you have to long call/goto when passing a (0x800)"page" boundary, and this doesn't appear to be needed in this context)

That must be it, then. Pages are every 0x200, so half page is 0x100. When I added that code, I pushed dummy from 0x4ff to 0x500+? And the first part of the code 0x000 to 0x100 is the bottom half of the page, not the top half, like what you might expect looking at the "page on your screen" lol. If that isn't right, then I'll never figure it out. "Pages" are a totally different can of worms with the midrange. Actually (if this is right), the baseline is easier... once you finally get over the dyslexia. If there is no pagesel and longcalls, then as long as it assembles without error, it will worky?

Error[120]   Z:\EMBEDDED DESIGN\FIRMWARE\CLIP\FRUIT CLIP.ASM 181 : Call or jump not allowed at this address (must be in low half of page)

[Electrofinn]

Wow OK I have a lot to go through here, it's was late over here in UK when I posted my last post so had to go to bed. Will report back when gone through it all.

When they said 3 bytes to spare I assumed they meant what was already there but I can not confirm nor deny this.

[KL27x]

Yeah, I'm learning a lot, too.

In fact, after chewing on what Ian said, I'm pretty sure I borked the code.

I pushed dummy subroutine to another page, and now I'm pretty sure it is not going to work. I think on these baseline, there is no way to cross page boundary with long calls and pagesel.... hmm...

sorry, been a long time.

Now that I see what I did wrong, I think "the 3 lines left of program memory" that you were told was because of this, exactly! So hmm.. I have to take another look here. What I posted up there is not going to work!

[KL27x]

Code addendum:

So, here it is... don't do step 5 from my previous code post

Do step 1 through 4. And do step 6 .

And you will get a error because I added a few lines of code to page 400f-600f. And this pushed "dummy" across a half page boundary. So to fix it, I have reclaimed 4 lines of code with these changes.

1.
timeout1
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
;    incf   counter_lo,f
;   btfss   status,zer
   incfsz   counter_lo,f            ;saves one line
;;;;;;;;;;;;;;;;;;;;;;;;;;;;
   goto   timend
;;;;;;;;;;;;;;;;;;;;;;;;;;;;
;   decf   counter_hi,f
;   btfss   status,zer
   decfsz    counter_hi,f             ;saves one line
;;;;;;;;;;;;;;;;;;;;;;;;;;;;
   goto   timend
   bcf   mode_byte,7
   bcf   mode_byte,0
   bsf   mode_byte,5         ; incf/decf affect status Z. But...
timend   retlw   00                ; incfsz doesn't affect status,Z. But timeout1 is called from extimeout, and I see no checks of Z after the return. So I think it's ok.

2.
sequence1   
      bsf      counter_lo,7
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
;      incf   counter_lo,f
;      btfss   status,zer           
      incfsz counter_lo,f          ;save one line of code. against have to figure out is status,Z is importance after the return. So maybe ok.
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
      goto   sequend
      incf   counter_hi,f
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
;insert this code
      movlw   .1                     ;this is the 3 lines I added in the previous post
      btfsc   counter_hi,4
      xorwf   new_byte,f
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
   bcf      counter_hi,4   ;this must go back to the original bcf counter_hi,4
   movf   credit_count,f
   btfsc   status,zer
   goto   sequend
   bcf   mode_byte,7
   bcf   mode_byte,5
   bsf   mode_byte,0
sequend   retlw   00                 ;sequence1 is a branch from sequence, and this is called from exsequence,
                                           ; and I see no checks of Z after this return. So I think it's fine that incfsz doesn't affect Z like incf does.

3.
start3
   clrf   fsr
   bcf   mode_byte,0      
   bsf   mode_byte,1
   bcf   portC,2
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
;   clrw
;   movwf   counter_hi
;   movwf   counter_lo
   clrf counter_hi                  ;save 1 line
   clrf counter_lo
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
   bcf   function_byte,7   
starend   retlw   00                  ;and retlw 00 loads 0 in W register. So clrw should not be missed for this reason. RETLW doesn't affect status Z;
                                              ;the clrw instruction sets Z bit. But... clrf also sets Z. So absolutely no worries on this one.
                                               


These changes bring "dummy" subroutine back to 0x4FE in program memory to stay on the same page as it is supposed to be, which I believe is page 2 (starting from 0). So now we have room for one more line of additional code in the future!!!

Also, I chanced upon STATUS, <6,5> are how pages are selected, and pages are 512 "words" which is simply more confusing to me. So yes, pages have to be selected, and it will be nice to keep everything on the page it started with, esp if you are a hack like me that can't figure out this nonsense. (Took me long enough to figure out the midrange). 

[Ian.M]

You still need PAGESEL and/or LCALL, LGOTO to cross baseline page boundaries.  (or use manual BSF/BCF instead of PAGESEL and save instructions when one of the page address bits STATUS<6:5> doesn't need changing).   The original slot machine code uses minimal BSF/BCF STATUS for page management which doesn't help readability.  Cleaning it up to use PAGESEL and LCALL would vastly help future maintainability.

Each page is 512 (0x200) instruction words.  The bottom half is accessible by CALL, the top half is not. i.e. If the most significant digit of the hex address is odd, CALL cant reach it.

If adding code pushes a subroutine label over a half-page boundary, you need to rename the label (e.g. append 'x') and find somewhere in the bottom half of the same page to add:
<label> GOTO <label>x
to provide an entry point CALL can reach.

[KL27x]

Each page is 512 (0x200) instruction words.

Ah, ok. Thanks. I totally spaced that.

I guess it's essentially the same as the midrange if lcall and pagesel work. Except you also have the half page thing AND the super small pages going on.

Good thing you posted that, else OP would have had issues with my half-baked solution, carelessly moving a subroutine.

OP, I'm now happy with my latest addendum. But you know what they say about last words.

So anyhow, if I had moved dummy to the next page, page 3, (or if you had done this already) it should require this change to work, properly:

exdummy   ;which is located on page 0
   bsf   status,6       ;CALL SUBROUTINE IN PAGE 400-5FF ; this is page 2, originally.
   call   dummy ;but this would be located on page 3, if you moved it to far side of org 600
   bcf   status,6

change this to:

exdummy   
   bsf   status,6
bsf status,5
   call   dummy
   bcf   status,6
bcf status,5


or
exdummy   
       pagesel dummy
   call   dummy
   pagesel $

or

exdummy
lcall dummy   ;beware this expands to two instructions when assembled (in midrange, this is how it always works for me but maybe I'm wrong?)
                     ;in case you are using relative jumps or btfss type instruction in front of it.
pagesel $       ;pagesel for some reason on midrange seems to only take one instruction??? Maybe I'm wrong, though.


So well, OP you have options to try.

[Electrofinn]

Oh crap I think I getting lost here, really sorry KL27x. trouble is the time of day here and when I get chance to get on this. so am I working from your latest addendum? scratch that, having read it again, I think I know what I need to do!

[Electrofinn]

As I came back to this after all the posts I did not do the step 5 you said to omit, so did not do the correction you mentioned in your last post about exdummy. step 6 really confused me though! testing now, so far so good!

[Electrofinn]

Ok had two instances in 30mins of playing that it run approx half the "pattern2" sequence on a win. Don't know whether this is relevant but I decided to plug the speaker in to test whether that was also effected and it is, it does half the amount of beeping along with half the "pattern2". Just wondered if code for peizo speaker could be stopping this from working properly maybe? I think its only doing this when I have a two reel win (when first two reels match and any on the third gives a 10 pence win) which is 2 five pence pieces since this machine only takes 5p's, this by the way is 4 servo actuations (1 actuation collects coin from coin stack and another actuation pays it into collection tray) so 4 all together for 2 coins. it seems to do this properly even if half the "pattern2" and half the beeps are run. but everything else is fine though.


There Is a real chance that somehow I messed up in the instructions but I don't think I did, I was going to suggest uploading my code, but I don't want to give you the extra task of having to go through mine as well. Or if I try your edited code? would that be easier? you have already worked so hard on this for me, I don't want to give you anything extra to do, unnecessarily. really appreciate this, you're not just creating code you're creating dreams. 


**edit played for an hour before it did it again. at least the changes haven't broke anything else.

[KL27x]

ASM file

If this has the same problem, I would suggest you try changing new_byte register to location 0x0F.

If that has the same problem, I would suggest you try the clip you originally uploaded to see if it didn't have this problem at the time I uploaded it?

Repeat Step 6.

[KL27x]

Hex file

[Electrofinn]

ok cool, just added extra text to above post btw.

[Electrofinn]

Lol ok one thing I noticed right of the bat is I put in my custom 32 light sequence into "pattern" and it doesn't seem to run it properly, it looks to me as its playing the first 24 and then repeating again. or repeating the second set of 8 twice if that makes sense
   

[Electrofinn]

this is mine that does work.