Assembly code Help! PIC16F57

[Electrofinn]

Before I start with what I need help with I feel a little back story will help. I am working on a fruit machine (Slot machine) project that was originally published in the EPE electronics magazine in two parts on December 1994 and January 1995 (a UK electronics magazine) The original project used a PIC16C57 and you had to order this pre-programmed at the time, more on this later. The project also utilised three, seven segment displays which are the reels of the fruit machine.
 
In 2012 I showed an interest in resurrecting and rebuilding the project as the first attempt was a messy one! So I had reached out to the magazines forum and to my amazement the original creator of this project had surfaced and was extremely helpful, he and his collaborative partner of the project kindly decided to open source the code and helped me find the now newer equivalent to the PIC16C57 which is the PIC16F57. With his help I was successful in programming the updated chip with no modification to the code (although it did list some warnings but worked anyway) However there were some differences from original program, for instance the LED demo sequence which Is displayed a couple of seconds after switch on, and is displayed until player inserts a credit, was different from the original. I informed the creator of this and concluded the code was a different version than that supplied on the PIC all those years ago. Unfortunately the creator couldn't find this newer version and the original PIC was code protected and couldn't be copied, but he was very helpful in telling me the parts of the code I could change to manipulate the sequence amongst other things. There was also another difference but more on this in a moment.

At the time I had some life issues going on and had to push the project to one side, I had stopped conversing with the creator and lost contact. Its only recently I have decided to pickit! Back up again and get this thing finished. There are things I am still trying to achieve with it but attempts to contact him for further help have failed. Though to be fair, even at the time of talking to him he had sort of moved on from it, I was extremely fortunate and grateful that he helped me as much as he did. He currently operates a fairly successful YouTube channel and in fact has posted a video about the fruit machine project with links to the original magazine article and source code, however he hasn't responded to my questions, and to be fair it was posted 4 years ago and other attempts to contact him are probably lost in a sea of other messages he now probably receives and has no time or even maybe no interest in responding, but I am understanding of that.

Link to the YouTube vid, the pdf article and source code files are in the vid's description... https://www.youtube.com/watch?v=GCGmK0gCCUI

There are two things I need help with, the first thing is still regarding the LED sequence. Now my assembly knowledge is extremely limited and need help with getting this to work. The LED sequence works by calling a lookup table named “pattern” using addwf and retlw, the original chip seems to display 32 different LED patterns/combinations and then loops around again, the source code version displays 16, however when I try to add more than 16 hex values to the table, they don't get used, it just loops 16 of them. Obviously as I can’t read the original chip and so I can’t see how he did it. There is a second table named “pattern2” this is the sequence displayed when a player has a win, this is the same as the original and I’m happy with that.

The second problem is that on the open sourced version, when you first switch the machine on and put the first credit in, it often gives the player the option to hold all the reels straight of the bat and because the reels are always on bars on the first switch on it gives the player an instant jackpot win! The original chip does not do this and the creator of the project had obviously fixed this in the final version, I don't possess the knowledge to fix this. If someone could please advise me or look at the source code on the YouTube vid, I would so appreciate it. Thank you in advance everybody.

[jpanhalt]

You will have to post your code for anyone to help.

[Electrofinn]

OK have attached code, Only changes I made are the list pic value from p=16C57 to p=16f57. added config bits, and I added a bunch of 0FF hex values to the "pattern" table for testing which should keep LEDS on solid but as previously stated only the first 16 I added actually work which is like a chasing/twinkle pattern.     

[Zero999]

Here's a tutorial on how to program in assembly for baseline PIC processors. I followed it myself a few years ago and managed to write a simple program within a day. The PIC16F57 has the same core as the ones described on the tutorial linked below, so it's a good fit.
https://www.gooligum.com.au/PIC-tutorials/baseline-PIC-tutorial

[Electrofinn]

Thank you so much for that, will try and give it a go but don't think I have a head for this, might end up  lol. And I can't think of an instant after this when I would use it again and therefore will probably forget it before long. will give me something to watch at bed time tonight though. you never know it might stick. thanks   

