Assembly code Help! PIC16F57

[KL27x]

It's coming I think, be it at a rediculously slow snails pace. Lol

After first playing with PIC AXE, it took me roughly 6 months to make a PIC blink using assembly. After stumbling through some crappy internet tutorials, I resorted to carrying around a PIC datasheet and slowly learning how to put read the tech speak.

I highly recommend you buy the "Gooligum PIC assembly" tutorials, if you find this stuff fun. They're like $5.00 each, if I recall. I stumbled across them several years too late, but I still learned more than a few tricks. And they are easy reading/doing and will probably be fun if you get them before you burn yourself out.

Once you safeguard the motor outputs, you could use your slot machine as a learning device and follow along the baseline tutorial, outputting to the LEDs and using the buttons as inputs. 

[Electrofinn]

Think it was sequence1, I have added it anyway just to try it. created separate asm save so I can mess about.

*In fact, another thing comes to mind. Where that bcf counter_hi,4 instruction was (I forget the title;header, but it was error/instance #11 of counter_hi, IIRC ). Because of the way that this register is used in auto1/2/3, and because of what it does in feature/pattern, this instruction should really be replaced by a bit mask.** If somewhere else, say in autoX, counter reaches 64 or higher, when it gets used to this spot that incf counter_hi and then clears bit 4... well, when pattern is called, that will cause a bad jump.

**
;;;;;;;;;;;;;;;;;;;;;;;;;;
;bcf counter_hi,4
;
movlw b'00001111'
andwf counter_hi,f
;;;;;;;;;;;;;;;;;;;;;;;;;

have noticed something else, when I run out of credits and after it waits the two seconds before displaying the "pattern" sequence, rather beginning at the start of the table it often starts somewhere in the middle of the table loop. just trying to figure out what this is telling me, it doesn't matter anyway it's just an interesting observation and I am thinking it has something to do with what ever is in the register at the time causing it to jump straight to that part of the table, sometimes it begins at the start of the pattern table.

Don't feel you have to respond or dig any deeper, you have already spent far too much of your time on this.

[KL27x]

Look at the subroutine sequence. You will see that the first time it is called, it is checking bit 7 of mode_byte. Presumably, this is to determine if this is the first time sequence is being called by the Loop, or not.

So if mode_byte 7 is clear, this subroutine clears counter_hi, which determines where pattern starts its "cycle." And it clears counter_lo, which determines when sequence progresses to the next point (the first time, at least). And it sets bit 7 of mode_byte, so that the next time it gets called, it will not repeat these steps. Then, when sequence has been completed, bit 7 of modebyte is cleared. So it is ready to start over. Apparently, it continues until credit_count is zero. This must occur in one of the other active subroutines.

Now, the problem here is that sequence, along with every single other subroutine which takes any significant amount of time, is not performed in one shot from beginning to end when it is called. Only one tiny slice of sequence gets performed on each cycle of Loop.* It will perform the same thing 256 times (as determined by counter_lo), before it progresses to the line in pattern. So counter_lo determines the speed that the LEDs change.

*You know this, automatically, because there is no interrupt routines (and the chip probably doesn't even have this feature). In order to keep the 7 segment displays refreshed, no subroutine may hold the program counter hostage for any significant amount of time, unless the 7 segment displays are either all off or displaying the same thing.

So while sequence is being executed, other things are also being executed. Some other part of the code might be setting bit 5 of modebyte to kill/hold sequence. Presumably if modebyte 5 is set at any other place in the code, bit 7 would also be cleared to "reset" sequence so it starts at the beginning next time. But maybe this has been missed, or maybe the authors prefer it to start at a random location. Who knows, maybe they use some of these registers as part of the random number generator. Or... to beat an old drum, here, there may be a bad memory share/handling regarding counter_hi.


;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
sequence   
      ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;   
      ;if mode_byte,5 is cleared, these subroutines are called by Loop
         ;sequence
         ;random
         ;flash
      ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
      clrf   fsr
      btfsc   mode_byte,7      ;TEST 'IN PROGRESS' BIT
   goto      sequence1
      clrf   counter_hi
      clrf   counter_lo
      bsf      mode_byte,7      ;SET 'IN PROGRESS' BIT

sequence1   
      bsf      counter_lo,7
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
;      incf   counter_lo,f
;      btfss   status,zer
      incfsz    counter_lo,f
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
   retlw .0
      incf   counter_hi,f
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
;insert this code
      movlw   .1
      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
   retlw   .0
      bcf      mode_byte,7
      bcf      mode_byte,5
      bsf      mode_byte,0
   retlw   .0
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;

[KL27x]

This structure of the subroutine is not too great. BTW, here, I will show you how to free up some extra space so you have more room on page 3.

I shuffled one of the subroutines and I added some new code to feature, so you can see how to change the rate of the LED pattern. See "feature".

So check out "slow" to see how to liberate some space if you run into the "half page error."


pages are program memory space
000-1ff page0
200-3ff page1
400-5ff page2
600-7ff page3

000-0ff is the bottom half of page0
100-1ff is the top half of page0

To see where any of your subroutines/labels are, you can open the list file.

Click "file" at the top left of the ide.
Select Open
And change "file type" box to find list file
And open the list file.

At the very bottom of this file, there are all your labels in alphabetical order. And in the leftmost column you will find the location of this label to see what page it is on.

[Electrofinn]

it's really annoying that he didn't have the final code but then I don't think It would of looked much better if this release is anything to go by.
 
can you confirm that I put the bit mask in correctly?

sequence1   
      bsf      counter_lo,7
;   v2.02
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
;      incf   counter_lo,f
;      btfss   status,zer
      incfsz counter_lo,f
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
      goto   sequend
      incf   counter_hi,f
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
;insert this code
      movlw   .1
      btfsc   counter_hi,4
      xorwf   new_byte,f
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
;   bcf      counter_hi,4   ;this must go back to the original bcf counter_hi,4
;
   movlw b'00001111'
   andwf counter_hi,f
;;;;;;;;;;;;;;;;;;;;;;;;;
   movf   credit_count,f
   btfsc   status,zer
   goto   sequend
   bcf   mode_byte,7
   bcf   mode_byte,5
   bsf   mode_byte,0
sequend   retlw   00
 

[Electrofinn]

oh ok just read your last post.

[KL27x]

^Yeah, that is fine. But note that when you did that, you used the last remaining line of program memory in page2. This is going to make further debugging hell. So see my last post and check out that asm file. 

If you want to learn how to write assembly, check out Gooligum PIC assembly. You will be writing assembly in a few days with the headstart you already have.

If you just want this thing to work, make another complete working slot machine and gift it to me.

[Electrofinn]

lol on a serious note I might just do that. I have loads of spare parts to make 5-10 of these things lol

you know what it's like when you order a part and end up having to order way more than one, its been like that for practically everything I have ordered lol

my next hurdle is I have to learn some adobe illustrator to produce a vector drawing so a can get the plastic parts laser cut for the pay mechanism. sod doing myself again, me and my dad made it happen back in the day but we ended up with very crude and unevenly cut mechanism which worked for a long time but was very difficult in the beginning to get done with limited tools.

I just spent the last four months teaching myself easyeda, I finally achieved it and had boards produced and have turned out excellent. I had to make some design and pinout changes but it turned out well.


also the sticker that went on the front of the box which back then you had to order, I have completely redone in photoshop with changes to the graphic around the buttons because the buttons are different shape to the original. so yeah still a bit to go but its coming together.


             

[KL27x]

Well, it sounds like you're well on your way to being featured on Make!

[Electrofinn]

lol I wish. I wish I had your knowledge though. I'd probably be doing things like changing the segment displays for oleds with real fruit graphics and the like. but as it is it's still a great toy and I am enjoying the nostalgia trip at the moment.

[KL27x]

Just to go way back, here:

Agree with others.   To edit code, you need to understand it.  There are a few obvious things of concern.   

carry   equ   0
zer   equ   2
ind   equ   00

Default radix is hexadecimal with Microchip.  (Choices are binary, octal (I think), decimal, hexadecimal, and ASCII.  You can declare the radix during your initialization.  RADIX DEC will make the default decimal.   It is an Assembler directive and is not part of the chip's instructions per se.) Thus, you have given "carry" and "ind" the same value/identifier.

In this case, the writers of the code are designating bit location of "carry" and "zer." While "ind" is designating a register address.
This is redundant if you include the .inc file for the part. Because these are already defined in the device .inc.

The .inc file by default defines C bit as 0 and Z bit as 2.
Address of INDF is by default 0x00 (same as 0 or .0 or b'00000000'; we just write it this way because it looks more like an address)

So there is no conflict, other than you can misuse any designated register or bit designation.

For instance, you can write 
  Movlw PORTA
and instead of moving the contents at this address to W, it will load the value of that address.

And you can write
  bsf PORTA,Z
this will set bit 2 of PORTA, even though the only reason Z bit was defined was to make using STATUS,Z easier to remember than STATUS,2 (and possibly more consistent between different devices).

or
 movlw Z
would put 0x02 in W.

This is the reason the IDE can't throw an error when you define multiple things with the same number. That can be an address, it can be a value/constant, or it can be a bit designator 0-7, and it all depends on how the coder uses it.  There are dozens of bit designators in the .inc header that will be the same value.

The assembler does not distinguish at all between address and literals/constants. We write them differently just to reinforce the difference, because it's up to the coder to know the difference.

This is why I always end constant names with ".c" for instance. Even though I like to define them thusly:

  constant name.c = 0x01

This is absolutely no differnet in result than writing
   name.c equ 1

Now, if you wanted to go down this road, you could make "zer" have more utility in a way.
#define zer STATUS,Z

and now you can write
  btfss zer
instead of
  btfss STATUS,Z

In this case, since STATUS is accessible from any bank, you don't need to worry about banking, so you don't have to remember what register zer is even associated with to do this.

[Electrofinn]

your last asm was probably just meant as an exercise to go with what your teaching me but just thought I'd let you know just in case, it sometimes, but not all the time, fails to run "pattern" on a win. lights just stay off.

[KL27x]

This is an example of how memory bugs happen. I didn't check the other subroutines that might be using counter_lo. So now we learn more, one tooth at a time.

So, let me guess, it also didn't change the speed of the sequence?
      bsf      counter_lo,7
I missed this line.

      bsf      counter_lo,7
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
;      incf   counter_lo,f
;      btfss   status,zer
      incfsz    counter_lo,f
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
   retlw .0

      movlw .128          ;this is b;10000000; if bit 7 gets set, anyway, then this is the same value as zero!* But this register is evidently used elsewhere
      movwf   counter_lo


*As far as this subroutine in isolation is concerned. But elsewhere in another subroutine, there is apparently something else. I'm guess there's a Z check of counter_lo in another subroutine**, and 0x80 is not the same thing as zero, as far as other part of the code might be concerned.

**similarly to how the coin_counter gets a Z check at the end of this "sequence" subroutine.



When writing code, I will end up breaking it many many times. When altering my own code, I'll break it many more times. When altering someone else's code, I'll break it pretty much nonstop. This is why trying to fix it over the internet is pretty much impossible for myself. If it were here, I'd break it just as often, but the iterations would happen much much faster.

[Electrofinn]

So, let me guess, it also didn't change the speed of the sequence?

You guessed right. Will have to look at your posts again tomorrow, this is blowing my mind a little bit lol finding this really hard. I think with this it's going to be like most things I find hard, the more time I expose myself to it the better.

This is why trying to fix it over the internet is pretty much impossible for myself. If it were here, I'd break it just as often, but the iterations would happen much much faster.

Yes absolutely understand that.

[KL27x]

Hard is an understatement. The way these guys used counter_hi and counter_lo is very, very, intertwined and obfuscating. This is made to fail. These counters are manipulated in all kinds of different ways in different subroutines.

If they (or he.. I bet anything there was one guy coding and one guy sharing credit ) were still coding today and wrote a similar program (on a baseline device, for some reason), they probably would have found a much better way to do this.

I imagine I would have a 2 byte counter, incremented only in the main Loop. And instead of Z checks in the individual subroutines, there ought to be compares, doing a subtraction of a literal and a C check. And they would have to submit a request for reset, which if the counter is in use by another routine, this should produce some obvious error code say on the output of the displays.

So, here's the rub. There's essentially zero chance you're gonna fix with google and intuition. Maybe in a matter of weeks, if you go through the Gooligum thing and don't lose interest.



 
 

[Electrofinn]

If they (or he.. I bet anything there was one guy coding and one guy sharing credit ) were still coding today and wrote a similar program (on a baseline device, for some reason), they probably would have found a much better way to do this.

I think you're absolutely right about that, obviously he or "they" managed to achieve it in the end, the PIC16C57 I have is proof of that, but even that, I bet if we were able to see to the contents of that pic there would still be the same theme of unorthodox coding practices going on.

It's shame really, I didn't think for a minute that it would be this hard to sort out.

[KL27x]

It's shame really, I didn't think for a minute that it would be this hard to sort out.

You're original question actually wasn't. Adding stuff is much easier than fixing stuff. Debugging code is why I have learned how to patch drywall.

So if shoring up those lookup tables doesn't patch things, there isn't much low-hanging fruit left to my knowledge.

"they" managed to achieve it in the end, the PIC16C57 I have is proof of that

Much like finishing a PCB and finishing the BOM, sourcing every last part, finishing a quasi professional schematic, and tweaking and verifying all the Gerber settings, and answering stupid questions from the PCB manufacturer/assembler... It's that last few details that suck the worst. If I could do things 95% and stop there, I would have the greatest job in the world.

I've actually lost a final production pcb file before; not yet done that with code, though. Luckily. Code can't be reverse engineered from the chip, unlike a pcb.

[Electrofinn]

lol, yeah bit of a bummer.  I do think its kind of curious how the writer couldn't find the final release but could find this one, especially given he seemed quite proud of the success of getting it published after working a year on it and in the video he has the magazine cover on the wall. Me personally would be backing it up everywhere, but then that's me, and maybe my background in computer maintenance tells me never to trust just one back up location. Either way it's frustrating because the original chip is flawless in it's operation, that is kind of why I wanted to at least recreate it because if this chip suddenly goes kaput! it's lost forever.


       

[Electrofinn]

Have a very positive update, as of the asm when we added a bitmask and an extra value to the "odds" table I am happy to report the following...

1. The machine seems to consistently run the correct "pattern2" table on a win, which is the problem I didn't realise existed in the source code.

2. The machine seems to consistently run the full and not just half the "pattern2" table on a win, full sound pattern too!

3. The machine did have 2 instances where it didn't run the "pattern" table after 2 seconds of running out of credits (returning to sequence mode), however I realised that if I just waited approx 30-45 seconds the pattern does actually begin to play, which I am actually fine with as it does actually do it and that's the main thing. but perhaps code wise you may know what might be causing that.

4. The machine doesn't always have the option to hold all reels when you insert the first credit upon first switch on, which is desirable as it adds a chance element to the game that the player may or may not be able to bag a jackpot win as the default position of the reels on first switch on is all bars.

I can take no credit for this as all credit goes to KL27x for his excellent work on taking such a borked code and turning it into a working one, as well as your excellent write up, It's been an education. Thank you so much.
 

       

[KL27x]

Glad to see it's improving!

3. The machine did have 2 instances where it didn't run the "pattern" table after 2 seconds of running out of credits (returning to sequence mode), however I realised that if I just waited approx 30-45 seconds the pattern does actually begin to play, which I am actually fine with as it does actually do it and that's the main thing. but perhaps code wise you may know what might be causing that.

Well, hmm. if it was NOT doing this before, then maybe we messed something up with the bitmasking. So in hindsight, I figure we can go back to the original version of the code and do a cleaner or more targeted fix.* We're gonna do the mask right in the lookup tables. This way, if his code illogically needs counter_hi (or w/e other registers are being loaded before these lookup tables are called) to be beyond the parameters of the table, this fix will allow for that coder oversight/duality.

*I intended the results of those bitmasks to only affect contents of w, and only where it was loaded just before calling these tables. But I found a spot where I did a mask to the register, itself, at least in my own code I was messing with. In sequence1. I don't know if I gave you that version. But the following fix on the original code will ensure these kinds of mistakes that I might have given you (or that you may have made) will be avoided. (OTOH, maybe a "mistake" was what fixed one or more of the other problems!)

If this works, you can use it like this and simply add just those steps we did in the post were we added "new_byte" in order to extend pattern (which was a "clean" modification), minus any place I put a bitmask in there (if I did so).

Code: [Select]

table
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
andlw 0x1F ;b'00011111' ;
;32 lines 0-31 ;
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
addwf pc,f
retlw 40 ;0
retlw 08
retlw 00
retlw 00 ;3

retlw 23 ;4
retlw 77
retlw 5C
retlw 08 ;7

retlw 01
retlw 60
retlw 38
retlw 10 ;11

retlw 00
retlw 01
retlw 62
retlw 3F ;15

retlw 54
retlw 08
retlw 00
retlw 21 ;19

retlw 73
retlw 5C
retlw 08
retlw 01 ;23

retlw 60
retlw 39
retlw 50
retlw 08 ;27

retlw 00
retlw 01 ;29
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
;add 2 redundat retlw's to complete this 5 bit table
retlw 01
retlw 01 ;31
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
odds
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
andlw 0x07 ;b'00000111' ;
;8 lines 0-7 ;
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
addwf pc,f
retlw 28 ;0
retlw 14 ;1
retlw 10 ;2
retlw 0C ;3
retlw 00 ;4
retlw 08 ;5
retlw 08  ;6
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
;add a redunant relw to complete this 3 bit table
retlw 08  ;7
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;

pattern
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
andlw 0x1F ;b'00011111' ;
;32 lines 0-31 ;
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
addwf pc,f
retlw 07f ;0
retlw 0bf
retlw 0df 
retlw 06f ;3

retlw 0b7
retlw 0db
retlw 06d
    retlw 0b6 ;7

retlw 0db
    retlw 0ed
retlw 0f6
retlw 0fb ;11

retlw 0fd
retlw 0fe
retlw 0ff
retlw 0ff ;15

retlw 0ff
retlw 0ff
retlw 0ff
retlw 0ff ;

retlw 0ff
retlw 0ff
retlw 0ff
retlw 0ff ;

retlw 0ff
retlw 0ff
retlw 0ff
retlw 0ff ;

retlw 0ff
retlw 0ff
retlw 0ff
retlw 0ff ;31

pattern2
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
andlw 0x0F  ;b'00001111' ;
    ;16 lines 0-15 ;
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
addwf pc,f

retlw 00 ;0
retlw 81
retlw 0C3
retlw 0E7

retlw 0FF
retlw 0E7
retlw 0C3
retlw 81 ;7

retlw 00
retlw 81
retlw 0C3
retlw 0E7

retlw 0FF
retlw 0E7
retlw 0C3
retlw 81 ;15

[Electrofinn]

You know it's funny you should mention the above because as I was trying think logically about how this might be working, I was thinking couldn't something be incorporated into those tables to do that job, but I just didn't know how or what or even if it would work. You see! There's hope for me yet! Lol

You made some major changes to the program loop and shuffled some stuff since then and made it look way neater, the attached asm is where I am at with code, can I not just remove the bitmask from "sequence1" and do the above modifications? only other things I have done is add my custom "pattern" and add tabs or removed tabs so it compiled without warnings. 

[Electrofinn]

just thought i'd try above changes while I gave you time to reply and without bitmask in sequence1 the lights do strange things but with the bitmask its fine. not done enough testing to see if I broke something else but then I didn't want to just in case your face palming about what I was asking lol


i think i spoke to soon about the pattern2 not doing the full pattern because its just done it on what i thought was the good code...damn it. i was playing it all night aswell..strange. think i might try what you originally told me to do from original code.


getting really confused about what was what with all the edits.

[KL27x]

New revision v21 attached

Well, I just looked at the code you attached, and it looks like all you did was to add that extra line to the odds table? You didn't add any bitmasks, at all, right?

So I did the changes in my last post to it and restored sequence1 to how it was, other than the part with new_byte.

i think i spoke to soon about the pattern2 not doing the full pattern because its just done it on what i thought was the good code...damn it. i was playing it all night aswell..strange. think i might try what you originally told me to do from original code.

This is where figuring out how to mess with the code and having the device is helpful. If you learned how to find where these issues correlate with the code and how to manipulate the code, you could write a test code between the initialization and the main Loop to set the mode_byte to where it needs to be to jump to this identified problem area. And you could put in test values into the pertinent registers to reproduce the error, without having to play a slot machine for hours.

Any rate, since you did not add the bitmasking and simply lengthened odds table, this new version v21 may actually behave differently, maybe even more correctly if we're lucky.

[Electrofinn]

Ok will test it. big headache this isn't it, I feel we getting closer though. because everything else seems ok. we will see.

[Electrofinn]

You didn't add any bitmasks, at all, right?

just that one in sequence1, which looking at v21 you have done away with.