Encoder is not responsive with long tasks in loop

[leelx78]

Hi @davetcc I open a card here because it looks like the issue is with the encoder class.

This is on ESP32-S3.

let's consider a case where there is a function in the main loop which takes a set amount of time (5.5ms in my case), e.g.

void loop()
{
  taskManager.runLoop();

  DEVICE.loopFunction(); // this function takes 5.5ms
}

In this case if I use a third party library for the encoder I can simply flag an "ENCODER_ROTATED_CW" event into an ISR whenever the encoder transition happens and then react to it at the next loop. Works well but unfortunately if I use this approach I would need to manually pass these events into MenuManager which is not ideal.

If I use your rotary encoder implementation (either polling or via interrupts) it seems to miss most of the events. From what I see if the transition happens during DEVICE.loopFunction() it is just ignored, while the other library correctly flags it down.

I had a look at your RotaryEncoder classes but I couldn't see the issue so far. Do you have any suggestions?

Thanks as always!

[davetcc]

Yes, this is down to the way it is currently implemented, even when interrupt-based, there is a need for the main loop to activate within a couple of milliseconds to get the next state. As I said before, at least as a temporary measure a potential workaround is to make a new implementation of the RotaryEncoder class that uses the other library; at least until I have chance to add this final support to IoAbstraction. I have to get 4.2 out first and fix a couple of long-standing outstanding bugs.

If you look at the master branch of 4.2, you'll see that I've started adding a state machine-based rotary encoder on master, but it is not yet ready. My plan is with this new encoder to handle all the rotational stuff within the interrupt handler itself, and only notify task manager to handle the callback. But like I say, it is not ready.

[davetcc]

So in short, go with the inconvenience for now, knowing that a proper solution is not far away. The rotary encoder in IoAbstraction has been a bugbear of mine for a while, I need to improve it for a couple of situations myself. Once the new state machine encoder is working, it should be near flawless.

[leelx78]

Thanks @davetcc . If that helps, I have tested this library

    https://github.com/caiofrota/cf-arduino-lib-pushbutton
    https://github.com/caiofrota/cf-arduino-lib-rotary-encoder

and works great so maybe you don't need to reinvent the wheel and just include it in your work? The ISR does nothing but flagging an event (CW/CCW/SW) which is then addressed in the main loop at the appropriate time.

[davetcc]

and works great so maybe you don't need to reinvent the wheel and just include it in your work? The ISR does nothing but flagging an event (CW/CCW/SW) which is then addressed in the main loop at the appropriate time.

Which version of IoAbstraction are you using out of interest?

After 4.2 goes live, I'm going to finish off the state-machine based encoder logic. The thing is TcMenu supports a few other environments other than Arduino, so I'll get the encoder support working for this case.

[leelx78]

Morning @davetcc I'm using your latest available on platformIO

davetcc/tcMenu@^4.1.1

looking forward for the encoder support! Hopefully the library I mentioned gave you some feedback on what is working well on ESP32 S3

Thanks!

[leelx78]

Hi @davetcc do you have an updated ETA for embedding the encoder class? My hack is sort-of-working but it's really not a good solution for production. I'll be more than happy to test a beta and give feedback if you have anything ready now.

Thanks!

[vzahradnik]

@davetcc what is the current state of this issue? Is the state machine-based encoder still in work in progress? I have some issue with the rotary encoder myself and I wonder whether I should report bugs and/or try to fix the code.