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

Independent timeout for the leader key #6420

Closed
2 of 4 tasks
danielo515 opened this issue Jul 26, 2019 · 12 comments
Closed
2 of 4 tasks

Independent timeout for the leader key #6420

danielo515 opened this issue Jul 26, 2019 · 12 comments

Comments

@danielo515
Copy link
Contributor

Feature Request Type

  • Core functionality
  • Add-on hardware support (eg. audio, RGB, OLED screen, etc.)
  • Alteration (enhancement/optimization) of existing feature(s)
  • New behavior

Description

I know that recently we (well, you ) implemented a new feature that allowed leader key to have per-key timeout or per-key timing. This was a great improvement, but I still miss something more: to specify a completely different timeout for the leader key.
This means that I may want a timeout of a couple of seconds after pressing the leader key and then just few milliseconds for the rest of the sequence.
The reasoning behind this is because the leader key is usually a bit apart from the rest of the keys, so the harder part of doing the sequence in time is reaching the first key of the sequence after hitting the leader key. Usually the rest of the keys on a combo are quite close or at least are keys that you are used to tap fast (the letters on a QWERTY layout for example).

This will improve a lot my usage of the leader key

Thanks.

@danielo515
Copy link
Contributor Author

Any chance to implement this?
Or a guidance about how to do it myself?

@noroadsleft
Copy link
Member

I think @drashna knows the way around that system.

@drashna
Copy link
Member

drashna commented Aug 1, 2019

There isn't exactly an easy way to do this.

However, the actual code is handled locally.

Some of the magic is here:
https://github.com/qmk/qmk_firmware/blob/master/quantum/process_keycode/process_leader.h#L35-L36

I think you'd want to have an array for the timeouts (eg uint8_t leader_timeout_array = { 300, 300, 200, 50, 3000 }), and then replace LEADER_DICTIONARY() with:

  if (leading && timer_elapsed(leader_time) > leader_timeout_array[leader_sequence_size])

@drashna
Copy link
Member

drashna commented Aug 1, 2019

@noroadsleft man, you just had to wait like 30 seconds :D I was almost done typing it out.... :D

@noroadsleft
Copy link
Member

@drashna 😆

@danielo515
Copy link
Contributor Author

danielo515 commented Aug 2, 2019

Oh wait, so those "magical" leader macros all that they do is define a bunch of external variables and thens nest everything into an if???!!! That leaves me a lot of freedom to do whatever I want!
So leader_sequence_size is the size of the leader sequence? does it include the leader key itself?
As far as I know, I can substitute the LEADER_DICTIONARY call with an if checking whatever I want right? So, if I want to give infinite amount of time to the first key on the leader sequence, all that I have to do is

  if (leading && leader_sequence_size > 1 && timer_elapsed(leader_time) > LEADER_TIMEOUT)

That is quite cool

@danielo515
Copy link
Contributor Author

Nop, it did not worked. Not sure what's wrong, but the leader sequence ends immediately if I add that check to the if.
Where is the leader_process function called? I can't see any reference.

@danielo515
Copy link
Contributor Author

@drashna any idea what's wrong with my code?

@danielo515
Copy link
Contributor Author

Ok, I found the problem (finally).
Now that it worked once I see where the problem is.
Because process_leader is only registering keys if the timeout has not ended (not sure why, this seems redundant) if you miss a sequence the leading boolean will never get reset and the process_leader function will never call qk_leader_start:

https://github.com/qmk/qmk_firmware/blob/master/quantum/process_keycode/process_leader.c#L51-L71

So first I need to understand why we stop registering keys if the timeout has passed. That is being already controlled by the matrix_scan_user leader section, so this seems redundant to me.
Another thing is that, instead of check if the leader key the latest thing, it should be the first one and have an early return.

What do you think? I feel tempted to add a new feature behind a flag like NO_LEADER_TIMEOUT to give the leader key infinite amount of time...

@danielo515
Copy link
Contributor Author

Definitely commenting out line 52 on process_leader.c and my extra check I got the behavior I was expecting.
I don't see a reason why this could not be the default behavior. In any case, let me know if you are open to at least let me implement it behind a flag. Or if you think this is nice as default behavior I can make a PR right now.

Regards

@austintrose
Copy link

Throwing in my 2-cents that I would quite like this feature! So once you hit the leader key you have all the time you need to start a sequence but once a sequence has begun you are confined by whatever timers are set.

@danielo515
Copy link
Contributor Author

@austintrose just for your information there is a MR for this feature: #6580
The only missing bit is the documentation, but if the suggestion gets approved I will for sure write it.
Regards

@zvecr zvecr closed this as completed Aug 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants