search

bjc / ArduinoCore-GD32

Arduino core for GD32 devices, community developed, based on original GigaDevice's core
Other
0 stars 1 forks source link

CDCACM::availabe() blocks #5

Closed bjc closed 2 years ago

bjc commented 2 years ago

Upstream issue: https://github.com/CommunityGD32Cores/ArduinoCore-GD32/issues/56

algernon commented 2 years ago

Hey!

A bit of an update on the issue, and why .available() is pretty much essential for Kaleidoscope: the protocol between the keyboard and the host implemented by Kaleidoscope is a bi-directional protocol, and the most often used method is a request-response one, initiated by the host.

To be able to handle that, we need to be able to tell whether there's data available via Serial. We can't just use a blocking read, because then the rest of the keyboard functionality would be lost until the host sends data. The issue is further complicated by the protocol being rather loose, and there are often scenarios where the firmware doesn't know how much data to read or expect, because data can end abruptly, and still be valid.

For example, to reprogram the keys on a keyboard, the command to send over serial is keymap.map <code> <code>..., where <code> is the keycode for a particular key. Any number of codes can be sent, and then the first N keys will be remapped. It's also common to just send the raw command, keymap.map, without arguments, to query the keymap. Both of these things require Kaleidoscope to know whether there's data available.

The implementation heavily relies on this. We could, in theory, come up with a protocol that doesn't, but that'd be a huge breaking change, and quite an undertaking.

We could also - temporarily - make the keyboard modal: either in keyboard mode, or in "chrysalis-mode", and in the second case, blocking wouldn't be an issue. But this would be a non-negligible amount of temporary code in Kaleidoscope.

Another idea I just had, perhaps a silly one, and I'm not even sure it is possible with Arduino: what if we did a blocking read every couple of seconds, but garded by an alarm of some kind, so it gets interrupted within a few milliseconds? If there's data, we'd get that into buffers, and .available() would return true. If not, then it'd simply return false. This would cause very tiny stalls from time to time, but wouldn't require changing the protocol, or introducing temporary code.

bjc commented 2 years ago

Fixed here: https://github.com/bjc/ArduinoCore-GD32/commit/be5d7bb7324882b2c90f13f8088828fe67cf52b8