Help with NewEncoder by gfvalvo on ESP32

Hey there gurus, I have been trying to use the rotary encoder library by gfvalvo to navigate through different menus on an ESP32. The issue is that I have not been able to use the 're configure routine' to change the min and max vales for the different menus once the encoder has been initialised. There are currently 8 menus with different number of entries selected by the switch on the rotary encoder. I would also like the encoder to have a circular nature (when increased at the max value the encoder wraps around to the min value). I would appreciate it if anyone has a working example that I could check syntax and get my code working.

@gfvalvo may be able to help

I guess by 're configure routine' you mean the newSettings() function that allows you to supply new values for min, max, and starting point:

bool newSettings(int16_t newMin, int16_t newMax, int16_t newCurrent, EncoderState &state);

What problem are you having with that function?

Doing that requires that you change the counting behavior of the encoder from the default of stopping at min or max. For this, you must create a new Class that inherits from NewEncoder and implements a custom version of the updateValue() function as described in the README:

See the 'CustomEncoder.ino' example that's included in the library. It implements a wraparound encoder.

And, of course, the standard advise: For detailed help, post your code.

Finally, are you using the most recent version of the library (v2.6)?

gfvalvo, thank you for the prompt reply; this forum has always amazed me by both speed and helpfulness of the replies.
My primary issue probably stems from the fact that I am updating an application that 'outgrew' the nano it was originally written for both in size of code and use of a TFT display and my unfamiliarity of the idiosyncrasies of the LOLIN D32 (ESP32) platform and that I just 'tinker' with microcontrollers in my latter years to keep the grey matter from atrophy and definitely NOT a C++ programmer.
I did see the custom encoder example but had mistakenly thought that the "SingleEncoderEsp32FreeRTOS" example was the one to base my revised encoder routines upon and it did indeed perform very well despite me having little idea how it seems to work. I have tried to add a couple of lines to add the button functionality using this code (from this source)

// for push swith make a collection of values
struct PushButton {
    const uint8_t PIN;
    uint32_t numberKeyPresses;
    bool pressed;
};

// and populate the collection
PushButton button1 = {enc_sw, 0, false};

//variables to keep track of the timing of recent interrupts
unsigned long button_time = 0;  
unsigned long last_button_time = 0; 

void IRAM_ATTR rot_button() {
button_time = millis();
if (button_time - last_button_time > 250)
{
Serial.printf("Change the menu to %i \n" ,crntItm );
//redrawMenu(crntItm);
  //This is where the currently visable menu is changed depending on the current item selected
//NewEncoder::configure(25, 26, 4, 10, 10);
 //       button1.numberKeyPresses++;
 //       button1.pressed = true;
       last_button_time = button_time;
}
}
// push switch to here

but when clicked it triggers panic mode. but I can't see how to modify the code so that I can change the min and max values 'on the fly' as different menus are selected nor add the 'wrap' effect.
I fully accept that my first task should have been to digest a decent C++ primer as there seems to be a lot I don't understand having mainly dabbled in Basic, VBA and javascript only to simplify the tasks in the day job but.....
I am also not able to install this library using the library manager in V2.21 of the Arduino IDE. I can manually add it but it then doesn't get updated (I was using 2.3 not 2.6 [2.5 in the library.properties file]) This library is faster than others I have tried and doesn't miss steps to boot. I have currently 8 separate .ino files inside the sketch folder each one for different functions but have embedded some of the encoder file above.

@mikeyBoyAus
If you don't mind, I'll address your points in separate replies as more bitesize chunks.

I have just fixed the version number issue with the library.properties (thanks for pointing that out!). So, you should not be using the latest issue v2.7. Correct, the NewEncoder library is not integrated into Arduino's library manager. So you must download the .zip file and either install it into the appropriate library folder manually or use the IDE's "add .zip library" feature. That's what it's called in IDE v1.x anyway. I'm not sure where to find it in IDE v2.x.

No, the "SingleEncoderEsp32FreeRTOS" example demonstrates how to use the library with the ESP32's built-in FreeRTOS real time operating system. I assume you're using the simpler Arduino setup() / loop() paradigm and not FreeRTOS.

So, the "Custom Encoder" example is the one you want to follow if you want to change the encoder's counting behavior (i.e. "wrap around" rather than stop counting at min or max).

As I mentioned in my first reply, you need to use the newSettings() function to change the min / max values, not the configure() function. You didn't include enough of your current code for me to tell you exactly how to use it. But, it would be something like:

  NewEncoder::EncoderState currentEncoderState;
  encoder.newSettings(newMin, newMax, newStartingValue, currentEncoderState);

As I said, it's documented in the library's README.

For more substantive help, please create and post an MRE. This is a small complete code that compiles and demonstrates the problem at hand. Do not include the unrelated clutter (TFT, etc) of your actually code, cut it to the bare essentials.

gfvalvo, many thanks for your help. I have had a go at using that as a starter. In the main file I use setup routines for different tasks one of which is the encoder, The encoder ino is below and uses pinmode and attachinterrupt to trap the button press on the encoder. This runs but randomly it causes the device to crash and I can't identify what is causing this.

#include "Arduino.h"
#include "NewEncoder.h"

#ifndef ESP32
#error ESP32 Only
#endif

// for push swith
struct Button {
    const uint8_t PIN;
    uint32_t numberKeyPresses;
    bool pressed;
};

Button button1 = {enc_sw, 0, false};

//variables to keep track of the timing of recent interrupts
unsigned long button_time = 0;  
unsigned long last_button_time = 0; 

void IRAM_ATTR ButtonIsr() {
button_time = millis();
if (button_time - last_button_time > 250)
{

//redrawMenu(crntItm);
  //This is where the currently visable menu is changed depending on the current item selected
 //       button1.numberKeyPresses++;
 //       button1.pressed = true;
       last_button_time = button_time;
}
}
// push switch to here

// Demonstrate creation of custom Encoder using inheritance.
// This encoder "wraps around" when it hits the min or max limit

class CustomEncoder: public NewEncoder {
  public:
    CustomEncoder() :
      NewEncoder() {
    }
    CustomEncoder(uint8_t aPin, uint8_t bPin, int16_t minValue, int16_t maxValue, int16_t initalValue, uint8_t type = FULL_PULSE) :
      NewEncoder(aPin, bPin, minValue, maxValue, initalValue, type) {
    }
    virtual ~CustomEncoder() {
    }

  protected:
    virtual void updateValue(uint8_t updatedState);
};

void ESP_ISR CustomEncoder::updateValue(uint8_t updatedState) {
  if ((updatedState & DELTA_MASK) == INCREMENT_DELTA) {
    liveState.currentClick = UpClick;
    liveState.currentValue++;
    if (liveState.currentValue > _maxValue) {
      liveState.currentValue = _minValue;
    }
      Serial.printf("Encoder: %i\n",liveState.currentValue);
  } else if ((updatedState & DELTA_MASK) == DECREMENT_DELTA) {
    liveState.currentClick = DownClick;
    liveState.currentValue--;
    if (liveState.currentValue < _minValue) {
      liveState.currentValue = _maxValue;
    }
     Serial.printf("Encoder: %i\n",liveState.currentValue);
  }
  stateChanged = true;
  crntItm=liveState.currentValue;
redrawMenu(crntItm);
}

CustomEncoder encoder(rot_enc1, rot_enc2, 1, 3, 1, FULL_PULSE);
int16_t prevEncoderValue;

void setup_encoder() {
  CustomEncoder::EncoderState state;

  Serial.println("Starting");
  if (!encoder.begin()) {
    Serial.println("Encoder Failed to Start. Check pin assignments and available interrupts. Aborting.");
    while (1) {
      yield();
    }
  } else {
    encoder.getState(state);
    Serial.print("Encoder Successfully Started at value = ");
    prevEncoderValue = state.currentValue;
    Serial.println(prevEncoderValue);
  }
  
// for push button
pinMode(button1.PIN, INPUT_PULLUP);
attachInterrupt(button1.PIN, ButtonIsr, FALLING);
}

not sure if this helps but here is a serial output after the crash


