gin66 / FastAccelStepper

A high speed stepper library for Atmega 168/328p (nano), Atmega32u4, Atmega 2560, ESP32, ESP32S2, ESP32S3, ESP32C3, ESP32C6 and Atmel SAM Due
MIT License
301 stars 70 forks source link

PCNT issue using FastAccelStepper 0.28.1 with ESP32Encoder 0.10.1 + workaround #148

Closed pit34 closed 1 year ago

pit34 commented 1 year ago

Hello, I have found an issue about PCNT using FastAccelStepper and ESP32Encoder library on same project. Behaviour seems indicate that both library use same PCNT unit, despite recent change done on each library about PCNT handling. Here is my code about encoder and stepper :

[...]
ESP32Encoder encoder;
FastAccelStepperEngine engine                   = FastAccelStepperEngine();
FastAccelStepper      *stepper                  = NULL;
[...]

void Setup(){
[...]
encoder.attachFullQuad(ENC_A_PIN, ENC_B_PIN);
engine.init();
stepper = engine.stepperConnectToPin(MOTOR_STEP_PIN);
[...]

This way the stepper motor can move and his behaviour is OK, but encoder 's count is reset proportionally to the stepper speed. I have tried to manually change the PCNT unit used by both library, but I am not competent enough to make it work.

Nevertheless, I have found a workaround to make it work. It consist of create a first encoder i.e. "encoder0" (which will work on same PCNT unit that the stepper motor) AND to attach pins to it. If .attach() is not used, the workaround doesn't work.

[...]
ESP32Encoder encoder0;
ESP32Encoder encoder;
FastAccelStepperEngine engine                   = FastAccelStepperEngine();
FastAccelStepper      *stepper                  = NULL;
[...]

void Setup(){
[...]
encoder0.attachFullQuad(ENC_A_PIN, ENC_B_PIN);
encoder.attachFullQuad(ENC_A_PIN, ENC_B_PIN);
engine.init();
stepper = engine.stepperConnectToPin(MOTOR_STEP_PIN);
[...]

Issue posted on ESP32Encoder repository :

https://github.com/madhephaestus/ESP32Encoder/issues/83#issue-1429011726

In hope this could be used to improve both libraries ;).

gin66 commented 1 year ago

Both libs are allocating pcnt units starting with 0. There is nothing wrong with this, but for an app developer a resource conflict will happen, if both libs are in use. The proposed workaround ensures pcnt unit 0 is not managed by the encoder lib. FastAccelStepper will then allocate pcnt unit 0 and overwrite encoder‘s isr handler for unit 0.So this works, just does not look pretty.

The problem is, that esp32 RTOS does not provide means to the libs for proper resource allocation. So the libs cannot avoid resource conflicts. The one, who is in position to resolve the conflict, is the app sw developer (or sw integrator).

Do both libs support the sw integrator in this task? ….Could be for sure better.

BTW: FastAccelStepper can make use of the rmt module without the need for pcnt unit. Just as of now FastAccelStepper allocates the module types in a fixed scheme: first pcnt/mcpwm combos and only then the rmt based modules. So this issue could be a trigger to provide the module choice to the app developer.

gin66 commented 1 year ago

The upcoming 0.29.2 extends stepperConnectToPin(), so that mcpwm/pcnt or rmt module can be directly selected.

daniel-frenkel commented 1 year ago

@gin66 Thanks for this amazing library btw. I've run into the same issue as @pit34 using the ESP32Encoder library. Would you mind explaining how to use stepperConnectToPin() to avoid this issue?

I'm going to use the workaround posted by @pit34 for now , but if you took the time to create a new function to fix this issue, I'd love to use it :)

gin66 commented 1 year ago

which esp32 do you use ?

The explanation is here

daniel-frenkel commented 1 year ago

I have the original ESP32 (ESP32-WROOM-32E)

Do I just update add the 0 like this in order to define DRIVER_MCPWM_PCNT?

stepper = engine.stepperConnectToPin(STEP_PIN, 0);

Do you suggest not using the PCNT module in order to not interfere with the encoder? Is there really any advantage to using it?

gin66 commented 1 year ago

Situation is a bit more complex. Currently there is no scheme to force FastAccelStepper to use a specific PCNT. So the solution from @pit34 is a good option.

FastAccelStepper offers another driver type, which is RMT. For this the call would be:

stepper = engine.stepperConnectToPin(STEP_PIN, DRIVER_RMT);

Is not recommended to use hardcoded values like 0 instead of the DRIVER_MCPWM_PCNT. If the library is changing the API, then with DRIVER_MCPWM_PCNTit will either continue to work or a compile error will appear. With a hardcoded value mis behavior is seen only if running on the target and then the debugging is much harder.

The RMT passes all tests, but #174 has revealed some bugs, on which I am working.