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

Remove microsleep after tapping or toggling keyboard #64

Merged
merged 1 commit into from
May 15, 2021

Conversation

ekrenzin
Copy link
Contributor

@ekrenzin ekrenzin commented Apr 12, 2021

What:

  • Remove micro sleep after keyboard taps and toggles

Why:

  • Issue 220
  • Keyboard delay is not behaving in an expected way.
  • The delay is unexpectedly 300ms for pressKey() and type() even when setting the keyboard delay to be less than 300ms.
  • Nut.js type() {@link String} is much faster than type() {@link Key} IE type('backspace') vs type(Key.Backspace)
  • It is therefor faster to execute keyboard.type('backspace') than it is to press the backspace key

Miscellaneous notes:

  • NOT TESTED
  • Potentially related robot js issue: 530

Update:

  • Tested on a forked branch, confirmed the problem stems from this code and can be resolved with this change. I'm not sure if there are repercussions for apps that utilize the keyboard delay.

@ekrenzin ekrenzin changed the title https://github.com/octalmage/robotjs/issues/530#issuecomment-756057632remove microsleep after tapping or toggling keyboard Remove microsleep after tapping or toggling keyboard Apr 12, 2021
@ekrenzin
Copy link
Contributor Author

@s1hofmann is there anything I can do about the ci failure or to push this PR along?

@s1hofmann
Copy link
Member

@ekrenzin The CI run failed due to networking issues, the restarted run succeeded. I just want to verify something before merging. Thanks for your contribution 👍

@ekrenzin
Copy link
Contributor Author

@ekrenzin The CI run failed due to networking issues, the restarted run succeeded. I just want to verify something before merging. Thanks for your contribution 👍

@s1hofmann sounds great! Let me know if there is anything else I can do to help, and thanks again for the fast replies, much appreciated.

@ekrenzin
Copy link
Contributor Author

ekrenzin commented May 15, 2021

We went ahead and forked the lib and optimized a couple of things because we could no longer wait for this to be merged.

This PR will definitely improve performance of libnut.

https://github.com/Desktop-Vision/libnut

@s1hofmann
Copy link
Member

s1hofmann commented May 15, 2021

Hi @ekrenzin 👋

Sorry for the delay, but I have limited time to work on my project since I started a new full time job.

Would be nice if you would contribute your further changes upstream

@ekrenzin
Copy link
Contributor Author

ekrenzin commented May 15, 2021

No worries @s1hofmann just wanted to let you know about our fork and progress. After fighting with the deploying and whatnot we can sympathize with the time it takes! Figured I'd let you know that we tested and confirmed the performance problems were related to this artificial delay.

We will continue to maintain our fork since we need it for a production app. Also it will allow us to strip out what we don't need or add things we do.

Hopefully I didn't come off as irritated, we are actually quite grateful for your work and time! Take care!

The only change made (that you would want) was the one in this PR. The other commits are simply changing package names and bumping versions so we can use the fork we maintain. So you should be able to simply merge this :)

@s1hofmann s1hofmann merged commit 9e5dd60 into nut-tree:develop May 15, 2021
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