[Electrofinn]

Have looked at the pdf linked to the lessons and I just can't see how I can apply this to an already written code and in the way it's been written. Strange thing is that the lookup table for the three seven segment displays is 30 hex values long but the tables for the 8 yellow Led's above my segment displays are only 16 hex value long, which incidentally are not only the demo display led's but are also the credit led's, inserting a coin effectively stops the light show and they light up one by one as each credit/coin is inserted to indicate credit total.


I just need someone to either edit the code or tell me I need to do this, this and this basically.

[Ian.M]

To edit the code successfully you need to understand it.  You'll probably have to reverse engineer it enough to draw up a flowchart to get anywhere with it, and will certainly need to be fluent in the 12 bit core subset of MPASM assembler.   It would probably be quicker and easier to write a formal specification for what the chip must do, then code your own slot machine firmware in a high level language (e.g. C).  However as compiled programs tend to be larger than hand-optimised assembler, it may not fit in the chip and as the PIC16F57 has a rather unique pinout, there's nothing newer with more memory that's pin-compatible, so you'd either need a daughter-board with a SMD modern PIC on it to convert the pinout, or to redesign the whole PCB.

If you want someone else to work on the code, expect to have to pay commercial rates.   It wont be cheap. . .

[jpanhalt]

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

Code: [Select]

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.  A series of equates is often used to identify user registers.   For that chip, user registers (General Purpose RAM) begin at 0x0A, so your equates would probably just substitute a 0x0, 0x2, at each occurrence of those identifiers.  Look at your code and see if that makes sense. 

Be aware that memory is banked.   Each bank of GP RAM is only 22 or 16 bytes.   Bank0 is 0x0A..0x1F, Bank1 is 0x30..0x3F, and so forth.  Here are your register definitions:

Code: [Select]

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

Therefore temp_reg1 and temp_reg2 do not point to user GP RAM.

In sum, I add my advice that it is probably easier to start from scratch or perhaps from a flowchart than to redo that code.  If you chose to do that you may wonder why this is written as such:

Code: [Select]

initialisation clrw ;START OF INITIALISATION ROUTINES
movwf portA
movwf portB
comf portB ;PORT B CONTAINS FF
movwf portC ;PORT C CONTAINS 00

clrw
tris portA ;PORT A ALL BITS AS OUTPUTS
tris portB ;PORT B ALL BITS AS OUTPUTS
The first instruction clears W, then uses movwf <reg name>.   Why not just clrf <reg name>?   The authors apparently do that to avoid what is called Read-Modify-Write (RMW) error (there is a lot written about it).  "clrf" , however, is not a RMW instruction; whereas, "comf" is.  "comf PORTB" is therefore subject to RMW despite the earlier attempts supposedly to avoid using clrf.    The last 3 instructions are a correct structure for avoiding RMW when dealing with TRIS.   Note, that "TRIS" instruction and MOVWF TRIS have the same effect.   

However, as you write your code for flashing LED's, you may want to focus on just getting it to work and worry much less about RMW.  Do be aware of RMW, though.

You hopefully will progress to the enhanced mid-range chips, e.g., 16F1xxx, where you have data latches on the ports for avoiding RMW in those instances where it is a problem.

Have fun.

[Electrofinn]

While the solutions suggested are interesting and sound fantastical they also seem unnecessary, you must see from my point of view that I have a working PIC16C57 with code on it that runs perfectly but I can’t access, and as I want to build another, I have a PIC16F57 which runs the slightly different source code, almost perfectly. Surely a complete rewrite or a new PIC is not necessary.

All I need to know is why the machine will not run any more than the 16 hex values in the “pattern” table i.e. 32 values and how I can stop the machine from giving me an option to hold the reels when I put the first credit in...That's it. 

I have attached the flow chart from the pdf magazine article if that helps. And the links below to the pdf magazine articles themselves if anyone is interested...

Part 1

http://www.epemag.wimborne.co.uk/EPE_Fruit_Machine_part_1Dec94.pdf



Part2

