Keyboard controller support #91
Conversation
|
Wow, what I nice and thorough merge request! I just read through it, and it looks like everything is there, even the Elm bits for the frontend. I am familiar with the keypad from my work on Stella, but I never bothered to add it to 6502.ts myself. I'll take a closer look at a few pieces the next days when I find more time, but this is really good work. I'll do a new release once i merge it --- I am afraid I have done preciously little work on 6502.ts over the last years, but this warrants one 😏 |
|
Thanks! I saw that I could basically follow the same patterns as the existing controllers, happy to make adjustments in case I misinterpreted anything. The elm bits in particular I did in my first pass through, before I really started testing with real ROMs, I believe they aren’t thoroughly tested.. |
|
Sorry for the late reply, real life was keeping me busy for the last two weeks. I have started to play with your code now and have found a few issues that still need sorting out, nothing big. My first findings:
I can support you and implement some or all of those things myself on the PR, but we can also share the load, or you can take over it completely. What would prefer. |
|
I'll take a look - I definitely didn't test out keyboard binding (was driving with click events) and yes it's not working for me either.. I might need some extra pointers wrt migration - is that about having for instance Cartridge metadata be backwards compatible, or updateing the data (somewhere) automatically? |
The easiest way is to register a new version with dexie.js and add a migration step that adjust the records accordingly. The corresponding code is here, and there is already a similar migration (that added TV settings). |
|
Took a run at this We need a data migration to support the new fields, otherwise old records are silently filtered out.
For some reason I can't get the keypad to work in Stellerator. I haven't debugged in depth yet, but it looks like the keyboard driver is using the wrong mappings.
There is a conflict between p as in "pause" and as a keypad key. Maybe it would be better to move the keypad mappings one column to the left
We should get rid of the "emulate paddles" option, it is superseded by your new controller settings
|
|
Sorry for the radio silence again. Seems to work fine now, thanks a lot! I just played a round of Alpha Beam with Ernie 😛 I'll do a review of the code over the weekend. |
| const currentTimestamp = this._cpuTimeProvider(); | ||
| let s = (currentTimestamp >= this._returnTimestamp) ? this._swcha_out : 0; | ||
| if (0 === pad) { | ||
| s = s >> 4; |
There was a problem hiding this comment.
I prefer logical right shifts >>> over arithmetic ones unless the sign has to be preserved.
src/machine/stella/KeypadsReader.ts
Outdated
| if (1 === (s & 0x01)) { | ||
| state = state || this._keypads[pad].getKey(row, column).read(); | ||
| } | ||
| s = s >> 1; |
There was a problem hiding this comment.
I prefer logical right shifts >>> over arithmetic ones unless the sign has to be preserved.
| @@ -947,6 +994,21 @@ export namespace Stellerator { | |||
| secam = 'secam', | |||
| } | |||
|
|
|||
| export enum ControllerType { | |||
There was a problem hiding this comment.
Docblocks here end up in the generated Typedoc documentation, so maybe add a line here for consistency (although ControllerType makes it hard to mistake it for something else 😏)
|
One weekend led to another, but I've finally gone through the code and only have two very minor comments. Thank you again for digging through the codebase and adding this, I'm looking forward to releasing it. One minor question: I gather that part of your goal was to expose On another note, I guess I'll sit down soon and port the codebase to the year 2025. Typescript was still young when I wrote that, and the way that I am using modules does not go down well with VSCode at all --- no usage search, no auto import, etc. Thanks again for your contribution! |
|
Thanks - I've gone ahead and fixed the nits. Wrt the actual peek/poke - I was originally thinking of backing the related changes out of the MR - let me know. I never completed the stellerator interface because figured I would need to understand the emulator better to see how it would really need to work. TLDR - dirty business. In my VCS Lisp project I access stellerator._emulationService directly to copy VCS Lisp programs to/from RAM. It accelerated my ability to test the Lisp evaluator by 100x, but there can be race conditions waiting for the emulated cartridge to reach a "good" state to do these operations. SoI should really have a system to catch the right place to pause the emulator and do a RAM swap. Fortunately/unfortunately peek/poke worked well enough that I never had to push hard on that side of things before I started looking at keypad support. |
|
... and merged 😏 I'll do a release later this week. Don't bother about removing your work on Regarding testing: a few years ago I (together with a few colleagues) experimented with unit tests against a VCS program (a simply binary clock) using 6502.ts. We rigged up a few test cases with mocha that prepare an initial state, run the emulator to a given point and then run assertions against the program state. Worked pretty well actually. If you are interested, the branch is still there: https://github.com/6502ts/6502.ts/commits/funky-testing/ |
This PR adds support for keypad controllers. I have tested this branch in my own embedded stellerator project (https://github.com/dchristianson/vcs-lisp) - there could be be gaps in testing for general use...
TLDR
* keypad controllers have 12 buttons arranged into 4 rows and 3 columns
* only one row can be read at a time for each controller by sending pulses via SWCHA
* discussion of the controllers + generic test rom here: https://forums.atariage.com/topic/247615-trying-to-figure-out-keyboard-controllers-inpt/
There is an additional change to add peek/poke methods for embedded client, this is used in my current homebrew project to be able to read/write to VCS RAM from outside stellerator