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 a variable to allow easy modification of the keypress limit. #4

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

Conversation

ZDBioHazard
Copy link

If you need to be able to press more than 6 keys on your virtual keyboard, and you don't mind losing BIOS compatibility, the changes in this branch enable you to simply change #define KEYS_ROLLOVER to however many keys you need, and it will alter the USB descriptor, resize the data structures and handle the extra keys automatically!

Even with the default 6-key configuration, this implementation is 34 bytes smaller. 😉 (on my Leonardo, at least)

I haven't thought of a sensible way to tweak KEYS_ROLLOVER from the user side, but at least people won't have to dig into the meat of this library to modify the rollover limit for their keyboard-emulating game controllers and whatnot.

Reorder some items, remove a redundant item, make spacing consistent.
If you need more than 6 keys of rollover on your USB keyboard,
now there's an easily editable value you can tweak.
@facchinm
Copy link
Contributor

facchinm commented Jun 9, 2016

LGTM! The code cleanup is enough to justify the merge, and the "configurable" keypress number could be useful for someone.
@NicoHood does your HID project handles more than 6 keypress?

@NicoHood
Copy link
Contributor

NicoHood commented Jun 9, 2016

Yes it does. The problem with this implementation is, that for u2 devices the HID reports are limited to 15 bytes + reportid. This means you should not set this variable higher than 15.

Other than 6 keys makes it bios incompatible, correct. But the arduino implementation is not bios compatible anyways.

My HID Project covers both needs: bios compatible and nkro keyboard with >100 keypresses at the same time. The question is, if the os can handle those.
https://github.com/NicoHood/HID/wiki/Keyboard-API#boot-keyboard
https://github.com/NicoHood/HID/wiki/Keyboard-API#nkro-keyboard

I am not against new usb functions, I really like to see people working on those things. But as the #define is just for proprietary use, it makes no sense to include it. And I think we (@facchinm and @cmaglie ) aggreed that the arduino libraries should keeps basic and simple and the HID Project should extend them.

If the implementation is not more reliable or smaller (you should test this!), I see no reason to add this. But if it does, feel free to open a PR at my project.

And also: This PR suffers from this missing feature too. I implemented it in the old times without arduino builder, but noone ever did for arduino-builder. It would make the code a lot simpler and compacter. This would give a breakable change to the IDE, but noone seems to care for now :( You might consider to finally have a look at this:
arduino/arduino-builder#15

@ZDBioHazard
Copy link
Author

Awesome, I hadn't run across the @NicoHood HID library. It took a little work to get it to play nice with arduino-makefile, but it works like a champ. SingleNROKeyboard somehow even seems to be 246 bytes smaller than the unmodified Arduino implementation with my simple ::press() and ::release() test app. 👍

If you like, I could go back and rework this patch into a general cleanup and resubmit it. Maybe change the #define to _REPORT_KEYS and remove the "you can edit this" comments, but keep it around so there aren't hard-coded sixes everywhere. Plus people could still edit it if they really want to. 😉 Maybe I can fix #5 while I'm at it.

@NicoHood
Copy link
Contributor

NicoHood commented Jun 9, 2016

I think i will not break the special bios compatiblity if the nkro is even smaller and better than this. If it works good, just use it like that ;)

If #5 also applies to my project, let me know.

@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@per1234 per1234 added topic: code Related to content of the project itself type: enhancement Proposed improvement labels Mar 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic: code Related to content of the project itself type: enhancement Proposed improvement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants