Various REL-file bugs (was: Not reporting invalid byte positioning in REL file)

[snhirsch]

This issue occurs with both .r00 files in the host directory and mounted disk images. I have a REL file copy program that determines record length by performing a simple linear search from 1..254, issuing a 'p' command for each value and stopping when an error is reported. On all physical devices (4040, SFD1001, Hardbox) this works fine, but XD2031 never throws an error and leads the program to always conclude the record length is 255.

Note I'm working exclusively with the os9-173 branch.

[fachat]

can you try branch 177-relpos?

[fachat]

Should now also fix the timeout bug reading REL files (65ms timeout when XD2031 is not quick enough to read a record)

[snhirsch]

Ah, sorry to have let this languish. I've been knee-deep in hardware troubleshooting (Hardbox). Will try the new code in the next day or so. Thanks for looking at the problems!

[snhirsch]

The record length is now properly respected and I'm able to detect it. However, reads are still glacially slow (probably 10x slower than SEQ or PRG) and I hit timeouts almost instantly. Doesn't seem different from earlier code in this respect.

[fachat]

Hm, would you be able to run the program with a patched VICE emulator as described here https://github.com/fachat/XD2031/blob/177-relpos/fwtests/README.vice and then send me the IEEE488 trace of it?

[snhirsch]

The patch I see is for VICE 3.4, which is the version I'm using. However it does not apply cleanly. Even after fixing some obvious things I'm getting a compile error:
Making all in parallel parallel.c: In function ‘WATN_ATN_true’: parallel.c:164:35: error: ‘parallel_debug’ undeclared (first use in this function); did you mean ‘parallel_emu’? #define Go(a) state=(a);parallel_debug?log_warning(LOG_DEFAULT,"Go->%d\n",(a)):0;return ^ parallel.c:225:5: note: in expansion of macro ‘Go’ Go(In1); ^~

[snhirsch]

If you are interested in the program behavior, I can elaborate. Currently using a modified version of CBM Commander. The copy function is doing:

while (not input eof) {
  read entire record (per discovered length) from input
  write entire record to output
}

Reads and writes are performed in "burst" mode using the CC65 runtime cbm_read and cbm_write functions.

In particular, I am not using any positioning commands on input or output as it is not required with any real CBM drive I've encountered (nor with the Hardbox). All real drives automatically advance to the next record.

[fachat]

The patch I see is for VICE 3.4, which is the version I'm using. However it does not apply cleanly. Even after fixing some obvious things I'm getting a compile error:
Making all in parallel parallel.c: In function ‘WATN_ATN_true’: parallel.c:164:35: error: ‘parallel_debug’ undeclared (first use in this function); did you mean ‘parallel_emu’? #define Go(a) state=(a);parallel_debug?log_warning(LOG_DEFAULT,"Go->%d\n",(a)):0;return ^ parallel.c:225:5: note: in expansion of macro ‘Go’ Go(In1); ^~

Hm, just downloaded a fresh copy, applied the patch (with "patch -p1 < .../patchfile" as described in README.vice - note the -p1 option), did "configure --disable-hwscale" (the option only for some unrelated compile issue with my kubuntu linux), and successfully compiled it.

Can you pls re-check and if the error persists, create a new issue for it where this can be discussed separately?

[fachat]

If you are interested in the program behavior, I can elaborate. Currently using a modified version of CBM Commander. The copy function is doing:

while (not input eof) {
  read entire record (per discovered length) from input
  write entire record to output
}

Reads and writes are performed in "burst" mode using the CC65 runtime cbm_read and cbm_write functions.

In particular, I am not using any positioning commands on input or output as it is not required with any real CBM drive I've encountered (nor with the Hardbox). All real drives automatically advance to the next record.

As I cannot re-create it so far, this is my assumption:

• the read works that it's a TALK/SECTALK, then GETBYTE until EOF, then UNTALK.
• after writing to another drive/channel, then start the loop again, i.e. TALK/SECTALK, GETBYTE until EOF, UNTALK, and repeat

This does not work in BASIC, so I'll have to create a separate testcase for it.
Edit: maybe if I use INPUT#....

[snhirsch]

On Mon, 26 Oct 2020, A. Fachat wrote: In particular, I am not using any positioning commands on input or output as it is not required with any real CBM drive I've encountered (nor with the Hardbox). All real drives automatically advance to the next record. As I cannot re-create it so far, this is my assumption:

Why would you expect to recreate the problem under Vice? I must be missing something.

* the read works that it's a TALK/SECTALK, then GETBYTE until EOF, then UNTALK. * after writing to another drive/channel, then start the loop again, i.e. TALK/SECTALK, GETBYTE until EOF, UNTALK, and repeat

Yes, that's what would be going on under the covers - exactly.

This does not work in BASIC, so I'll have to create a separate testcase for it.

I'm not quite parsing that statement. What doesn't work in BASIC?

