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<DriveNumber, Drive>`, 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.
This commit is contained in:
Ian Flanigan 2022-08-31 18:06:38 +02:00 committed by GitHub
parent 4688cae5b2
commit 5e224006e4
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 66 additions and 67 deletions

View File

@ -312,8 +312,8 @@ function setDriveState(state: DriveState) {
*/
export default class DiskII implements Card<State>, MassStorage<NibbleFormat> {
private drives: Drive[] = [
{ // Drive 1
private drives: Record<DriveNumber, Drive> = {
1: { // Drive 1
format: 'dsk',
encoding: ENCODING_NIBBLE,
volume: 254,
@ -325,7 +325,7 @@ export default class DiskII implements Card<State>, MassStorage<NibbleFormat> {
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<State>, MassStorage<NibbleFormat> {
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<State>, MassStorage<NibbleFormat> {
/** 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<State>, MassStorage<NibbleFormat> {
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<State>, MassStorage<NibbleFormat> {
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<State>, MassStorage<NibbleFormat> {
}
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<State>, MassStorage<NibbleFormat> {
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<State>, MassStorage<NibbleFormat> {
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<State>, MassStorage<NibbleFormat> {
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<State>, MassStorage<NibbleFormat> {
// 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<State>, MassStorage<NibbleFormat> {
} 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<State>, MassStorage<NibbleFormat> {
}
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<State>, MassStorage<NibbleFormat> {
};
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<State>, MassStorage<NibbleFormat> {
} 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<State>, MassStorage<NibbleFormat> {
{
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<State>, MassStorage<NibbleFormat> {
// 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<State>, MassStorage<NibbleFormat> {
// 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;
}

View File

@ -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', () => {