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

Tweak OS detect, add OS_DETECTION_SINGLE_REPORT #24379

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

andrebrait
Copy link
Contributor

@andrebrait andrebrait commented Sep 9, 2024

Description

  • Default OS_DETECTION_DEBOUNCE bumped from 200ms to 250ms
  • Add OS_DETECTION_SINGLE_REPORT to prevent undesired multiple reports
  • Prevents random stability issues on ARM MacBooks after switching via KVM
  • Works for every device I could test, including ARM MacBooks
  • Disabled by default to keep current behavior
  • Add Troubleshooting section on documentation
  • Tweak reset logic to prevent a freeze with some KVMs

The USB stack on ARM MacBooks is more similar to that of iOS and,
for some reason, it seems to like sending packets that influence
the OS detection and results in a second OS_MACOS report being sent
at a random period of time after plugging the keyboard back. This
does not always happen and the consequences of this vary based on
what the user is doing in the callback, but since this is not
obvious and it's hard to debug, I've decided to add a flag for
those affected by such issue. The stability issue I had in mine was
a combination of factors and I found the actual cause being my own
bad math when changing the default layer, but this change alone is
also confirmed to fix it. Lastly, soem KVMs seem to leave the USB
controlled in a suspended state when cold-booting Windows, meaning
the keyboard would hang and the reset logic would not work. This
tunes it so that it can get out of such state. Also retested for
compatibility with my old KVM to ensure the logic works for both.

Types of Changes

  • Core
  • Bugfix
  • New feature
  • Enhancement/optimization
  • Keyboard (addition or update)
  • Keymap/layout/userspace (addition or update)
  • Documentation

Issues Fixed or Closed by This PR

  • None

Checklist

  • My code follows the code style of this project: C, Python
  • I have read the PR Checklist document and have made the appropriate changes.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • I have tested the changes and verified that they work and don't break anything (as well as I can manage).

* Default OS_DETECTION_DEBOUNCE bumped from 200ms to 250ms
* Add OS_DETECTION_SINGLE_REPORT to prevent undesired multiple reports
* Prevents random stability issues on ARM MacBooks after switching via KVM
* Works for every device I could test, including ARM MacBooks
* Disabled by default to keep current behavior
* Add Troubleshooting section on documentation
* Tweak reset logic to prevent a freeze with some KVMs

The USB stack on ARM MacBooks is more similar to that of iOS and,
for some reason, it seems to like sending packets that influence
the OS detection and results in a second OS_MACOS report being sent
at a random period of time after plugging the keyboard back. This
does not always happen and the consequences of this vary based on
what the user is doing in the callback, but since this is not
obvious and it's hard to debug, I've decided to add a flag for
those affected by such issue. The stability issue I had in mine was
a combination of factors and I found the actual cause being my own
bad math when changing the default layer, but this change alone is
also confirmed to fix it. Lastly, soem KVMs seem to leave the USB
controlled in a suspended state when cold-booting Windows, meaning
the keyboard would hang and the reset logic would not work. This
tunes it so that it can get out of such state. Also retested for
compatibility with my old KVM to ensure the logic works for both.
@drashna drashna requested a review from a team September 10, 2024 03:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants