JorgeGzm opened a new pull request, #19586:
URL: https://github.com/apache/nuttx/pull/19586
## Summary
This is the change asked for in the review of apache/nuttx#19579, done on
its own rather than inside a board port.
`usbhost_hidkbd` kept a character device, a ring buffer, a poll waiter list
and an encoder of its own, in parallel with everything the keyboard upper
half already provides. A USB keyboard was therefore the one keyboard an
application could not read like any other. It now registers with
`keyboard_register()` and reports with `keyboard_event()`, which removes the
private character device and the four hundred lines that served it.
While looking at why six of the nine drivers that register a keyboard never
report a special key, the answer turned out to be in the header:
```c
/* include/nuttx/input/keyboard.h, before this change */
#define KEYBOARD_PRESS 0
#define KEYBOARD_RELEASE 1
```
The type field of `struct keyboard_event_s` has four values, not two. The
other two live in `kbd_codec.h` under a different prefix. Somebody writing a
driver reads `keyboard.h`, sees two types and implements two types, and the
failure is silent: the build is clean and the symptom is a key that does
nothing. All four are declared here now, as aliases of the `kbd_decode()`
return values so that a driver can feed both interfaces from one source.
The rest follows from that:
* `kmatrix` reported every key as a plain press, so a board whose matrix has
arrows had no way to say so. A keymap entry wrapped in `KMATRIX_SPECIAL()`
now declares that it holds a keycode rather than a character.
* `sim_keyboard` reported any X11 keysym its table did not know as an
ordinary key press carrying the raw keysym, so an application looking for
a byte of text got 65307 for Escape and 65289 for Tab, and the modifiers
arrived as 65505 and friends. NXDoom on the simulator has therefore had no
menu, no map and no fire, which is most of the game. Escape and Tab are
control characters and are reported as such, the modifiers get the
keycodes the codec now has for them, and an unknown keysym above the
Latin-1 range is not reported at all, which closes the case rather than
the three instances of it.
* The keyboard driver documentation described the byte stream codec and
nothing else. It never mentioned `keyboard_register()`, so there was
nowhere to look up the interface that every keyboard actually uses. The
contract is written down now, along with why the event type matters, what
to name the device, how to get a matrix keyboard working without writing a
driver, and how to test the result with or without the hardware.
* The M5Stack Cardputer reported its Fn cursor cluster as ordinary presses
carrying 0x80 to 0x83, chosen to sit above the printable range so that an
application could tell them apart from characters. That is the same
collision, solved by hand. They are keycodes now.
This is the first of three parts, and it stands on its own.
apache/nuttx-apps#PENDING is the application half and needs this one merged
first, since it uses the keycodes added here. A follow-up here will then
update the two board configurations that name an application option.
## Impact
**Nine configurations have an application that reads the USB keyboard as a
byte stream.** They keep working through `INPUT_KEYBOARD_BYTESTREAM`, which
renders each event with the keyboard codec instead of copying the event
structure. Only presses are rendered, which is what a keyboard reporting
through a character device has always delivered, so this reproduces the
previous behaviour rather than adding to it. The option is selected in those
nine defconfigs here.
**Out of tree, `NSH_USBKBD` and `MICROWINDOWS_KBD_RAW` also read the byte
stream** and need the same option. Both now depend on it, in the
applications PR, so Kconfig refuses the combination that cannot work. No
in-tree configuration selects either.
**`HIDKBD_ENCODED` and `HIDKBD_NODEBOUNCE` are gone**, with the code they
guarded. Encoding is inherent to the event now. The previous report is no
longer an optimisation either: a HID keyboard reports the keys that are
down rather than the transitions, so it is what tells a new press from a key
still held, and what tells that a key has been released.
**Reporting the modifiers as keys is new**, so it is behind
`HIDKBD_REPORT_MODIFIERS` and off by default. It matters for a game, where
fire, run and strafe are bound to Ctrl, Shift and Alt, and not for a
terminal.
**`USBHOST_HIDKBD` now selects `INPUT` and `INPUT_KEYBOARD`**, the same way
`USBHOST_HIDMOUSE` already selects `INPUT` and `INPUT_MOUSE`.
**`INPUT_KMATRIX_DEVPATH` defaults to `/dev/kbd0`** instead of
`/dev/keypad0`. Applications look for a keyboard under the former, so the
matrix driver was out of reach of all of them. No in-tree configuration
enables `INPUT_KMATRIX`.
`LAST_KEYCODE` moves to the end of the enumeration, which it has to: the
codec range checks against it, so the new keycodes would otherwise trip an
assertion in `kbd_specpress()` and be rejected by `kbd_decode()`.
## Testing
Validated on hardware in two independent backends, a USB HID keyboard on a
Linum STM32H753BI and X11 on the simulator, in both the event model and the
byte stream compatibility mode.
Counted rather than eyeballed. Typing normally on the Linum produced 40
presses and 40 releases, perfectly paired, with no key pressed twice while
held, no orphan release, nothing stuck down at the end, and four keys held
at once reported in the right order. The arrows, Ctrl and Shift gave 29
press and release pairs on the same terms, including two arrows held
together, and arrived as keycodes 5 to 8, 88 and 90.
On the simulator, dumping the events while typing shows Escape as 27, Tab as
9 and the modifiers as keycodes 88 to 95, none of which were reachable
before. NXDoom runs there and plays.
Two things worth knowing for anybody repeating this, both now documented:
Without `HIDKBD_NOGETREPORT` the driver samples the keyboard over the
control pipe every 40 ms, and a key pressed and released between two samples
is simply not there. Six keys arrived in seventy five seconds of typing.
That is the sampling rate, not the reporting, and it behaves the same way on
master. The Linum configuration enables it.
`UINPUT_KEYBOARD_BUFNUMBER` counts events rather than keys, so its default
of eight holds four keystrokes, and a console hands over a whole line at
once. The upper half overwrites the oldest event when the buffer is full, so
a typed line arrives with its beginning missing and nothing says so. This is
pre-existing and only shows up when uinput is actually used as a keyboard.
The Linum configuration raises it.
Built every configuration in the tree that enables `USBHOST_HIDKBD`,
`INPUT_KEYBOARD`, `INPUT_KMATRIX`, `UINPUT_KEYBOARD`, `EXAMPLES_LVGLTERM`,
`EXAMPLES_KEYBOARD`, `EXAMPLES_HIDKBD` or `SYSTEM_KBD`. 21 of them were also
built inside the CI container, which covers the Xtensa and x86_64 toolchains
that are not on my machine, and all 21 pass. That includes both Cardputer
configurations, which is the only way I could check the driver change above,
since the board was not available.
The remaining failures are `stm32butterfly2:nsh` and `:nshusbhost`, which
select `ARM_TOOLCHAIN_BUILDROOT` and need a compiler that is not in the CI
container either (the CI builds them with `ARM_TOOLCHAIN_CLANG`), and
`ci20:jumbo`, which needs MIPS. All three fail the same way on master.
`sim:nxdoom` builds and runs. `nxstyle` is clean on every file touched.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