…

--

[snhirsch]

    The patch I see is for VICE 3.4, which is the version I'm using. However it does not apply cleanly. Even after fixing some obvious things I'm getting a compile error:
    Making all in parallel parallel.c: In function ‘WATN_ATN_true’: parallel.c:164:35: error: ‘parallel_debug’ undeclared (first use in this function); did you mean ‘parallel_emu’? #define Go(a) state=(a);parallel_debug?log_warning(LOG_DEFAULT,"Go->%d\n",(a)):0;return ^ parallel.c:225:5: note: in expansion of macro ‘Go’ Go(In1); ^~

Hm, just downloaded a fresh copy, applied the patch (with "patch -p1 < .../patchfile" as described in README.vice - note the -p1 option), did "configure --disable-hwscale" (the option only for some unrelated compile issue with my kubuntu linux), and successfully compiled it.

Can you pls re-check and if the error persists, create a new issue for it where this can be discussed separately?

Before we go any further, can you be specific about exactly what you downloaded and from where? I am working with the HEAD revision of VICE 3.4, which is r38822. There are many applications with 'fuzz' and a handful of hard fails. Something tells me you're working with a different revision of VICE. I'm referencing this URL for checkout: https://vice-emu.svn.sourceforge.net/svnroot/vice-emu/

((note: sorry I clicked "edit" instead of "reply quote" ... no idea why it actually lets you do that.... trying to restore -- André))

[fachat]

Before we go any further, can you be specific about exactly what you downloaded and from where? I am working with the HEAD revision of VICE 3.4, which is r38822. There are many applications with 'fuzz' and a handful of hard fails. Something tells me you're working with a different revision of VICE. I'm referencing this URL for checkout: https://vice-emu.svn.sourceforge.net/svnroot/vice-emu/

I'm working on the 3.4 release version, as downloaded from the sourceforge project web site. I'm sorry I don't have the time to follow every commit...
https://sourceforge.net/projects/vice-emu/

[fachat]

Why would you expect to recreate the problem under Vice? I must be missing something.

Not recreating the problem - but the access pattern from the programs you mentioned, that trigger the problem on the real hardware. To record and be able to replay on XD2031, like all the other test cases (and maybe even find something to check for the problem)
See my mail. Will have a look later today.

• the read works that it's a TALK/SECTALK, then GETBYTE until EOF, then UNTALK. * after writing to another drive/channel, then start the loop again, i.e. TALK/SECTALK, GETBYTE until EOF, UNTALK, and repeat
  Yes, that's what would be going on under the covers - exactly.
  This does not work in BASIC, so I'll have to create a separate testcase for it.
  I'm not quite parsing that statement. What doesn't work in BASIC?

If you're using GET# it will pull each byte as TALK/SECTALK/GET/UNTALK sequence.
Only INPUT# (as noted in the P.S. above) pulls in multiple bytes, only with a different end condition (CR instead of EOI).

Created some tests with INPUT# and already found some, say, interesting, results even without looking at the timing... Will have a look here too

…

[snhirsch]

Before we go any further, can you be specific about exactly what you downloaded and from where? I am working with the HEAD revision of VICE 3.4, which is r38822. There are many applications with 'fuzz' and a handful of hard fails. Something tells me you're working with a different revision of VICE. I'm referencing this URL for checkout: https://vice-emu.svn.sourceforge.net/svnroot/vice-emu/

I'm working on the 3.4 release version, as downloaded from the sourceforge project web site. I'm sorry I don't have the time to follow every commit...
https://sourceforge.net/projects/vice-emu/

I had a feeling that might be the case. Unfortunately the release version has itself some serious issues with REL file handling.

[snhirsch]

I downloaded and built the release of 3.4. Patch applies cleanly and it built with no problem. I have not tried running in the xd2031 test framework yet, but my modified CBM Commander is working correctly in it. I run CBM cmd from a D64 image and can copy files around to my heart's content. It's very odd that it doesn't work for you.

[fachat]

I downloaded and built the release of 3.4. Patch applies cleanly and it built with no problem. I have not tried running in the xd2031 test framework yet, but my modified CBM Commander is working correctly in it. I run CBM cmd from a D64 image and can copy files around to my heart's content. It's very odd that it doesn't work for you.

Which PET model and disk drive emulation are you using?

[snhirsch]

I am running 'xpet' with SuperPET specified in the configuration dialogs. I find the Vice drive dialogs extremely confusing, but here's how it's setup:

• Peripheral --> Drives --> Drive 9: True Drive, 4040, Disk Image
• " -> Filesystem Device --> Drive 9: Directory where the D64 lives

Then I enter Alt-9 and double-click on the D64 image file. I start CBM Commander from Device 9 and it works fine - reporting the disk as CBM DOS 2.0.0. If you have a specific command line you'd like me to use, I'll try that.

[fachat]

