zmkfirmware / zmk

ZMK Firmware Repository
https://zmk.dev/
MIT License
2.68k stars 2.74k forks source link

refactor(hog): Cleanup ble message sending #2363

Open khoek opened 3 months ago

khoek commented 3 months ago

We deduplicate a bunch of code removing about 80 lines, fixing a very minor copy-paste typo in the process. No functional change.

Stacked on top of other pull since we use a struct from there. Tested on an Adv360.

petejohanson commented 1 week ago

@khoek Any chance you are still interested in working on this?

khoek commented 1 week ago

@petejohanson Sorry about the radio silence, I forgot about your question, and remember interpreting it as slightly more contemplative at the time. Sure, I'll have a play around and see if I can get rid of them.

By the way while I have your attention: I've been having a bunch of trouble with applications correctly interpreting some taps coming out of hold-tap because of their very fast duration (coincidentally the same phenomenon which triggered the race condition bug which got me poking around in the first place)---mainly this is happening in e.g. video game engines which poll the key states themselves instead of handling events, and I guess the key is being press-released within a single frame. Anyhow, is there a natural way to integrate a configurable tiny delay into the current way ZMK works to alleviate this problem?---if that would be a bit awkward, is this more motivation for me to take another look at unblocking the main thread and adding a message queue for outbound USB messages (of course queuing a message to be sent later after some delay is then very cheap to implement in that setting)?

khoek commented 1 week ago

By the way, besides the upsides about queuing multiple messages instead of waiting which I evangelized in that other thread, the only downside I could think of was that, naively implemented, there would be a tiny latency slowdown which people interested in maximum responsiveness wouldn't like. So I just wanted to note that I was attentive to that and I'd be changing the USB send function to immediately commit a USB transaction if there wasn't a pending operation in progress, and only queue in the alternative case (of if e.g. the message was to be transmitted after some delay).

petejohanson commented 1 week ago

@petejohanson Sorry about the radio silence, I forgot about your question, and remember interpreting it as slightly more contemplative at the time. Sure, I'll have a play around and see if I can get rid of them.

By the way while I have your attention: I've been having a bunch of trouble with applications correctly interpreting some taps coming out of hold-tap because of their very fast duration (coincidentally the same phenomenon which triggered the race condition bug which got me poking around in the first place)---mainly this is happening in e.g. video game engines which poll the key states themselves instead of handling events, and I guess the key is being press-released within a single frame. Anyhow, is there a natural way to integrate a configurable tiny delay into the current way ZMK works to alleviate this problem?---if that would be a bit awkward, is this more motivation for me to take another look at unblocking the main thread and adding a message queue for outbound USB messages (of course queuing a message to be sent later after some delay is then very cheap to implement in that setting)?

You'd need to modify the hold-tap behavior to add a devicetree configurable release-delay and implement a sleep for that amount in the decide code for hold-tap.