Adding full ASCII support #1
Conversation
There was a problem hiding this comment.
Thank you for the PR!
I think it's a nice addition to support full ASCII. Personally, I think it would even be okay to insert BACKSPACE and DEL characters. If they actually appear on a barcode, then why not put them in the resulting string?
Of course, there are still ASCII characters without a corresponding key. It might be interesting to see what barcode scanners do when they read them. However, I don't think this needs to be done for this PR :)
|
I though about adding in DEL and BACKSPACE but placing them in a barcode returns a different set of keys being pressed by the scanner, depending on what barcode format I used. So that suggests to me that putting them in a barcode might be undefined behavior, or might be handled differently depending on the barcode spec. |
Yeah, not all barcode formats support the full ASCII range. I'm fine with ignoring them now. If it makes sense we can always handle them later. |
| match key_name { | ||
| evdev::Key::KEY_LEFTSHIFT => left_shift_pressed = event.value() == 1, | ||
| evdev::Key::KEY_RIGHTSHIFT => right_shift_pressed = event.value() == 1, | ||
| evdev::Key::KEY_CAPSLOCK => capslock_on = event.value() == 1, |
There was a problem hiding this comment.
Is this correct? Does an evdev device report a key-up the second time you press capslock?
There was a problem hiding this comment.
It looks like capslock is reported as usual: press and release events match the actual key state. Not the "is capslock enabled" bit.
If I use a real keyboard, I do receive LED events. It may be some other components doing that: the Xorg server or the Wayland compositor. But that's fine. I think we should just follow the LED events and not look at the KEY_CAPSLOCK events.
There was a problem hiding this comment.
Although maybe this isn't true when you have an exclusive grab.. Hmm..
Maybe we should just ignore capslock support entirely.. Sorry for the turmoil.. What do you think?
There was a problem hiding this comment.
I would just say to drop it. Caps lock working differently to pressing shift would require more complexity in handling it anyway.
Also to my understanding the LED events come from whatever is handling the keyboard. The keyboard is not responsible for tracking the LED states, usually Xorg or a Wayland compositor.
There was a problem hiding this comment.
Right, sounds good to me. I highly doubt barcode scanners would use capslock anyway. And if they do they shouldn't >.<
|
|
||
| // Map key_name to the number or char. | ||
| if event.value() == 1 { | ||
| if let Some(c) = key_to_str(key_name, left_shift_pressed || right_shift_pressed || capslock_on) { |
There was a problem hiding this comment.
This isn't totally correct: capslock only shifts letters to their uppercase variant. It doesn't shift numbers or symbols.
I changed up the code for converting the keys returned by evdev to actual
chars. It now takes holding down shift into account, so that upper-case is possible. Also I added in some missing character to allow full support of all printable ASCII characters minus DEL (so character code 32-126).Also I bumped the version number.