Add Disk II write support, and a way to make a blank disk to write to - #284
Merged
Conversation
The drive was read-only: IsWriteProtected returned true unconditionally and $C0EF was a no-op, so saving, INIT, copying and high scores all failed. Most visibly for VisiCalc, which is booted rather than injected precisely because its /S and /L commands call DOS - /L worked and /S could not. This is the host-agnostic half: the machine can now write, and what it writes comes back out as sectors. Nothing here touches a file. The soft-switch path. $C0EF enters write mode, a store to $C0ED loads the data register, and $C0EC shifts the byte out. Reading and writing $C0ED do different things - the read is the write-protect sense - so the written byte has to reach the controller, and Apple2SoftSwitches was discarding it: it had the value at Write(address, value) and dropped it before Access(). Threaded through now, and that page needs it nowhere else. Bytes go down at the cadence they come up, one per 32 CPU cycles. A machine storing faster than the surface moves has its early store dropped rather than laying down a byte the hardware could not have written. Same reasoning as the read path, and for the same reason: RWTS measures this. Decoding back to sectors. TryDecodeSector and TryDecodeAddressField move into Disk2NibbleCodec from the test project, where the algorithm already existed and was round-trip tested - From44 has carried the comment "used by tests and future write support" since the drive was written. They return false rather than throwing, because a track caught part-way through a write is legitimately inconsistent and half a sector must never reach the image. ApplyNibbleTrackToImage scans a track circularly. RWTS finds a sector by its address field and then writes only the data field, so where that data lands depends on where the head was - a rewritten sector does not have to sit where the original did, and a field wrapping the end of the buffer is still a good field on a disk that has no end. Two decisions worth stating: - The inserted image is now cloned. InsertDiskImage kept the caller's array by reference, which was harmless while nothing wrote to it. It is not harmless now: that array can be a snapshot's embedded bytes or a download cache entry, and writing through it would corrupt someone else's data. 140 KB, so the cost is nothing. Pinned by a test. - The image is decoded on leaving write mode, which for RWTS is once per sector. That gets per-sector durability without having to recognise a completed data field mid-stream, so a crash costs at most the sector in flight. Write protection is real state, default protected. The host decides when to uncover the notch, gating it on whether the backing file can be written at all - a separate question from whether the user wants it writable. 10 new tests: encode/decode round trip, corrupt byte and corrupt checksum both rejected, full-track round trip, a field straddling the buffer end, garbage leaving the image untouched, a written track reaching the image with other tracks untouched, the caller's array staying pristine, write-protect ignoring writes, writing faster than the disk turns dropping the early byte, and reading back a written byte a revolution later. 1812 tests pass, 73 projects build with no warnings.
Completes the write feature started in the previous commit, which stopped at the library boundary. Nothing there touches a file, because that library is shared with hosts that have no filesystem. The picked file's path was being thrown away before anything could use it: AppPickedFile carried Name and Bytes only. It now carries LocalPath too - optional, so no existing call site changed - filled from TryGetLocalPath() on desktop and null in the browser, where a picked file genuinely has no handle back to it. Write-enable is two separate questions, and merging them gives a checkbox that offers something it cannot deliver: does the user want this disk writable, and can the file be written at all. FileWritability.Probe answers the second by opening the file for writing and closing it again. Deliberately not FileInfo.IsReadOnly, which only reports the DOS read-only attribute and says nothing about POSIX permissions, read-only media, or a file another process holds open - all of which end in the same failed write. It reports distinct reasons so the tooltip can say which one applies. So DiskWriteEnabledCheckBox is disabled only when the disk is backed by a file this host cannot write, with the reason in its tooltip. A disk with no file behind it - downloaded, remote-inserted, or any disk in the browser - is written in memory and saved out explicitly, so nothing blocks it. That is one rule rather than a set of per-host special cases. Persistence is per sector, driven by the controller's DiskImageWritten event. A failure surfaces in the drive status line rather than throwing into the emulation thread. SaveDiskImageButton covers everything else through the existing IAppFileSaver, which already falls back to a Blob download in the browser because the File System Access API is Chromium-only. The drive status now says "modified, not saved" so an unsaved browser disk is visible rather than silently lost on tab close. Also adds diagnostic logging to the drive - motor on/off with read and write counts, head seeks, entering and leaving write mode, write-protect sense answers, and dropped or refused writes. This is not speculative: it turned a stalled investigation into a definite answer in a single run, establishing that VisiCalc's failing save never attempts a write and never even probes the write-protect notch. Kept at Debug and Trace so it costs nothing normally. New Basic sample DiskSaveLoad, wired into the Apple II examples list. It writes a text file through DOS, reads it back, and prints DISK ROUND TRIP OK only if the string and the number both survive - self-checking rather than something to eyeball. Tokenized by typing the listing into a real emulated Apple II and dumping $0801 to PRGEND, so the repo's own machine produced it and no external tool was needed. Verified end to end by an outside tool: the emulated machine ran SAVE TEST1 and CiderPress II reads the resulting image as a valid DOS 3.3 volume containing an Applesoft file TEST1 at $0801. An emulator agreeing with itself would prove much less. 1812 tests pass, 73 projects build with no warnings.
There was no way to get an empty disk to write to, which made write support hard
to use: every test needed a disk that already had files on it.
The authentic way is to boot DOS and type INIT, and that is what the hardware
and other emulators do - AppleWin creates the 140 KB container and leaves the
formatting to the emulated machine. It does not work here, and the measurement
is worth recording rather than rediscovering:
track buffer 6208 (our layout) INIT wrote 121512 bytes, then I/O ERROR
track buffer 6656 (real track) INIT never terminated, 9381036 bytes
The second run refutes the obvious explanation. Buffer capacity is not the
constraint: INIT writes 10-bit self-sync bytes so its verify pass can bit-align
to the field prologs afterwards, and this drive models nibbles as plain bytes
with no bit layer to express that in. It is the same gap that rules out
.nib/.woz media and copy protection, and it belongs with that work rather than
with write support.
So BlankDiskImageBuilder writes the volume structures directly, which is what
disk-image tools do: the VTOC with its catalog pointers, geometry, allocation
cursor and free-sector bitmap, and the catalog chain running down from sector 15
to sector 1. Tracks 0-2 and 17 are reserved, giving 496 free sectors. There is
no DOS on it, so it is a data disk and will not boot - the same as any tool-made
blank. Boot a DOS disk first, then insert this.
Verified two ways, because a structure that satisfies a parser is not
necessarily one DOS will accept:
- CiderPress II reads it as "140KB DOS 3.3 Vol 254, 496 sectors free".
- The emulated machine booted DOS, had this blank swapped in, and SAVEd a
program onto it - 2485 bytes, none dropped - after which CiderPress II reads
the file back and the free count is down to 494.
The button inserts the blank already write-enabled, since a disk you just made
exists to be written to. It registers with no file path, so it falls into the
same "save explicitly" branch as a downloaded or browser disk: the drive reports
"modified, not saved" and Save disk image is how it is kept. One rule, not a
special case.
4 tests assert what DOS actually reads - the VTOC fields, the 496 free sectors,
a catalog chain that walks 15 sectors and terminates, and a custom volume
number.
971 tests pass in Systems.Tests, 73 projects build with no warnings.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The Disk II drive was read-only —
IsWriteProtectedreturned true unconditionally and$C0EFwas a no-op — so saving,INIT, copying and high scores all failed. This makes the machine able to write, persists what it writes, and adds a way to get an empty disk to write onto.Builds on the cycle-paced read path from #282: writes go down at the same 32-cycle cadence, and building this on the old per-access model would have baked in the same class of bug that made boots take 95 seconds.
Three commits, three layers
5e807e5f3db3689908debeefWrite path.
$C0EFenters write mode, a store to$C0EDloads the data register (a read of the same address is the write-protect sense — which is why the written byte had to be threaded throughApple2SoftSwitches, which had it and was dropping it), and$C0ECshifts it out.TryDecodeSector/TryDecodeAddressFieldmove into production from the test project, returning false rather than throwing because a track caught mid-write is legitimately garbage.ApplyNibbleTrackToImagescans circularly: RWTS finds a sector by its address field and writes only the data field, so a rewritten sector needn't land where the original did.Write-enable is two questions, not one — does the user want this disk writable, and can the file be written at all. Merging them gives a checkbox that offers what it cannot deliver.
FileWritability.Probeanswers the second by opening the file for writing, deliberately notFileInfo.IsReadOnly, which sees neither POSIX permissions, read-only media, nor a file another process holds open. The checkbox is disabled only when a backing file can't be written, with the reason in its tooltip. A disk with no file behind it — downloaded, remote-inserted, or any disk in the browser — is written in memory and saved explicitly. One rule rather than per-host special cases.Blank disks are generated directly, not formatted by the machine. The authentic route is
INIT, and it does not work here. Measured:The second run refutes the obvious explanation — capacity isn't the constraint.
INITwrites 10-bit self-sync bytes so its verify pass can bit-align to the field prologs, and this drive models nibbles as plain bytes with no bit layer. Same gap that rules out.nib/.wozand copy protection; it belongs with that work.Verified externally, not by self-agreement
CiderPress II was the judge for every disk result:
SAVE TEST1on the System Master → a valid volume containingTEST1at$0801140KB DOS 3.3 Vol 254, 496 sectors freeDiskSaveLoadsample writing a text file through DOS →TESTDATApresent on the user's real.dsk, confirming per-sector persistence to a file (the link no test harness exercised)Live-verified in the desktop app: the sample prints
DISK ROUND TRIP OK, with the drive trace showing seven write-mode entries across tracks 15 and 17 and 16 of 16 sectors intact on every decode. The write-protect gating was observed working in both directions — refused withsense → PROTECTEDbefore ticking, permitted after.Known not to work
VisiCalc
/Sstill fails. It never calls RWTS ($BD00: zero calls), never enters write mode, and never probes the write-protect notch — it reads 266,000 bytes through its own routines and gives up. Not a write-support defect; tracked separately. Worth stating because VisiCalc was the original motivation for this feature.Notes for review
DiskSaveLoad, tokenized by typing the listing into a real emulated Apple II — the repo's own machine, no external tool.971 tests pass in Systems.Tests (14 new), 73 projects build with no warnings.