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

Configurable threshold to prevent tiny movements activating auto mouse layer #563

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

okuRaku
Copy link

@okuRaku okuRaku commented Apr 22, 2024

Hello,

Thank you for efforts supporting this great device. I have been enjoying the feature added in #508 and noticed that sometimes tiny movements, especially typing quickly, were accidentally activating the mouse layer.

I noticed that the qmk firmware mainline had a setting for this, detailed here qmk/qmk_firmware#21398 and added in version 0.24.0. Based on the code there, I tried implementing it myself in our keyball fork.

I am not sure about a good starting value though.

@okuRaku
Copy link
Author

okuRaku commented Apr 23, 2024

Sorry I did not run the size comparison yet. I see the script for it but need to set up my environment more to run it.

@ljmill
Copy link

ljmill commented Jun 12, 2024

This seems close to a good solution. However, with this implementation, if you activate AML by moving the cursor and then toggle scroll mode (kb 6): you become locked out of the AML because scrolling does not seem to be activating the AML.

How can we modify it so that scrolling causes the AML to activate as well?

@okuRaku
Copy link
Author

okuRaku commented Jun 12, 2024

@ljmill Thanks for your comment! That's a really interesting point. I didn't encounter it myself, I think? It could be due to my layout configuration. I have scroll mode bound to my K key on the mouse layer, so I use AML to activate it, then press and hold K, and the mouse layer will stay active until I release.

I'm not sure of a way to handle that, if you think of a way feel free to add it in!

@ljmill
Copy link

ljmill commented Jun 12, 2024

@okuRaku I have made some modifications to the pull request and pushed them to my fork. It now checks for scrolling and activates the auto mouse layer.

Commit 1

@okuRaku
Copy link
Author

okuRaku commented Jun 12, 2024

@okuRaku I have made some modifications to the pull request and pushed them to my fork. It now checks for scrolling and activates the auto mouse layer.

Commit 1

Nice! That seems good to me. If you could put your commit into a branch on your fork I think I can merge it into my branch that's in this PR.

However, I have the feeling the leads are not so interested in this one, or maybe we will be on a new version of QMK soon anyway and need to redo. But, just in case, I'd like to put your changes in this PR too with the proper attribution.

@ljmill
Copy link

ljmill commented Jun 12, 2024

@okuRaku Apologies. I am still getting familiarized with using Git/Github. I have created a branch of my fork here.

I also removed your keymap from my fork and added my own (oops). Hope it doesn't cause issues. Please let me know if you need me to do anything else. Thank you!

@okuRaku
Copy link
Author

okuRaku commented Jun 12, 2024

That's okay! It seems like your fork may be from my main instead of my upstream branch used in this PR. Let me have a look and see if I can get it arranged where you still get commit credit!

@okuRaku
Copy link
Author

okuRaku commented Jun 12, 2024

Ahhhh hmm. I also see the .DS_Store files etc. What may be easiest for both of us, if you could just make a brand new branch in your fork, that is based on okuRaku/keyball:upstream and add one commit that just has the changes you want in keyball.c, then I can definitely merge that back to okuRaku/keyball:upstream which should have it show up here.

civitaspo added a commit to civitaspo/keyball that referenced this pull request Sep 20, 2024
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