posting code in hopes of peer reviews.

Discuss technical or other issues relating to programming the Nintendo Entertainment System, Famicom, or compatible systems.

Moderator: Moderators

User avatar
FrankenGraphics
Formerly WheelInventor
Posts: 2032
Joined: Thu Apr 14, 2016 2:55 am
Location: Gothenburg, Sweden
Contact:

posting code in hopes of peer reviews.

Post by FrankenGraphics » Mon May 06, 2019 3:13 pm

I plan to post code occasionally to see if i can get feedback on what to do better, what won't work, if you can spot any bug, or remarks about better practices. They might as well be collected into one thread.

So here it goes, first function:

A function to interface with a custom peripheral. It uses 4 daisy-chained serial-to-parallel 8bit registers, which in turn feeds 8 BCD to decimal converters, in turn relaying 8x10 nixie tube cathodes.

Now, the hardware side of the NES is always a bit hard to wrap my head around.. so let's check if all my assumptions are OK, and if my code actually corresponds well enough to those assumptions. What do you think?

Code: Select all

;assumptions:
;-port 2 clk can be used to clock the shift registers (rising edge type).
;-port 2 data 1 is used to feed serial data to shift registers (in turn feeding bcd to decimal decoders)
;-port 2 data 2 is used to enable input on shift registers 
;-port 2 data 0 is unused as to not strobe controller 1. maybe pedantic. 
;-instead, we may tie data 0 to "output enable", ie. results are not shown on shift register outs until controller is strobed once per frame as an automated quantizer to reduce update rate on the nixie tubes.
;-this means we might be able to update score at any point in the loop; results are still updated on the displau itself on a per frame basis. 
;which could save us the "displayPending" var. 
;-we're only storing one BCD per array entry. We could bitpack one in each nybble, but i don't care to at this time. 
;-cable is known to have all three data lines. 

;======

lda displayPending ;might not be warranted to keep a pending var. 
beq skipDisplay
dec displayPending ;is now 0 again 

ldx #0 
ldy #0

displayBigLoop:
lda myScore,y ;y can be 0-7; asserted in loop. each value holds just 0-9 in the lower nybble. 
sta temp ;now holds our score buffer

displaySmallLoop:
lda temp 
and #1
asl ;moves from bit 0 to bit 1; which we use to feed the shift register with data.
ora #%00000100 ;we use data 2 as input enable on the shift registers. 
sta $2016 ; doesn't affect controller 1, and reversely, controller 1 strobes don't affect nixie display
lda #0
sta $2016 ; input enable goes low. 
lsr temp  
inx  
cpx #32 ;done with all 8 digits (4 bits * 8 places) in our binary coded decimal? 
beq skipDisplay: 
txa ;used to check for absence of modulo. 
and #7 
tay ;preparatory for fetching the next myScore,y
and #3 ;check for absence of modulo - if there is none; we're done with one binary coded decimal. 
beq displayBigLoop
jmp displaySmallLoop

skipDisplay: 	
;i might tie data 0 to "output enable" on the shift registers so that score changes only switch at max once per frame, each time controller code strobes the ports. 
;else, output enable is meant to be tied high. 
rts
note:
the shift register i'm using:
http://www.ti.com/lit/ds/symlink/cd4094b.pdf


What do you think? Anything i should work on improving?

Edit: added in "and #7" after posting

Edit 2: I suppose that jmp at the bottom is replacable with bne.
http://www.frankengraphics.com - personal NES blog

lidnariq
Posts: 9138
Joined: Sun Apr 13, 2008 11:12 am
Location: Seattle

Re: posting code in hopes of peer reviews.

Post by lidnariq » Mon May 06, 2019 4:22 pm

