Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Suggestion to fix age-old CBM Kernal bug around Listen, Talk and IOStatus #333

Open
pzembrod opened this issue Aug 29, 2022 · 4 comments
Open

Comments

@pzembrod
Copy link

I ran into this when adapting VolksForth from the R38 to the R41 X16 Kernal. The Kernal version dependencies were all around use of Kernal variables which, for C64 and C16 was perfectly safe but for the X16 isn't as they, for good reasons, can still move from Kernal version to Kernal version.

The right thing to do was, of course, to replace the Kernal variable references with calls to the Kernal API, and in almost all cases I was successful doing that (thanks at this point for the new keyboard API). The one exception I couldn't yet fix was a reference to IOStatus (C64: $90, X16: currently $0289), for the purpose of clearing it before calls to LISTEN and TALK, respectively. As it turns out, IOStatus is not cleared at the beginning of these two Kernal routines, so one must clear it manually before calling them, or IEC error detection won't work properly. And it turns out there's no way to clear IOStatus in a reasonable way through Kernal API calls.

I have therefore two proposals:

  1. Fix this old Kernal bug: Clear IOStatus at the beginning of LISTEN and TALK.
  2. Add an API call to clear IOStatus. It seems kinda wrong that one cannot do this.

WDYT?

@BruceMcF
Copy link

BruceMcF commented Sep 1, 2022

I cannot off the top of my head imagine why I would want to use the Kernel calls at the LISTEN and TALK level, which tells me that my preference is for the first solution, since if I AM doing that, it was likely something that caught me by surprise, and the simpler it is to use these Kernel functions, the better.

@pzembrod
Copy link
Author

pzembrod commented Sep 7, 2022

By default I would also usually go via OPEN, CHKIN, CKOUT, BASIN and BSOUT.
However, VolksForth, as it was designed on the C64 35 years ago, offers a LISTEN/TALK based API/wordset consisting of busopen, busclose, busin, busout, bus@, bus!, bustype and businput. And this API is actually very convenient. An advantage it has over OPEN/CLOSE based channel I/O is that you can mix screen and file I/O without any need for switching between channels; I've grown to appreciate that, I must say.

@mist64
Copy link
Collaborator

mist64 commented Oct 24, 2022

Of course, I'm all for cleaning up the API so that it is generally useful without resorting to zero page variables! And this problem seems like something that needs fixing.

The two solutions you're mentioning differ philosophically, so I think we need to be really careful when deciding which route to go.

Clearing the status before all operations

This sounds clean, but has the side effect that the API can no longer be used to accumulate errors. I am not sure whether it was designed like this on purpose, or used by anyone like this, or even useful at all, but: What if I invoked several of these API calls, and wanted to test whether any of them failed?

The X16 isn't supposed to be a C64 clone or C64 compatible, but I would want reasonable compatibility between well-written PET/VIC/TED/C64/C128 assembly code and the X16. Is it reasonable to rely on the accumulating side effect of the status byte?

Add an API to clear the status

Should only be done if we decide that we don't want to change the existing behavior.

@pzembrod
Copy link
Author

Thanks for your considerations, and I agree that the 2 proposals differ philosophically. I am not sure whether I would consider them alternative solutions; I think I at least initially thought of them more as complementing each other than as alternatives. But that is of course only a partial view of things. Here are my thoughts regarding the two options:

Clearing the status before all operations

It's really just at the beginning of LISTEN and TALK that I would clear the status byte. I think it is quite reasonable to expect a sequence of LISTEN, SECOND, CIOUT and UNLSN or a sequence of TALK, TKSA, ACPTR and UNTLK to accumulate errors in the status byte, as they together each form one semantic operation on the bus. But I can't really think of a reasonable situation where I would want an error from e.g. one TALK+ operation to carry over to the next one. If I were to wish for such a behaviour, I would in truth rather consider myself trying to take a shortcut I shouldn't take, i.e. implicitly aggregating the status outcomes of two operations. And would I want the second TALK+ to happen if the first one failed? Or not? This would surely be a decision to be taken on a case by case basis, and is not a decision that should be delegated to the behaviour of a set of API functions; rather, I should then be constructing my if-else sequence according to my desired error behaviour.

Therefore my suggestion to clear the status byte at the beginning of LISTEN and TALK, and only of those two.

I would, however, also suggest that we try to reach out to some of the old Commodore engineers such as are still around, and run this past them; maybe they care and maybe they have some interesting insights; I would be willing to take that task.

Add an API to clear the status

I'm on the fence regarding this one. Of course, if we decide against the above-mentioned LISTEN/TALK change, then I'd need this new API to be KERNAL var independent. But what if we do change LISTEN and TALK? Then on the one hand not being able to reset the status byte that you later want to check does feel like it leaves room for trouble. On the other hand adding an API call that one is normally not expected to need could also lead to unnecessarily long code, if e.g. everyone starts to call SETSTAT before each LISTEN/TALK or maybe even before each OPEN call. So that API call would need to come with a clear caution in its documentation.

Let's examine some potential cases:

  • Assume someone calls LISTEN which returns a non-zero IOStatus. Could that someone then want to still call SECOND and then get a fresh IOStatus, i.e. reset IOStatus before the SECOND call and get a clear status value only of the SECOND call, independent of the LISTEN call? Seems not too likely to me.
  • How about a call to ACPTR returns an EOF flag in IOStatus. Could one want to call ACPTR again and expect a guaranteed fresh value of the EOF flag? In principle I could see that happen e.g. with the new Seek() equivalent that I think I saw in the pipeline elsewhere. But, IIUC that Seek() would be implemented by sending something to the command channel, which in turn would require an UNTLK on the open file channel, a LISTEN etc. on the command channel and then a TALK again on the open file channel, so the next ACPTR on that channel would profit from the IOStatus reset by the TALK call.

So overall it looks to me at the moment more like an YAGNI (you ain't gonna need it), but I would keep the idea on the back burner in a discoverable way, so that if a legit use case that needs it after all arises, the idea can be picked up again.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants