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 keyboard skipping 'o' on lemp11 #359

Closed
wants to merge 1 commit into from
Closed

Conversation

Ivshti
Copy link

@Ivshti Ivshti commented May 30, 2023

Summary

I had an issue on my Lemur Pro 11 where the keyboard was skipping the o when typing fast for every word that ended in "sion".

This issue is well documented in support ticket 123465

I played around with different debounce values. I don't fully understand the logic but when I disabled debouncing caused by ghost keys (see #58 (comment)) I got the occasional ghost key (eg 'version' comes out as 'vers5ion')

The perfect compromise seems to be (from my tests) a low debounce value such as 3ms. Now I can type perfectly without ghost keys or missed keys.

Resume from suspend issue 1

I first tried flashing with the current master (82f091e) and resume from suspend broke: it worked normally most of the time but on some occasions the laptop didn't resume: upon attempting a resume, the screen remained off, LED solid green, no keyboard combos work and holding the power button didn't do anything either, I had to disconnect the battery.

Then I went back to ef6ea32 - this time, the laptop did not suspend AT ALL.

Then I went back to e032c5f and now the laptop seems to be functioning flawlessly.

I always performed internal flashing via make BOARD=system76/lemp11 internal_flash

## Summary

I had an issue on my Lemur Pro 11 where the keyboard was skipping the o when typing fast for every word that ended in "sion". 

This issue is well documented in support ticket 123465 

I played around with different debounce values. I don't fully understand the logic but when I disabled debouncing caused by ghost keys (see system76#58 (comment)) I got the occasional ghost key (eg 'version' comes out as 'vers5ion')

The perfect compromise seems to be (from my tests) a low debounce value such as 3ms. Now I can type perfectly without ghost keys or missed keys.

## Resume from suspend issue 1
I first tried flashing with the current master (82f091e) and resume from suspend broke: it worked normally most of the time but on some occasions the laptop didn't resume: upon attempting a resume, the screen remained off, LED solid green, no keyboard combos work and holding the power button didn't do anything either, I had to disconnect the battery.

Then I went back to ef6ea32 - this time, the laptop did not suspend AT ALL.

Then I went back to e032c5f and now the laptop seems to be functioning flawlessly.
@crawfxrd
Copy link
Member

crawfxrd commented Jun 2, 2023

A better change description would be "Decrease debounce to 3ms", since this change affects everything and not just lemp11. The suspend issue is unrelated and shouldn't be included in the commit message.

Does a larger value like 8 or 10 work? 3 seems too low to trust (without testing on every keyboard).

Other things I'm thinking of:

  • Would an asymmetrical debounce (separate times for up/down) be useful?
  • Should debounce time be set per keyboard?

@Ivshti
Copy link
Author

Ivshti commented Jun 2, 2023

A better change description would be "Decrease debounce to 3ms", since this change affects everything and not just lemp11. The suspend issue is unrelated and shouldn't be included in the commit message.

Does a larger value like 8 or 10 work? 3 seems too low to trust (without testing on every keyboard).

Other things I'm thinking of:

* Would an asymmetrical debounce (separate times for up/down) be useful?

* Should debounce time be set per keyboard?

I can rebase but I wanted to get your opinion on it first. I tested 5 and it seems fine, 10 skips but way more rarely than the default. I would suggest using 5 specifically for lemp11 and keeping the other value for other keyboards as I also haven't tested on other models of system

@crawfxrd
Copy link
Member

crawfxrd commented Jun 2, 2023

5ms seems to be what QMK uses as a default debounce time. I still think we should use something a bit higher since we're dealing with laptop scissor keys though. If you're can accept 8ms, we can probably make that the default for everything.

It should be changed so boards can override the value if needed. The usual ifndef/define would be enough.

@Ivshti
Copy link
Author

Ivshti commented Jun 4, 2023

5ms seems to be what QMK uses as a default debounce time. I still think we should use something a bit higher since we're dealing with laptop scissor keys though. If you're can accept 8ms, we can probably make that the default for everything.

It should be changed so boards can override the value if needed. The usual ifndef/define would be enough.

8ms is certainly way better than 15.

@crawfxrd
Copy link
Member

crawfxrd commented Jun 4, 2023

I am attempting to clean up the kbscan logic, starting with #362.

The datasheets say:

Supports Schmitt trigger input pin

I'm no EE, but presumably that means we have hardware debouncing already. How much do we really need to debounce in software? I'd say it's okay to change this to use the QMK default of 5ms.

@Ivshti
Copy link
Author

Ivshti commented Jun 13, 2023

I am attempting to clean up the kbscan logic, starting with #362.

The datasheets say:

Supports Schmitt trigger input pin

I'm no EE, but presumably that means we have hardware debouncing already. How much do we really need to debounce in software? I'd say it's okay to change this to use the QMK default of 5ms.

I tried no debouncing at all on lemp11 and I did get ghost keys, most notably "5".

@jackpot51
Copy link
Member

Adjusted in #378. 5ms was chosen to match QMK's default, which is used successfully on the Launch keyboard from System76.

@jackpot51 jackpot51 closed this Jul 17, 2023
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.

3 participants