firmata / ConfigurableFirmata

A plugin-based version of Firmata
GNU Lesser General Public License v2.1
153 stars 72 forks source link

RP Pico RP2040 ConfigurableFirmata Encoder Interrupts not working #133

Closed ale-novo closed 1 year ago

ale-novo commented 1 year ago

Hello! been using ConfigurableFirmata V2.10.1 based from Firmatabuilder.com sketches successfully with Arduino.

Ive added support for RP2040 how is described here: https://github.com/firmata/ConfigurableFirmata/blob/3a1747953150b6589b7d5bf1f3aa92c5aca3c365/BoardSupport.md

Everything works fine however ive notice that if i plug in an encoder it does not work accurately like it does in the Arduino nano port 2/3 that are interrupts pins.

In theory all RP2040 digital pins are interrupts but they are not behaving like interrupt pins but like normal pins therefore making the encoder not read accurately.

ive modified the encoder library here: https://github.com/PaulStoffregen/Encoder/search?q=RP2040

to use #elif defined(TARGET_RP2040) || defined(TARGET_RASPBERRY_PI_PICO) instead of #elif defined(ARDUINO_NANO_RP2040_CONNECT)

Its compiling fine however not working accurately, please help!! thanks

pgrawehr commented 1 year ago

Hi @ale-novo

I've just recently had a similar issue on the ESP32 (which is quite similar to the RP2040). There's a bug in the DigitalInputHandller that may affect the interrupt behavior, depending on the board.

In src/DigitalInputFirmata.cpp, line 121 try changing PIN_MODE_INPUT to just INPUT. That could fix your issue.

ale-novo commented 1 year ago

Hi thanks for your answer however im confused. im using ConfigurableFirmata V2.10.1 (compatible with firmatabuilder.com) in that version that line is already 'INPUT' as you can see here:

https://github.com/firmata/ConfigurableFirmata/blob/2.10.1/src/DigitalInputFirmata.cpp#L97

the if statement however in yours is

if (mode == PIN_MODE_INPUT) {

and mine is

if (mode == INPUT) {

what im not seeing in v2.10.1 is the INPUT or PIN_MODE_INPUT definition

https://github.com/firmata/ConfigurableFirmata/blob/2.10.1/src/ConfigurableFirmata.h#L104

perhaps this is not properly define in the RP2040 board definition?

pgrawehr commented 1 year ago

True, I forgot about this. The error I mentioned is not in that version.

Firmata doesn't really use the interrupt capabilities of the pins at all. It sends PIN_CHANGE notifications based on polling only (for all board types). This is by design not really intended for fast-changing pins, such as to read encoder values. Are you really saying the same encoder works on an Arduino Nano but not on an RPI2040? That would confuse me, since the later is much faster.

One possible solution would be to use the Frequency module in the latest version. That does internally use interrupt capabilities and is exactly intended to support pins that are used as "tachometers". What client library are you using?

ale-novo commented 1 year ago

Hi, my findings are as follows With Arduino, an encoder will work in any pin, but will perform poorly if the pins used are not interrupt pins. for example in arduino nano pins 2/3 if interrupt pins are used it works perfectly.

here its described in the docs the use of interrupts in the firmata protocol https://github.com/firmata/protocol/blob/master/encoder.md

i have practical experience with arduino boards and encoders using interrupt and non interrupt pins.

For RP2040 like i mentioned the encoder kind of works but poorly meaning it randomly reads values, increase decrease, its not reliable. i suspect the issue is firmata is using the pins as normal pins and not interrupt pins.

im new to RP2040 but i suspect in order to enable the interrupt pins a special set up must be done and this is not being done by firmata since its a new board

my client library is perl-firmata and like i said i have good results using arduino nano pins 2/3 with encoders and this library it reads the encoders perfectly via USB and Ethernet

pgrawehr commented 1 year ago

Firmata doesn't use the interrupt capability at all. It internally just polls all input pins in a loop, so I'm really confused that you observe a different behavior based on which pins you use. Maybe your client library adds some magic by adding some extra polling for the relevant pins.

You could try to reduce the samplingValue in FirmataReporting.cpp to a smaller value (unless it is already set by your library) to improve the situation. But as I have said above: The DigitalInput module is not designed for this. If the frequency on the pin is faster than the polling interval, you'll always loose events.

ale-novo commented 1 year ago

Hi, perhaps firmata does not but the encoder library running underneath does: please look:

#Encoder Feature

Provide incremental encoders support, for both [linear](http://en.wikipedia.org/wiki/Linear_encoder) and [rotary](http://en.wikipedia.org/wiki/Rotary_encoder#Incremental_rotary_encoder) encoders.

This feature is based on based on [PJRC's implementation](http://www.pjrc.com/teensy/td_libs_Encoder.html). See [this article](http://www.pjrc.com/teensy/td_libs_Encoder.html) for more informations about how it works (well explained!).

Current implementation supports 5 encoders at the same time (#[0-4]) and you can activate automatic position reports every (SAMPLING_INTERVAL)ms. Reports are disabled by default.

For best Performances, connect only interrupt pins.

https://github.com/firmata/protocol/blob/master/encoder.md

Encoders have 2 signals, which must be connected to 2 pins. There are three options.

Best Performance: Both signals connect to interrupt pins.
Good Performance: First signal connects to an interrupt pin, second to a non-interrupt pin.
Low Performance: Both signals connect to non-interrupt pins, [details below](https://www.pjrc.com/teensy/td_libs_Encoder.html#polling).

https://www.pjrc.com/teensy/td_libs_Encoder.html

Like i said ive tested this with both optical and mechanical encoders and interrupt pins work best

ale-novo commented 1 year ago
Low Performance Polling Mode
If neither pin has interrupt capability, Encoder can still work. The signals are only checked during each use of the read() function. As long as your program continues to call read() quickly enough, you will get accurate results. Low resolution rotary encoders used for dials or knobs turned only by human fingers are good candidates for low performance polling mode. High resolution encoders attached to motors usually require interrupts!

I can try with polling mode with lower values however its not ideal, and like i said interrupts work fine in arduino perl-firmata i dont think does any magic and for encoders i use a callback method, not polling.

ale-novo commented 1 year ago

please also look:

Interrupts are currently used behind the scenes. In fact if you use StandardFirmata an Arduino Uno or similar board all of the interrupts are currently in use. PWM uses interrupts, Servo uses interrupts, millis() uses an interrupt, etc. This has been one of the larger problems when trying to scale Firmata. We're working on a new firmata example called ConfigurableFirmata (see the [configurable branch](https://github.com/firmata/arduino/tree/configurable) and the new [utility directory](https://github.com/firmata/arduino/tree/configurable/utility) as well as the [ConfigurableFirmata example](https://github.com/firmata/arduino/tree/configurable/examples/ConfigurableFirmata)). With ConfigurableFirmata you select features for a particular application rather than the general purpose StandardFirmata approach. This could free up a timer as long as conflicting features are not chosen. For example if you don't need a Servo, then you don't include the Servo class and that frees up a timer or if you don't need PWM you could free up multiple timers. Then we could add a RotaryEncoder feature class, but you'd need to sacrifice an existing feature in order to add a new feature that relies on interrupts so you couldn't use both a rotary encoder and a servo or a rotary encoder and pwm for example.

For more capable boards such as the Due this may not be an issue as there may be additional timers available.

https://github.com/firmata/arduino/issues/63

ale-novo commented 1 year ago

Interrupt pins: https://github.com/firmata/ConfigurableFirmata/commit/d8f9a75c419fab6230c0f9a10e7afed4464378e5

pgrawehr commented 1 year ago

I have never worked with encoders, and hence I have not looked at the encoder module. Can you check that the macro PIN_TO_INTERRUPT evaluates to something > 0 for the pins you intend to use on the RP2040? Of course, it's also possible that the Encoder module needs an update for the new board.

ale-novo commented 1 year ago

ive modified the encoder library here: https://github.com/PaulStoffregen/Encoder/search?q=RP2040

to use #elif defined(TARGET_RP2040) || defined(TARGET_RASPBERRY_PI_PICO) instead of #elif defined(ARDUINO_NANO_RP2040_CONNECT)

in regards to PIN_TO_INTERRUPT im not sure how to evaluate this

pgrawehr commented 1 year ago

You can just add some test code (e.g. in the main setup() method)

Serial.print("Interrupt for pin 3:");
Serial.println(PIN_TO_INTERRUPT(3));
ale-novo commented 1 year ago

Thanks i will try.

i can confirm that the issue is NOT ConfigurableFirmata

ive run the encoder basic sketch and i can reproduce the issue. On arduino NANO works perfect, on RP PICO does not https://github.com/PaulStoffregen/Encoder/blob/master/examples/Basic/Basic.ino

Although the issues is not ConfigurableFirmata im happy to take suggestions on this thread. cheers

ale-novo commented 1 year ago

seems this is the issue i have: https://github.com/PaulStoffregen/Encoder/issues/69

ale-novo commented 1 year ago

https://github.com/arduino/ArduinoCore-mbed/issues/253

pgrawehr commented 1 year ago

Did that last thread help you solve the issue? I do not have an RP2040, so I cannot do any testing.

ale-novo commented 1 year ago

No, this seems to be a bug with some Arduino Code somewhere, this exceeds my capabilities at the moment.

ive seen some solutions with micropython and rotary encoders that seem to work but since my goal is to use ConfigurableFirmata i have to use the same Encoder library that has the problem, so at the moment im stuck with this

igfasouza commented 1 year ago

I got a error about PIN_SPI_SCK and PIN_SPI_MOSI and PIN_SPI_MISO and PIN_SPI_SS are not defined.

this line on "Boards.h"

define IS_PIN_SPI(p) ((p) == PIN_SPI_SCK || (p) == PIN_SPI_MOSI || (p) == PIN_SPI_MISO || (p) == PIN_SPI_SS)

pgrawehr commented 1 year ago

@igfasouza Please create a new issue for this. Your problem is completely unrelated to this one (and seems quite easy to fix).

ale-novo commented 1 year ago

whis can be closed as the issue is not with configurable firmata but with the encoder library or arduino