Encoder inputs

[vegetate7]

Just a curious, yet.
Why not to use ESP32's hardware pulse counter for encoders?
May be discussed / tried ealier, and there is reason for it.

[kachurovskiy]

I've tried it but it takes more lines of code, more complex and still uses interrupts internally because even hardware pulse counter ignores pulses during the time we're saving preferences. Not sure what the benefit is there.

[vegetate7]

Hit another bug. While spinle running, RPM periodically jumps to extremely high value.

spindle_encoder_bug2.mp4

If, for example, Gearbox mode running, the Z stepper makes 'clank" sound in this moment. I think it's somehow related to PCNT mode. may be 'int64_t' and 'long' types messing. Can not figure out yet.

Upd: It' definitely not caused by pref. save delay. Bug is happens when Z stepper running. I also disabled SaviIfChanged for be sure. Did not help.

Upd2: Ok, it is not 'int64_t ' to 'long' issue. It is not collision with DRO counters read (i've disable it). Also tried to set interval between Encoder reads, not help.
I made some Serial debug if 'delta' greater than ENCODER_STEPS_INT and:

22:19:07.050 -> EncErr: delta 32766 Prv 458724 New 491490
22:19:14.376 -> EncErr: delta 32766 Prv 688086 New 720852
22:19:19.625 -> EncErr: delta 32766 Prv 851916 New 884682
22:19:20.656 -> EncErr: delta 32766 Prv 884682 New 917448
22:19:30.105 -> EncErr: delta 32766 Prv 1179576 New 1212342
22:19:35.344 -> EncErr: delta 32766 Prv 1343406 New 1376172

Encoder counter is unresetting signed int64_t, this enough for 200000 years of run at 10k RPM.
'Prv' and 'New' are values of it for processSpindlePosDelta(), 'delta' is unprocessed counts.
Errors throwed same time as RPM display jumps, and 'delta' is always same,
and is something magic "max signed int16 - 1".
So the bug is somewhere in the ESP32Encoder library. :(
There is another version of this library, but it is int32_t and not protected for overflow.

[vegetate7]

Added monitoring for abs(delta):

20:44:53.741 -> EncErr: delta 32764 Prv -458722 New -425958
20:44:53.741 -> EncErr: delta -32786 Prv -425958 New -458744
20:44:59.742 -> EncErr: delta 32764 Prv -753616 New -720852
20:44:59.742 -> EncErr: delta -32783 Prv -720852 New -753635
20:45:01.060 -> EncErr: delta 32764 Prv -819148 New -786384
20:45:01.060 -> EncErr: delta -32783 Prv -786384 New -819167
20:45:02.384 -> EncErr: delta 32764 Prv -884680 New -851916
20:45:02.384 -> EncErr: delta -32779 Prv -851916 New -884695

Clearly visible MAX_INT16 spikes. This can be issue with overflow handling in the library. I tried to fix, but without success.
But it's also show a way for workaround: just re-read counter, if difference higher than, let's say, 32000.
I did so and the problem is gone:

    int64_t spindlePcntNew = pcntEncoderSpindle.getCount();
    delta = spindlePcntNew - spindlePcntPrev;
    if (delta == 0){
      return;
    }
    if (abs(delta) > 32000){
	    // Try to re-read
	    spindlePcntNew = pcntEncoderSpindle.getCount();
	    delta = spindlePcntNew - spindlePcntPrev;
    }

[kachurovskiy]

If I read the code correctly, you aren't using hardware PCNT either since you don't define SOC_PCNT_SUPPORTED when using the library - https://github.com/madhephaestus/ESP32Encoder/blob/master/src/ESP32Encoder.cpp#L10

IIUC the library listens to the CHANGE using a regular interrupt like we do. The only material difference that I noticed is ignoring pulses shorter than 10 nanoseconds which probably explains why you no longer see drifting - https://github.com/search?q=repo%3Amadhephaestus%2FESP32Encoder%20US_DEBOUNCE&type=code

It might only require 4 new lines in h4.ino to do the same in spinEnc() in h4.ino:

void IRAM_ATTR spinEnc() {
  static uint32_t lastTime = 0;
  uint32_t currentTime = micros();
  if (currentTime - lastTime < 10) return;
  lastTime = currentTime;
  spindlePosDelta += digitalRead(ENC_B) ? -1 : 1;
}

[vegetate7]

"SOC_PCNT_SUPPORTED" defined in the boad specific includes "soc/soc_caps.h". It's enabled for ESP32, ESP32S3, ESP32S2, and not for ESP32C3

[kachurovskiy]

Good catch, thanks! Shared my code below.

[duebelpapst]

Since I also have problems with the encoder, I would like to try your solution with the PCNT library, but I don't have a DRO.
Does your software work without DRO?

Regards,
Stefan

[vegetate7]

It should. Do not enable axis _POS_BY_DRO , _BACKLASH_BY_DRO. and PULSE_1_DRO/PULSE_2_DRO
Also I didn't test PCNT mode for rotary encoder on A1/A2 inputs. They can be used in Interrupt mode by disabling PULSE_1_PCNT / PULSE_2_PCNT.

And get latest code, I did some bugfixes.

[duebelpapst]

I'll test it

[duebelpapst]

Hi,

I tested the software today and it works.
My encoder has 600 pulses/revolution, driven 1:1, so I set ENCODER_STEPS_INT = 2400 and ENCODER_BACKLASH = 8.

All other settings, except for those concerning the DRO, were the same as in the original software, and the hardware was also unchanged.

All DRO settings are set to false.

I have cut several threads and they were all OK.
Even when I did several spring passes on the same thread the tool stayed in pitch.
I then loaded the original software and immediately had an offset of about 0.1mm towards the tailstock on each pass.

I hope Maxim, at least, adopts your changes in relation to the encoder.

In the next days I do some more tests, to see if it is stable.

Regards,
Stefan

[kachurovskiy]

I've tried using PCNT but I'm getting confusing results. Turning the spindle exactly 1 turn yields a weird number of impulses - something like 9/10 or 9/20 of the expected - 161 or 342 degrees depending on how I multiply the encoder PPR - instead of 360 degrees.

h4.txt

Here are the main changes:

#include "driver/pcnt.h"

void taskAttachInterrupts(void *param) {
  pcnt_config_t pcntConfig;
  pcntConfig.pulse_gpio_num = ENC_A;
  pcntConfig.ctrl_gpio_num = ENC_B;
  pcntConfig.channel = PCNT_CHANNEL_0;
  pcntConfig.unit = PCNT_UNIT_0;
  pcntConfig.pos_mode = PCNT_COUNT_INC;
  pcntConfig.neg_mode = PCNT_COUNT_DEC;
  pcntConfig.lctrl_mode = PCNT_MODE_REVERSE;
  pcntConfig.hctrl_mode = PCNT_MODE_KEEP;
  pcntConfig.counter_h_lim = 32767;
  pcntConfig.counter_l_lim = -32768;
  pcnt_unit_config(&pcntConfig);
  pcnt_set_filter_value(PCNT_UNIT_0, 2);
  pcnt_filter_enable(PCNT_UNIT_0);
  pcnt_counter_pause(PCNT_UNIT_0);
  pcnt_counter_clear(PCNT_UNIT_0);
  pcnt_counter_resume(PCNT_UNIT_0);

  if (PULSE_1_USE) attachInterrupt(digitalPinToInterrupt(A12), pulse1Enc, CHANGE);
  if (PULSE_2_USE) attachInterrupt(digitalPinToInterrupt(A22), pulse2Enc, CHANGE);

  vTaskDelete(NULL);
}

void processSpindlePosDelta() {
  int16_t delta;
  pcnt_get_counter_value(PCNT_UNIT_0, &delta);
  pcnt_counter_clear(PCNT_UNIT_0);
  if (delta == 0) {
    return;
  }

  unsigned long microsNow = micros(); // unchanged from this line

[vegetate7]

Should be ok. May be set filter value much higher. I think something around 500 should be enough for 8000PPR@1200RPM.
Also not sure about "get_counter / clear_counter" strategy. We can miss pulses here. The ESP32Encoder library does same, but only on counter overflow interrput, so if it miss pulse, it will be much less probablity, and less loss (1/32k)

[kachurovskiy]

Thanks @duebelpapst and @vegetate7, looks like I got it working too, clearing the counter as infrequently as possible was indeed key.

https://gist.github.com/kachurovskiy/e8842a36b95ad179654dc50b461511ce

[vegetate7]

Looks very good. And promising. The ESP32Encoder libary is over-complicated because universal, and use lot of _CRITICAL_SECTION locks.
Would you like to change PULSE1/PULSE2 encoders too?

[vegetate7]

proposed changes to this pre-release. Just moved PCNT initialisation to function. This allow less changes for future adding PULSE1/PULSE2 PCNT encoders.

https://gist.github.com/vegetate7/f6d6b918ca7f3bed4ac7f0c93bec65aa

[vegetate7]

I feel we can get rid of PCNT interrupts completely. even for DRO mode. The 'getAndResetPulses()' called from continously loops in taskMoveZ / taskMoveX, so counters can be read same manner as for spindle encoder.

[kachurovskiy]

Turned out using ESP32 PCNT really helps in some situations where extra pulses are detected - in the latest software version 12 we've switched to it. Huge thanks to @vegetate7 and @duebelpapst 🎉

[vegetate7]

BTW, updated my branch to interruptless PULSE1/PULSE2 counters and DRO.