FrankenGraphics wrote:Now, the hardware side of the NES is always a bit hard to wrap my head around.. so let's check if all my assumptions are OK, [...]
  • ;assumptions:
  • port 2 clk can be used to clock the shift registers (rising edge type).
  • port 2 data 1 is used to feed serial data to shift registers (in turn feeding bcd to decimal decoders)
  • port 2 data 2 is used to enable input on shift registers
  • port 2 data 0 is unused as to not strobe controller 1. maybe pedantic.
The NES only has three general purpose outputs, and only one is available on the controller port. If you make an expansion port peripheral, however, this is fine. Do note that "OUT0" isn't specific to the port, but is instead shared among both jacks.
sta $2016 ; doesn't affect controller 1, and reversely, controller 1 strobes don't affect nixie display
$4016.
txa ;used to check for absence of modulo.
**
and #7
tay ;preparatory for fetching the next myScore,y
and #3 ;check for absence of modulo - if there is none; we're done with one binary coded decimal.
beq displayBigLoop
This won't do the thing you want. You'll end up emitting 4 bits from byte 0, and then 4 bits from byte 4, and repeat that three more times.
I think just inserting "LSR / LSR" where I put the ** should make it work?

User avatar
rainwarrior
Posts: 7743
Joined: Sun Jan 22, 2012 12:03 pm
Location: Canada
Contact:

Re: posting code in hopes of peer reviews.

Post by rainwarrior » Mon May 06, 2019 4:55 pm

I think I'd want OUT connected to your shift register input, and CLK connected to your shift register clock?

1. store the bit you want to write to $4016 (OUT holds $4016.0 at both ports)
2. then read $4017 to shift that bit into your shift register (generates CLK low pulse, second port only)
3. Repeat for as many bits are in your shift register chain.

Afterward (or before) you can separately use $4016 to strobe and then read the first controller port without affecting your shift registers, since they will be ignoring OUT unless it gets a CLK.

hackfresh
Posts: 100
Joined: Sun May 03, 2015 8:19 pm

Re: posting code in hopes of peer reviews.

Post by hackfresh » Mon May 06, 2019 5:10 pm

Just an example of making the code slightly more readable... with the comments all lined up and commands indented from labels. And getting rid of a few of the "magic numbers". Well everything is not lined up perfectly in my example because using the code tag is a bit wonky but you get the idea.

Code: Select all

;assumptions:
;-port 2 clk can be used to clock the shift registers (rising edge type).
;-port 2 data 1 is used to feed serial data to shift registers (in turn feeding bcd to decimal decoders)
;-port 2 data 2 is used to enable input on shift registers 
;-port 2 data 0 is unused as to not strobe controller 1. maybe pedantic. 
;-instead, we may tie data 0 to "output enable", ie. results are not shown on shift register outs until controller is strobed once per frame as an automated quantizer to reduce update rate on the nixie tubes.
;-this means we might be able to update score at any point in the loop; results are still updated on the displau itself on a per frame basis. 
;which could save us the "displayPending" var. 
;-we're only storing one BCD per array entry. We could bitpack one in each nybble, but i don't care to at this time. 
;-cable is known to have all three data lines. 

;======

NES_JOY1                                 = $4016
BCD_BITS_PER_DIGIT		= $04
BCD_NUMBER_OF_PLACES		= $08 

lda displayPending 						;might not be warranted to keep a pending var. 
beq skipDisplay
dec displayPending 						;is now 0 again 

ldx #0 
ldy #0

displayBigLoop:
	lda myScore,y 						;y can be 0-7; asserted in loop. each value holds just 0-9 in the lower nybble. 
	sta temp 							  ;now holds our score buffer

displaySmallLoop:
	lda temp 
	and #1
	asl 								    ;moves from bit 0 to bit 1; which we use to feed the shift register with data.
	ora #%00000100 					  ; we use data 2 as input enable on the shift registers. 
	sta NES_JOY1 						 ;doesn't affect controller 1, and reversely, controller 1 strobes don't affect nixie display
	lda #0
	sta NES_JOY1 						 ; input enable goes low. 
	lsr temp  
	inx  
	cpx #(BCD_BITS_PER_DIGIT * BCD_NUMBER_OF_PLACES)
	beq skipDisplay: 

check_modulo:	
        txa 								   ;used to check for absence of modulo. 
	and #7 
	tay 								   ;preparatory for fetching the next myScore,y
	and #3 							   ;check for absence of modulo - if there is none; we're done with one binary coded decimal. 
	beq displayBigLoop
	jmp displaySmallLoop

skipDisplay:    
									       ;i might tie data 0 to "output enable" on the shift registers so that score changes only switch at max once per 
									       ;frame, each time controller code strobes the ports. 
									       ;else, output enable is meant to be tied high. 
rts

User avatar
FrankenGraphics
Formerly WheelInventor
Posts: 2032
Joined: Thu Apr 14, 2016 2:55 am
Location: Gothenburg, Sweden
Contact:

Re: posting code in hopes of peer reviews.

Post by FrankenGraphics » Tue May 07, 2019 12:45 am

Thanks, everybody.
lidnariq wrote:The NES only has three general purpose outputs, and only one is available on the controller port. If you make an expansion port peripheral, however, this is fine.
At some point i thought i'd use the expansion port, but then didn't feel like modifying the unit, so that approach might rest on the shelf until/if a proper expansion port connector ever becomes available. I completely missed the two other outs aren't available on the controller ports!
lidnariq wrote:I think just inserting "LSR / LSR" where I put the ** should make it work?
Nice catch! I think i need to restore A afterwards for the beq BigLoop to function, like so?

Code: Select all

txa
lsr ;divide by 4
lsr 
and #7
tay ;preparatory for reading the next myScore,y.
txa ;refresh a after the above division. 
and #3
rainwarrior wrote:I think I'd want OUT connected to your shift register input, and CLK connected to your shift register clock?
Ohh, i never figured CLK:s were separately signalled, but that makes total sense for controllers to function. :lol: That should simplify things considerably.

So, basically:

Code: Select all

displaySmallLoop:
lda temp 
sta $4016 ;latches 0st bit to both controller ports, but we're only going to clock the second port. 
lda $4017 ;generates a clock on the second port; feeds one bit into the shift register.
lsr temp
[...]
only thing is i have no way to control the output enable, but i don't think that matters. I'll just tie it high and see how that works for me.

Also in that case i'll probably keep the pending thing so it is always called right after/before controller reads, in order to not have the nixie display output reset controller 1 in the middle of a joypad read.
hackfresh wrote:And getting rid of a few of the "magic numbers".
thanks. using constants would for sure have saved me from the double typo lidnariq catched.
http://www.frankengraphics.com - personal NES blog

User avatar
rainwarrior
Posts: 7743
Joined: Sun Jan 22, 2012 12:03 pm
Location: Canada
Contact:

Re: posting code in hopes of peer reviews.

Post by rainwarrior » Tue May 07, 2019 1:25 am

BIT $4017 might be appropriate for generating a CLK pulse from the read without clobbering A.

User avatar
FrankenGraphics
Formerly WheelInventor
Posts: 2032
Joined: Thu Apr 14, 2016 2:55 am
Location: Gothenburg, Sweden
Contact:

Re: posting code in hopes of peer reviews.

Post by FrankenGraphics » Tue May 07, 2019 1:37 am

nice! it gets overwritten in a few lines, but it seems a better practice in general. thanks!

New version:

Code: Select all

;assumptions:
;-it seems okay in theory to call this routine at will. If an NMI interrupts,
;there doesn't seem to be a way joypad reading and nixie output could interfere.
;-all this is assuming we just want a simple score printout of 8 digits, and not some other mixture of HUD vars. 
;hypothetically, one or two digits could be tied to output "0" as a score base and relieve logic to output lives, keys, a thematic radiation meter or whatever else. 
;-a 1 player solution. a version could be made to use expansion port to allow for 2 players and a score display.

