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

OneShot: Feature request: Add support for triggering multiple modifier OneShot #414

Closed
algernon opened this issue Oct 18, 2018 · 10 comments · Fixed by #1024
Closed

OneShot: Feature request: Add support for triggering multiple modifier OneShot #414

algernon opened this issue Oct 18, 2018 · 10 comments · Fixed by #1024
Labels
enhancement New feature or request plugin Issues related to otherwise unlisted plugins

Comments

@algernon
Copy link
Contributor

Originally by @joeynguyen, as keyboardio/Kaleidoscope-OneShot#41.

@algernon algernon added enhancement New feature or request plugin:oneshot labels Oct 18, 2018
@algernon
Copy link
Contributor Author

I finally got around to start going through existing issues, and I believe this is a duplicate of #420 (or the old keyboardio/Kaleidoscope-OneShot#38, rather). The root issue is that OneShot.inject() is not a particularly good API in its current form.

Instead of providing a new API to achieve what OneShot.inject() was supposed to help with, I'm just going to fix .inject() instead. That makes both issues have the same root cause.

@algernon algernon added the duplicate This issue or pull request already exists label Oct 21, 2018
@algernon
Copy link
Contributor Author

I'm reopening this, because there are two issues at play, and this one needs a bit more thought.

@algernon algernon reopened this Oct 21, 2018
@algernon algernon removed the duplicate This issue or pull request already exists label Oct 21, 2018
@algernon
Copy link
Contributor Author

Ok, so the OneShot.inject() API is enough to do this, as you found. I'm unsure about what APIs I could provide to make this kind of thing easier. You see, OneShot keys should also support being held, and not timeout while held. It currently keeps track of the last pressed oneshot key to figure out if it needs to go sticky, so even if we inject two oneshots, the next time we inject them, the sticky mechanism won't trigger. We need special handling for that...

So... ugh. I'm afraid that the code you posted @joeynguyen is about the best we can do at the moment.

Unless... hmm... I may have an idea.

@algernon
Copy link
Contributor Author

Yeah, that idea didn't work. So for now, your code is pretty much the easiest way to accomplish this. I'll update the example sketch to show this kind of use too.

@joeynguyen
Copy link

joeynguyen commented Oct 21, 2018

I recommend using the latest version of my macro - https://github.com/joeynguyen/dotfiles/blob/master/Model01-Firmware/Model01-Firmware.ino#L212-L277.

I fixed a bug with my previous implementation.

You could maybe clean it up a bit more by using else if and else statements instead of the return in each if statement. I forget if I had tried that or not.

@zedmango
Copy link

That link no longer works - is there a new one?

@gedankenexperimenter
Copy link
Collaborator

This could be addressed, at least in part, by #905, with OneShot.enableAutoModifiers(), which would make OneShot treat any modifier key (including one with other modifier flags applied) into a OneShot key. It would still be possible to have non-OneShot modifiers, by using LSHIFT(Key_NoKey), which would not be detected as a "modifier" key.

@joeynguyen
Copy link

joeynguyen commented Oct 6, 2020

That link no longer works - is there a new one?

Sorry @zedmango for not replying earlier and for removing the file from that location. I decided to make that repo private. Here's a gist though so it won't happen again - https://gist.github.com/joeynguyen/d1e6d485e648c443ae44c3a5c6e5a4e0#file-model01-firmware-ino-L212-L277. Although this implementation may be out of date and not work with the latest firmware.

@joeynguyen
Copy link

I was the one who initially filed the bug, but I've moved away from using OneShot in my workflow so I'm okay with closing this issue if you all agree. The bug was initially filed many firmware versions ago so it may have been fixed now like @gedankenexperimenter said and wouldn't work with my previous implementation anymore.

@gedankenexperimenter
Copy link
Collaborator

I think this one should stay open, because it's still a problem. Especially with #905 open, which can be considered to constitute a solution.

@algernon algernon added the plugin Issues related to otherwise unlisted plugins label Jun 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request plugin Issues related to otherwise unlisted plugins
Projects
None yet
4 participants