Core  1 register dump:
PC      : 0x4008b3da  PS      : 0x00060d35  A0      : 0x8008a352  A1      : 0x3ffbf21c  
A2      : 0x3ffbd544  A3      : 0x3ffbd3d4  A4      : 0x00000004  A5      : 0x00060d23  
A6      : 0x00060d23  A7      : 0x00000001  A8      : 0x3ffbd3d4  A9      : 0x00000018  
A10     : 0x3ffbd3d4  A11     : 0x00000018  A12     : 0x3ffc2734  A13     : 0x00060d23  
A14     : 0x007bf4a8  A15     : 0x003fffff  SAR     : 0x00000006  EXCCAUSE: 0x00000006  
EXCVADDR: 0x00000000  LBEG    : 0x40086bed  LEND    : 0x40086bfd  LCOUNT  : 0xffffffff  
Core  1 was running in ISR context:
EPC1    : 0x400e224b  EPC2    : 0x00000000  EPC3    : 0x00000000  EPC4    : 0x00000000


Backtrace: 0x4008b3d7:0x3ffbf21c |<-CORRUPTED


Core  0 register dump:
PC      : 0x4008b577  PS      : 0x00060035  A0      : 0x80089f7b  A1      : 0x3ffbebcc  
A2      : 0x3ffbf4a8  A3 �

curiously the other version I was using didn't do this. I am sure that, in my ignorance, I am doing something wrong but would appreciate an expert eye to spot it.

That code does not compile. Thus I can't try it myself and suggest fixes. That's why I requsted that you post an MRE:

BTW this is the kind of clutter that should be left out of your MRE:

   redrawMenu(crntItm);

We should just be dealing with the encoder code here. Print progress messages to the Serial Monitor only.

There is absolutely no reason to use interrupts for detecting butting pushes:

   attachInterrupt(button1.PIN, ButtonIsr, FALLING);

Trying to do so only complicates matters and is fraught with hidden risks for Newbies. Just poll the button (i.e. digitalRead()) in the loop(). If the loop() is repeating too slowly, you'll need to re-architect your code to use non-blocking techniques.

Speaking of interrupts, recall from my original reply (with emphasis added this time):

But your code:

void ESP_ISR CustomEncoder::updateValue(uint8_t updatedState) {
  if ((updatedState & DELTA_MASK) == INCREMENT_DELTA) {
    liveState.currentClick = UpClick;
    liveState.currentValue++;
    if (liveState.currentValue > _maxValue) {
      liveState.currentValue = _minValue;
    }
      Serial.printf("Encoder: %i\n",liveState.currentValue);
  } else if ((updatedState & DELTA_MASK) == DECREMENT_DELTA) {
    liveState.currentClick = DownClick;
    liveState.currentValue--;
    if (liveState.currentValue < _minValue) {
      liveState.currentValue = _maxValue;
    }
     Serial.printf("Encoder: %i\n",liveState.currentValue);
  }
  stateChanged = true;
  crntItm=liveState.currentValue;
  redrawMenu(crntItm);
}

... does two things that you should not be doing in interrupt context:

  1. Serial.print()
  2. Calls redrawMenu()

Please check again and see if there was anything printed above (before) the line:

Core  1 register dump:

There usually is.

gfvalvo, thanks again for the time taken to make sense of my fumbling in the dark. By a process of elimination I have modified my code and this part now seems to work as expected but I have yet to integrate this MRE into my code. You were right that it was the inclusion of Serial.print statements in the ESP_ISR routines that messed things up, I had always thought that the loop routine should be kept as free as possible and allow other 'more efficient' routines to handle processes especially interrupts. It can also get messy when the main file has the loop file and elements from all the other files that handles the menus, display, calendar etc are processed in it.

#include "Arduino.h"
#include "NewEncoder.h"


// for push swith
struct Button {
    const uint8_t PIN;
    uint32_t numberKeyPresses;
    bool pressed;
};

Button button1 = {16, 0, false};

//variables to keep track of the timing of recent interrupts
unsigned long button_time = 0;  
unsigned long last_button_time = 0; 

void IRAM_ATTR ButtonIsr() {
button_time = millis();
if (button_time - last_button_time > 250)
{
//redrawMenu(crntItm);
  //This is where the currently visable menu is changed depending on the current item selected
      //  button1.numberKeyPresses++;
        button1.pressed = true;
       last_button_time = button_time;
}
}
// push switch to here
// Demonstrate creation of custom Encoder using inheritance.
// This encoder "wraps around" when it hits the min or max limit

class CustomEncoder: public NewEncoder {
  public:
    CustomEncoder() :
      NewEncoder() {
    }
    CustomEncoder(uint8_t aPin, uint8_t bPin, int16_t minValue, int16_t maxValue, int16_t initalValue, uint8_t type = FULL_PULSE) :
      NewEncoder(aPin, bPin, minValue, maxValue, initalValue, type) {
    }
    virtual ~CustomEncoder() {
    }

  protected:
    virtual void updateValue(uint8_t updatedState);
};

void ESP_ISR CustomEncoder::updateValue(uint8_t updatedState) {
  if ((updatedState & DELTA_MASK) == INCREMENT_DELTA) {
    liveState.currentClick = UpClick;
    liveState.currentValue++;
    if (liveState.currentValue > _maxValue) {
      liveState.currentValue = _minValue;
    }
  } else if ((updatedState & DELTA_MASK) == DECREMENT_DELTA) {
    liveState.currentClick = DownClick;
    liveState.currentValue--;
    if (liveState.currentValue < _minValue) {
      liveState.currentValue = _maxValue;
    }
  }
  stateChanged = true;
   //   Serial.printf("Encoder: %i\n",liveState.currentValue);
}

CustomEncoder encoder(26, 25, 0, 20, 10, FULL_PULSE);
int16_t prevEncoderValue;

void setup() {
  CustomEncoder::EncoderState state;

  Serial.begin(115200);
  delay(2000);
  Serial.println("Starting");
  if (!encoder.begin()) {
    Serial.println("Encoder Failed to Start. Check pin assignments and available interrupts. Aborting.");
    while (1) {
      yield();
    }
  } else {
    encoder.getState(state);
    Serial.print("Encoder Successfully Started at value = ");
    prevEncoderValue = state.currentValue;
    Serial.println(prevEncoderValue);
  }
// for push button
pinMode(button1.PIN, INPUT_PULLUP);
attachInterrupt(button1.PIN, ButtonIsr, FALLING);
}

void loop() {
  int16_t currentValue;
  CustomEncoder::EncoderState currentEncoderState;
  if(button1.pressed==true){
    Serial.printf("Button pressed \n");
    button1.pressed=false;
    NewEncoder::EncoderState currentEncoderState;
    encoder.newSettings(1, random(4,15), 1, currentEncoderState);
  }

  if (encoder.getState(currentEncoderState)) {
    currentValue = currentEncoderState.currentValue;
    if (currentValue != prevEncoderValue) {
      Serial.print("Encoder: ");
      Serial.println(currentValue);
      prevEncoderValue = currentValue;
    }
  }
}

As you can see I have left the attachinterrupt for the button press and at the moment it works as expected. Your advise that it Is not recommended to be used is that specifically for ESP32's or all Arduino's ? I have used it successfully on mega and nano projects in the past.

You were correct in that I hadn't included the first line of the crash report here.

Guru Meditation Error: Core  1 panic'ed (Interrupt wdt timeout on CPU1). 

Once again thank you for the help and guidance.

Regards

No. In basic Arduino-style coding, the vast majority of the work should be done in the loop() function. BUT .... that doesn't mean loop() should take a long time to execute. It should run hundreds or (depending on the processor) thousands of times per second. For that to happen, you have to learn non-blocking coding techniques where you break long tasks into smaller chunks that just do a little bit each time the loop() function runs. And, to do periodic tasks based on so-called "millis() timing" ... an internet search on that term will yield lots of information.

A major problem is the "Arduino Method" of breaking large projects into separate .ino files. I won't go into the issues it causes here, I'll just say it doesn't provide true modularity for your code. If you're interested in a much better technique for creating modularity in large programs, take a look at My Post #5 in this Thread.

The general advise is that Arduino newbies shouldn't jump in and attempt to use interrupts. They cause more trouble than their worth in the form of unexpected problems. You already got bit with one of those and I see two more lurking in the latest code you posted that will come back and cause you problems.

Your code is a perfect of example of the unnecessary use of interrupts. You detect the button press and set a flag in the ISR to be picked up in the loop() function. That is a good technique in general for handling very quick, short events, but pointless for handling button presses which occur on a human (not microsecond) timescale. It would be just as effective (and a lot less prone to errors) if you just did a digitalRead() in the loop() function.

I suspected that. It means that your code was executing a very long task while interrupts were disabled and the Watchdog Timer expired, triggering a processor reset. That's a consequence of the functions you were calling in the updateValue() function.

This topic was automatically closed 180 days after the last reply. New replies are no longer allowed.