Callback function data pointer is not a pointer to the ESP32Encoder instance

[matthewbeckler]

Hello, thanks for this great library. Hard to find an encoder library that uses interrupts or the PCNT hardware peripheral, and also supports registering a callback function.

However, there's a bug here (or at least a documentation bug) regarding the pointer passed to the callback function.

Per the docs: https://github.com/madhephaestus/ESP32Encoder/blob/master/src/ESP32Encoder.h#L32 "@param enc_isr_cb callback executed on every encoder ISR, gets a pointer to the ESP32Encoder instance as an argument"

Per the example: https://github.com/madhephaestus/ESP32Encoder/blob/master/examples/Encoder_interrupt_display/Encoder_interrupt_display.ino#L8 The ISR here casts the void* v to ESP32Encoder*, but this is always a null pointer.

The instantiation of the ESP32Encoder class here: https://github.com/madhephaestus/ESP32Encoder/blob/master/examples/Encoder_interrupt_display/Encoder_interrupt_display.ino#L18 ESP32Encoder encoder(true, enc_cb); does not provide a third parameter here, hence the null pointer passed into the callback function.

We could fix this by changing this line: https://github.com/madhephaestus/ESP32Encoder/blob/master/src/ESP32Encoder.cpp#L88 esp32enc->_enc_isr_cb(esp32enc->_enc_isr_cb_data); to something like esp32enc->_enc_isr_cb(esp32enc->_enc_isr_cb_data ? esp32enc->_enc_isr_cb_data : esp32enc); where it could return the constructor-set _enc_isr_cb_data if one was provided at construction time, otherwise it would return a pointer to the ESP32Encoder instance.

I can prepare a PR for this if you like. Thanks!

[guidoschreuder]

maybe better to do it in the constructor? like: if (enc_isr_cb_data == nullptr) { _enc_isr_cb_data = this; }

[ztjona]

maybe better to do it in the constructor? like: if (enc_isr_cb_data == nullptr) { _enc_isr_cb_data = this; }

In the case somebody does not know where. It must be added between these lines: https://github.com/madhephaestus/ESP32Encoder/blob/99797223eda058758bb239689606047559851f0c/src/ESP32Encoder.cpp#L45-L46