MiSTer-devel / ZX-Spectrum_MISTer

45 stars 33 forks source link

ZX-Spectrum Recreated Bluetooth keyboard input support #36

Closed MilanPolle closed 1 year ago

MilanPolle commented 1 year ago

This code, developed by pgimeno, adds optional support for the ZX-Spectrum Recreated Bluetooth keyboards. https://www.recreatedzxspectrum.com/

The idea developed in this thread: https://misterfpga.org/viewtopic.php?t=3491

I made this pull request, because pgimeno left GitHub, after Microsoft bought it.

sorgelig commented 1 year ago

Well, it's a doubtful change.. It clutter the code a lot while adding some marginal HW which is definitely hard to use with other cores... Probably it's better to keep it in fork for those couple people who will decide to integrate MiSTer with single ZX core inside that case.

MilanPolle commented 1 year ago

Thank you for your reply. Do you mean other cores are also using the code of this core?

sorgelig commented 1 year ago

MiSTer is originally multi-core system. So, keyboard connected to MiSTer is used in other cores too. How you will use this keyboard in such cores as AO486 or Minimig for example? So you have to connect another keyboard which makes it inconvenient. Replica of ZX keyboard is good only for ZX, so probably it's good to have a separate de10-nano (or even other FPGA system) just for single ZX core tuned specifically for this keyboard.

However ghosting mode should be ok to be merged, although i'm not sure if this effect is really used somewhere. Otherwise it will only make this core harder to compile due to large combinatorial logic. And ZX core is already very hard for Quartus. Sometimes it fails to work.

MilanPolle commented 1 year ago

Thank you for your explanation. I don't think anyone is expecting to be able to use this keyboard with other cores than the ZX Spectrum, ZX81 (there's already a fork for that as well) or Jupiter Ace. For these two cores, the keyboard allow the typical kind of input of these computers (single press for a basic command etc.).

If you are not following the discussion at the forum, this is the response of the author, pgimeno:

Clutters the code? The Recreated code is clearly delimited, it doesn't cause any confusion with non-Recreated code. Part of what was done is a cleanup of the somewhat fishy pre-existing keyboard code. It actually reduces clutter.

The ghosting commit was not kept separate in the PR from the Recreated support commit. That may have caused some confusion. If kept separate, the Recreated support could be reviewed independently of the Ghosting support, and maybe the impression of clutter would be reduced. I separated them because they are functionally different changes. Ghosting support is not that essential, and if keeping it out makes the patch be accepted, I'd be willing to sacrifice it.

I've now read Sorg's other reply. I keep both a PC keyboard and a Recreated keyboard connected through USB; the hub has room for both and it's not a problem, I keep the Recreated behind the normal one while not in use (there's room between the PC keyboard and the monitor) and I switch them depending on the core. Similarly I have three controllers connected, and I choose one depending on the core I'm playing. So I can't follow his reasoning.

The ZX81 and Jupiter Ace cores could take advantage of this keyboard as well. In fact the ZX81 patch is already written.

A normal PC keyboard is not too suitable for the Spectrum core; the Symbol Shift key in particular (and the space bar at times) is in an inconvenient position.

Rhester72 writes:

Sorgelig I beg you to reconsider. The market for these keyboards may not be huge, but most of us are NOT using MiSTer as a dedicated Speccy solution - that doesn't make a (togglable) input device undesirable or not useful! I don't think anyone advocating for this wants to use it with other cores - we just want to use it when we run THIS core! It's a USB keyboard, just mapped very specifically - why can't we have this code and swap keyboards as/when we wish?

NOBODY expects this keyboard to work with ANY other core. We intend to hot-swap it when we use the ZX core, that's all.

sorgelig commented 1 year ago

With ZX81 core it will be especially fun when ZX80 mode has completely different basic commands map.