open-simh / simh

The Open SIMH simulators package
https://opensimh.org/
Other
479 stars 90 forks source link

RK05 image created by SIMH will not attach properly - complains of non-existent parameter RK05 #81

Closed decuser closed 10 months ago

decuser commented 2 years ago

(base) nebula:open-simh-newest wsenn$ BIN/pdp11 PDP-11 simulator V4.0-0 Current git commit id: 65410851 sim> att rk0 rk0 %SIM-INFO: RK0: Creating new file: rk0 sim> q Goodbye

(base) nebula:open-simh-newest wsenn$ BIN/pdp11

PDP-11 simulator V4.0-0 Current git commit id: 65410851 sim> att rk0 rk0 %SIM-ERROR: RK device: Non-existent parameter - RK05 sim> q Goodbye

expected behavior: (base) nebula:simh-newest wsenn$ BIN/pdp11 PDP-11 simulator V4.0-0 Current git commit id: 2673530d sim> att rk0 rk0 %SIM-INFO: RK0: Creating new file: rk0 sim> q Goodbye (base) nebula:simh-newest wsenn$ BIN/pdp11 PDP-11 simulator V4.0-0 Current git commit id: 2673530d sim> att rk0 rk0 sim>

actual behavior:

(base) nebula:open-simh-newest wsenn$ BIN/pdp11 PDP-11 simulator V4.0-0 Current git commit id: 65410851 sim> att rk0 rk0 %SIM-INFO: RK0: Creating new file: rk0 sim> q Goodbye

(base) nebula:open-simh-newest wsenn$ BIN/pdp11

PDP-11 simulator V4.0-0 Current git commit id: 65410851 sim> att rk0 rk0 %SIM-ERROR: RK device: Non-existent parameter - RK05 sim> q Goodbye

pkoning2 commented 2 years ago

Does it work if you ask for ./rk05 ?

decuser commented 2 years ago

No. It's the same error: PDP-11 simulator V4.0-0 Current git commit id: 65410851 sim> att rk0 rk0 %SIM-ERROR: RK device: Non-existent parameter - RK05 sim> att rk0 ./rk0 %SIM-ERROR: RK device: Non-existent parameter - RK05

I also tried with the full absolute path, same issue.

markpizz commented 2 years ago

Try with the simulator at the https://github.com/simh/simh repo

decuser commented 2 years ago

@markpizz - the most recent there works as expected, it looks like you have made a couple of commits related to pdp11_rk.c in the last couple of months (between Feb and now).

decuser commented 2 years ago

also, after reading the help more carefully and digging into it, it looks like it's actually attaching, so maybe it's just an erroneous error?

PDP-11 simulator V4.0-0 Current git commit id: 65410851 sim> show rk0 RK0 1247KW, not attached, write enabled RK05, autosize, AUTO detect format sim> att rk0 rk0 %SIM-ERROR: RK device: Non-existent parameter - RK05 sim> show rk0 RK0 1247KW, attached to rk0, write enabled RK05, autosize, RAW format

pkoning2 commented 2 years ago

I don't know what causes that particular defect, but the trigger is the presence of the MarkP metadata appendage.

Do "zap rk0" to remove that; the message should not appear afterwards.

decuser commented 2 years ago

Hmm.. something up with autosize is my guess. I worked around it with:

(base) nebula:v6 wsenn$ pdp11

PDP-11 simulator V4.0-0 Current git commit id: 65410851 sim> set rk0 en noautosize sim> att rk0 rk0 %SIM-INFO: RK0: Creating new file: rk0 sim> q Goodbye (base) nebula:v6 wsenn$ pdp11

PDP-11 simulator V4.0-0 Current git commit id: 65410851 sim> set rk0 en noautosize sim> att rk0 rk0 sim> show rk0 RK0 1247KW, attached to rk0, write enabled RK05, noautosize, RAW format sim> q Goodbye

Dunno if this is ok, but it works?

decuser commented 2 years ago

Is the metadata stuff what causes: %SIM-INFO: and %SIM-ERROR: That is present by default isn't it? Should it normally be disabled?

markpizz commented 2 years ago

A while back, there was user confusion between informative messages vs error messages. I added the display of %SIM-INFO for informative messages and %SIM-ERROR for errors.

In general messages can be suppressed if the command that caused them is invoked with the -Q switch.

al20878 commented 1 year ago

AFAICT, the problem here is that when AUTOSIZE is on, sim_disk_attach_ex2() tries to iterate over all disk types passed in the last parameter (for RK, there's only "RK05" in that array), and to issue the "SET RKn RK05" command internally. (Also, at the end, it tries to revert to the original drive type, which was also passed as "RK05", in a dtype parameter.) The RK module does not recognize such a SET command, even though it recognizes "SHOW RKn TYPE" one. I think there's a logic error in using the drive types in this particular case. Or, at least it should have checked that the "dtype" was matching the entry from "drivetypes" and not attempted to use it -- since it's already the current drive type.

Anyways, the AUTOSIZE logic in that function is extremely convoluted, so no wonder it just back fires at the corner cases.

pkoning2 commented 1 year ago

The obvious answer is that autosize is not meaningful, and should not be done, for devices that come in only one size. Unless someone adds RK02 support, that is true for the RK emulation. One reason why this makes sense is because for single-size devices, SET is not useful and unlikely to be implemented.

Interesting. It seems to be the only PDP-11 disk controller for which that is true. (If we add Pro support, it would also be true for the Pro floppy controller.)

al20878 commented 1 year ago

@decuser : can you please check that the issue has been addressed in the current codebase? Thx

decuser commented 12 months ago

I've not been following this lately, but sure, I'll check it out in the next day or so.

decuser commented 11 months ago

Oops. This completely dropped off my radar. Yes, it works now and appears to be fixed in the current build

PDP-11 simulator Open SIMH V4.1-0 Current git commit id: cc76d9a7

al20878 commented 10 months ago

Thanks for checking!

So I suppose this can be closed then?