Remove resetting of key states after sending a report

[lonesometraveler]

This pull request aims to improve the behavior of the keyboard events. Currently, the report.keycodes is being cleared immediately after sending a report. However, this approach can lead to the loss of the current state of pressed keys, hindering the ability to handle multiple key presses.

The proposed change removes the resetting of report.keycodes after sending a report. By retaining the current state of pressed keys until they are released, we ensure that the report.keycodes accurately reflects the keyboard's state, enabling better management of key presses and releases.

[HaoboGu]

This few lines cancel previous pressed key if there is a new key stroke. It prevents a rare case that one key is kept pressed forever when somehow the release report is not sent successfully.

Theoretically it has nothing bad for multi-key processing because the keyboard actually holds a keystate matrix which always reflects the real state of matrix scanning.

[lonesometraveler]

Maybe I should explain my use case. I'm using a custom keyboard as a joystick for playing games. Currently, it is not possible to press "N" (jump key) while pressing "D" (move-to-right key). This is because the matrix doesn't accurately reflect the current state of the pressed keys.

Here's what is happening:

When "D" is pressed

• The report ["D", 0, 0, 0, 0, 0] is sent
• The local keycodes are cleared [0, 0, 0, 0, 0, 0]
• The host still thinks "D" is pressed.

When "N" is pressed,

• "N" is added to the keycodes and the report ["N", 0, 0, 0, 0, 0] is sent.
• "D" is ignored because key_state is not changed (still held down).
• Since "D" is not in the report, the host assumes "D" is released, and "N" is pressed.

[HaoboGu]

Ah, that's normal for a keyboard IMO. I just tried on my macbook's keyboard: first I pressed "N" and held, "N" was kept triggered. Then I tapped "D" while "N" was pressed. In this case "N" was cancelled.

You could also try your own keyboard to verify this. I don't know much about joysticks, so in your case this behavior might be not expected. That makes sense.

[lonesometraveler]

Each application processes reports differently, so one app's behavior does not necessarily reflect the standard HID behavior. We should evaluate the debug outputs and see how the reports look like. I believe adhering to the HID protocol involves changing registered values to 0 only when the corresponding keys are released.

If our concern is preventing keys from getting stuck, resetting keycodes each cycle makes sense. But if we do that, we should adjust how we read the matrix. We need to ensure that held-down keys are reinserted into the keycodes along with newly pressed keys.

[HaoboGu]

If our concern is preventing keys from getting stuck, resetting keycodes each cycle makes sense. But if we do that, we should adjust how we read the matrix. We need to ensure that held-down keys are reinserted into the keycodes along with newly pressed keys.

Agreed, do you have time to take a look? I'm doing some refactoring about storage/keymap, after that I could get some time on this issue.

[lonesometraveler]

Since one of my projects now relies on this crate, improving it is my priority. 😀

I need to study the codebase first, and I may send small PRs as I learn. Some of them may not seem directly related to this issue, but please understand that this is how I learn a new codebase.

[lonesometraveler]

This few lines cancel previous pressed key if there is a new key stroke. It prevents a rare case that one key is kept pressed forever when somehow the release report is not sent successfully.

Shouldn't we try to resend the report if it fails? I've made a commit demonstrating that. Currently, write_serialize suppresses errors without propagating them. However, by propagating errors, we could implement retry logic, as shown in the code. Wouldn't this address our concern about keys getting stuck?

[HaoboGu]

write_serialize returns error when there is an error of writing stage. I guess the reason of stuck keys might due to packet loss in transmission(especially for BLE). In this case, writing is successful.

For example, when writing via USB, write_serialize returns errors in only two situations: BufferOverflow and UsbDisabled, both of them couldn't be solved by retrying.

[HaoboGu]

I'm doing some refactoring to improve it. I'm merging it first. Thanks @lonesometraveler !