http://www.epemag.wimborne.co.uk/EPE_Fruit_Machine_part_2Jan95.pdf


Unfortunately the forum I was on when I was able to get hold of the owner has now closed but I did save some of
the things he said…

“We had to write the code as a real-time multitasking operating system. Again this is to ensure no coins are ignored. You probably know that you can insert coins at any time regardless of what the machine is doing. This
was very important to us. The machine had to log every single inserted coin - without fail”

He also said..

“Have a look through the program code. Sorry it's a bit ugly - this was the first PIC code I ever wrote”

However the code worked anyway so I wasn’t complaining about its aesthetic or efficiency.

[jpanhalt]

All I need to know is why the machine will not run any more than the 16 hex values in the “pattern” table

It is a jump table, i.e., the position it goes to is relative to the current program counter position.  Do a "disassembly" list and review it. There you can see PCL (program counter low or as you call it, PC) and where each of the entries your table is located.

BTW, 16C57's are available on eBay, but you may not be able to program it with your tools.   A PicStart Plus can probably do it. (I haven't checked, but I have programmed other 16Cxx chips with that programmer.)

[Electrofinn]

Ok thanks will try that. Yeah the 16C57 is not supported by the pickit3, which I own, and owner used the PICstart originally. it was through his advice that we arrived at its direct replacement 16F57 of which I have 5 spare. As previously mentioned they work great with the source code, its just those little things I need to get changed in the code that he provided as he couldn't find the final version of the code that he supplied on the pre-programmed chips back in the day. before I had chance to flesh these things out about the code I had lost contact with him.

it's just frustrating because I just don't possess the programming knowledge to achieve this and attempts to learn this is proving very hard for me. I know enough to build the projects and program the PIC's and maybe mess with a few variables in code but it's like gobbledy goop to me. I have had new PCB's made by JLPCB using original schematic (with changes made because of unavailability of the original push buttons and new pinout arrangement for the segment displays) and I have built it up and works superbly with both the original chip and the new 16F57. I am so close grr! just need help!

You see this I like a restoration project for me and I am trying to have the code act like the original, I grew up with this machine after having and watching it being built by my dad, now I am building it for my son and my dad would also like to do this again, so I was going to give him the bits he needs as a kit. 

[jpanhalt]

First, I made an error in my previous post.   By accident I was looking at the 16F59, not the 16F57.   The latter's GP Ram does begin at 0x08, not 0x0A, which is the 16F59.

Since your compiler seems to accept PC for PCL, that is probably OK.   But, you might want to try PCL

[Electrofinn]

looked a the disassembly list and I'm at a loss as to what I am looking at or how I am to conclude why this won't run the 17th - 32nd entry of the "pattern" table. please except my apology but I'm just like    here!,


yeah just tried renaming the PC to PCL at the top of code and on all the tables but has made no difference, but will leave it that way since every example I have read seems to have it named that way.

[Electrofinn]

The other thing is I don't know whether I should be using MPLAB IDE v8.83 or MPLAB X IDE, seem to be able to use both without too much pain since I know where most things are in MPLAB X now!

[jpanhalt]

I use MPLab 8.92.  If 8.83 works, that is what I would use.  Either is easier when using Assembly, if the chip is supported.   That is not to say MPLab X won't work, and with newer chips, that is the only option.   The problem is very unlikely to be the Assembler.

Can you snippet the Disassembly and your code for the jump table side by side and post?

[KL27x]

Electrofinn

I think I can help, but this code totally sucks. Fortunately, my own code mostly totally sucks, so I am pretty good at working backwards to fix things.

You have identified that the look up table named "pattern" is what produces the sequence of 16 frames that you wish to extend to 32. I'm assuming you are correct in identifying this.

