Update patch, add basic test prg, rewrite TX test code

Makefile

@@ -4,13 +4,16 @@ MEMCFG := mem.cfg
 
 .PHONY: all clean
 
-all: rs232.prg
+all: rs232.prg test.prg
 
 clean:
-	rm -f rs232.bin *.o *.lst *.map
+	rm -f *.o *.lst *.map *.prg
 
 rs232.prg: driver.o
 	cl65 -v -C ${MEMCFG} -m rs232.map -o $@ $^
 
+test.prg: test.bas
+	petcat -w40 -o $@ -- $^
+
 %.o: %.a65
 	ca65 -v -l $(patsubst %.o,%.lst,$@) -t ${MACHINE} -o $@ $<

driver.a65

@@ -20,7 +20,7 @@ PIA2_CRB := PIA2+$3
 ; 1MHz phase2 clock
 PHI2_CLOCK = 1000000
 ; don't set the baud rate too high, or other operations will be starved
-BAUD_RATE = 300
+BAUD_RATE = 600
 
 ; parameters and return values can reside in an unused area of the zero page
 .zeropage
@@ -40,8 +40,9 @@ BAUD_RATE = 300
 	rs_status: .byte 0
 	; TODO add a state variable that can be monitored by the BASIC WAIT command
 
-; this is load address, for generating PRG files
+; this is for the load address, so we can generate PRG files.
-; works, as long as the code segment comes right after these two bytes
+; works as long as the code segment comes right after these two bytes,
+; i.e. as long as the LOADADDR segment resides at $load_address - 2
 .segment "LOADADDR"
 	.export LOADADDR
 	LOADADDR:
@@ -49,6 +50,10 @@ BAUD_RATE = 300
 
 ; entry points
 .code
+	; for development and testing
+	.export rs_test
+	rs_test:
+		jmp test
 	; these are convenience entry points right at the beginning of the page,
 	; to reduce dependency on code size.
 	; relocatable code would be perfect, but that's a lot more work.
@@ -94,8 +99,108 @@ BAUD_RATE = 300
 ; main code follows
 .code
 
+;FIXME BEGIN
+	; test code:
+	; single-byte RS-232 transmission
+	; this code runs asynchronously in a VIA T2 interrupt handler
+	; the handler is installed and uninstalled automatically until
+	; transmission is complete.
+	; do not call this too quickly in succession, or you will lock
+	; up the CPU in an endless loop.
+	test:
+		sei
+
+		lda #10
+		sta rs_available
+		lda rs_data
+		asl a
+		sta rs_data
+		lda #%00000001
+		rol a
+		sta rs_data+1
+
+		lda VIA_DDRB
+		ora #%00001000
+		sta VIA_DDRB
+
+		lda IRQVec
+		ldx IRQVec+1
+		sta oldvector
+		stx oldvector+1
+		lda #<irqtest
+		ldx #>irqtest
+		sta IRQVec
+		stx IRQVec+1
+
+		lda VIA_CR
+		and #%11011111
+		sta VIA_CR
+		lda #%10100000
+		sta VIA_IER
+		testperiod = PHI2_CLOCK/BAUD_RATE
+		lda #<testperiod
+		ldx #>testperiod
+		sta VIA_T2CL
+		stx VIA_T2CH
+		rts
+
+		cli
+		rts
+
+	irqtest:
+		sei
+		cld
+		pha
+		txa
+		pha
+
+		lda VIA_IFR
+		and #%00100000
+		beq @irqtestend
+
+		ldx rs_available
+		beq @irqtestrestore
+		dex
+		stx rs_available
+
+		;lda #<testperiod
+		ldx #>testperiod
+		;sta VIA_T2CL
+		stx VIA_T2CH
+
+		lsr rs_data+1
+		ror rs_data
+		lda VIA_PB
+		bcc @irqtestblank
+		ora #%00001000
+		bcs @irqtestwrite
+	@irqtestblank:
+		and #%11110111
+	@irqtestwrite:
+		sta VIA_PB
+
+	@irqtestend:
+		pla
+		tax
+		pla
+		jmp (oldvector)
+
+	@irqtestrestore:
+		lda #%00100000
+		sta VIA_IER
+		lda oldvector
+		ldx oldvector+1
+		sta IRQVec
+		stx IRQVec+1
+		clv
+		bvc @irqtestend
+;FIXME END
+
 	; driver installation, must be called once to set up IRQs, etc.
 	install:
+		; FIXME this doesn't work if we're loading from ROM
+		lda #1
+		sta initialized
 		; check if we're already initialized
 		lda initialized
 		beq @hwinit
@@ -107,7 +212,7 @@ BAUD_RATE = 300
 
 	@hwinit:
 		; disable interrupts, so we're not disturbed
-		cli
+		sei
 		
 		; save the previous handler
 		lda IRQVec
@@ -115,9 +220,9 @@ BAUD_RATE = 300
 		lda IRQVec+1
 		sta oldvector+1
 		; assign our custom interrupt handler to the IRQ vector
-		lda #>irqhandler
-		sta IRQVec
 		lda #<irqhandler
+		sta IRQVec
+		lda #>irqhandler
 		sta IRQVec+1
 
 		; VIA timer 2 cannot be enabled and disabled on the fly, so we'll
@@ -163,7 +268,7 @@ BAUD_RATE = 300
 		lda #rs_status_ok
 		sta rs_status
 		; fire away
-		sei
+		cli
 		; return
 		rts
 
@@ -181,7 +286,7 @@ BAUD_RATE = 300
 
 	@hwuninit:
 		; disable interrupts, so we're not disturbed
-		cli
+		sei
 
 		; restore the previous handler
 		lda oldvector
@@ -210,7 +315,7 @@ BAUD_RATE = 300
 		lda #rs_status_ok
 		sta rs_status
 		; the rest of the system will probably want interrupts enabled
-		sei
+		cli
 		; return
 		rts
 		
@@ -230,7 +335,7 @@ BAUD_RATE = 300
 
 	@dowrite:
 		; disable interrupts while we write into the buffer
-		cli
+		sei
 
 		; test if there's space in the buffer first
 		lda #16
@@ -275,7 +380,7 @@ BAUD_RATE = 300
 
 	@wrdone:
 		; and re-enable
-		sei
+		cli
 
 		; return
 		rts
@@ -295,7 +400,7 @@ BAUD_RATE = 300
 
 	@doread:
 		; disable interrupts while we read the buffer
-		cli
+		sei
 
 		; test if we have any data first
 		lda inqlen
@@ -339,7 +444,7 @@ BAUD_RATE = 300
 
 	@rddone:
 		; and re-enable
-		sei
+		cli
 
 		; return
 		rts
@@ -348,9 +453,9 @@ BAUD_RATE = 300
 	starttimer:
 		; arm VIA timer 2 by latching the counter
 		period = PHI2_CLOCK/BAUD_RATE
-		lda #>period
-		sta VIA_T1CL
 		lda #<period
+		sta VIA_T1CL
+		lda #>period
 		sta VIA_T1CH
 		; return
 		rts
@@ -368,7 +473,7 @@ BAUD_RATE = 300
 	.interruptor irqhandler
 	irqhandler:
 		; disable interrupts
-		cli
+		sei
 		; clear decimal flag to avoid unexpected behavior
 		cld
 		; save registers
@@ -546,9 +651,9 @@ BAUD_RATE = 300
 		pla
 		tax
 		pla
-		; enable interrupts
+		; we don't need to re-enable interrupts here,
-		; FIXME should we do this here? what's common practice? is this compatible with other interrupt handlers
+		; this will happen automatically when flags are restored later
-		sei
+		;cli
 		; jump to previous handler
 		; we don't need to return or restore flags, this will be done by the chained interrupt handler
 		jmp (oldvector)

test.bas

@@ -0,0 +1,15 @@
+10 print "initializing driver"
+20 poke 238, asc("h")
+30 sys 28672
+120 end
+200 print "sending data"
+210 poke 238, asc("a")
+220 sys 28681
+230 s = peek(239)
+240 if s = 0 then goto 40
+250 print "status "; s
+260 goto 40
+300 print
+310 print "disabling driver"
+320 sys 28675
+430 end

vice-tape-rs232.patch

@@ -12,10 +12,10 @@ index cd38b6d..cf10121 100644
 +	tape-rs232.h
 diff --git a/src/tapeport/tape-rs232.c b/src/tapeport/tape-rs232.c
 new file mode 100644
-index 0000000..2b66748
+index 0000000..680a1c4
 --- /dev/null
 +++ b/src/tapeport/tape-rs232.c