;========================================
;only use these if i need to keep it last in the NMI. 
;---------------------------------------
;lda nixiePending ;might not be warranted to keep a pending var. 
;beq skipNixie 
;dec nixiePending ;is now 0 again 
;========================================

	ldx #0 
	ldy #0
nixieBigLoop:

	lda myScore,y ;x can be 0-7. each value holds just 0-9. 
	sta temp ;holds our score buffer

nixieSmallLoop:
	lda temp 
	sta $4016 ;moves 0st bit into both port out latches, but we're only clocking the second port. 
	bit $4017 ;generates a clock on the second port; feeds one bit into the shift register. 
	lsr temp  
	inx  
	cpx #32 ;done with all 8 digits in our score printout? 
	;- (4 bits * 8 places = 32 shift register bits -modify this circumstances change)
	beq skipNixie 
		txa   	;+
		lsr   	;|
		lsr   	;|-- preparatory for getting the next myScore,y.
		and #7	;|
		tay   	;+
		
		txa 	       	  ;+
		and #3  	     	;| 
				 	   		;|- check if we're done with a binary coded decimal.  
		beq nixieBigLoop	;|
		bne nixieSmallLoop ;+

skipNixie: 	
	rts
http://www.frankengraphics.com - personal NES blog

User avatar
FrankenGraphics
Formerly WheelInventor
Posts: 2032
Joined: Thu Apr 14, 2016 2:55 am
Location: Gothenburg, Sweden
Contact:

Re: posting code in hopes of peer reviews.

Post by FrankenGraphics » Tue May 07, 2019 2:55 am

A more hardware related question - would it be reasonable to ask the NES +5v to supply 4 shift registers and 8 bcd decoders? I'm using a separate supply but feel a bit like i'm wearing both belt and braces.

If i read the spec correctly, the bcd decoders are using 1mA each for supply and a neglible ammount for the data in:s. So that's a little above 8mA. That seems to be the case for the shift registers as well, so a little above 12mA, then.

https://tubehobby.com/datasheets/k155id1.pdf
http://www.frankengraphics.com - personal NES blog

lidnariq
Posts: 9138
Joined: Sun Apr 13, 2008 11:12 am
Location: Seattle

Re: posting code in hopes of peer reviews.

Post by lidnariq » Tue May 07, 2019 11:55 am

The datasheet says that the typical draw per 7441 (or Soviet equivalent) is 11 (74141) or 21 (7441) mA, not 1mA... Eight of those is ... not certain. Probably ok? I mean, the NES has a 1A regulator, and my measurements show only about 400mA are consumed internally.

I would be a little paranoid about the 7441 / K155s handling high voltage and would add a some high voltage protection on the NES side... Probably something like:

NES+5V ---|>|--- external +5V (any diode that has a peak reverse voltage greater than the supply used for the nixies)
NESsignal ---3k--- external circuitry

The NES controller ports already have over-/under- voltage protection diodes inside (from ground to signal and from signal to +5V), but there's no current limiting for them.

User avatar
FrankenGraphics
Formerly WheelInventor
Posts: 2032
Joined: Thu Apr 14, 2016 2:55 am
Location: Gothenburg, Sweden
Contact:

Re: posting code in hopes of peer reviews.

Post by FrankenGraphics » Fri May 10, 2019 6:26 am

This is what i use to get individual bits from 32 byte chunks. I think it is fine, but could use a critical look. I have this particular problem where i use this routine to get a semi-persistent state for any screen ID, but some particular screens on my map don't register a change. I've gone through this piece a few times and don't think the problem is here... but it's also my first bitwise handler and you might know of better practices. And if i'm proven wrong, that's great too.

myTemp1 could just as well be myTemp+1, but anyway...

Code: Select all

