-
Notifications
You must be signed in to change notification settings - Fork 44
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
Comments
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. |
By default I would also usually go via OPEN, CHKIN, CKOUT, BASIN and BSOUT. |
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 operationsThis 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 statusShould only be done if we decide that we don't want to change the existing behavior. |
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 operationsIt'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 statusI'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:
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. |
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:
WDYT?
The text was updated successfully, but these errors were encountered: