#438 Fixes BASICODE loading from Vic-20 datasette

[Malcolm Tyrrell]

This could also be viewed as an enhancement: Support for vic 20 loaders that expect rising edges from datasette

Cause: The BASICODE loader expects interrupts to be issued on rising as well as falling edges. Unlike the C64, the Vic-20 has support for observing both edges. The kernel loader doesn't need that support and presumably neither did most loaders from back in the day. This is probably why the issue hadn't been noticed until now.

Use of AI: I used AI to help me debug the problem and disassemble the loader. The fix is my own.

Attached files (in the zip):

• BasicodeMazezaM.wav (a BASICODE wav file - tested on C64 and ZX Spectrum)
• basicode.d64 - The BASICODE loading and saving program for Vic 20. These were obtained from Chip Shop Basicode 2 cassette, circa 1984, and put on disk for convenience.
• vic20risingEdge.patch - My proposed patch for this issue.
• BasicodeMazezaM-vic20pal-v2.tap - An example of what a hypothetical Vic-20 v2 Tap file would look like.

Additional files attached for convenience:

• BasicodeMazezaM.tap (a v1 tap file of BasicodeMazezaM.wav)
• vice_snapshot_vic20_load.vsf - A snapshot of the BASICODE loader, post initialization.
• basicode-loader-disassembly.asm - AI assisted disassembly of the loader.

Reproduction steps:

• If possible, load the snapshot. Otherwise, load the basicode loader from the d64. Run it and type SYS 7168.
• Using the monitor, add a breakpoint at $1D95. (See the disassembly for details.)
• type SYS 7203 to start the loader
• Attach the BasicodeMazezam.tap file.
• Press play.
• Loading should time out. The breakpoint is never hit.

The fix:

• Because the Vic-20 ecosystem is currently based on v1 tap files, the only realistic way of fixing this at the moment is to emulate the rising edges by assuming they fall half-way between the falling edges. Fortunately, BASICODE uses symmetric waves, so this fixes the problem for its loader (and probably most other obscure loaders too, if there are any).
• Good news: Support for C16 v1 tap files already supported this behaviour. The only real difference is that the C16 support has a power of two applied to the cycle count, which isn't suitable for the Vic-20. (It looks to me like that power of two might be neutralized by other C16 code later, but I didn't didn't want to change any of that.)
• The change makes Vice behaviour consistent with real hardware, so regular loaders continue to work.
• There was a chunk of duplicated code in datasette_read_gap, so I refactored the function to remove the duplication. I think it's cleaner now.
• The change should be essentially a no-op for non-Vic-20 platforms

Tests performed:

• A few downloaded Vic-20 tap files load successfully
• BASICODE Vic-20 tap file loads into BASICODE loader successfully
• A few downloaded C16 v2 tap file loads successfully in xplus4
• C16 v1 tap file (obtained by saving a BASIC program from Vice) loads successfully in xplus4
• A few downloaded C64 tap file loads successfully in x64sc

Note:

• The true fix for this issue would be to switch to using "v2" tap files for Vic-20. This would support hypothetical Vic-20 loaders whose waves had an asymmetric duty cycle. In fact, with the proposed patch, such a file gets supported out of the box and at zero-cost, so I attach a v2 Vic-20 file just as a demonstration. Support would need to be available widely in emulators before conversion programs could start adopting this.

[gpz]

Interesting! Didn't know vic20 can actually sense both edges.

So yes, we should probably support v2 for vic20 - and perhaps make that the default when creating files in VICE.

One thing makes me wonder though.... how is there so much code removed in your patch? Somehow it feels dubios :) Please explain

It would also be good if you updated the documentation (vice.texi) accordingly, ie add the exact details on how a VIC20 v2 file would look like (exactly like plus4 one?)

[gpz]

Forgot: does this also support writing? (A test program for that would be nice to have)

[Malcolm Tyrrell]

Thanks for your response!

does this also support writing?

You know, I never thought of testing BASICODE saving, since I just assumed that it would work out of the box. I've just tested it and sadly it doesn't :(. It would be shame not to try to fix saving at the same time as loading, so I'll look into it.

we should probably support v2 for vic20 - and perhaps make that the default when creating files in VICE.

It's definitely something worth considering, but I would suggest that the emulator adopt the compromise approach initially, where v2 can be loaded, but saving is still v1.

My reasoning is:

1. v1 with synthesized rising edges supports all currently known loaders
2. If vice saves v2, someone might save a tap file and then be confused or frustrated when they find that none of the available tap-to-wav conversion programs (or other emulators) support it.

how is there so much code removed in your patch?

If you look at the old version of the datasette_read_gap code, you'll see three copies of the same ~20 lines of code. I've refactored the function so that code happens in just one place. (As a consequence of the refactor, the C16 v2 case now sets the fullwave_gap value, which is harmless.)

It would also be good if you updated the documentation (vice.texi)

Yes, it makes sense to add the information to the documentation. I'll get on that, although for now I'll limit the details to observing that the v1 format has a limitation, but rising edges can be sythesized as a compromise.

exact details on how a VIC20 v2 file would look like (exactly like plus4 one?)

I think that's basically correct, although the opening bytes would probably stay "C64" (C16 files sometimes rewrite that as "C16").

I'll follow up with a new patch with documentation and hopefully fixing the save problem.

Thanks again for considering the patch,

Malcolm