Closed gardners closed 9 months ago
The PETSCII scanner does retrurn 1,2,4,8,... for shift, mega, ctrl, alt, and 255 for no-scroll. This is contrary to the ASCII scanner implementation. Was this a conscious decision?
At least once I got a simple peek poke loop on the petscii scanner stop working by pressing shift-1 to 9. Something is still off there.
D619 PETSCIIKEY was supposedly added last year (including to iomap and documentation), but when I PEEK($D619)
from BASIC in a loop it only returns 1 no matter what key is pressed, in 683-cartflash,20230612.07,cae3b0b. I see this issue is still open so maybe it is known that this isn't working yet. I'm just posting an update because issue comments elsewhere imply that this was completed.
10 PRINT PEEK($D619):GOTO 10
For what it's worth, I'm only getting 63 out of PEEK($D610)
. Can anyone repro an issue there?
@dansanderson
It works like the good old $D610
, ie, needs to write any value back (just the write event counts not the actual written value) to $D619
to signal to "pop up" the item from the queue so you'll read the next. Also at the beginning at your program you should clear the queue by writing some value to $D619
till it reads empty to make sure no old keys are stored in the queue still.
So $D610
and $D619
works almost the same just the meaning of read data (ASCII vs PETSCII) is different.
GS $D610 UARTMISC:ASCIIKEY Last key press as ASCII (hardware accelerated keyboard scanner). Write to clear event ready for next.
GS $D619 UARTMISC:PETSCIIKEY Last key press as PETSCII (hardware accelerated keyboard scanner). Write to clear event ready for next.
Note about the Write to clear event ready for next. part.
I guess this is a good demonstration (though it lacks the initial clear the queue part, what you can see when you start the program and you'll see values popping up, they were queued before, this also shows the need of emptying the queue at the beginning of the program to start from empty state):
10 A=PEEK($D619)
20 IF A=0 THEN 10 : REM no event queued if zero is read
25 REM there is some queued event
30 PRINT A
40 POKE $D619,0 : REM clear event ready for next (only the write event itself counts not the data written!)
50 GOTO 10
Your one-liner won't work, since it never issues a write event to $D619
, so always reads the same value.
The problem is not that it does not work, just that it works differently when it comes to the modifier keys, which are in D611, and are not giving keypresses in D610. But it does with D619 (see my last comment).
wow, very cool! It only took me a minute to write an assembly program to compare typing speed and accuracy between the kernel's getin
and $d619. Hardware accelerated is much more accurate, as expected. I'm impressed that it even does its own key-repeat handling. (I wonder if we need to tweak delays to match a bit better. Wishlist item: configurable key repeat delay! :) )
I need to mess with it a bit more before it's a drop-in replacement for keyboard_scan
—as you mentioned the modifier keys need handling—but it's very promising so far.
Is there a pattern for the d619 values for keys with modifiers, or does this need improvement?
I don't know how to interpret the d619 return values for other modified keys:
Should I be making a chart of all of the return values, or does this represent an issue that needs improvement before programs base any logic on it? Certainly Shift + 5 not emitting a key event is an issue.
Btw, IIRC $D611
bit 7
can be set/reset to enable/disable the modifiers. As far as I remember it works in a way, that with disabled modifiers, the returned value in both of (?) $D610
and $D619
(ASCII and PETSCII "hardware accelerated keyboard scanner") returns only the base key, ie a
even if shift+a
is pressed, though reading the modifier keys can be done without change. So a program can choose if they want a "fully decoded" version (eg, a
or A
depending on a
or shift-a
is used) or only the "base key" is returned. I am not sure, however, how accurate my description is. Again, reading the modifier keys (by reading $D611
) should work anyway regardless of modifier being enabled or disabled.
This is also shows btw, that programs using $D610
or $D619
should not only empty the queue before starting to use the hardware accelerated keyboard scanner functionality, but they should always set/reset the modifier disable/enable bit to their needs, otherwise surprise can happen if previous programs alters that bit, and it's not what the program's author had in their mind.
The problem is that the matrixpetscii* tables are known to be wrong, and need fixing.
The $D615-$D617 virtual key facility could be used to write an automated test for to check the mapping.
I have located the bug with the matrix_petscii_shifted
table and am preparing a PR. I have tested every key combination and compared them to the output of the kernel's getin
to confirm that they match as much as possible.
I propose a change to the D619 codes emitted when modifier keys are pressed. These should return FF, similar to other keystrokes that have no PETSCII equivalent. Previously they were returning 01, 02, 04, and 08, but these overlap with Ctrl-A, Ctrl-B, Ctrl-D, and Ctrl-H, and distinguishing between them would require detecting changes to D611. A caller of D619 has no use for a non-PETSCII code that identifies a keypress of modifiers, in part because there is no event emitted on releasing the key. I believe the caller can simply read D610 (without consuming a buffer byte) when it sees FF on D619 if it cares what it was. (I'll test this.)
I understand that D619 must emit a code for each modifier key because of how this is implemented, and I think it's fine to ask callers to ignore FF bytes and proceed with processing the buffer. I agree it should be a non-0 byte because some programs might want to process the entire buffer before moving on.
Below is the full spec for D619 as I understand it. I'll document our final result in a new chapter I'm writing on hardware interfaces.
getin
routine translates function key presses to KEY macros. Naturally, D619 does not do this: it emits the PETSCII code for the function key.OK, so far so good. A few final things:
I'm not sure any of these matter to callers of D619. Any program that needs a more precise understanding about which keys are pressed can use the other registers. For example, a text editor probably wants to know about modifiers + cursor keys to offer advanced text navigation. Such a program probably needs D610 and D611 for that.
(Added from discord discussion) D610 does not emit keyboard events if only a modifier key is pressed. Only a combination will do that. Modifiers are found in D611. I suspect that the function to emit keypresses in D619 for modifiers only was added on request, so that the ROM can always look for a keypress.
As nobody is using the feature yet, we can decide which way we go:
D619 would replace most of keyboard_scan
, an IRQ routine that populates the kernel's keyboard buffer that feeds the default input stream. So far, I've been assuming that keyboard_scan
would continue to use other methods, such as D611 and CIA lines, for whatever it does other than populate the buffer. At first glance, I don't see how D619 emitting bare modifier key-down events would make this function faster or simpler, especially when the currently held modifier keys are on D611. I'll see if I can do a more intensive code review of keyboard_scan
tomorrow, and also dig into the BASIC screen editor to see if it's doing anything outside of getin
/keyboard_scan
on its own.
There are a few other places in the ROM where CIA lines are tested directly to detect a down key, such as several tests for Run/Stop. These would not use d619.
A few things keyboard_scan
does outside of PETSCII translation and key repeat:
shflag
, only used in a couple of places; powers an interesting feature involving booting from disks while holding the Alt keyThere appear to be two kernel extension vectors within keyboard_scan
that might be difficult to support as written. I'm not shy about suggesting that we rewrite the extension API to fit our needs, considering that it's extremely unlikely that anything depends on them. The ability to add keyboard handlers to the BASIC editor is a great idea; I've always wanted to build a BASIC help system that activates when the user presses the Help key from the editor. The provided extension points appear to be a good way to do that, and if we nix these we should consider designing new ones.
@dansanderson Also worth to mention (for me) that speaking about emulation, it's kinda hard to do "load PRG file" and things like that. C64 emulators usually do this by having a trap on a certain ROM address, however that assumes a ROM which never changes. So my approach is kinda a complex, I force RESET, then I try to catch when the screen memory contains the "READY." prompt (and waiting for some cursor blinks on the next line to be sure). So this way I am independent of the ROM, and I only rely on the visual screen content more-or-less. Then I overwrite video RAM on the next line with the command I want, and I simulate a RETURN keypress. You see, it is kind of complicated and error-prone process. However if the ROM had ability to "inject" keypresses more easily, then it can be used by the emulator as well for this purpose. Surely it's useful for anything else too. It was just a quick thought of mine, maybe not worth the trouble, but this has just come into my mind.
OK, I now understand the idea behind having D619 emit an event for a modifier key. It has to do with preserving the original extension API within keyboard_scan
. My preference is for d619 to only emit events for characters and not modifier keys, and redesign the kernel extension APIs.
The current implementation detects either a character key or a non-character key, sets up variables, then follows the keyvec
extension vector if a key press is detected, once per IRQ. An extension has the opportunity to override the key index before returning control to kernel code that tests for No Scroll, Mega + Shift, and Ctrl-S. This vector is called once per IRQ, if and only if a key is being pressed. (Modifier keys are stored in a kernel variable, and this variable would have to be formalized to allow keyvec
to override modifiers. It can override the key index via the accumulator.)
We're no longer dealing in key indexes, so we're changing the keyvec
API regardless. With d619 emitting a separate event for each modifier key-down, we can support an API with a similar call pattern. For each key event, keyvec
is invoked with A=d619 and X=d611. The extension can read and modify the CPU registers, and those modifications are honored by the rest of keyboard_scan
. (Note that d619 should still return FF for all modifier key presses in this design. There's no reason for modifier keys to emit key codes that collide with Ctrl combos.)
I would prefer that d619 not emit events for modifier keys, and design a different extension API in a similar spirit. For example, we could have two extension points, one for overriding modifier keys once per IRQ, and one for overriding each PETSCII code emitted by d619, in a tight loop that consumes the entire hardware queue per IRQ. There is already a separate extension point for characters (keychk
) that takes place after the check for Ctrl-S, but I don't know if that's a useful distinction and maybe we delete that one.
One thought I would add: D610/D619 is a queue of 5 key events. D611 is the momentary state of the modifier keys.
So mixing those two up might have not the same effect as making modifier key press events real key events.
True. I wasn't thinking the lesser option would try to associate an FF with a d611 value. If we really wanted to return events for modifier key-downs, we'd have to pick some unused byte values to represent the modifier keys, but there are actually quite a few available, surprisingly. d619 won't return anything greater than DF, and there are even a bunch lower than that also unused by d619.
For keyboard_scan
purposes, it should be sufficient to read d611 once per IRQ for No Scroll and Mega + Shift, then separately process only real PETSCII chars from d619 to populate the kernel key buffer (including testing for Ctrl-S). I have a partially-working version so far that does the first part correctly.
Still trying to figure out why d619 isn't emitting every key event to the IRQ handler the way it does to my test program. 😕
Here's the general idea: https://github.com/MEGA65/mega65-rom/commit/00c9c74f0fe04418123120b63ec02184577e138d
(The tight d619 reading loop is not working, but I'm confident this is a ROM issue and not a core issue. I can only get key events out of the loop if I mash a bunch of keys really quickly, as if it's competing with something to consume d619 events. This is the only place in the kernel that touches it, which I know from the fact that the same loop in a user program works fine. There's something about this IRQ handler needing to be re-entrant, which is what keyboard_lock
is for. I'll keep messing with it...)
I'm glad I dropped the link! lydon fixed me. I had assumed that a null value needed to be cleared with a sta d619, but it actually goes non-null on its own.
I now have a working fast-typing ROM. This version doesn't need d619 events for modifier keys; it just ignores FF. (It does need all mods to be FF. With the dev core that emits 1, 2, 4, and 8, it confuses those for Ctrl sequences that do things like change the color.)
We should tweak the repeat delay a bit, it's slower than the ROM at the moment. I'll see if I can find an appropriate value and add it to my PR.
I'll keep testing with the new ROM, and also see how much ROM code I can delete.
Heads up, we need a spec change: d619 must return FF for "no event" because Ctrl-At is PETSCII 0. It will otherwise work the way 00 does for d610: reads will return FF until there is a character to consume, then reads will return the next character. Writing to d619 will advance to the next character in the queue, and when all characters are consumed, d619 will return FF again. I'll work on this core change.
Repro: Type PRINT "
followed by Ctrl-@. The working version should emit a reversed @ character.
To be really honest, I was always a bit puzzled about the idea to having both of ASCII and PETSCII "accelerated hardware keyboard scanner". I mean, there was the ASCII one first. OK, we need PETSCII, but it's kind of trivial programs to have a table to convert themselves. My fear here: if there is a bug in the core what values should be returned (or some design problem like with the zero here, where there is not even possible to have a change which does not break compatibility with other programs!) the fix means to change core, and possibly break other programs then. When with "DIY convert" you modify your conversion table. But really it's only a thought, just I think it's possible to always find something which would require to have core modification (or even behaviour change like now 00->FF). Surely this is not even exactly correct what I've said, as it assumes ASCII one is already perfect and have all the keys/etc which is not so much true either ;) So probably it's not far from me to say this.
Several good points, yeah. The hardware "acceleration" comes from the hardware key buffer, not so much the interpretation. In general, moving a feature from the ROM to the core exposes it as a low-level API surface and makes it more difficult to develop.
I can imagine a different design that enqueues 16-bit key events (a key code with modifier bits) and lets callers translate as needed. That would give the most flexibility and still offload a useful chunk of typing-style key handling logic. Neither ASCII nor PETSCII can represent strokes like Ctrl-CursorLeft, which you might want when implementing a modern-style text editor. d619 presents Ctrl-CursorLeft as "no key." We could change this to "CursorLeft" and have the caller check d611, but d611 represents the state of the modifiers at the time it is read, not at the time the character at the top of the queue was pressed.
I don't know what motivated the ASCII version of this feature originally. Given the utility of a PETSCII version and the presence of the ASCII feature, I can see why it would be considered trivial to just put the translation PETSCII table into the core instead of asking callers to maintain their own ASCII-to-PETSCII translation tables. I would agree that it's more logic that needs testing, but in this case, we knew the PETSCII feature was incomplete, which is why we didn't document it or resolve this issue. $FF was a late discovery but it was part of the finishing process, as was fixing the Shift codes. IMO, it's feasible to get this feature correct enough to define it in hardware.
(If nobody is using d610 we could nix it and recover the register, but it is actually documented in the Compendium, so I'd want to be extra careful.)
Note: we can never change D610/D611, as this is used in all internals. As far as I know the feature was implemented to not have to do it in assembly in all the internals (HYPPO, MEGAFLASH, etc). In addition D610/D611 is a widely know feature and used by nearly every assembly program I know.
What we can do is change how D619 works, as this is a new and unfinished feature that was solely implemented to reduce ROM space for keyboard scanning and make typing speed faster (as far as I know the history). For example we could make 10 bytes long and let it emit 16 bit pairs of keyscan codes...
(The $00 thing also currently bites my brain, while trying to change MEGAFLASH to not use the ROM for printing stuff. It certainly also collides with zero terminated strings)
I'm having some difficulty figuring out how to get d619 to default to FF. I've set a bunch of VHDL things to FF, but I must be missing some because I'm still getting continuous zeroes. lmk if you see anything I missed. https://github.com/dansanderson/mega65-core/commit/fd072dd0567874a95f6d547f1a11b595b8fdd194#diff-20cda91b319ee173c3068877493ecbcfd21405a44d4d462be8075aeb825e3213
If $D619 is really not so much used by any software (yet) other than ROM (or it will use) for sure, it's kind of OK to change the behaviour as @lydon42 stated. Really, my only fear here, that relying on that by the ROM without any transition table etc, it must be "perfect" otherwise a very troublesome core+ROM dependency will be introduced, in a much more tightly way than usual. Also the PETSCII based scanner may be picked up by other programs meanwhile, and then it's really hard to change the core because the ROM would need it, and without breaking those programs. So anyway, indeed: if we want to change $D619 behaviour, we should do it ASAP (before other programs start to use it for example) and must be "perfect" to avoid newer and newer refinements on that later again and again.
Having D619 as an alternative does not remove the "normal" way of scanning the keyboard, it is in addition to that. So all old programs keep working as long as the ROM GETCH will return the same codes.
@lydon42 Of course, I haven't stated that. I meant that new programs adopting using D619 directly, which is - in theory - possible, currently it's not so much the situation but who knows for the future: if some feature is available at hardware level, it will be used by someone after a while :) On a "modern" system (ie, PC, etc) it does not matter since modern OSes would not allow for user programs interacting with the hardware directly (typically), so only the OS/OS driver is affected. Surely, on a "retro computer" it is kind of different.
I very much want to avoid tight coupling between the ROM and the core for exactly the reasons you're describing. It's worth the investment to make this design clean, well-defined, intuitive, and generally useful.
For example: The ROM needs a distinction between Ctrl-S and Home, which have the same PETSCII code. The cheapest solution to implement would be to assign an unused but non-PETSCII code to Home, so the ROM can read both codes from d619 and react accordingly. But then PETSIIKEY becomes the "whatever the ROM needs" register. If a future ROM needs a similar distinction between two other keys with the same PETSCII code, we'd be stuck with either an inconsistent design or a backwards incompatible design.
The most likely solution is for the ROM to not use d619 to make this distinction. The ROM could read Ctrl-S off of the CIA line once per IRQ, and if detected would ignore $13 from d619 during that IRQ. This is janky with some edge cases (pressing Ctrl-S and Home in succession really quickly), but it's probably sufficient, and it avoids corrupting the PETSCIIKEY register design with exceptions.
I still need to figure out how to get PETSCIIKEY to use $FF to represent the empty queue. As with Ctrl-S, reassigning Ctrl-@ to a different code would be a cheap and easy workaround, but makes the hardware design ugly. I can't imagine using $FF for empty is impractical, we just need to figure out the correct VHDL change.
Here's a write-up of my "typing event queue with three views" design, which I think is an improvement over the current PETSCIIKEY design. If I had a better VHDL development workflow I'd attempt an implementation. It doesn't look difficult, it's just a larger code change. As follows:
A typing event occurs when the user presses a non-modifier key along with zero or more modifier keys being held. The typing event queue stores up to five event records with the key number (0-71) and a bitfield of held modifiers. The event queue handler implements debounce and key repeat logic similar to the C65 ROM's CIA-based keyboard scanner.
The top of the queue is presented on three sets of registers, each with a different interpretation of the value:
When the queue goes from empty to non-empty, these views update immediately with the new top event. The non-empty presentation latches. When the caller writes any value to any of the queue-view registers, the top event is popped, and all views update to the next event or the empty state.
Advantages of this design:
Disadvantages:
I have a jank implementation of Ctrl-S that doesn't require special treatment from d619. It scans d611 and CIA lines for Ctrl-S once per IRQ, then ignores $13 in the d619 queue during an IRQ where Ctrl-S was detected. Ctrl-S and Home are both functional. As expected, this occasionally emits $13 instead of Ctrl-S if the CIA lines are clear at the top of the IRQ handler but PETSCII $13 is added to the queue before the handler exits. Without the Typing Event Queue design (or different d619 codes for the two cases), I can't disambiguate the PETSCII $13 event at the time the event occurred.
Unfortunately I've discovered a new unrelated issue where a BASIC Syntax Error toggles 40/80 column mode, so I've got some work to do. 😅 (Fixed: I was overzealous in deleting unused code.)
I have a successful change for making $FF the empty queue value for PETSCIIKEY. (I must have been having some workflow issues before. The final code change was as expected.)
I still want to pursue a unified typing event queue design. Even if we don't implement raw key event registers (which I still like), having ASCIIKEY and PETSCII use the same queue will keep this possibility open in the future. I don't see a reason for them to have separate queues. Alas, I can't disambiguate between Ctrl-S and Home using just ASCIIKEY and PETSCIIKEY because they both return $13 in both cases. I would still need raw typing event registers to do this reliably.
If we keep PETSCIIKEY undocumented and private for ROM use, we can launch fast typing without a unified typing event queue. Adding it later would not change the behavior of the documented ASCIIKEY. It would be polite to unify the queues before publicizing PETSCIIKEY.
New really good idea from @lydon42 ! Instead of separate registers for ASCIIKEY, PETSCIIKEY, key number and modifier views of the queue:
d619[5:7] are read-write mode registers that determine whether d610 is ASCIIKEY (boot-time default), PETSCIIKEY, or raw key number. d619[0:4] are always the modifiers of the top of the queue. Writing to d610 (and not d619) advances the queue. The caller can change the modes to change the view of the top of the queue in d610.
Full disambiguation, full hardware translation, efficient and intuitive use of only two registers, and backwards compatible with the existing ASCIIKEY. Thoughts?
On scenario that will break (if it is a reasonable scenario):
Possible solution: ROM switches back to ASCII on SYS? Still problematic for programs which sue both D610 using ASCII and ROM...
Addition to the possible bitfield: one bit is CLRKEYQ, flushing the queue (saving us the peek/poke loop).
(Copying from chat:) The ROM will only read this during an IRQ and the IRQ routine already has to stash and restore some things, so it can stash and restore the mode bits also.
There's actually a worse problem that by having them all use the same queue, the new kernel IRQ is going to clear out the hardware queue every IRQ, so a program that leaves the kernel IRQ in place will miss most key events. It sounds like ASCIIKEY is pretty widely used, so maybe it has to keep its own queue after all. I still like this idea for a new register that can switch between PETSCII and raw key number. We just have to assign a new register.
Some good discussion with @gardners and @lydon42. I'll attempt to summarize:
Proposal:
This is close enough to good that I will start an implementation. Then we can play with it and see if any further revisions are needed. Thanks all for the great discussion!
This PR now has the PETSCIIKEY fixes and an implementation of MODKEY and MODKEYVIEW. It's ready for review. https://github.com/MEGA65/mega65-core/pull/720
RAWKEY is still a good idea but I'd like to defer it to a separate PR to reduce launch risk.
The PR has been merged. I believe this Issue can be resolved. (I don't have permission to resolve it.)
Is your feature request related to a problem? Please describe. The existing KERNAL keyboard scanner doesn't get called often enough to prevent dropped characters with fast typists. The $D610 ASCII input method doesn't have this problem. Let's add something equivalent for PETSCII. It would also save Bitshifter about 800 bytes in the ROM, which could be used for more interesting things, like 80x50 text mode.
Describe the solution you'd like $D610 or another similar register allows hardware accelerated reading of PETSCII key presses. USing $D610 would be problematic, because existing software expects it to provide ASCII chars, not PETSCII ones.
Describe alternatives you've considered Do nothing.
Additional context