AppleWin
AppleWin copied to clipboard
AppleWin
In the disk Status getting ?? for Sectors in ProDOS
On most ProDOS disks,
I am getting ?? in S when track T is getting updated. But later after more disk activity it gets populated.
But on some ProDOS disks, the status always stays as ??.
Compared with a previous version of AppleWin (1.30.11.0) this does not happen -- it always shows S as non-?? whenever T is updated.
This is probably related to #1134 which was release in version 1.30.13.0.
Can you provide a simple reproducible for this? EG. attach (or link to) a ProDOS disk, and then describe the steps where it used to work (1.30.11.0) and now doesn't (1.30.13.0).
Thanks.
Once such example is PLASMA2-SYS.PO.
Please make sure to run Apple IIe Enhanced, Authentic speed, and disable Enhanced disk access (to follow the status updates).
It starts with a regular ProDOS boot, and S keeps showing ?? throughout the whole boot sequence.
Then at the prompt type +CAT return, which has a lot of disk activity, and still also only ??.
Whereas in 1.30.11.0 it shows all the actual S numbers.
The need for this feature is to understand what sectors it is reading to identify, which files are accessed. So to the same goal, it would be great to have a logging feature for all disk activity in the form of
- R/W TT:SS (decimal and/or hex).
@tomcw Win32Frame::GetTrackSector() was a bit of a clusterfuck with memory probing. I've cleaned this up for non-.WOZ caching the last Track & Sector read ~~with the caveat that this shows the Physical Sector and not the Logical Sector like previous versions used to.~~ You can find this in branch 1215-in-the-disk-status-getting-for-sectors-in-prodos
@tingtron Why does track 23 have a VolumeTrackSectorChecksum of Vol:3C Trk:3C Sec:3C Chk:3C ?
@tingtron Why does track 23 have a VolumeTrackSectorChecksum of Vol:3C Trk:3C Sec:3C Chk:3C ?
I am not sure what you are asking: the referenced disk PLASMA2-SYS.PO above doesn't have any issues, and also opens fine in CiderPress incl. Disk Sector Viewer.
Right... it's just a 140KiB image, so all the nibblisation is generated by AppleWin.
@Michaelangel007 - when you say "track 23" is this hex 23? ie. dec 35... which isn't a valid track number (only 0-34 are valid).
The simple fix is just to remove the check against nProDOSslot:
https://github.com/AppleWin/AppleWin/blob/f8da683d4598c97a4784bc55a0ae450d1b0173ce/source/Windows/WinFrame.cpp#L665
For this ProDOS v1.9 (16-JUL-90), the value of nProDOSslot gets read from memory as $FF. So I guess this $BE3C(DEFSLT=Default Slot) location changed for this ProDOS version.
EDIT: We can't just remove this slot check, since if we have Disk II controllers in s6 and s5, then the T/S UI gets updated for both controllers!
But I agree, @Michaelangel007, relying on magic DOS3.3/ProDOS memory locations for track & sector is brittle.
@Michaelangel007 - when you say "track 23" is this hex 23? ie. dec 35... which isn't a valid track number (only 0-34 are valid).
No, 23 decimal.
If you add this assert()
assert(m_VolTrkSecChk[2] <= 0xF);
to line 273 of DecodeLatchNibble()
#if LOG_DISK_NIBBLES_READ
const bool chk = (m_VolTrkSecChk[0] ^ m_VolTrkSecChk[1] ^ m_VolTrkSecChk[2] ^ m_VolTrkSecChk[3]) == 0;
if (!bIsWrite)
{
BYTE addrPrologue = m_bAddressPrologueIsDOS3_2 ? (BYTE)kADDR_PROLOGUE_DOS3_2 : (BYTE)kADDR_PROLOGUE_DOS3_3;
m_strReadD5AAxxDetected = StrFormat("read D5AA%02X detected - Vol:%02X Trk:%02X Sec:%02X Chk:%02X %s",
addrPrologue, m_VolTrkSecChk[0], m_VolTrkSecChk[1], m_VolTrkSecChk[2], m_VolTrkSecChk[3], chk?"":"(bad)");
assert(m_VolTrkSecChk[2] <= 0xF);
// NOTE: We can NOT: assert(m_VolTrkSecChk[2] <= 0xF);
// Since some disks have bogus sector numbers: PLASMA2-SYS.PO
if (!m_bSuppressReadD5AAxxDetected)
{
LOG_DISK("%s\r\n", m_strReadD5AAxxDetected.c_str());
}
}
#endif
and boot PLASMA2-SYS.PO it will trigger shortly after displaying MEM FREE:$8FD9 with this log:
0095AF5A: track $17.00 magnet-states 0000 phase 0 off address $C0E0 last-stepper 0.034ms
spin: revs=0, nibbles=1563
read D5AA96 detected - Vol:3C Trk:3C Sec:3C Chk:3C
pDrive has these values
- m_phasePrecise | 46.0000000 | float
- m_phase | 0x0000002e | int
@tomcw It looks Disk.h is missing including "Log.h". If I turn on LOG_DISK_ENABLED I get a compile error (MSVC 2019) in DiskLog.h due to LOG not being defined.
#include "Card.h"
#include "Log.h" // <--- missing include
#include "DiskLog.h"
I've added this with ba1dc880 over in PR 1212
@tomcw We probably should spin off the following FR since this issue is about a regression.
So to the same goal, it would be great to have a logging feature for all disk activity in the form of R/W TT:SS (decimal and/or hex).
If you add this assert()
assert(m_VolTrkSecChk[2] <= 0xF);to line 273 of DecodeLatchNibble()
I did this, and it works fine, ie. the assert doesn't trigger. NB. It looks like you have some logging enabled, whereas I don't.
OK, triggering if: m_enhanceDisk == false (ie. 'Enhance Disk Access Speed' is off).
Probably a bug in DecodeLatchNibble(). I'll take a look...
Thank you for looking into the disk logging feature. This would really be invaluable for debugging actual apps that use files and optimizing etc. It would also be great if such disk logging is run-time selectable as opposed to compile-time. It would be used along with Authentic/Custom CPU speed and "Enhanced disk access speed" diagnose various disk and file issues.
Probably a bug in DecodeLatchNibble(). I'll take a look...
@tomcw Looks like the bug is on line 264
m_VolTrkSecChk[i] = ((m_VolTrkSecChk4and4[i*2] & 0x55) << 1) | (m_VolTrkSecChk4and4[i*2+1] & 0x55);
Should be:
m_VolTrkSecChk[i] = ((m_VolTrkSecChk4and4[i*2] << 1) | 1) & m_VolTrkSecChk4and4[i*2+1];
I'm not sure why this bitstream is showing up on a DOS 3.3 System Master.WOZ ??
FF F7 BF BF BF BF DE BE in DecodeLatchNibble() ??? Using Copy ][+ nibble editor I don't see these nibbles anywhere on disk.
| Decoded |
|---|
| FF 3F 3F BC |
Normally this should be in the range:
| Decoded |
|---|
| FE tt 0s Fx |
Looks like the bug is on line 264
m_VolTrkSecChk[i] = ((m_VolTrkSecChk4and4[i*2] & 0x55) << 1) | (m_VolTrkSecChk4and4[i*2+1] & 0x55);Should be:m_VolTrkSecChk[i] = ((m_VolTrkSecChk4and4[i*2] << 1) | 1) & m_VolTrkSecChk4and4[i*2+1];
I think those two expressions always give the same result (but your way is the standard Apple II way of decoding two "4 and 4" nibbles into a byte).
I think those two expressions always give the same result (but your way is the standard Apple II way of decoding two "4 and 4" nibbles into a byte).
OK, just verified that they do.
int val = ((in44[0] & 0x55) << 1) | (in44[1] & 0x55);
Sector 0 00 01 02 03 04 05 06 07
ADDR: FF F7 BF BF BF BF DE BE
VTSC: FF 3F 3F BC
int val = ((in44[0] << 1) |1) & in44[1];
Sector 0 00 01 02 03 04 05 06 07
ADDR: FF F7 BF BF BF BF DE BE
VTSC: F7 3F 3F BC
i.e.
#include <stdio.h>
int char2hex(char c)
{
const int CHAR_2_HEX[128] =
{ //0 1 2 3 4 5 6 7 8 9 A B C D E F
0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, // 0x00..0F @ABCDEFGHIJKLMNO
0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, // 0x10..1F PQRSTUVWXYZ[\]^_
0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, // 0x20..2F !"#$%&`()*+.-/
0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 0, 0, 0, 0, 0, 0, // 0x30..3F 0123456789:;<=>?
0,10,11,12,13,14,15, 0, 0, 0, 0, 0, 0, 0, 0, 0, // 0x40..4F @ABCDEFGHIJKLMNO
0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, // 0x50..5F PQRSTUVWXYZ[\]^_
0,10,11,12,13,14,15, 0, 0, 0, 0, 0, 0, 0, 0, 0, // 0x60..6F 'abcdefghijklmno
0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0 // 0x7F..7F pqrstuvwxyz{|}~
};
return CHAR_2_HEX[c & 0x7F];
}
const char* letters2hex(const char *pSrc, char *pDst)
{
if (!pSrc) return 0;
int val = (char2hex( pSrc[0] ) << 4)
| (char2hex( pSrc[1] ) << 0);
*pDst = val;
return pSrc + 3;
}
int decode44(const char *in44 )
{
//int val = ((in44[0] << 1) |1) & in44[1];
int val = ((in44[0] & 0x55) << 1) | (in44[1] & 0x55);
return val;
}
void decodeVTSC( const char *str)
{
char nibs[8];
char VTSC[4];
const char *tmp = str;
for( int byte = 0; byte < 8; ++byte )
tmp = letters2hex( tmp, &nibs[byte] );
printf( " ADDR: " );
for( int byte = 0; byte < 8; ++byte )
printf( "%02X ", (unsigned char) nibs[ byte ] );
for( int byte = 0; byte < 4; ++byte)
VTSC[byte] = decode44( nibs + 2*byte );
printf( "\n VTSC: " );
for( int byte = 0; byte < 4; ++byte )
printf( "%02X ", (unsigned char) VTSC[ byte ] );
printf( "\n" );
}
int main()
{
const char *SECTOR[] =
{
"FF FE AA AA AA AA FF FE", // 0 6C2E 845E
"FF FE AA AA AA AB FF FF", // 1 6DB0
"FF FE AA AA AB AA FE FE", // 2 6F32
"FF FE AA AA AB AB FE FF", // 3 70B4
"FF FE AA AA AA AE FF FA", // 4 7236
"FF FE AA AA AA AF FF FB", // 5 73B8
"FF FE AA AA AB AE FE FA", // 6 753A
"FF FE AA AA AB AF FE FB", // 7 76BC
"FF FE AA AA AE AA FB FE", // 8 783E
"FF FE AA AA AE AB FB FF", // 9 79C0
"FF FE AA AA AF AA FA FE", // A 7B42
"FF FE AA AA AF AB FA FF", // B 7CC5
"FF FE AA AA AE AE FB FA", // C 7E46
"FF FE AA AA AE AF FB FB", // D 7FC8
"FF FE AA AA AF AE FA FA", // E 814A
"FF FE AA AA AF AF FA FB" // F 82CC
};
const int SECTORS = sizeof(SECTOR) / sizeof(const char *);
for( int sector = 0; sector < SECTORS; ++sector )
{
printf( "Sector %1X 00 01 02 03 04 05 06 07\n", sector );
decodeVTSC( SECTOR[ sector ] );
}
return 0;
}
I don't understand why we are getting this false positive? FF F7 BF BF BF BF DE BE in DecodeLatchNibble() ?
@tomcw OK, I think I have this fixed for both .WOZ and .DSK/.PO:
In Disk2InterfaceCard::DataLatchReadWOZ() I was expecting this to work:
#if LOG_DISK_NIBBLES_READ
// May not actually be read by 6502 (eg. Prologue's CHKSUM 4&4 nibble pair), but still pass to the log's nibble reader
m_formatTrack.DecodeLatchNibbleRead(m_floppyLatch);
newLatchData = true;
#endif
The solution was to only chain if we have a valid disk nibble:
// GH #1215
if (m_floppyLatch & 0x80)
m_formatTrack.DecodeLatchNibbleRead(m_floppyLatch);
Why does track 23 have a VolumeTrackSectorChecksum of Vol:3C Trk:3C Sec:3C Chk:3C ? :
Probably a bug in DecodeLatchNibble(). I'll take a look...
Your assert only fires if m_enhanceDisk == false.
Log output: (similar to what your recorded above)
0095CE74: track $17.00 magnet-states 0000 phase 0 off address $C0E0 last-stepper 0.034ms
spin: revs=0, nibbles=1563
read D5AA96 detected - Vol:3C Trk:3C Sec:3C Chk:3C
That "spin..." log msg is from Disk2InterfaceCard::ReadWrite() - basically it's a "fast forward" of 1563 nibbles, and this spin emulation is only done when m_enhanceDisk == false. NB. This is for .DSK/NIB images only - WOZ is handled much more accurately.
But the FormatTrack state isn't being reset here (or at least those 1563 nibbles could be fed one by one into the FormatTrack::DecodeLatchNibbleRead() function). IE, the disk nibble stream has jumped forward 1563 nibbles, so all the FormatTrack vars (eg. m_trackState, m_uLast3Bytes, m_4and4idx) needs to be invalidated.
Calling m_formatTrack.Reset() fixes this specific issue, ie here:
if (!m_enhanceDisk && pDrive->m_spinning)
{
const ULONG nCycleDiff = (ULONG) (g_nCumulativeCycles - m_diskLastCycle);
m_diskLastCycle = g_nCumulativeCycles;
if (nCycleDiff > 40)
{
// 40 cycles for a write of a 10-bit 0xFF sync byte
uSpinNibbleCount = nCycleDiff >> 5; // ...but divide by 32 (not 40)
ULONG uWrapOffset = uSpinNibbleCount % pFloppy->m_nibbles;
pFloppy->m_byte += uWrapOffset;
if (pFloppy->m_byte >= pFloppy->m_nibbles)
pFloppy->m_byte -= pFloppy->m_nibbles;
m_formatTrack.Reset(); <--- call this to invalidate formatTrack state
#if LOG_DISK_NIBBLES_SPIN
UINT uCompleteRevolutions = uSpinNibbleCount / pFloppy->m_nibbles;
LOG_DISK("spin: revs=%d, nibbles=%d\r\n", uCompleteRevolutions, uWrapOffset);
#endif
}
}
@tomcw Thanks for the analysis. If you want to take the 1215-in-the-disk-status-getting-for-sectors-in-prodos branch for a spin ... =P
The reason it is a bigger patch is because I ripped out that hack of sporadically updating the T/S status and have it only update when we actually read a T/C. This way the display is always up-to-date. It is pretty cool to boot disks now and see this change in real-time.
I funneled .woz disks into FormatTrack::DecodeLatchNibbleRead() so it works for them too. I didn't do any performance testing so this may slow .woz disks down?
I also added a new disk logging defined: LOG_DISK_NIBBLES_ADDR in case someone wants to just see the VTSC stuff without ALL the disk nibbles.
I won't get a chance to run AppleWin-Test until next weekend.
Tom when you get a chance could you look at Mockingboard - Crowther and Woods Adventure.woz from Great Hierophant's .WOZ Archive?
Not sure why this assert(track <= 80); is getting triggered in Line 108 of Disk.cpp.
I don't know, but the nibbles in the m_VolTrkSecChk4and4[8] buffer is junk - that nibble sequence doesn't appear on that track (T$0E).
NB. I fixed the code (in the branch) so that DumpTrackWOZ() works again. This is invaluable for debugging woz-related issues.
OK, I fixed it in the branch.
Just a reminder: your use of FormatTrack is beyond the original design, so expect more issues! :-)
Just pulled latest on the branch. Booting DOS 3.3 System Master.WOZ is again showing FF 3F 3F BC for m_VolTrkSecChk. :-/ Will investigate later.
-
pc= 0xc640 -
addr= 0xc0e4
Mini-Log:
T00.50: 17D0->1843, Len=C800
Track seam: T00.50: FF/10 (none)
000BBE5D: track $00.50 magnet-states 0000 phase 1 off address $C0E2 last-stepper 19.280ms
000BBE6C: track $00.00 magnet-states 0001 phase 0 on address $C0E1 last-stepper 0.015ms
T00.00: 2B7D->2AC1, Len=C480
Track seam: T00.00: FF/10 (run=16), start=0000, end=009F
000C0B47: track $00.00 magnet-states 0000 phase 0 off address $C0E0 last-stepper 19.280ms
000C0B56: track $00.00 magnet-states 1000 phase 3 on address $C0E7 last-stepper 0.015ms
read skipped latch data: 07B3 = EE
read skipped latch data: 07B4 = 9F
read skipped latch data: 07B5 = E7
read skipped latch data: 07B6 = F9
read skipped latch data: 07B7 = FE
read skipped latch data: 07B8 = FF
read skipped latch data: 07B9 = FF
read skipped latch data: 07BB = FF
read skipped latch data: 07BC = D5
read skipped latch data: 07BD = AA
read skipped latch data: 07BE = 96
read skipped latch data: 07BF = FF
000C5831: track $00.00 magnet-states 0000 phase 3 off address $C0E6 last-stepper 19.280ms
000C5840: track $00.00 magnet-states 0100 phase 2 on address $C0E5 last-stepper 0.015ms
read skipped latch data: 0A1B = F7
read skipped latch data: 0A1C = BF
read skipped latch data: 0A1D = BF
read skipped latch data: 0A1E = BF
read skipped latch data: 0A1F = BD
read skipped latch data: 0A20 = DE
Assertion failed!
I also set the assert() to trigger sooner:
void Disk2InterfaceCard::SetLastReadTrackSector(int track, int physical)
{
assert(track <= 40);
DumpTrackWOZ() works again.
Oooh, this is REAL nice. Thanks! I (locally) tweaked the output slightly for readability:
0000:ff#2 ff#2 ff#2 ff#2 ff#2 ff#2 ff#2 ff#2 ff#2 ff#2 ff#2 ff#2 ff#2 ff#2 ff#2 ff#2 D5 AA 96 FF FE AA AA AA AA FF FE DE AA EB ff#2 ff#2 ; read D5AA96 detected - Vol:FE Trk:00 Sec:00 Chk:FE
0124:ff#2 ff#2 ff#2 ff#2 ff#2 D5 AA AD B6 DB DC F4 F3 BB BD CF 97 9A AE AE 96 AD AC 9A AB 97 B2 B2 AD AB 9A 9B
022E:AB 9F 97 B3 9A B3 9E 97 9F B3 96 AC AE A7 9A B2 96 AD AC 9B B2 A6 97 9F AF AC A7 AB 97 9F A6 9E
032E:97 9E A7 AF 9E AE 9E AE 9F 9B 97 9B B2 AF B3 AE AC 9A B3 B2 AC A7 AC 97 AB AB BA F4 EA AD 9E E5
042E:D6 FB ED F5 EF EC DA BD 96 96 96 B4 EF B5 EB DC FD F5 EB AB EA B9 FD BE DB FD D7 CD FD E5 B9 FD
052E:B2 AB E6 FC B5 DA EB FC AE FD E5 B9 FD DA DF FA AE FD E7 DA B5 B9 B3 FB 9D FD F9 9D FD AC E6 CE
062E:F6 E9 CB F6 9B F4 BC DA B5 DB FD 9A 9B 97 96 9B 96 97 96 9B 96 97 96 9B 96 97 9B 9B A7 B5 B4 DC
072E:EB DF E6 E6 AB F3 BF E5 F3 FB BF FC CF DE FD D9 FC BF EE FE F9 B9 D9 F9 F3 FB B9 D9 DE E6 DA F3
082E:BF CF EF EC EA AE D9 9E D9 DA 97 EF EF CB AE BC D6 E9 DF FC DB EE F6 9A F4 EE AB BA EB E6 FC F5
092E:B3 DB FF 9A F6 F2 AB EB FD DE D7 BC CD F3 BF F9 AB FD 96 96 96 96 96 96 96 96 96 96 96 96 96 96
0A2E:96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96
0B2E:96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 AF B3 9A DE
Thinking about the current local architecture for FormatTrack, my preference is that this class doesn't call back/out to these functions directly:
m_pCard->SetLastReadTrackSector(m_VolTrkSecChk[1], m_VolTrkSecChk[2]);
GetFrame().FrameDrawDiskStatus();
Instead do it from Disk2InterfaceCard by creating a new wrapper function, eg:
Disk2InterfaceCard::DecodeLatchNibbleReadAndUpdateUI(BYTE latch)
{
m_formatTrack.DecodeLatchNibbleRead(latch)
BYTE trk, sec;
if (m_formatTrack.IsNewVTSC(trk, sec)) <--- new function
{
m_pCard->SetLastReadTrackSector(trk, sec);
GetFrame().FrameDrawDiskStatus();
}
}
Just pulled latest on the branch. Booting DOS 3.3 System Master.WOZ is again showing FF 3F 3F BC for m_VolTrkSecChk. :-/
I fixed this in the branch.
All these issues are when there's a discontinuity (eg. track change or long gap between $C0EC accesses in this case).
I'm trying all the WOZ test images from AppleSauce, here: http://evolutioninteractive.com/applesauce/woz_images.zip
"First Math Adventures - Understanding Word Problems.woz" only shows T$00 (even though if you switch to the debugger, it'll show eg. T$05). Same for "Rescue Raiders - Disk 1, Side B.woz" And "Wings of Fury - Disk 1, Side A.woz" only shows T$01.
Maybe use the physical Track (Phase) for T$ ?
Maybe use the physical Track (Phase) for T$ ?
Yeah, I was thinking about doing that -- just hadn't gotten to it yet -- since it doesn't make much sense to display the Track from the VTSC when we have a physical track. (Plus there has to be a game or two that lies about the track number. I probably should ask qkumba if he has run across any...)
Hi @Michaelangel007 - the work on this issue looks to have stalled, so I've made a quick fix to the main line to fix the regression at 1.30.13.0.
I'll leave this issue open, as you have a branch that looks like a more involved and robust fix (to remove the DOS / ProDOS memory probing).