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

Add optional mod delay #19405

Open
wants to merge 7 commits into
base: develop
Choose a base branch
from

Conversation

sambaum
Copy link
Contributor

@sambaum sambaum commented Dec 22, 2022

Description

I've copied the solution from #1037 suggested by IBNobody and integrated it into the newest version. Apparently there have been plans to create a PR, but it never got to that. So I've decided to give it a shot, because this solution really helped me and other people are also looking for a solution.
See more information here: #1037
For me the issue usually happens in the following circumstance:
Usage of nested remote access solutions, i.e. Remote-access VPN to office, then site-to-site VPN to customer, then perhaps nested RDP and/or Citrix connections.

I've been working with this solution for many weeks now and haven't encountered any issues with it. Config that I use: KEYBOARD_MOD_PACKET_DELAY 20

If the code requires changes, this PR will need the help of others, because I only have very little knowledge about C coding.

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

Checklist

  • My code follows the code style of this project: C, Python (not my code, but as far as I can tell, it's following the conventions)
  • 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).

@github-actions github-actions bot added the core label Dec 22, 2022
Copy link
Member

@drashna drashna left a comment

Choose a reason for hiding this comment

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

I'm not sure that this is the correct way to handle the RDP/etc issue here.

Also, formatting:

quantum/action_util.c Outdated Show resolved Hide resolved
quantum/action_util.c Outdated Show resolved Hide resolved
quantum/action_util.c Outdated Show resolved Hide resolved
@drashna drashna requested a review from a team December 23, 2022 06:47
sambaum and others added 3 commits December 23, 2022 14:09
Co-authored-by: Drashna Jaelre <[email protected]>
Co-authored-by: Drashna Jaelre <[email protected]>
Co-authored-by: Drashna Jaelre <[email protected]>
@sambaum
Copy link
Contributor Author

sambaum commented Dec 23, 2022

I'm not sure that this is the correct way to handle the RDP/etc issue here.

Ideally the key presses should be handled by the OS/Application correctly, but they don't, and there isn't much we can do about it, at least I didn't figure anything out. The problem does not exist with a "normal keyboard", because no one presses keys that fast.
It is kind of a workaround, which would be disabled by default. But this fix made me very happy, because my beloved custom keymap was usable again with those remote connections.

@github-actions
Copy link

Thank you for your contribution!
This pull request has been automatically marked as stale because it has not had activity in the last 45 days. It will be closed in 30 days if no further activity occurs. Please feel free to give a status update now, or re-open when it's ready.
For maintainers: Please label with bug, awaiting review, breaking_change, in progress, or on hold to prevent the issue from being re-flagged.

@github-actions github-actions bot added the stale Issues or pull requests that have become inactive without resolution. label Feb 17, 2023
@github-actions github-actions bot removed the stale Issues or pull requests that have become inactive without resolution. label Feb 28, 2023
@github-actions
Copy link

Thank you for your contribution!
This pull request has been automatically marked as stale because it has not had activity in the last 45 days. It will be closed in 30 days if no further activity occurs. Please feel free to give a status update now, or re-open when it's ready.
For maintainers: Please label with bug, awaiting review, breaking_change, in progress, or on hold to prevent the issue from being re-flagged.

@github-actions github-actions bot added the stale Issues or pull requests that have become inactive without resolution. label Apr 15, 2023
@tzarc tzarc requested review from drashna and a team April 15, 2023 07:33
@tzarc tzarc removed the stale Issues or pull requests that have become inactive without resolution. label Apr 15, 2023
Copy link
Member

@tzarc tzarc left a comment

Choose a reason for hiding this comment

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

Just some general paranoia.

quantum/action_util.c Outdated Show resolved Hide resolved
quantum/action_util.c Outdated Show resolved Hide resolved
quantum/action_util.c Outdated Show resolved Hide resolved
sambaum and others added 3 commits May 5, 2023 23:05
@sambaum sambaum requested a review from tzarc May 28, 2023 10:46
@github-actions
Copy link

Thank you for your contribution!
This pull request has been automatically marked as stale because it has not had activity in the last 45 days. It will be closed in 30 days if no further activity occurs. Please feel free to give a status update now, or re-open when it's ready.
For maintainers: Please label with bug, awaiting review, breaking_change, in progress, or on hold to prevent the issue from being re-flagged.

@github-actions github-actions bot added the stale Issues or pull requests that have become inactive without resolution. label Jul 13, 2023
@TristanCacqueray
Copy link

Thank you, that seems to work well!

@drashna drashna removed the stale Issues or pull requests that have become inactive without resolution. label Jan 16, 2024
drashna
drashna previously approved these changes Jan 16, 2024
@drashna drashna requested review from drashna and a team January 16, 2024 20:02
xenoscr added a commit to xenoscr/qmk_firmware that referenced this pull request May 10, 2024
@dstretch
Copy link

Is this dead? I'm getting the same RDP -> RDP problem with keystrokes not registering properly...

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.

5 participants