PaulStoffregen / Encoder

Quadrature Encoder Library for Arduino
http://www.pjrc.com/teensy/td_libs_Encoder.html
540 stars 239 forks source link

add int pin and macro check for Raspberry Pi Pico and W #85

Open jjsch-dev opened 1 year ago

jjsch-dev commented 1 year ago

For a 3D printing filament dryer project, I had to use the encoder library with the Raspberry Pi Pico, so I added the IRQ pins. https://github.com/jjsch-dev/filament_dryer_rp2040

ale-novo commented 1 year ago

Hi, ive tested this with RP2040 and using the "examples/Basic/Basic.ino" sketch.

its working but not properly, it is skipping reads, also sometimes runs away meaning starts increasing for ever without moving the knob. Sometimes it counts in the incorrect way.

im using a mechanical encoder. the same hardware with a arduino NANO works perfectly.

have you tested this?

jjsch-dev commented 1 year ago

Hi @ale-novo, I didn't test it with Basic.ino, I used the EncoderButton library which is kind of a wrapper for this library. If it helps you, although it is still under construction, you can review the code of the project that I am developing.

ale-novo commented 1 year ago

Hi, ive tested this with RP2040 and using the "examples/Basic/Basic.ino" sketch. with a simple mechanical encoder connecting the common to ground and A, B to interrupt pins, works very unreliable. with a KY-040 encoder connecting 5v, gnd and a,b to interrupt pins works better but still misses or counts incorrectly. with a optical encoder did not test but i suspect that there is a problem with interrupts

all encoders work perfectly with arduino nano pins 2,3 (only interrupt pins)

jjsch-dev commented 1 year ago

I use the KY-040 but connected to 3.3V of the berry pico; my version of the rotary encoder has the 10K ohm pull-ups resistor included, attached I send you a video of my application that controls a GUI.

https://user-images.githubusercontent.com/55675185/225114212-e9598685-02aa-4b1f-89e1-098f8f8c0085.mp4

PaulStoffregen commented 1 year ago

Has anyone else tested? I'm reluctant to merge with "works very unreliable.with a KY-040 encoder". Need more feedback from people actually using RP2040.

jjsch-dev commented 1 year ago

Hi @PaulStoffregen, honestly I don’t check the basic.ino with the Pico Berry, but tomorrow I will test it and upload the results and pictures of the oscilloscope

jjsch-dev commented 1 year ago

@PaulStoffregen, @ale-novo, I tested the encoder connected to both 5V and 3.3V on the Raspberry Pi Pico and the practical result is the same. I also added two capacitors to the data and a clock pin to test if there is an improvement with glitch, but the result is the same.

The main difference between basic.ino and my code is that I need to modify the class declaration. This is because the constructor initiates the interrupt of the pins, and in the Arduino Pico porting, this is not allowed because the init() function is executed after the configuration and clears the initialization pin. My solution was to declare the class as a pointer and use new in the setup() function. This way, the memory is allocated dynamically, and the constructor is called after the init(), which solves the problem.