Steps.
1. Look at "pattern." It is a table that adds the contents of W to the PCL to determine where to jump and exit from. The contents of W must be between 1 and 16 (or is it 0-15? I'm not sure. In modern PIC I use BRW, in this case it would be 0-15, not 1-16; but I digress) or else the PCL will jump to a point that is outside of the contents of pattern. Hence you must figure out what is loaded into the W register before pattern is called. If you simply add 16 more retlw instructions after the original 16, they will never be executed.

2. Figure out where "pattern" is called. To do this, first locate "pattern" by right clicking on the code and selecting "goto" from the dropdown menu in MPLAB. Then find header "pattern" in the dropdown list.

3. Now you have "pattern" in front of you. Type ".fuckall" after it. Then reassemble. You will get a list of errors where the code is trying to call or goto "pattern." In this case, you will get only one error. But before you click on it, be sure to change "pattern.fuckall" back to "pattern." Then click on the error and it will take you to
 where "pattern" is called from. It is called from "feature."

4. In "feature," you will find:
   movf   counter_hi,W
   call   pattern

   every single time "pattern" is called, the W register is first loaded with the value that is contained in register designated as "counter_hi."

  So unless the code is broken, the writer has assured that the value of  counter_hi is always something between 1 and 16 (or 0-15?). If it were not, this code would more than likely produce a stack error.

5. Find every instance of "counter_hi" in the code.
5A - locate where the coder defined counter_hi. In this case, it's a short code, and it is easily found at the top. But if it were more complex there are two ways.
    A trick I use is to scroll to the very top of the main .asm file and type "counter_hi equ 0" 
    Reassemble.
    You will get an error that says "counter_hi previously defined"
    delete the "counter_hi equ 0" you just typed, then click on the error to jump to where the coder has defined "counter_hi"
    The good part of this is it will find it no matter what file it was placed in, which it could be one of many .inc files and/or .asm files.

You will find it here:
unused2   equ   14
hold_counter   equ   15
counter_hi   equ   16
counter_lo   equ   17
delay_counter   equ   18

So this means that the coder has defined it to be register 0x16 in general memory. Do not change this to 32. This is a fortuitous coincidence, only. This is an address, not a value.

You might have forgotten by now, but the point of locating it was to find out every place it is used in the code. So change the name to produce an error.
write "ASS." in front of counter_hi. So it reads "ASS.counter_hi equ 16"
reassemble. You will find numerous errors of "counter_hi not defined."

Again, be sure to erase the "ASS." before you click on the errors.

Scrolling through the errors one at a time, I get to the eleventh error when I find this code

sequence1
      bsf      counter_lo,7
      incf   counter_lo,f
      btfss   status,zer
   retlw   00
      incf   counter_hi,f
      bcf      counter_hi,4
      movf   credit_count,f
      btfsc   status,zer
   retlw   00
      bcf      mode_byte,7
      bcf      mode_byte,5
      bsf      mode_byte,0
   retlw   00



     
If you notice, this code increments counter_hi, but it clears bit 4. Bits go from 0-7. So bits 0-3 are the only things used. Anything that rolls over into bit 4 gets clipped off. This is how the coder is assuring that the contents of counter_hi, and hence the addw PCL instruction in "pattern" do not overflow past 16. In fact, he clips the range to 0-15. 16 is not included, which is why maybe he has redundant value in the spot 15 and 16 in his pattern table. At 16, counter_hi just rolls back to zero.

If you change this 4 to a 5, you will double the range of counter_hi. And your 16 retlw valued you added to the original 16 in "pattern" should now be included in the mix
To be clear, change
bcf counter_hi, 4
to
bcf counter_hi, 5



There may be more things which you must take into account and change, but this will get you started.

[Electrofinn]

wow yes This looks like something I can follow, excellent thank you. will work through your post and report back. yes the original writer had admitted that his code was ugly but hey it works so me personally couldn't argue with it. As I said I grew up playing with this fruit machine for hours and hours without a hitch.

would be marvellous if I had a fluent understanding of coding, because believe me if I did, the perfectionist in me would completely re-write this and make it the most efficient code it could possibly be. sadly, I have trouble with coding... and mathematics for that matter.   

[KL27x]

You're welcome.

Just to add: if this were important, at all, you would want to carefully check all the errors past #11, too, to make sure that increasing the range of counter_hi is not going to cause some other stack error.

A perfectionist would have to completely rewrite this code. This code has very illogical/inefficient use of the stack.*  But this was a first attempt at coding in assembly. And even good, efficient assembly code is rarely readable. These "tricks" I have shown are pretty much necessary to do anything complex in assembly, IMO. Without manipulating error codes, it would be near impossible for me. This is why I avoid macros. Trying to make assembly readable is essentially a lost cause; but in some ways it breaks the chain of error codes.

*I take that back. This part probably only has 4 deep stack to begin with. This makes is very hard to write assembly!
edited to add

Do a "disassembly" list and review it.

Why on god's earth would you disassemble assembly when you are looking at the source code?

[Electrofinn]

OK well sort of a success, increasing the counter_hi does now display the 32 hex values with the pattern like the original so yay! to that one, but playing the machine for around 10 mins I noticed 2 things.

1. occasionally on a win it doesn't seem to be playing the correct light sequence, which for a win is the "pattern2" table. sometimes it plays the first 16 of the "pattern" table and other times it runs the correct one.

2. and I had one instance when I had ran out of credits and after 2 seconds the "pattern" is meant to be run but the lights stayed off. again this doesn't happen every time.   

Thank you so much for your help by the way, I really appreciate this.   


   

[KL27x]

I still have this up on my computer, so error 10 shows

      movf   counter_hi,W
      call   pattern2

So pattern2 is analogous to pattern, but it only has the original 16 values, not 32. So when counter_hi is higher than 15, it is leaving pattern2 and producing a stack error. We call this "going off into the weeds."

What I would try is locate feature1 (right click on the code, goto, find feature1)
feature1   
      btfss   mode_byte,4
   goto      feature2
      movf   counter_hi,W
      call   pattern2

and change it to be:

feature1   
      btfss   mode_byte,4
   goto      feature2
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
;   movf   counter_hi,W  ;the semicolon in front removes this from the code but leaves it there for posterity, for your own reference. In case this doesn't work.
;
movlw 0x0f
andwf counter_hi,w
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
      call   pattern2

and see what happens.

[jpanhalt]

Why on god's earth would you disassemble assembly when you are looking at the source code?

In my experience, it can be quite quite useful to see how the assembler assembled the code, particularly when there are questions about memory usage and duplicate equates.  That original code is a mess, disassembly may help sort it out.  I am assuming he is using MPLAB SIM to know where the code fails. 

We each have our own way to troubleshoot unusual problems, assuming you write Assembly often, that is one way.  Please define your use of "source code."  Isn't C or any other high-level language also source code?

[KL27x]

Yeah, but C gets compiled. Disassembly shows you what the compiler wrote in assembly.
If you disassemble assembly source code, you get assembly but without headers and tabbing/spacing and variable/constant names that was present in the source code!
In this case "source code" means having the assembly in human form, rather than dissembled from reverse from just the hex file.

You can view the list file if you want to see how the assembly code is placed in sequence by the assembler, rather than disassembling. This would be useful if you are afraid your addPCL instruction might be rolling over past a byte boundary, for instance. But in this case, the coder has org 200 before and  org 300 after those pcl instructions. So this is already proofed in in the source code, which is preferable.

[KL27x]

OP: scrolling further, counter_hi is also used in other areas: auto1/2/3 and win2

So I can write you a foolproof solution, if this doesn't work, by utilizing an additional bit of general memory that is only used by feature/pattern. It is often easier to do it this way and potentially use up more memory and code than to reverse engineer existing code.

[Electrofinn]

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

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

[rstofer]

And yet, sometimes, it is helpful to have a core dump whether a disassembly or actual dump.  That way you are forced to think about every instruction.  There is no slipping past something because the 'name' seems right.  You are dealing only with actual hardware addresses.

No, it wasn't fun on the 8080s and Z80s back in the day.  Digital Research's Dynamic Debugging Tool (DDT) was my best friend.  Such as it was...

And, no, I don't want to go back to those days but on the mid-range PICs you are almost there anyway.  What an ugly architecture!