I don't think that works - maybe in the upstream VICE, but not in 3.4, as there the dual drives are not emulated fully (drive 1 on unit 8 takes the place of drive 0 on unit 9 - this should be fixed in the next release thanks to the great VICE team).

I now can use
xpet -drive8type 4040 -drive10type 4040 +sound -superpet -8 cbm2.d64 -10 cbm.d64
and this allows me to change drives, select, delete files, etc - however, I simply cannot read a file using neither "5", "c", "C" nor "return" when the cursor is on the file.

[fachat]

Could you in the meantime try the 177-* branch again on the real hardware? I fixed a couple of read/write bugs with rel files, so I would like to see if your problem persists.

[snhirsch]

On Tue, 27 Oct 2020, A. Fachat wrote: I don't think that works - maybe in the upstream VICE, but not in 3.4, as there the dual drives are not emulated fully (drive 1 on unit 8 takes the place of drive 0 on unit 9 - this should be fixed in the next release thanks to the great VICE team). I now can use xpet -drive8type 4040 -drive10type 4040 +sound -superpet -8 cbm2.d64 -10 cbm.d64 and this allows me to change drives, select, delete files, etc - however, I simply cannot read a file using neither "5", "c", "C" nor "return" when the cursor is on the file.

I'll take your word for it. I'm unable to get xpet from release 3.4 to work. I have no idea which specific ROMs it wants, but if I add

…

-directory ../data/PET:../data/DRIVES:../data/common it will start. Once started I get a rapidly flashing cursor and the UI refuses to take keyboard input. This is one of the many reasons I'm using the development version.

--

[greg-king5]

I'll take your word for it. I'm unable to get xpet from release 3.4 to work. I have no idea which specific ROMs it wants, but if I add "-directory ../data/PET:../data/DRIVES:../data/common", it will start. Once started, I get a rapidly flashing cursor, and the UI refuses to take keyboard input. That is one of the many reasons I'm using the development version.

If you copy the xpet executable file into the "data/" subdirectory, then you should be able to start it from there without that "-directory" option.

I remember that vsync speed instability. I think that I managed to get VICE to settle down, sometimes, by clicking on the speed widget in the statusbar. I switched the Maximum speed to "Unlimited" for a moment. Then, I changed it back to "100%".

[snhirsch]

Thanks, Greg. I was able to get VICE cooperating by following your advice and can duplicate what Andre' reported. The misbehavior is almost certainly a bug in VICE 3.4 (release), since the code in question works on hardware and on current development VICE.

[fachat]

I've pushed an experimental IEEE488 trace patch for VICE r38891 (HEAD). I think it needs to be applied with "patch -p0".
Both the revision you used (r38822) and HEAD (r33891) don't compile for me on my system (see e.g. https://sourceforge.net/p/vice-emu/bugs/1342/ ).

[fachat]

Ok, I've now tried the patch, but I found that the IEEE488 handling for truedrive disk emulation has changed, and I have no idea how to fix it. Maybe with help of the VICE team. So no IEEE488 traces after 3.4 release for now.

[snhirsch]

I tried copying out a REL file from a D82 image and am still getting instant IEEE bus timeouts. If I suppress the timeout it will eventually complete but is extremely slow. I don't really see much of a change, to be honest. SEQ and PRG file throughput seems fine.

[fachat]

Hm, without a re-creatable test case that is becoming difficult.

Can you see from some logs whether the timeout happens immediately after the open (basically the first record to read) or later?

I have changed something to read the next record when the end of the previous record is reached. This should(*) prevent timeouts when trying to read the next record. Can you check again?

As for the speed ... I think that comes because each record read or write has two round-trips, one to set the position, the other to transfer the buffer. Changing this will probably take longer, so I'm focusing on the timeout right now.

(*) again, without means of verification

[snhirsch]

A thought: Could you arrange for the firmware on the PetSD to toggle an otherwise unused output just prior to bus activity of interest? I could use that to trigger the logic analyzer and avoid having to dig through a lot of wiggles. Unfortunately the analyzer software presents IEEE data bus state in hex with no ASCII translation. Makes it a royal PITA to interpret.

[snhirsch]

Found another issue. I tried copying a large REL file into a D82 image mounted as u10,d1 in xd2031. Besides being slow (required turning off IEEE timeouts), the target file is badly corrupted and a bit shorter than it should be.

[fachat]

Bummer. I was looking into such a problem on XS1541 in Issue #178 - I guess I have to change the title of that one as you are using a PetSD.

[fachat]

I have been looking through this thread and it's unclear to me what is left unfortunately. I see these points still open:

1. slow speed of REL file operations
2. missing bytes when copying a large REL file
   I would, if you agree, close this issue and open two separate new issues. Is anything else missing?
   @snhirsch @greg-king5

[snhirsch]

Both of those issues are still in evidence here, so would be good to move them to open topics.

[fachat]

replaced by #179 and #180