Skip to content
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

fix: adding option to separate implicit mod release from key release #2334

Merged
merged 2 commits into from
Jun 24, 2024

Conversation

Timoyoungster
Copy link
Contributor

This adds a new config value ZMK_HID_SEPARATE_MOD_RELEASE_REPORT where, if enabled, the report for a key release is sent separately to the accompanying modifier release signals, which are then sent in a second report.

This fixes an issue where certain applications are unable to work with implicitly modified keys (e.g. colon) due to them registering the modifier release prior to the actual key release.

Have tested this on my personal keyboard and wev now shows the signals in the correct order.
=> Previously: LSHIFT (pressed) -> colon (pressed) -> LSHIFT (released) -> **semi**colon (released)
=> Now: LSHIFT (pressed) -> colon (pressed) -> colon (released) -> LSHIFT (released)

@Timoyoungster Timoyoungster requested a review from a team as a code owner June 13, 2024 23:20
app/src/hid_listener.c Outdated Show resolved Hide resolved
Copy link
Contributor

@petejohanson petejohanson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code seems reasonable, just a comment on an unrelated file change that snuck in. Thanks for the PR!

@@ -4897,11 +4897,12 @@
}
},
"node_modules/braces": {
"version": "3.0.2",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please take this file change out of this commit/PR.

Copy link
Contributor Author

@Timoyoungster Timoyoungster Jun 14, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll try👍
Sorry about that.

This adds a new config value `ZMK_HID_SEPARATE_MOD_RELEASE_REPORT`
where, if enabled, the report for a key release is sent separately to
the accompanying modifier release signals, which are then sent in a
second report.

This fixes an issue where certain applications are unable to work with
implicitly modified keys (e.g. colon) due to them registering the
modifier release prior to the actual key release.

Have tested this on my personal keyboard and `wev` now shows the signals
in the correct order.

=> **Previously:** ```LSHIFT (pressed) -> colon (pressed) -> LSHIFT
(released) -> **semi**colon (released)```

=> **Now:** ```LSHIFT (pressed) -> colon (pressed) -> colon (released)
-> LSHIFT (released)```

(This time without accidental files)
Copy link
Contributor

@petejohanson petejohanson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just one comment on the docs portion.

docs/docs/config/system.md Outdated Show resolved Hide resolved
@Timoyoungster
Copy link
Contributor Author

@petejohanson is the phrasing ok now?

Copy link
Contributor

@petejohanson petejohanson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@petejohanson petejohanson merged commit 29599e8 into zmkfirmware:main Jun 24, 2024
47 checks passed
@Timoyoungster Timoyoungster deleted the implicit-mods branch June 26, 2024 18:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core Core functionality/behavior of ZMK hid
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants