Skip to content

Fix uinput-echo example #39

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jan 5, 2025
Merged

Conversation

edsko
Copy link
Contributor

@edsko edsko commented Jan 5, 2025

In the setup of the device, the keys were initialized to

[Codes.KeyA .. Codes.KeyZ] ++ [Codes.Key1 .. Codes.Key0]

But that is incorrect: KeyA corresponds to value 30 and KeyZ corresponds to value 44; this therefore excludes keys such as KeyB (48) and others. This can be observed by running the example and typing abcde; only ad will be echoed back.

Perhaps the better fix would be to modify the Enum instance, but that might result in difficult to debug backwards incompatibility problems, so this PR simply fixes the example.

In the setup of the device, the keys were initialized to

```
[Codes.KeyA .. Codes.KeyZ] ++ [Codes.Key1 .. Codes.Key0]
```

But that is incorrect: `KeyA` corresponds to value 30 and `KeyZ`
corresponds to value 44; this therefore excludes keys such as `KeyB`
(48) and others. This can be observed by running the example and typing
`abcde`; only `ad` will be echoed back.

Perhaps the better fix would be to modify the `Enum` instance, but that
might result in difficult to debug backwards incompatibility problems,
so this PR simply fixes the example.
@georgefst
Copy link
Owner

georgefst commented Jan 5, 2025

🤦‍♂️ Not sure how I failed to notice this!

I'm not convinced that an alternative Enum instance would be any less surprising in general than the current one, which maps keys to their actual integer representation. But I agree that backwards compatibility concerns make it a non-starter regardless.

Perhaps the library could provide "event groups", like alphaNumericKeys :: [Key]? I don't think there's any such notion in libevdev or the kernel, so this would be a bit ad hoc, but it would be useful.

Anyway, thanks for the fix!

@georgefst georgefst merged commit 367ca25 into georgefst:master Jan 5, 2025
1 check passed
@edsko edsko deleted the edsko/fix-uinput-echo branch January 7, 2025 12:37
@edsko
Copy link
Contributor Author

edsko commented Jan 7, 2025

Indeed, such groups would be useful and might even prevent some bugs, certainly had me stumped for a while why a was working just fine but b was not 😬

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants