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

Subjugator keyboard bugfix #78

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

Conversation

TheRickyZhang
Copy link
Contributor

Previous bug was caused by key repeats sometimes having delay longer than default set 150ms. Since handling input could be a little finniky i used ncurses to handle more reliably, and it seems to work locally, but not sure how to integrate it into the monorepo package system.

@alexoj46 alexoj46 self-requested a review March 14, 2025 00:17
@alexoj46 alexoj46 marked this pull request as ready for review March 23, 2025 15:08
Copy link
Contributor

@alexoj46 alexoj46 left a comment

Choose a reason for hiding this comment

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

@TheRickyZhang Thanks for your continued efforts on this! When I checkout your code and run it, the bug still seems to be present on my machine, at least. It also seems that the instructional log messages show up in an odd format now. What steps do you use to test that the issue if fixed? It may be something that differs based on architecture, but if you could show a video or show me in person it working on your end that would be a helpful start! If the bug is still present, I think we should see if there is a fix that does not involve a third party library like ncurses, and see what in the code could be causing it in the first place.

Also, for future work on this issue, if you could use the updated code on main that would be awesome! We had refactored the package to divide code between a header file, cpp file, and main function, just to keep things more organized and standard for C++ packages. I figure you may not have seen those changes, so wanted to bring it to your attention.

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