-@@ -0,0 +1,268 @@
+@@ -0,0 +1,270 @@
 +/*
 + * tape-rs232.h: RS-232 interface for the PET, connected to the tape port
 + *
@@ -130,7 +130,7 @@ index 0000000..2b66748
 +        dev->read_shift = 0;
 +        dev->write_shift = 0;
 +        /* TODO make the baud rate customizable */
-+        dev->baudrate = 300;
++        dev->baudrate = 600;
 +        if (dev->write_alarm != NULL) {
 +            alarm_destroy(dev->write_alarm);
 +        }
@@ -186,7 +186,7 @@ index 0000000..2b66748
 +        alarm_unset(dev->read_alarm);
 +    }
 +}
-+
++CLOCK last_maincpu_clk = 0;
 +static void tape_rs232_write(int port, int write_bit)
 +{
 +    if (port < 0 || port >= TAPEPORT_MAX_PORTS) {
@@ -201,7 +201,9 @@ index 0000000..2b66748
 +    /* FIXME RS-232 bit-clocking must be done on a time base, not on level change interrupts */
 +    /* write_bit is actually the contents of VIA_PB, and the write bit is PB3, i.e. 0x08 */
 +    dev->write_level = (write_bit & 0x08) != 0 ? 1 : 0;
-+    log_debug("tape_rs232: write port %d level %d", port, dev->write_level);
++    long delta = (long) maincpu_clk - last_maincpu_clk;
++    last_maincpu_clk = maincpu_clk;
++    log_debug("tape_rs232: write port %d level %d clock %lu delta %ld", port, dev->write_level, maincpu_clk, delta);
 +    /* write in progress? */
 +    if (dev->write_shift == 0) {
 +        /* no, start a new transmission - but only if this was a high->low transition (start bit) */
@@ -212,7 +214,7 @@ index 0000000..2b66748
 +            /* arm the first baud rate timer: sample in the middle of each bit,
 +             * so the delay must be half a baud clock cycle */
 +            clock_t delay = machine_get_cycles_per_second() / (dev->baudrate * 2);
-+            log_debug("tape_rs232: schedule start bit alarm after %d clock cycles", delay);
++            //log_debug("tape_rs232: schedule start bit alarm after %ld clock cycles", delay);
 +            alarm_set(dev->write_alarm, (CLOCK) (maincpu_clk + delay));
 +        }
 +    }
@@ -224,24 +226,24 @@ index 0000000..2b66748
 +        return;
 +    }
 +    tape_rs232_state_t *dev = (tape_rs232_state_t *) data;
-+    /* is this the first bit of a transmission? */
++    /* prepare the next alarm, at baudrate interval */
-+    if (dev->write_shift == 10) {
-+        /* yes, reconfigure the timer to run at baudrate intervals */
 +    alarm_unset(dev->write_alarm);
 +    clock_t delay = machine_get_cycles_per_second() / dev->baudrate;
-+        log_debug("tape_rs232: schedule regular bit alarm after %d clock cycles", delay);
++    //log_debug("tape_rs232: schedule regular bit alarm after %ld clock cycles", delay);
 +    alarm_set(dev->write_alarm, (CLOCK) (maincpu_clk + delay));
-+    }
++    /* shift in the next bit, from the head */
-+    /* shift in the next bit */
++    dev->write_register = (dev->write_register >> 1) | (dev->write_level ? 0x200 : 0x000);
-+    dev->write_register = (dev->write_register << 1) | dev->write_level;
 +    /* and decrement the counter */
 +    --dev->write_shift;
++    long delta = (long) maincpu_clk - last_maincpu_clk;
++    last_maincpu_clk = maincpu_clk;
++    log_debug("tape_rs232: sample %d reg 0x%04x offset %lu clock %lu delta %ld shift %u", dev->write_level, dev->write_register, offset, maincpu_clk, delta, dev->write_shift);
 +    /* transmission complete? */
 +    if (dev->write_shift == 0) {
 +        /* yes, unarm the timer */
 +        alarm_unset(dev->write_alarm);
 +        /* check if start and stop bits are valid */
-+        if ((dev->write_register & 0x0200) == 0x0000 && (dev->write_register & 0x0001) == 0x0001) {
++        if ((dev->write_register & 0x0200) == 0x0200 && (dev->write_register & 0x0001) == 0x0000) {
 +            /* yes, output data byte */
 +            uint8_t wbyte = (uint8_t) (dev->write_register >> 1);
 +            /* utf-8 codes generated by the petcii conversion routine can be max. 4 bytes (plus terminator) */