From 5e224006e4cb35309ab61b27073b9078655319ab Mon Sep 17 00:00:00 2001 From: Ian Flanigan Date: Wed, 31 Aug 2022 18:06:38 +0200 Subject: [PATCH] Make `DiskII.drives` a `Record` instead of an array (#154) Before, the `drives` field was an `array[0..1]` of `Drive`, but all of the methods took a `DriveNumber`, which was `[1..2]`. This meant that code everywhere was always subtracting 1 from the drive number. Now, `drives` is a `Record`, which means tha it has indexes `1, 2` and there's no need to subtract 1 everywhere. This change updates the `DiskII` class and its tests. The motivation for this change is to slowly split the WOZ disk implementation from the nibble disk implementation. I've tried twice, but the change has always grown too big and hairy, so I'm starting very small this time and working my way up. --- js/cards/disk2.ts | 49 +++++++++++----------- test/js/cards/disk2.spec.ts | 84 ++++++++++++++++++------------------- 2 files changed, 66 insertions(+), 67 deletions(-) diff --git a/js/cards/disk2.ts b/js/cards/disk2.ts index e2caca9..e21188c 100644 --- a/js/cards/disk2.ts +++ b/js/cards/disk2.ts @@ -312,8 +312,8 @@ function setDriveState(state: DriveState) { */ export default class DiskII implements Card, MassStorage { - private drives: Drive[] = [ - { // Drive 1 + private drives: Record = { + 1: { // Drive 1 format: 'dsk', encoding: ENCODING_NIBBLE, volume: 254, @@ -325,7 +325,7 @@ export default class DiskII implements Card, MassStorage { readOnly: false, dirty: false, }, - { // Drive 2 + 2: { // Drive 2 format: 'dsk', encoding: ENCODING_NIBBLE, volume: 254, @@ -336,7 +336,8 @@ export default class DiskII implements Card, MassStorage { phase: 0, readOnly: false, dirty: false, - }]; + } + }; /** * When `1`, the next nibble will be available for read; when `0`, @@ -361,7 +362,7 @@ export default class DiskII implements Card, MassStorage { /** Current drive number (1, 2). */ private drive: DriveNumber = 1; /** Current drive object. */ - private cur = this.drives[this.drive - 1]; + private cur = this.drives[this.drive]; /** Nibbles read this on cycle */ private nibbleCount = 0; @@ -673,7 +674,7 @@ export default class DiskII implements Card, MassStorage { case LOC.DRIVE1: // 0x0a this.debug('Disk 1'); this.drive = 1; - this.cur = this.drives[this.drive - 1]; + this.cur = this.drives[this.drive]; if (this.on) { this.callbacks.driveLight(2, false); this.callbacks.driveLight(1, true); @@ -682,7 +683,7 @@ export default class DiskII implements Card, MassStorage { case LOC.DRIVE2: // 0x0b this.debug('Disk 2'); this.drive = 2; - this.cur = this.drives[this.drive - 1]; + this.cur = this.drives[this.drive]; if (this.on) { this.callbacks.driveLight(1, false); this.callbacks.driveLight(2, true); @@ -756,7 +757,7 @@ export default class DiskII implements Card, MassStorage { } private updateDirty(drive: DriveNumber, dirty: boolean) { - this.drives[drive - 1].dirty = dirty; + this.drives[drive].dirty = dirty; if (this.callbacks.dirty) { this.callbacks.dirty(drive, dirty); } @@ -780,7 +781,7 @@ export default class DiskII implements Card, MassStorage { this.writeMode = false; this.on = false; this.drive = 1; - this.cur = this.drives[this.drive - 1]; + this.cur = this.drives[this.drive]; } for (let idx = 0; idx < 4; idx++) { this.q[idx] = false; @@ -803,9 +804,8 @@ export default class DiskII implements Card, MassStorage { on: this.on, drive: this.drive }; - this.drives.forEach(function (drive, idx) { - result.drives[idx] = getDriveState(drive); - }); + result.drives[1] = getDriveState(this.drives[1]); + result.drives[2] = getDriveState(this.drives[2]); return result; } @@ -817,18 +817,17 @@ export default class DiskII implements Card, MassStorage { this.on = state.on; this.drive = state.drive; for (const d of DRIVE_NUMBERS) { - const idx = d - 1; - this.drives[idx] = setDriveState(state.drives[idx]); - const { name, side, dirty } = state.drives[idx]; + this.drives[d] = setDriveState(state.drives[d]); + const { name, side, dirty } = state.drives[d]; this.callbacks.label(d, name, side); this.callbacks.driveLight(d, this.on); this.callbacks.dirty(d, dirty); } - this.cur = this.drives[this.drive - 1]; + this.cur = this.drives[this.drive]; } getMetadata(driveNo: DriveNumber) { - const drive = this.drives[driveNo - 1]; + const drive = this.drives[driveNo]; return { format: drive.format, volume: drive.volume, @@ -842,7 +841,7 @@ export default class DiskII implements Card, MassStorage { // TODO(flan): Does not work on WOZ disks rwts(disk: DriveNumber, track: byte, sector: byte) { - const cur = this.drives[disk - 1]; + const cur = this.drives[disk]; if (!isNibbleDrive(cur)) { throw new Error('Can\'t read WOZ disks'); } @@ -865,7 +864,7 @@ export default class DiskII implements Card, MassStorage { } else { const disk = createDiskFromJsonDisk(jsonDisk); if (disk) { - const cur = this.drives[drive - 1]; + const cur = this.drives[drive]; Object.assign(cur, disk); this.updateDirty(drive, false); this.callbacks.label(drive, disk.name, disk.side); @@ -876,7 +875,7 @@ export default class DiskII implements Card, MassStorage { } getJSON(drive: DriveNumber, pretty: boolean = false) { - const cur = this.drives[drive - 1]; + const cur = this.drives[drive]; if (!isNibbleDrive(cur)) { throw new Error('Can\'t save WOZ disks to JSON'); } @@ -894,7 +893,7 @@ export default class DiskII implements Card, MassStorage { }; this.worker.postMessage(message); } else { - const cur = this.drives[drive - 1]; + const cur = this.drives[drive]; Object.assign(cur, jsonDecode(json)); } return true; @@ -925,7 +924,7 @@ export default class DiskII implements Card, MassStorage { } else { const disk = createDisk(fmt, options); if (disk) { - const cur = this.drives[drive - 1]; + const cur = this.drives[drive]; const { name, side } = cur; Object.assign(cur, disk); this.updateDirty(drive, true); @@ -952,7 +951,7 @@ export default class DiskII implements Card, MassStorage { { const { drive, disk } = data.payload; if (disk) { - const cur = this.drives[drive - 1]; + const cur = this.drives[drive]; Object.assign(cur, disk); const { name, side } = cur; this.updateDirty(drive, true); @@ -969,7 +968,7 @@ export default class DiskII implements Card, MassStorage { // TODO(flan): Does not work with WOZ or D13 disks getBinary(drive: DriveNumber, ext?: NibbleFormat): MassStorageData | null { - const cur = this.drives[drive - 1]; + const cur = this.drives[drive]; if (!isNibbleDrive(cur)) { return null; } @@ -1005,7 +1004,7 @@ export default class DiskII implements Card, MassStorage { // TODO(flan): Does not work with WOZ or D13 disks getBase64(drive: DriveNumber) { - const cur = this.drives[drive - 1]; + const cur = this.drives[drive]; if (!isNibbleDrive(cur)) { return null; } diff --git a/test/js/cards/disk2.spec.ts b/test/js/cards/disk2.spec.ts index 90df011..663454e 100644 --- a/test/js/cards/disk2.spec.ts +++ b/test/js/cards/disk2.spec.ts @@ -21,14 +21,14 @@ const PHASES_PER_TRACK = 2; function setTrack(diskII: DiskII, track: number) { const initialState = diskII.getState(); - initialState.drives[0].track = track * STEPS_PER_TRACK; - initialState.drives[0].phase = (track * PHASES_PER_TRACK) % 4 as Phase; + initialState.drives[1].track = track * STEPS_PER_TRACK; + initialState.drives[1].phase = (track * PHASES_PER_TRACK) % 4 as Phase; diskII.setState(initialState); } function setWriteProtected(diskII: DiskII, isWriteProtected: boolean) { const initialState = diskII.getState(); - initialState.drives[0].readOnly = isWriteProtected; + initialState.drives[1].readOnly = isWriteProtected; diskII.setState(initialState); } @@ -71,9 +71,9 @@ describe('DiskII', () => { state.latch = 0x42; state.on = true; state.writeMode = true; - state.drives[1].tracks[14][12] = 0x80; - state.drives[1].head = 1000; - state.drives[1].phase = 3; + state.drives[2].tracks[14][12] = 0x80; + state.drives[2].head = 1000; + state.drives[2].phase = 3; diskII.setState(state); expect(diskII.getState()).toEqual(state); @@ -175,8 +175,8 @@ describe('DiskII', () => { diskII.ioSwitch(0x84); // coil 2 off const state = diskII.getState(); - expect(state.drives[0].phase).toBe(0); - expect(state.drives[0].track).toBe(0); + expect(state.drives[1].phase).toBe(0); + expect(state.drives[1].track).toBe(0); }); it('allows head positioning when the drive is on', () => { @@ -192,8 +192,8 @@ describe('DiskII', () => { diskII.ioSwitch(0x84); // coil 2 off const state = diskII.getState(); - expect(state.drives[0].phase).toBe(2); - expect(state.drives[0].track).toBe(4); + expect(state.drives[1].phase).toBe(2); + expect(state.drives[1].track).toBe(4); }); it('moves the head to track 2 from track 0 when all phases are cycled', () => { @@ -213,8 +213,8 @@ describe('DiskII', () => { diskII.ioSwitch(0x80); // coil 0 off const state = diskII.getState(); - expect(state.drives[0].phase).toBe(0); - expect(state.drives[0].track).toBe(2 * STEPS_PER_TRACK); + expect(state.drives[1].phase).toBe(0); + expect(state.drives[1].track).toBe(2 * STEPS_PER_TRACK); }); it('moves the head to track 10 from track 8 when all phases are cycled', () => { @@ -235,8 +235,8 @@ describe('DiskII', () => { diskII.ioSwitch(0x80); // coil 0 off const state = diskII.getState(); - expect(state.drives[0].phase).toBe(0); - expect(state.drives[0].track).toBe(10 * STEPS_PER_TRACK); + expect(state.drives[1].phase).toBe(0); + expect(state.drives[1].track).toBe(10 * STEPS_PER_TRACK); }); it('stops the head at track 34', () => { @@ -261,14 +261,14 @@ describe('DiskII', () => { diskII.ioSwitch(0x80); // coil 0 off const state = diskII.getState(); - expect(state.drives[0].phase).toBe(0); + expect(state.drives[1].phase).toBe(0); // The emulated Disk II puts data for track n on the // 4 quarter-tracks starting with n * STEPS_PER_TRACK. // On a real Disk II, the data would likely be on 3 // quarter-tracks starting with n * STEPS_PER_TRACK - 1, // leaving 1 essentially blank quarter track at the // half-track. - expect(state.drives[0].track).toBe(35 * STEPS_PER_TRACK - 1); + expect(state.drives[1].track).toBe(35 * STEPS_PER_TRACK - 1); }); it('moves a half track when only one phase is activated', () => { @@ -283,8 +283,8 @@ describe('DiskII', () => { diskII.ioSwitch(0x86); // coil 3 off const state = diskII.getState(); - expect(state.drives[0].phase).toBe(3); - expect(state.drives[0].track).toBe(15 * STEPS_PER_TRACK + 2); + expect(state.drives[1].phase).toBe(3); + expect(state.drives[1].track).toBe(15 * STEPS_PER_TRACK + 2); }); it('moves backward one track when phases are cycled in reverse', () => { @@ -301,8 +301,8 @@ describe('DiskII', () => { diskII.ioSwitch(0x80); // coil 0 off const state = diskII.getState(); - expect(state.drives[0].phase).toBe(0); - expect(state.drives[0].track).toBe(14 * STEPS_PER_TRACK); + expect(state.drives[1].phase).toBe(0); + expect(state.drives[1].track).toBe(14 * STEPS_PER_TRACK); }); it('moves backward two tracks when all phases are cycled in reverse', () => { @@ -323,8 +323,8 @@ describe('DiskII', () => { diskII.ioSwitch(0x84); // coil 2 off const state = diskII.getState(); - expect(state.drives[0].phase).toBe(2); - expect(state.drives[0].track).toBe(13 * STEPS_PER_TRACK); + expect(state.drives[1].phase).toBe(2); + expect(state.drives[1].track).toBe(13 * STEPS_PER_TRACK); }); it('does not move backwards past track 0', () => { @@ -345,8 +345,8 @@ describe('DiskII', () => { diskII.ioSwitch(0x84); // coil 2 off const state = diskII.getState(); - expect(state.drives[0].phase).toBe(2); - expect(state.drives[0].track).toBe(0); + expect(state.drives[1].phase).toBe(2); + expect(state.drives[1].track).toBe(0); }); it('moves backward one half track', () => { @@ -361,8 +361,8 @@ describe('DiskII', () => { diskII.ioSwitch(0x82); // coil 1 off const state = diskII.getState(); - expect(state.drives[0].phase).toBe(1); - expect(state.drives[0].track).toBe(14.5 * STEPS_PER_TRACK); + expect(state.drives[1].phase).toBe(1); + expect(state.drives[1].track).toBe(14.5 * STEPS_PER_TRACK); }); // The emulated Disk II is not able to step quarter tracks because @@ -378,8 +378,8 @@ describe('DiskII', () => { diskII.ioSwitch(0x87); // coil 3 on const state = diskII.getState(); - expect(state.drives[0].phase).toBe(3); - expect(state.drives[0].track).toBe(15 * STEPS_PER_TRACK + 1); + expect(state.drives[1].phase).toBe(3); + expect(state.drives[1].track).toBe(15 * STEPS_PER_TRACK + 1); }); // The emulated Disk II is not able to step quarter tracks because @@ -395,8 +395,8 @@ describe('DiskII', () => { diskII.ioSwitch(0x83); // coil 1 on const state = diskII.getState(); - expect(state.drives[0].phase).toBe(1); - expect(state.drives[0].track).toBe(14.25 * STEPS_PER_TRACK); + expect(state.drives[1].phase).toBe(1); + expect(state.drives[1].track).toBe(14.25 * STEPS_PER_TRACK); }); }); @@ -478,21 +478,21 @@ describe('DiskII', () => { it('writes a nibble to the disk', () => { const diskII = new DiskII(mockApple2IO, callbacks); diskII.setBinary(1, 'BYTES_BY_TRACK', 'po', BYTES_BY_TRACK_IMAGE); - let track0 = diskII.getState().drives[0].tracks[0]; + let track0 = diskII.getState().drives[1].tracks[0]; expect(track0[0]).toBe(0xFF); diskII.ioSwitch(0x89); // turn on the motor diskII.ioSwitch(0x8F, 0x80); // write diskII.ioSwitch(0x8C); // shift - track0 = diskII.getState().drives[0].tracks[0]; + track0 = diskII.getState().drives[1].tracks[0]; expect(track0[0]).toBe(0x80); }); it('writes two nibbles to the disk', () => { const diskII = new DiskII(mockApple2IO, callbacks); diskII.setBinary(1, 'BYTES_BY_TRACK', 'po', BYTES_BY_TRACK_IMAGE); - let track0 = diskII.getState().drives[0].tracks[0]; + let track0 = diskII.getState().drives[1].tracks[0]; expect(track0[0]).toBe(0xFF); diskII.ioSwitch(0x89); // turn on the motor @@ -501,7 +501,7 @@ describe('DiskII', () => { diskII.ioSwitch(0x8F, 0x81); // write diskII.ioSwitch(0x8C); // shift - track0 = diskII.getState().drives[0].tracks[0]; + track0 = diskII.getState().drives[1].tracks[0]; expect(track0[0]).toBe(0x80); expect(track0[1]).toBe(0x81); }); @@ -510,7 +510,7 @@ describe('DiskII', () => { const diskII = new DiskII(mockApple2IO, callbacks); diskII.setBinary(1, 'BYTES_BY_TRACK', 'po', BYTES_BY_TRACK_IMAGE); let state = diskII.getState(); - state.drives[0].dirty = false; + state.drives[1].dirty = false; diskII.setState(state); jest.resetAllMocks(); @@ -522,7 +522,7 @@ describe('DiskII', () => { expect(callbacks.dirty).toHaveBeenCalledWith(1, true); state = diskII.getState(); - expect(state.drives[0].dirty).toBeTruthy(); + expect(state.drives[1].dirty).toBeTruthy(); }); }); @@ -557,11 +557,11 @@ describe('DiskII', () => { diskII.ioSwitch(0x84); // coil 2 off const state = diskII.getState(); - expect(state.drives[0].phase).toBe(2); + expect(state.drives[1].phase).toBe(2); // For WOZ images, the number of tracks is the number in the image. // The DOS3.3 System Master was imaged on a 40 track drive, so it // has data for all 40 tracks, even though the last few are garbage. - expect(state.drives[0].track).toBe(40 * STEPS_PER_TRACK - 1); + expect(state.drives[1].track).toBe(40 * STEPS_PER_TRACK - 1); }); it('spins the disk when motor is on', () => { @@ -572,14 +572,14 @@ describe('DiskII', () => { diskII.setBinary(1, 'DOS 3.3 System Master', 'woz', DOS33_SYSTEM_MASTER_IMAGE); let state = diskII.getState(); - expect(state.drives[0].head).toBe(0); + expect(state.drives[1].head).toBe(0); diskII.ioSwitch(0x89); // turn on the motor cycles += 10; diskII.tick(); state = diskII.getState(); - expect(state.drives[0].head).toBeGreaterThan(0); + expect(state.drives[1].head).toBeGreaterThan(0); }); it('does not spin the disk when motor is off', () => { @@ -590,13 +590,13 @@ describe('DiskII', () => { diskII.setBinary(1, 'DOS 3.3 System Master', 'woz', DOS33_SYSTEM_MASTER_IMAGE); let state = diskII.getState(); - expect(state.drives[0].head).toBe(0); + expect(state.drives[1].head).toBe(0); cycles += 10; diskII.tick(); state = diskII.getState(); - expect(state.drives[0].head).toBe(0); + expect(state.drives[1].head).toBe(0); }); it('reads an FF sync byte from the beginning of the image', () => {