Skip to content

T-Lora Pager: Fully fix rotary encoder and speaker fuzzing/popping#7986

Merged
thebentern merged 17 commits into
meshtastic:developfrom
WillyJL:fix/tlora-pager-rotary-amplifier
Oct 2, 2025
Merged

T-Lora Pager: Fully fix rotary encoder and speaker fuzzing/popping#7986
thebentern merged 17 commits into
meshtastic:developfrom
WillyJL:fix/tlora-pager-rotary-amplifier

Conversation

@WillyJL

@WillyJL WillyJL commented Sep 14, 2025

Copy link
Copy Markdown
Contributor
  • Rotary encoder now uses an interrupt + freertos thread + queue mechanism:
    • It sounds overengineered but hear me out
    • Reading the gpio pins in the interrupt proved unreliable and missed some inputs randomly all the time
    • The LilyGoLib factory example uses a freertos thread to poll the gpio every 2ms which is inefficient
    • This instead uses an interrupt to detect that something happened, which wakes the freertos idle background thread to poll the gpio immediately just once so it wont get missed, and dispatch the inputs to a queue
    • The usual "OSThread" mechanism polls the queue only, not gpio, every 10ms as before to send the events to GUI (sending events from the freertos thread causes crashes) (sidenote, the "OSThread" name is a bit deceiving, I spent more than I'd like to admit getting mad at why waiting for queue items in a "thread" froze the whole system, just to realize "OSThread" is actually a protothread ran in sync and yielding needs to be done manually and all system components including input, keyboard, and gui are done in this manner, and its not compatible with freertos synchronization primitives)
    • Overall, now rotary encoder is fully fixed, it responds very well to scrolling and never skips a tick (except scrolling fast, that might detect 2 ticks when 3 were scrolled for example, but it will not skip ticks out of the blue)
  • New InputPollable system in InputBroker that allows for polling inputs right after an interrupt so no events are missed, which standardizes the process explained above so other input sources can use it (eg TCA8418 in TCA8418, T-Lora Pager, T-Deck Pro: Keyboard improvements #7989, the code for this is the same, feel free to merge either PR first)
    • Basic idea is:
    • InputEvent sources inherit from InputPollable (like they did with Observable<const InputEvent *>
    • Define a pollOnce() method on the input source class, this will be run from userspace so it can do whatever
    • Inside pollOnce() add events with inputBroker->queueInputEvent(&event) (instead of this->notifyObservers(&event) with previous system)
    • During setup attach an interrupt for the device and in the interrupt handler call inputBroker->pollSoonRequestFromIsr(this) (this being the input event source class which inherits from InputPollable)
    • With this setup, when the interrupt is hit the class will be polled using pollOnce() by InputBroker as soon as the interrupt handler exits, bypassing cooperative multitasking delay of OSThread, providing much better responsiveness and avoiding missed inputs, and avoiding unnecessary periodic polling
    • After the input events are collected with inputBroker->queueInputEvent(&event) in pollOnce(), they will be processed by GUI and other modules on the main thread when possible via inputBroker->processInputEventQueue() which is shared among all input event sources that use this paradigm
    • Note that this relies on FreeRTOS, so it will not work on platforms that don't use FreeRTOS
  • Speaker fuzzing and random popping fixed by cutting power to the amplifier when no sound is being played
  • I'm not sure how to measure battery consumption in idle / sleep, would be nice if someone could check that, I'm not entirely sure if any changes here could affect that and if it would need some changes for proper sleep

🤝 Attestations

  • I have tested that my proposed changes behave as described.
  • I have tested that my proposed changes do not cause any obvious regressions on the following devices:
    • Heltec (Lora32) V3
    • LilyGo T-Deck
    • LilyGo T-Beam
    • RAK WisBlock 4631
    • Seeed Studio T-1000E tracker card
    • Other (please specify below)
      • LilyGo T-Lora Pager

@thebentern thebentern requested a review from mverch67 September 15, 2025 10:59

@mverch67 mverch67 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was also thinking to use a queue to dispatch events from the interrupt handler but the current behavior was sufficient for me so I didn't spent the effort.

Comment thread src/AudioThread.h Outdated
@mverch67

Copy link
Copy Markdown
Collaborator

I'm not sure how to measure battery consumption in idle / sleep, would be nice if someone could check that, I'm not entirely sure if any changes here could affect that and if it would need some changes for proper sleep

The measured power consumption during light sleep was 36mA. Switching off the peripherals before light sleep didn't save any power. In deep sleep it's 65μA.

@QuietImCoding

Copy link
Copy Markdown

eargerly awaiting this being merged, rotary encoder as it stands is really broken

@QuietImCoding

Copy link
Copy Markdown

Ah, need a tag for "hardware-support" to pass pull requirements

@thebentern thebentern added the hardware-support Hardware related: new devices or modules, problems specific to hardware label Sep 16, 2025
@WillyJL

WillyJL commented Sep 17, 2025

Copy link
Copy Markdown
Contributor Author

possibly on hold, would like feedback on this #7989 (comment) as it would provide a standard interface for other input subsystems to do what this pr does to get more reliable inputs without duplicating code and having multiple background threads

@QuietImCoding

Copy link
Copy Markdown

Interesting thread, definitely like the idea of centralizing input events in a more streamlined way, should help deal with a greater variety of form factors going forward.

@WillyJL WillyJL marked this pull request as ready for review September 20, 2025 00:42
@WillyJL

WillyJL commented Sep 20, 2025

Copy link
Copy Markdown
Contributor Author

i reworked it based on the system i made for #7989 , seems to work great and means the 2 systems will not duplicate necessary resources for thread/queue etc...

they have 1 commit in common implementing the underlying system, thats why it may look similar. merging either PR first will be fine, then can just merge develop into the other PR.

@WillyJL

WillyJL commented Sep 23, 2025

Copy link
Copy Markdown
Contributor Author

not sure if i need to do something to fix CI on this and keyboard improvements PR

EDIT: ah i see its a build fail, i hadnt scrolled up enough

@giannoug

Copy link
Copy Markdown
Contributor

I'm so glad I came across this PR! I had actually started working on these fixes myself. I managed to get the rotary encoder working by refactoring RotaryEncoderImpl to behave like LILYGO’s examples. The encoder worked fine, but navigation felt a bit laggy (yours is much smoother!). I wasn’t sure if that was a Meshtastic, display, or hardware limitation, so I stopped there. The popping noise though...

This PR works perfectly on my hardware and fixes both of the issues I was experiencing! Navigation is smooth, and there’s no popping.

I’m also interested in adding a Greek font for displaying messages and a Greek keyboard layout. Before I spend time on it and possibly hit a dead end, should I start working on it now or wait for the -tft release (or something else)? Maybe I could open a new issue so we can discuss it there?

@WillyJL

WillyJL commented Sep 28, 2025

Copy link
Copy Markdown
Contributor Author

Regarding Greek font and layout, probably best to open an issue for discussion

@hylaride

hylaride commented Oct 2, 2025

Copy link
Copy Markdown

FWIW, I merged this into my own repo that was synced off of today's develop branch and the scroll wheel is SO MUCH BETTER.

@WillyJL

WillyJL commented Oct 2, 2025

Copy link
Copy Markdown
Contributor Author

FWIW, I merged this into my own repo that was synced off of today's develop branch and the scroll wheel is SO MUCH BETTER.

i have my 4 currently open PRs for tlora pager and cardputer adv and the multi message storage branch from meshtastic repo merged in my fork's develop branch as ive been dogfooding them on my device, if you wanna try the keyboard improvements and rtc too

@thebentern thebentern merged commit c48a64e into meshtastic:develop Oct 2, 2025
82 checks passed
@mverch67

Copy link
Copy Markdown
Collaborator

fixes #8258

@jeremyd

jeremyd commented Nov 7, 2025

Copy link
Copy Markdown

I'm having some kind of timing issues with heltecv3 and rotary encoder running the latest firmware (master branch), but it's that, it goes the wrong direction 30-40% of the time. Any pointers on how this was tested on heltecv3? Pins used? I read somewhere I need to make sure the pins support interrupt.. Tried different pins and different encoder parts, and even setting a couple different deBounceMs delays in the code to no avail. It's the GPIO encoder type.

@WillyJL

@WillyJL

WillyJL commented Nov 7, 2025

Copy link
Copy Markdown
Contributor Author

@jeremyd this PR is specific to T-Lora Pager, afaik no other devices currently use the same FSM rotary encoder. also, heltecv3 has no rotary encoder, so there is nothing to test. if you added your own rotary encoder to your specific device, thats not something that someone else can test.

@jeremyd

jeremyd commented Nov 8, 2025

Copy link
Copy Markdown

@jeremyd this PR is specific to T-Lora Pager, afaik no other devices currently use the same FSM rotary encoder. also, heltecv3 has no rotary encoder, so there is nothing to test. if you added your own rotary encoder to your specific device, thats not something that someone else can test.

Oh ok, sorry, I was reading it and it had the " I have tested that my proposed changes do not cause any obvious regressions on the following devices: heltecv3". No worries, yeah I think the rotary encoder I'm trying to use, does not act the same I suppose, as what the firmware is expecting.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

hardware-support Hardware related: new devices or modules, problems specific to hardware

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants