kmk_firmware
kmk_firmware copied to clipboard
KMKfw
Support for tap behavior autorepeat for modtap and layertap keys.
This is an implementation of QMK-like autorepeat for tap-hold keys.
Assuming you have a tap hold key, you can simulate a long press of the tap behavior (which should trigger the autorepeat feature if set in the OS). To this purpose: within the hold delay, tap the key once, then press it again and keep it pressed as long as you want the autorepeat to last.
The behavior can be disabled by passing the optional argument repeat = False.
I did not write the doc yet (what are the concerned files? only modtap.md and layers.md?)
make fix-formatting fix-isort test should fix most things, and show you if you pass locally without even pushing BTW
make fix-formatting fix-isort testshould fix most things, and show you if you pass locally without even pushing BTW
Ah thank you, I will try this.
This may be an ignorant question: isn't that the same as the tapdance KC.TD(KC.MT(KC.X, KC.LSFT), KC.X) for example?
This may be an ignorant question: isn't that the same as the tapdance
KC.TD(KC.MT(KC.X, KC.LSFT), KC.X)for example?
I am the ignorant one! I am still getting used to KMK and still polluted with the QMK way of thinking.
In QMK, such a combination is not possible (for the sake of memory efficiency, the arguments of a tap dance or a hold-tap can only be basic keycodes).
So maybe what I proposed is unnecessary.
Ok, I just tried the alternative proposed by xs5871. So, it doesn't work well. 2 issues:
- the tapdance and holdtap timeouts add up, entailing a quite big latency
- for a reason I can't explain yet, the tap keycode often ends up stuck in pressed state (causing endless repeats).
So I would still support the patch I proposed.
Another question: should autorepeat be the default behavior?
Con: before the patch, autorepeat was disabled. Existing keyboards would thus change their behavior if they updated to a newer KMK version.
Pros:
- the feature is enabled by default in QMK, so it would be a smoother migration
- if you don't know about this feature and it is enabled, even so it would not typically trigger bad or unexpected behaviors. The impact on existing keyboards would be almost negligible (also when updating the firmware, you know you can expect some behavioral changes anyway....).
The answer to the latter question will impact the way I should fix the tests and what new tests should be added (currently, having autorepeat enabled make the tests fail, probably for legitimate reasons).
I still think this would be better suited as a special case of tapdancing. We can improve tapdance to resolve the last key in a sequence immediately, that's a good idea. The stuck keys, yeah, we need to fix that anyway / will fix it as part of #467.
I just noticed that the immediate resolve already exists: KC.TD(KC.MT(...), KC..., tap_time=0).
I just noticed that the immediate resolve already exists:
KC.TD(KC.MT(...), KC..., tap_time=0).
Ok this fixes the delay issue, but not the key getting stuck. But I agree this is an already existing, separate, bug which needs to be fixed anyway.
I really want this, so I tried to merge it with master in my fork and it seemed fine at first, but I found a case when it breaks:
- tap any modtap or layertap key
- wait more than tap timeout, even a few seconds
- hold the same key - it repeats it instead of holding I don't see where I could have merged it wrong, but I am willing to help however I can.
This appears to be the same issue as mentioned in #593, which isn't at all clear if you're not familiar with that feature and the frankly terrible feature names of QMK. The code here has a couple of bugs, gotchas, has now merge conflicts, and doesn't pass unit tests. I accidentally re-implemented this functionality for #593, with (probably) all those fixed. It's original code, but by some miracle the code is 90% identical, except for those fixes. @ald0o, how do you want to proceed?
I accidentally re-implemented this functionality for #593
#593 doesn't have PRs or branches linked. Where is this code?
This appears to be the same issue as mentioned in #593, which isn't at all clear if you're not familiar with that feature and the frankly terrible feature names of QMK. The code here has a couple of bugs, gotchas, has now merge conflicts, and doesn't pass unit tests. I accidentally re-implemented this functionality for #593, with (probably) all those fixed. It's original code, but by some miracle the code is 90% identical, except for those fixes. @ald0o, how do you want to proceed?
If your implementation does the same think without the bugs and passes the tests, isn't it reasonable to merge yours?
This appears to be the same issue as mentioned in #593, which isn't at all clear if you're not familiar with that feature and the frankly terrible feature names of QMK. The code here has a couple of bugs, gotchas, has now merge conflicts, and doesn't pass unit tests. I accidentally re-implemented this functionality for #593, with (probably) all those fixed. It's original code, but by some miracle the code is 90% identical, except for those fixes. @ald0o, how do you want to proceed?
If your implementation does the same think without the bugs and passes the tests, isn't it reasonable to merge yours?
When the maintainers are happy with the quality of the code, it will be merged. We are currently either not happy with it, or too busy with our real lives to get it worked on and/or merged. This is open source work done for free, and none of us are paid to work on KMK. If you have worthwhile things to say about that PR, please make notes on that. This is off topic, and I really would prefer if this gets back on topic. If you disagree with what we have on upstream, feel free to fork and run whatever code you wish there, or give us useful feedback, or code, and we'll get to it as we have time. Thank you.
When the maintainers are happy with the quality of the code, it will be merged. We are currently either not happy with it, or too busy with our real lives to get it worked on and/or merged. This is open source work done for free, and none of us are paid to work on KMK. If you have worthwhile things to say about that PR, please make notes on that. This is off topic, and I really would prefer if this gets back on topic. If you disagree with what we have on upstream, feel free to fork and run whatever code you wish there, or give us useful feedback, or code, and we'll get to it as we have time. Thank you.
There is probably a misunderstanding here. I was only answering @xs5871's direct question to me, not trying to press anyone to do anything. If I was in such a hurry I would already have fixed the patch I submitted by myself!
I am sorry if my previous comment was formulated in a way that led you to think otherwise!
When the maintainers are happy with the quality of the code, it will be merged. We are currently either not happy with it, or too busy with our real lives to get it worked on and/or merged. This is open source work done for free, and none of us are paid to work on KMK. If you have worthwhile things to say about that PR, please make notes on that. This is off topic, and I really would prefer if this gets back on topic. If you disagree with what we have on upstream, feel free to fork and run whatever code you wish there, or give us useful feedback, or code, and we'll get to it as we have time. Thank you.
There is probably a misunderstanding here. I was only answering @xs5871's direct question to me, not trying to press anyone to do anything. If I was in such a hurry I would already have fixed the patch I submitted by myself!
I am sorry if my previous comment was formulated in a way that led you to think otherwise!
That was a general statement, not targeting any specific user. You got caught in the crossfire there, so nothing personal. Was likewise a misunderstanding on my part. 👍
If your implementation does the same think without the bugs and passes the tests, isn't it reasonable to merge yours?
Because of proper attribution :) I can take your work, fix it up, and submit another PR. No additional work for you, but some of your contribution will not show up in the logs. Other option is: I share my code and you merge/copy paste it onto your existing work. More work for you, but you keep control over your contribution.
Thanks for the thought ! People are waiting for this feature, so I am OK if you fire first and make a new PR. If you do not fire, I might... or not find a few minutes to fix mine.