/* Encoder Library - Basic Example

include

// Change these two numbers to the pins connected to your encoder. // Best Performance: both pins have interrupt capability // Good Performance: only the first pin has interrupt capability // Low Performance: neither pin has interrupt capability Encoder* myEnc; //(6, 7); // avoid using pins with LEDs attached

void setup() { Serial.begin(9600); delay(5000);

myEnc = new Encoder(6, 7);

Serial.println("Basic Encoder Test:"); }

long oldPosition = -999;

void loop() { long newPosition = myEnc->read(); if (newPosition != oldPosition) { oldPosition = newPosition; Serial.println(newPosition); } }

On stack overflow and on the arduino forum he explains this behavior.

This is a picture of the signal with 3.3V and a capacitor of 0.01uF. 3v3_cap_0 01

This is a picture of the signal with 3.3V. 3v3

This is a picture of the signal with 5V. 5v

To compare the reaction of the rotary encoder I built the device with an Arduino Nano and the unmodified basic.ino, the results as seen in the video are very similar to those of the berry pico.

https://user-images.githubusercontent.com/55675185/225435094-aaaf36cf-cabf-4847-8575-0daea52e3bd7.mp4

https://user-images.githubusercontent.com/55675185/225435134-10bccc15-e8ec-4e45-ba46-5a6ca89e4aea.mp4

ale-novo commented 1 year ago

@jjsch-dev @PaulStoffregen Im testing with "examples/Basic/Basic.ino" sketch included in this library. for this PR im using the corresponding fork. This is the simplest test we can do. I go against testing with any other wrapper library, as it may introduce unexpected behaviour to the mix.

with Arduino nano it reads perfectly. Also have tested with Teensy LC and works flawlessly

With RP PICO however its not the case with a simple mechanical encoder connecting the common to ground and A, B to interrupt pins, works very unreliable. with a KY-040 encoder connecting 5v, gnd and a,b to interrupt pins works better but still misses or counts incorrectly. KY-040 has external components to improve the signal. with a optical encoder did not test . This should work the best as there is no "bounce" problem in the signal, or noise.

When i say it works unreliably i mean it misses counts or counts backwards sometimes.

i suspect the problem is encoder signal noise. Of course if you add external components like resistors or capacitors to filter the noise it will work fine. However im expecting the same behaviour of arduino nano or teensy lc where it does not need external components to work. I believe they may be an issue with this library and RP2040 compatibility. perhaps we can compare with the official RP2040 encoder library to see how its handling things.

ale-novo commented 1 year ago

@jjsch-dev @PaulStoffregen

Ive realized i had Arduino Mbed OS RP2040 boards 3.4.1 in tools->board->board manager

now ive updated to v4.0.2

naked encoder: still unreliable and runs away (please see at the end of video). same encoder works perfect with Arduino nano

https://user-images.githubusercontent.com/36439993/226759261-7bd0714f-0c16-4de2-bc91-91316bb5d53c.mp4

KY-040 : works much better with 3.3v and 5v still see some missed steps like:

-3
-2
-1
0
2
1
2

or

4
3
1
0
-1
-2
-3
-4

but is minimal, but we can call it good.

ale-novo commented 1 year ago

Also please consider this alternative implementation ive found online, as its slightly different than the one proposed in this PR im not sure if its better or worse: file: direct_pin_read.h

#elif defined (ARDUINO_ARCH_RP2040) 
#define IO_REG_TYPE                     pin_size_t  
#define PIN_TO_BASEREG(pin)             (0) 
#define PIN_TO_BITMASK(pin)             (pin)   
#define DIRECT_PIN_READ(base, pin)      (gpio_get(pin) ? 1 : 0) 

in this PR:

#define DIRECT_PIN_READ(base, pin)      digitalRead(pin)
jjsch-dev commented 1 year ago

@ale-novo the I am using the arduino-pico port from Earle F. Philhower. You could try? arduino_pico_version

ale-novo commented 1 year ago

ive tried Earle F. Philhower and i cant get a encoder response on the serial monitor for either the naked encoder or KY-040 back to the Arduino rp2040 official board definition, encoders works as described before

jjsch-dev commented 1 year ago

@ale-novo with the Earle F. Philhower porting, you need to change the code as I suggest in a previous reply.

The main difference between basic.ino and my code is that I need to modify the class declaration. This is because the constructor initiates the interrupt of the pins, and in the Arduino Pico porting, this is not allowed because the init() function is executed after the configuration and clears the initialization pin. My solution was to declare the class as a pointer and use new in the setup() function. This way, the memory is allocated dynamically, and the constructor is called after the init(), which solves the problem.

/*
 * Encoder Library - Basic Example
 * http://www.pjrc.com/teensy/td_libs_Encoder.html
 * This example code is in the public domain.
 */
#include <Encoder.h>

// Change these two numbers to the pins connected to your encoder.
// Best Performance: both pins have interrupt capability
// Good Performance: only the first pin has interrupt capability
// Low Performance: neither pin has interrupt capability
Encoder* myEnc; //(6, 7);
// avoid using pins with LEDs attached

void setup() {
   Serial.begin(9600);
   delay(5000);

   myEnc = new Encoder(6, 7);

   Serial.println("Basic Encoder Test:");
}

long oldPosition = -999;

void loop() {
   long newPosition = myEnc->read();
   if (newPosition != oldPosition) {
      oldPosition = newPosition;
      Serial.println(newPosition);
   }
}
ale-novo commented 1 year ago

@jjsch-dev i had the same results with Earle F. Philhower and Arduino MBED os RP2040

jjsch-dev commented 1 year ago

@ale-novo , Thanks for the comments, I only tried with the ky-40 encoder, can you send me a link of the 600 ppr optical encoder, I'll see if I can get one in Argentina to test.

ale-novo commented 1 year ago

The optical encoder is https://articulo.mercadolibre.com.ar/MLA-1124457150-encoder-rotativo-incremental-600-pulsos-fotoelectrico-hobb-_JM?quantity=1

I think the problem why it does not work is its because its 5v and RP PICO is 3.3v. i have ordered a logic level converter and i will try when it arrives.

jjsch-dev commented 1 year ago

@ale-novo thanks for the link, it is advertised on this site that the encoder output is open collector type, so you don't need a level converter, with the right pull-up you could do the level adapter. If you say it works with the nano but not with the pico, it's possible that the internal pull-up is the problem (for the nano the value is about 20K ohms, and for the berry pico about 50K ohms). Could you try an external pull-up to 3.3V of 4k7 ohm for the clock and data pins?

Pull-Up resistance for Berry Pico CPU rp2040

Pull-Up resistance for Nano CPU atmel_nano

ale-novo commented 1 year ago

ive tested an optical encoder 600 ppr (5v signal) with a logic level converter to 3.3v and works fine with RP2040 no missed steps. KY-040 works fine for RP2040 most of the time buy sometimes misses steps or counts 1 unit backwards. But its usable for non precise applications naked encoder totally unusable for RP2040

jjsch-dev commented 1 year ago

@ale-novo I'm glad you were able to solve your problem.

ale-novo commented 1 year ago

please merge this pr

Deadeye5589 commented 1 year ago

I can confirm that this is working with the Earle F. Philhower board definition and the basic example sketch.

@jjsch-dev how would I need to modify a EncoderButton library project to also initialise the constructor after the init function and make it work?

jjsch-dev commented 1 year ago

@Deadeye5589 This is the modified EncoderButton example.


/**
 * A basic example of using the EncoderButton library.
 */
#include <EncoderButton.h>

/**
 * Instatiate an EncoderButton.
 * For Arduino Uno, the hardware interrupts are pins 2 & 3
 * For Teensy, you can use any digital pin.
 * Probably better to pick a more meaningful name than 'eb1'...
 * Encoder+button:
 * EncoderButton(byte encoderPin1, byte encoderPin2, byte switchPin);
 * Encoder only:
 * EncoderButton(byte encoderPin1, byte encoderPin2);
 * Button only:
 * EncoderButton(byte switchPin);
 */
EncoderButton *eb1; //(2, 3, 4);

/**
 * A function to handle the 'clicked' event
 * Can be called anything but requires EncoderButton& 
 * as its only parameter.
 * I tend to prefix with 'on' and suffix with the 
 * event type.
 */
void onEb1Clicked(EncoderButton& eb) {
  Serial.print("eb1 clickCount: ");
  Serial.println(eb.clickCount());
}

/**
 * A function to handle the 'encoder' event
 */
void onEb1Encoder(EncoderButton& eb) {
  Serial.print("eb1 incremented by: ");
  Serial.println(eb.increment());
  Serial.print("eb1 position is: ");
  Serial.println(eb.position());
}

void setup() {
  // put your setup code here, to run once:
  Serial.begin(9600);
  delay(500);
  Serial.println("EncoderButton Basic Example");

  eb1 = new EncoderButton(2, 3, 4);
  //Link the event(s) to your function
  eb1->setClickHandler(onEb1Clicked);
  eb1->setEncoderHandler(onEb1Encoder);
}

void loop() {
  // put your main code here, to run repeatedly:
  // You must call update() for every defined EncoderButton.
  // This will update the state of the encoder & button and 
  // fire the appropriat events.
  eb1->update();
}
Deadeye5589 commented 1 year ago

Thank you for providing the example code. Works fine as well. No jumps or missing counts, using internal pull up only. From my point of view the code update can be merged into the libarary

jjsch-dev commented 1 year ago

@Deadeye5589 Thanks for the comment, I'm glad I helped you.

doctea commented 1 year ago

I'm having what sounds like similar problems using this on a Seeeduino XIAO RP2040 with the earlephilhower core, wondered if there is anything I'm missing?

I've got my encoder wired to D2 and D3 of the RP2040, which seem to be GPIO pins 28 and 29.*

I've tried the changes in this patch and that I found mentioned in this thread. I've also modified interrupt_pins.h to check for _defined(ARDUINO_ARCHRP2040), and also defined CORE_INT28_PIN and CORE_INT29_PIN.

I am also using the 'gpio_get' version of DIRECT_PIN_READ.

If I instantiate the Encoder statically then the encoder 'works' but is very unreliable as described here as interrupts don't seem to be used for the reasons described.

If I instantiate the Encoder using new in setup(), then the same thing happens no matter what combination of changes I try: no reaction at all to the encoder being turned.

However if I configure the CORE_INT28_PIN and CORE_INT29_PIN, use the gpio_get version of DIRECT_PIN_READ, AND if I set a breakpoint in Encoder::update in my debugger, then the first time the knob is turned, the interrupt triggers and the breakpoint is reached twice - once leading to a position +=2, and then again, resulting in no change. Subsequent turns of the encoder give no reaction.

Running this same version of the code but without the debugger attached doesn't even trigger the breakpoint once, so I'm not sure if that could be a red herring?

Is there anything else I might be missing here, or something else I could try to see what the problem is?

Thanks

jjsch-dev commented 1 year ago

@doctea What kind of encoder are you using, the output is open collector?, the XIAO-RP2040 needs 3.3V at the input.

For general I/O pins: Working voltage of MCU is 3.3V . Voltage input connected to general I/O pins may cause chip damage if it' higher than 3.3V .

For power supply pins: The built-in DC-DC converter circuit able to change 5V voltage into 3.3V allows to power the device with a 5V supply via VIN-PIN and 5V-PIN.

doctea commented 1 year ago

@doctea What kind of encoder are you using, the output is open collector?, the XIAO-RP2040 needs 3.3V at the input.

For general I/O pins: Working voltage of MCU is 3.3V . Voltage input connected to general I/O pins may cause chip damage if it' higher than 3.3V .

For power supply pins: The built-in DC-DC converter circuit able to change 5V voltage into 3.3V allows to power the device with a 5V supply via VIN-PIN and 5V-PIN.

Am using the 3.3V from the XIAO, with some capacitors to ground on each encoder pin to smooth the signal a bit (without these it was behaving completely crazy).

I'm not sure how to check what kind of encoder it is. However, this project has been working with the encoder mostly OK for months, if I don't use the Encoder interrupts or if I instantiate, but the encoder clicks don't correspond well to the events received - its usable, but only just. It is only if I use interrupts that I get the behaviour described where the encoder basically doesn't work at all, except for the one trigger of interrupt.

I was hoping there might be something about the XIAO pin/interrupt configuration that differs from the Pico, that I might be missing and need to reflect in the Encoder headers in order to get this to work?

I've also been using a variation of the same code+circuit on a Teensy 4.1, where the encoder events correspond well to the clicks of the encoder, so I believe this to be some problem with the RP2040 specifically. If worst comes to worst I can attempt to try the same thing using a Pico and see if the same problem occurs, but my PCBs use the XIAO so this is a little bit of a pain - building the circuit on a breadboard was too noisy to tell whether the encoder is working properly :)

jjsch-dev commented 1 year ago

@doctea Sorry for my English, by type of encoder I mean the model of the encoder, please send me a picture or link of the product. I didn't use the XIAO, but it uses the same RP2040 CPU. A capacitor is a good help to reject low frequency noise, use with care because it limits the rotation speed of the encoder. If the encoder is mechanical, it's most likely an open collector device, and you need the pull-up resistor and for 3.3V use in the range from 10K to 4K7.

doctea commented 1 year ago

@jjsch-dev ah sorry, the encoder is one of these: "EC11 360 Degree Rotary Encoder, Code Switch", if that helps? https://fr.aliexpress.com/item/33022441687.html

Just to be sure I understand, the pull-up resistor should go from 3.3V, to each of the signal pins? I shall try this, and see if it either improves reliability in non-interrupt mode, or allows for interrupts to be triggered correctly. Thanks for your help!

jjsch-dev commented 1 year ago

@doctea thanks for the link, and this device, although the RP2040 has internal pulls, it works better with external pull-ups on the clock and data pins, I'll send you a picture, the range for R2 and R3 can be from 4k7 to 10K for 3.3V. Capacitors C3 and C9 are optional, but they definitely help eliminate false triggering. encoder_resistor

doctea commented 1 year ago

encoder_resistor

Thank you for this! I added 10K pull-ups to the encoder pins as suggested, and now it works perfectly :D.

jjsch-dev commented 1 year ago

@doctea I'm glad it worked. :thumbsup:

madias123 commented 1 year ago

got it working with this code AND enabled pullup: (standard chinese encoders) pinMode(pin1, INPUT_PULLUP); pinMode(pin2, INPUT_PULLUP); Minor thing: I had to divide /4 for getting a correct value on my pi pico. Keep in mind, that here is a much better approach for the pico(tested): https://www.upesy.com/blogs/tutorials/rotary-encoder-raspberry-pi-pico-with-arduino-code# drawback: You will loose every compatibility with other MCUs

@ale-novo with the Earle F. Philhower porting, you need to change the code as I suggest in a previous reply.

The main difference between basic.ino and my code is that I need to modify the class declaration. This is because the constructor initiates the interrupt of the pins, and in the Arduino Pico porting, this is not allowed because the init() function is executed after the configuration and clears the initialization pin. My solution was to declare the class as a pointer and use new in the setup() function. This way, the memory is allocated dynamically, and the constructor is called after the init(), which solves the problem.

/*
 * Encoder Library - Basic Example
 * http://www.pjrc.com/teensy/td_libs_Encoder.html
 * This example code is in the public domain.
 */
#include <Encoder.h>

// Change these two numbers to the pins connected to your encoder.
// Best Performance: both pins have interrupt capability
// Good Performance: only the first pin has interrupt capability
// Low Performance: neither pin has interrupt capability
Encoder* myEnc; //(6, 7);
// avoid using pins with LEDs attached

void setup() {
   Serial.begin(9600);
   delay(5000);

   myEnc = new Encoder(6, 7);

   Serial.println("Basic Encoder Test:");
}

long oldPosition = -999;

void loop() {
   long newPosition = myEnc->read();
   if (newPosition != oldPosition) {
      oldPosition = newPosition;
      Serial.println(newPosition);
   }
}
jjsch-dev commented 1 year ago

@madias123 thanks for the feedback. I read the tutorial and it's a good use of the PIO to decode the quadrature encoder and reduce interrupts in the program. And regarding division by 4, what model of encoder are you using?

madias123 commented 1 year ago

I bought years ago 100 pieces of such standard aliexpress rotary encoders, like this: https://www.aliexpress.com/item/1569311119.html

doctea commented 1 year ago

FWIW, I've always been dividing the result of read() by 4 in order to get accurate results too - both on RP2040 with the modifications described above, and on my original Teensy project too. I always assumed there was a setting in the library that I'd missed, but it was a simple enough workaround to do knob.read()/4 so I never looked into it further. I linked the encoders I used in this thread above too.

jjsch-dev commented 1 year ago

@madias123 thanks for the link, the EC11 is a standard rotary encoder, they usually have 20 steps per turn. Are you using external pull-up resistors (typically 10Kohm)? 50 Kohm from the internal pull-ups isn't much for this encoder.

jjsch-dev commented 1 year ago

Thanks for the information about (steps/4) I will take it into account for future developments, but in the projects that I have used this library the precision of the steps does not matter, because what I need is to know if it increases or decreases when the user operates the encoder.

JKTUNING commented 11 months ago

I was able to get this working using the suggestions above.

Using the TT Electronics EN05 Series encoder - EN05HS1212NHH

Encoder* volEncoder;  //(26, 27);
Encoder* micEncoder;  //(28, 29);

void setup() {
    volEncoder = new Encoder(26,27);
    micEncoder = new Encoder(28,29);
}

void loop() {
    newVolPos = volEncoder->read();
    newMicPos = micEncoder->read();
}

direct_pin_read.h

#define IO_REG_TYPE                     pin_size_t
#define PIN_TO_BASEREG(pin)             (0)
#define PIN_TO_BITMASK(pin)             (pin)
#define DIRECT_PIN_READ(base, pin)      (gpio_get(pin) ? 1 : 0)
jjsch-dev commented 11 months ago

Hi @JKTUNING I'm glad it works for you, I didn't know about the EN05HS1212NHH, it's a very small device.

kallaspriit commented 8 months ago

I was able to get this working using the suggestions above.

Using the TT Electronics EN05 Series encoder - EN05HS1212NHH

Encoder* volEncoder;  //(26, 27);
Encoder* micEncoder;  //(28, 29);

void setup() {
    volEncoder = new Encoder(26,27);
    micEncoder = new Encoder(28,29);
}

void loop() {
    newVolPos = volEncoder->read();
    newMicPos = micEncoder->read();
}

direct_pin_read.h

#define IO_REG_TYPE                     pin_size_t
#define PIN_TO_BASEREG(pin)             (0)
#define PIN_TO_BITMASK(pin)             (pin)
#define DIRECT_PIN_READ(base, pin)      (gpio_get(pin) ? 1 : 0)

Can also confirm that this works, tested with earlephilhower core. Merge would be nice :)

jjsch-dev commented 8 months ago

@kallaspriit, thanks for the feedback.

NuclearPhoenixx commented 4 months ago

Maybe also adding ARDUINO_GENERIC_RP2040 to interrupt_pins.h would be a good idea.

Other than that, it works just fine for me with a pointer to the encoder.