Kernel/HID: Introduce initial USB keyboard support

[Olekoop]

It was tested with qemu's usb-kbd. The code is based on USB Mouse driver

[gmta]

Just from a cursory glance: you repeat the below pattern a lot of times even though it doesn't matter which one of the four if-branches it takes:

if (packet_raw.keypressX == 0x54)
    event.code_point = '/';

You should lift these up out of the branches and put it at the end.

[Olekoop]

Fixed that

[gmta]

You have a lot of code duplication for these 6 keys. Can you reduce this to a single code block that you execute for each of the 6 keys you track?

[Olekoop]

Fixed that. Now it is an array and the code iterates through it

[supercomputer7]

Not fond of this. All other HID drivers (USB mouse, PS/2 keyboard and PS/2 mouse) don't do this.
Please find a way to separate USBKeyboardDevice from the KeyboardDevice inheritance tree.

[Olekoop]

Fixed that, Now it inherits from HIDDevice

[supercomputer7]

This is not what I meant - look at how the USB mouse driver has a USBMouseDevice which holds a MouseDevice:

serenity/Kernel/Devices/HID/USB/MouseDevice.h

Lines 34 to 37 in 4d5823a

	USBMouseDevice(USB::Device const&, NonnullOwnPtr<USB::InterruptInPipe> pipe);
	NonnullOwnPtr<USB::InterruptInPipe> m_interrupt_in_pipe;
	NonnullRefPtr<USB::Device> m_attached_usb_device;

I'd want the same to happen here.

[Olekoop]

Well, I don't quite understand.
It seems to me like the USB Mouse class inherits from MouseDevice and uses itself to process packets

serenity/Kernel/Devices/HID/USB/MouseDevice.h

Lines 22 to 23 in 4d5823a

	class USBMouseDevice final : public MouseDevice {
	friend class DeviceManagement;

serenity/Kernel/Devices/HID/USB/MouseDevice.cpp

Line 43 in 4d5823a

handle_mouse_packet_input_event(packet);

While the PS/2 drivers need to have either KeyboardDevice or MouseDevice passed on

serenity/Kernel/Devices/HID/PS2/KeyboardDevice.h

Line 31 in 4d5823a

PS2KeyboardDevice(SerialIOController const&, SerialIOController::PortIndex port_index, ScanCodeSet scan_code_set, KeyboardDevice const&);

serenity/Kernel/Devices/HID/PS2/MouseDevice.h

Line 30 in 4d5823a

PS2MouseDevice(SerialIOController const&, SerialIOController::PortIndex, MouseDevice const&);

and use it to process input. They inherit from SerialIODevice.
If you want to have just a sepreate KeyboardDevice. I have fixed that. If you want something else, tell me.

[supercomputer7]

What I wanted is essentially a replicate of the PS2KeyboardDevice or PS2MouseDevice class.
But now that I look at this again, we should keep it the way it was for now, so you should revert to inherit from KeyboardDevice and to not have a separate device, as it doesn't make sense to both inherit and have the same device class as a member.

What we should aim is to have some sort of model like in the PS/2 keyboard and mouse code, but it will take some time to write the proper modifications, so let's not do this for now.

[Olekoop]

Okay, it should be fixed

[supercomputer7]

Are we guaranteed to have the mouse device at endpoint 0?

[Olekoop]

I think you mean a keyboard device but yes.
The driver checks if the device is a keyboard and if it uses Boot Protocol.
Also endpoint 0 is always a control endpoint and we are guaranteed that it is available.

[supercomputer7]

This looks mostly OK now, although I didn't test this. I'd suggest merging this and extensively testing this before making this device a default of some sort.