Prepare_32byteBitwise:
;===================================================================
;prepare the prerequisites for looking up/writing to a single bit
;in a 32-byte array of flags from a single byte ID.  
;-------------------------------------------------------------------
;Written to be called mostly at screen loads
;or to help with occasional object decisionmaking
;-------------------------------------------------------------------
;expects an ID of 0-255 in A. 
;returns a bitmask in myTemp1, and an index ($0-$1f) in X.
;tampers with A,X,Y,myTemp,myTemp1.
;===================================================================
sta myTemp ;used to restore A a few lines down 


ldy #1 ;prep first bitmask option
sty myTemp1 


and #$7 ;used to get the 3 least significant bits. 
tay
lda myTemp ;used to get the 5 most significant bits 
lsr
lsr
lsr 
tax
  
 
;this loop makes bitmasks out of the last 3 bits of the 1-byte ID

@loop:
cpy #0
beq @skip ;if the three bits are 0, the correct bitmask is already loaded in myTemp 
    asl myTemp1 ;shifts bitmask up
    dey 
    
    bcc @loop ;brA 
    @skip:
    
    ;ldy myTemp1 ;uncomment if you want the bitmask loaded into Y as well. 
Prepare_32byteBitwiseEnd: ;Safety label to limit the scope of "@" labels in asm6
rts
http://www.frankengraphics.com - personal NES blog

lidnariq
Posts: 9138
Joined: Sun Apr 13, 2008 11:12 am
Location: Seattle

Re: posting code in hopes of peer reviews.

Post by lidnariq » Fri May 10, 2019 10:46 am

Your code looks fine to me.

I'd be tempted to replace the loop with a lookup table, which would be faster and I think might be clearer, but it shouldn't matter.

supercat
Posts: 161
Joined: Thu Apr 18, 2019 9:13 am

Re: posting code in hopes of peer reviews.

Post by supercat » Fri May 10, 2019 11:52 am

lidnariq wrote:Your code looks fine to me.

I'd be tempted to replace the loop with a lookup table, which would be faster and I think might be clearer, but it shouldn't matter.
I don't think dey affects condition flags, so the loop exit condition is wrong.

Also, if y holds a value 0 to 7 and the accumulator holds a byte, the loop:

Code: Select all

loop:
  lsr
  dey
  bpl loop
will exit with the value of bit "y" in the carry flag, with no further masking or tests required.

User avatar
rainwarrior
Posts: 7743
Joined: Sun Jan 22, 2012 12:03 pm
Location: Canada
Contact:

Re: posting code in hopes of peer reviews.

Post by rainwarrior » Fri May 10, 2019 1:22 pm

supercat wrote:I don't think dey affects condition flags, so the loop exit condition is wrong.
DEY affects zero flag and sign/negative flag, but not carry. (All the INC/DEC instructions work this way.)

http://www.obelisk.me.uk/6502/reference.html#DEY

User avatar
FrankenGraphics
Formerly WheelInventor
Posts: 2032
Joined: Thu Apr 14, 2016 2:55 am
Location: Gothenburg, Sweden
Contact:

Re: posting code in hopes of peer reviews.

Post by FrankenGraphics » Fri May 10, 2019 1:30 pm

Oh, i see dey only affects n and z, according to http://www.6502.org/tutorials/6502opcodes.html#DEY

but

asl myTemp1

sets carry,
which is why it has seemingly worked for the most time. but it seems i can end up with a fully negating bitmask that way on every 8th incoming ID that way - that would explain my troubles.
http://www.frankengraphics.com - personal NES blog

lidnariq
Posts: 9138
Joined: Sun Apr 13, 2008 11:12 am
Location: Seattle

Re: posting code in hopes of peer reviews.

Post by lidnariq » Fri May 10, 2019 1:33 pm

FrankenGraphics wrote:asl myTemp1 sets carry,
It should have ended up terminating always with myTemp1 = 0 and C=1...

Post Reply