TCA8418, T-Lora Pager, T-Deck Pro: Keyboard improvements#7989
Conversation
- Pass key parameter to release(), allows tracking multiple held keys - Move KeyState enum and member out of TCA8418KeyboardBase class - Process all FIFO events in trigger(), remove overrides of trigger() - Remove all unused/unrelated code from TDeckPro/TLoraPager Keyboard - TDeckPro/TLoraPager Keyboard track held keys individually, can press multiple keys together and they are registered in order of release, helps greatly with typing fast registering inputs correctly - TDeckPro/TLoraPager Keyboard modifiers persist while held, even if a non-modifier key is pressed, like you would expect from a keyboard - Keep TCA8418Keyboard (multitap) handling one key at a time, wouldn't make much sense to handle multiple held keys on a multitap keyboard - Make event maps const so they do not take up RAM
- Restore behavior of single modifier keypress influencing next keypress - Holding modifier and clicking other keys/modifiers still combines them as you would expect from a real keyboard - But for ease of use, can press just one modifier on its own to make it apply to the next keypress
|
Hi @WillyJL! The "cardputer adv" keyboard is also TCA8418. There is no PR yet. |
Of course! That was the next thing I wanted to try and figure out, since a YouTuber and friend of mine got a cardputer adv and noted how typing felt quite sluggish. How should we go about it? Make a PR for cardputer adv first and once that's merged I update this PR for it? Or merge this first, and I'll provide the updated versions of the relevant files for cardputer adv for when that PR is opened? |
I am not a PR expert. I don't want to start being one either. I'm afraid I would do more harm than good. :) |
|
@Szetya just to confirm, are you the person that uploaded "Meshtastic For Cardputer-Adv" to M5Burner? i could not find a source for that firmware on M5Burner, and since it seems to work mostly well it would make sense to go off from that code to get it merged into here. if it is you, and its based on the files you shared in #7985 (comment), then i can open a PR for you with you as commit author, and based on whatever PR gets merged first i will update the other. |
No, I didn't upload it. |
interesting. so it would seem this "ozbk" guy on M5Burner is in breach of Meshtastic's GPLv3 license. |
I tried searching for the user on GitHub. I was unsuccessful, or perhaps I wasn't persistent enough. 😉 |
There was a problem hiding this comment.
I tested with a T-Deck Pro. The key combinations work except backspace: Typing backspace 2x within a short time reacts like escape and goes back to the main screen. Typing it slowly with a larger pause deletes the previously typed characters as expected.
Edit: I noticed when typing quickly a longer text several typed characters are missing. I believe this wasn't the case before this PR.
|
Interesting, I can't replicate either of those on T-Lora Pager. I had that behavior, where pressing backspace multiple times quickly caused it to switch view (not quit the text input, but switch frame/tab in the ui to the clock one I think, and from there pressing backspace again kept deleting characters in the text input view even though I couldn't see it until all text was deleted where the next backspace removed the text input view from the tab list/switcher), before this PR, but not at all with these changes. Also, typing fast on it before I missed many inputs because I tend to click the next key before releasing the previous one, which with this PR is not a problem and registers correctly so I can type reliably much faster with this PR. I'll reread the T-Deck Pro code in case I missed something but other than that debugging will be difficult for me as I don't have the device |
I asked in the Meshtastic discord, someone asked then in the M5Stack discord, they said:
Which seems scummy to me. Anyway, I have built the code you shared in your issue and had my friend sn0ren test it with his cardputer adv (I don't have one), he said it works so I'll open a PR for it shortly with you as commit author so there's proper support without some third party gatekept build |
I experienced this behavior with the backspace key just yesterday when using the original files (on the cardputer). I can repeat it over and over again. At first, I thought it was a feature. |
We need to agree on which display driver to use. I don't want to decide that myself. |
|
@WillyJL: this is 2.7.7 alpha compared to the PR branch, quick typing test on the T-Deck Pro. Some characters get eaten. 2.7.7 IMG_0579.mp4PR: IMG_0578.mp4 |
|
@mverch67 thanks for the videos. The fact the screen refreshes slower makes me think it might be related. The TCA8418 has a 10 event FIFO, so I'm guessing the issue here is that there are more than 10 key press/releases before the main loop polls it again due to the gui sleeping (I'm assuming) longer to not redraw as fast on the epaper display, and it would make sense since it's at about 4-5 typed keys (so ~10 key events) not being drawn yet that it starts losing the last key not yet registered. If my conclusion is correct, I see 2 ways to handle it. One is similar to the other PR I made, have a freertos thread that gets woken up by the interrupt and submit into queue, so the inputs don't depend on the main loop. Other is changing how display delays drawing, if my assumption of it sleeping longer is the case, to not sleep but instead just no-op and keep track of the last time to drew to the screen, so it can draw less often without sleeping on the main loop. |
im thinking of ways to implement this fix, one way of course would be to do essentially the same thing i did for the rotary encoder PR, but that means 2 background threads doing nothing until an interrupt happens... i was thinking maybe its worth standardizing this? eg move this freertos thread to input broker and have an interrupt-safe method in input broker to give it a class with its own method to run ASAP in the freertos thread that would then queue events into a input broker queue and input broker would process them on the main thread in loop(). for example:
(names arent great can always change them, but i hope you get the idea) this way gpio can be polled as soon as something happens to register the events so they are not lost (like they are here on tdeck pro), while handling them later when system is ready, and other input systems can use this model too without spawning each their own freertos thread and queue... @mverch67 if you too think this is a good idea i will implement it and update #7986 to use it too one concern i have tho is im not sure what the state of freertos is on nrf52 based devices, is it possible to use freertos on them? |
|
@WillyJL Regarding nrf52 I found this from last year which indicates that freeRTOS is available for the nordic platform. But care must be taken about the code size as nrf52 firmware is at its flash size limits. |
i dont think it should: the way inputs reach the UI is the same as before, they are processed on the main loop, so for example if a screen refresh takes long the inputs will still be processed after they happen. this system however makes sure the inputs are recorded when they happen, even if the system is not ready to process them, so they do not get lost. AFAICT things like long press are handled in input sources, like TCA8418Keyboard class, which will emit different input events based on timing. this does not change, actually it would mean they are more responsive because it does not poll inputs with 300ms interval, but as soon a the hardware reports that an event has happened, so the logic in TCA8418Keyboard class to calculate time between inputs will be even more precise now. a concrete example of this is what you mentioned with pressing backspace 2 times quickly, for me on develop branch it often switches tab/frame instead of deleting 2 characters, while with this PR it always registers as deleting 2 characters, my guess is because it records the inputs right as they happen, not delayed by polling, thus the input timing is more accurate, even if they are processed by the UI later anyway. as for FreeRTOS on nrf52, i noticed unfortunately i do not have any other device on different architectures so i cannot test those without freertos besides "it compiles", but i assume its fine since i dont think there is any overlap between devices without freertos and devices with these input modules that im porting to a interrupt + freertos task + queue approach. |
This reverts commit cea129c.
|
@mverch67 is it possible to reopen this? Or should I make a new PR? |
|
sigh |
KeyStateenum and field moved toTCA8418Keyboardclass (Nokia multitap keyboard) so classes derived fromTCA8418KeyboardBasecan handle key hold state their own waykeyparameter torelease()allowing T-Lora Pager and T-Deck Pro to track multiple held keys at oncetrigger()implementation intoTCA8418KeyboardBase, and fix it to process all available FIFO key events correctlyKB_INTpin to trigger on interrupts, thus no more 300ms typing delay on T-Lora Pager and T-Deck Pro keyboardsInputPollablesystem inInputBrokerthat allows for polling inputs right after an interrupt so no events are missed, this is what TCA8418 now uses for interruptsInputEventsources inherit fromInputPollable(like they did withObservable<const InputEvent *>pollOnce()method on the input source class, this will be run from userspace so it can do whateverpollOnce()add events withinputBroker->queueInputEvent(&event)(instead ofthis->notifyObservers(&event)with previous system)inputBroker->pollSoonRequestFromIsr(this)(thisbeing the input event source class which inherits fromInputPollable)pollOnce()byInputBrokeras soon as the interrupt handler exits, bypassing cooperative multitasking delay of OSThread, providing much better responsiveness and avoiding missed inputs, and avoiding unnecessary periodic pollinginputBroker->queueInputEvent(&event)inpollOnce(), they will be processed by GUI and other modules on the main thread when possible viainputBroker->processInputEventQueue()which is shared among all input event sources that use this paradigmUnfortunately I do not have a T-Deck Pro to test, but being so similar to the T-Lora Pager keyboard I'm guessing it should be fine.
I also have no way of testing regressions on
TCA8418Keyboard, which AFAICT is for Nokia 5130 with multitap (click same key multiple times to cycle what character it produces); since it's multitap I did not add N-key rollover to it, I tried to keep it equivalent behavior to current, but can't test for regressions from the refactoring.🤝 Attestations