pmarques-dev / PicoEncoder

High resolution quadrature encoder using the PIO on the RP2040
BSD 2-Clause "Simplified" License
3 stars 0 forks source link

Unable to use with certain additional libraries. #2

Open Jediknite1101 opened 1 month ago

Jediknite1101 commented 1 month ago

Using a Pi Pico and the Earlephilhower board manager profile, I had a very simple sketch adding one library and defining one object from that library at a time. (no other code) With each iteration adding one more library and one more object declaration. I got to the point where I had serial prints from both core0 & core1, the joystick library with one button, and a NeoPixel object. As soon as I added this library and added [encoderName].begin(); I would get an error that I could only best describe as a silent fail. Arduino IDE would say that the sketch would compile but fail to upload b/c the board's COM port would just no longer exist. I would have to reflash the board with anything else in order to use it again.

I tried the "Export Compiled Binary" from the IDE. After copying the UF2 manually once again the board would just disappear. If I commented out the [encoderName].begin(); line the sketch would compile and run. If I commented out anything sent to second core and the [NeopixelObject].begin(), the sketch would compile and run. I'm not exactly sure what's going on. But this was the first encoder library for RP2040 / Pi Pico I have any sort of success with.

#include "PicoEncoder.h"

#include <Joystick.h>
#include <Adafruit_NeoPixel.h>

Adafruit_NeoPixel centerStrip(60, 28, NEO_GRB + NEO_KHZ800);

PicoEncoder VolKnobL;
PicoEncoder VolKnobR;

#define ENCODER_L_A 3
#define ENCODER_L_B 2 // For some reason my encoder OUTB needs to be pinned first read properly
#define ENCODER_R_A 5
#define ENCODER_R_B 4
#define ENC_STEP_MX 2400 // Incremental encoder's #PPR *4 This is maxium pulses your encoder to spin a full 360

int VolLCnt, VolRCnt = 0;
int OldLVolPos, OldRVolPos;

void setup() {
  // Core 0 setup
  //VolKnobL.begin(2);
  //VolKnobR.begin(4);

  /*
  OldLVolPos = VolKnobL.step;
  OldRVolPos = VolKnobR.step; */

  Joystick.begin();

}

void setup1() {
  // Core 1 setup

  centerStrip.begin();
  centerStrip.show();
  centerStrip.setBrightness(50);

}

void loop() {
  // put your main code here, to run repeatedly:
  Serial.println("Core0 firing.");
  delay(500);
}

void loop1() {
  // Core 1 Loop function
  rainbow(10);
  //Serial.println("Core 1 firing.");
  //delay(2000);
}

// Rainbow cycle along whole strip. Pass delay time (in ms) between frames.
void rainbow(int wait) {
  // Hue of first pixel runs 5 complete loops through the color wheel.
  // Color wheel has a range of 65536 but it's OK if we roll over, so
  // just count from 0 to 5*65536. Adding 256 to firstPixelHue each time
  // means we'll make 5*65536/256 = 1280 passes through this loop:
  for(long firstPixelHue = 0; firstPixelHue < 5*65536; firstPixelHue += 256) {
    // strip.rainbow() can take a single argument (first pixel hue) or
    // optionally a few extras: number of rainbow repetitions (default 1),
    // saturation and value (brightness) (both 0-255, similar to the
    // ColorHSV() function, default 255), and a true/false flag for whether
    // to apply gamma correction to provide 'truer' colors (default true).
    centerStrip.rainbow(firstPixelHue);
    // Above line is equivalent to:
    // strip.rainbow(firstPixelHue, 1, 255, 255, true);
    centerStrip.show(); // Update strip with new contents
    delay(wait);  // Pause for a moment
  }
}
pmarques-dev commented 1 month ago

I can try running this example code myself later to see what is going on, but in the meanwhile I can give a few pointers that may help debugging the problem. I think this is a problem with some conflict on the usage of the PIO. The Neopixel library also uses PIO and claims one SM right at the constructor. PicoEncoder only claims the PIO at begin time and returns an error code if it is unable to get an available PIO. You could start by checking the return value from the encoder being method. If that's negative, then calling update afterwards on an Encoder that wasn't properly initialized will block indefinitely with interrupts disabled, which would be consistent with what you are seeing. I can add a protection in the code to avoid this. It would still not work, but at least it wouldn't hang forever. In theory, the PicoEncoder library needs a full PIO, but it should be able to handle 4 encoders out of that PIO, so you should still have one full PIO available for other code.

Actually I just had a look at the NeoPixel code and I think I understand what is going on:

If this is the sequence of events that is making this fail, then using the NeoPixel to send something to the LED's before calling the PicoEncoder begin should work around the problem. The NeoPixel PIO usage is a bit fragile, but I can also make the PicoEncoder sequence more robust as it was assuming that if it could fit a 32 instruction program in a PIO, then all SM must have been available on that PIO, and that isn't the case here.

pmarques-dev commented 1 month ago

After thinking about it some more, I think the problem is even worse: once PicoEncoder is told it can load its program to PIO0, it assumes all SM's are available, claims all of them and just starts using SM0, which was in fact claimed by NeoPixel and will be used in some way by NeoPixel later. In any case, the workaround I described above should still work.

Jediknite1101 commented 1 month ago

A lot of this is a bit over my head. But thank you for providing an easy to understand possible solution. I will research more on PIO later today. I dropped out of the engineering track and became a visual designer b/c debugging in college plagued me. So please forgive my ignorance. Is there a way to target PIO1? Or is the board suppose roll code the second PIO chip once the first chip is claimed?

pmarques-dev commented 1 month ago

So, here is a summary of the way this works:

What is happening is that the NeoPixel library claims one SM but doesn't immediately load code into the instruction memory. So when the PicoEncoder asks the sdk if the PIO has enough space for a 32 instruction program, the sdk says yes and the PicoEncoder assumes that means the PIO is unused and it can take over it.

If the NeoPixel had already loaded the code, the sdk would answer no and the PicoEncoder code would then move to PIO1 and install itself there.

That's why I suggested that changing the order of operations should work around the issue. If you send something to the LED's using NeoPixel before calling PicoEncoder::begin, the NeoPixel will load its code and PicoEncoder should move to PIO1 and everything should work

Of course it would be better if things just work, so I'll make changes to the library so that we don't need to work around this any more, but in the meanwhile you can just try this. The change I'll do is to check if I can actually claim all 4 SM's before assuming I can use that PIO, and if that fails, release the SM's already claimed and move to the other PIO. That should interact properly with libraries like the NeoPixel, without the user having to worry about initialization order.

BTW, in the pico-examples repository, I've published PIO code that reads an encoder and "only" uses 24 instructions, thus leaving some room available for other users. That code doesn't do the substep speed estimation that the PicoEncoder does though, and I didn't want to have to maintain two libraries just to save 8 PIO instructions.

Jediknite1101 commented 1 month ago

Thank You! I know you really didn't have to go through so much trouble, but I really do appreciate it. And hopefully I can finally finish this project within the week.

Jediknite1101 commented 1 month ago

So the work-a-round that you suggest works initially. I sent a show command to NeoPixel and added 3sec delay before calling the Encoder. The code compiles and uploads to the board. It will then run for a few seconds and then the board seems like it locks up. Once again it becomes unusable till it's re-flashed.

#include <Joystick.h>
#include <Adafruit_NeoPixel.h>

#include "PicoEncoder.h"

Adafruit_NeoPixel centerStrip(28, 28, NEO_GRB + NEO_KHZ800);

PicoEncoder VolKnobL;
PicoEncoder VolKnobR;

#define ENCODER_L_A 3
#define ENCODER_L_B 2 // For some reason my encoder OUTB needs to be pinned first read properly
#define ENCODER_R_A 5
#define ENCODER_R_B 4
#define ENC_STEP_MX 2400 // Incremental encoder's #PPR *4 This is maxium pulses your encoder to spin a full 360

void setup() {
  // Core 0 setup
  Joystick.begin();

  centerStrip.begin();
  centerStrip.show();
  centerStrip.setBrightness(50);

  delay (3000);

  VolKnobL.begin(2);
  //VolKnobR.begin(4);
}

void setup1() {
  // Core 1 setup

}

void loop() {
  // put your main code here, to run repeatedly:
  Serial.println("Core0 firing.");
  delay(1000);

}

void loop1() {
  // Core 1 Loop function
  rainbow(10);
  //Serial.println("Core 1 firing.");
  //delay(2000);
}

// Rainbow cycle along whole strip. Pass delay time (in ms) between frames.
void rainbow(int wait) {
  // Hue of first pixel runs 5 complete loops through the color wheel.
  // Color wheel has a range of 65536 but it's OK if we roll over, so
  // just count from 0 to 5*65536. Adding 256 to firstPixelHue each time
  // means we'll make 5*65536/256 = 1280 passes through this loop:
  for(long firstPixelHue = 0; firstPixelHue < 5*65536; firstPixelHue += 256) {
    // strip.rainbow() can take a single argument (first pixel hue) or
    // optionally a few extras: number of rainbow repetitions (default 1),
    // saturation and value (brightness) (both 0-255, similar to the
    // ColorHSV() function, default 255), and a true/false flag for whether
    // to apply gamma correction to provide 'truer' colors (default true).
    centerStrip.rainbow(firstPixelHue);
    // Above line is equivalent to:
    // strip.rainbow(firstPixelHue, 1, 255, 255, true);
    centerStrip.show(); // Update strip with new contents
    delay(wait);  // Pause for a moment
  }
}
pmarques-dev commented 1 month ago

I hadn't time yet to try your code, but if the code above is all it takes to lock up then it's hard to imagine it being caused by PicoEncoder as that code doesn't even have a call to update(). So, all Pico Encoder is doing at that point is running stuff on the PIO itself without any interrupts or DMA or anything that could interfere with code execution.

One thing I've noticed is that you are playing a little fast and lose with the multicore nature of the RP2040. Yes it does have 2 cores and you can run code on both simultaneously, but you need to be careful with synchronization. For instance, from your code I can't see any guarantee that the calls to rainbow on core1 will be made after the setup of the NeoPixel library finishes on setup(). Both setup functions are called at the same time and the first loop call is made immediately after the setup function finishes. Since setup1() is empty, that will finish immediately and will go on to loop1() very likely while centerStrip.begin() is still running on core0.

I would start by simplifying things and not use a setup1 / loop1 at all and just run everything on the first core. Multi-threaded synchronization is not something that is easy to get right, so I would just avoid it, unless you have a very compelling reason to use it. At the very least don't mix data between the two cores: anything used by core0 should be initialized on core0 and the same for core1. Any data that you want to share must be properly synchronized using things like mutexes, etc.

BTW, you don't need any delay between calling the NeoPixel methods and the PicoEncoder begin call, because as soon as the NeoPixel methods return the code space is already allocated on the PIO and things should just work

Jediknite1101 commented 1 month ago

I've already tried eliminating any other code than defining each object and a serial.print. I'm not entirely sure how to use that quadrature_encoder in your pico examples but even before that I can't get MinWG to install without an error to even try to compile it to a *.h file.

I'm willing to chalk this project as a partial loss. I can still finish building arcade style controller all this headache is for. Before looking in to adding any "extra effects"... your encoder library was flawless. I was impressed how easy it was to grab a count/step from a pair 600ppr encoders and get those vals scaled down to a 10-bit value to near perfectly emulate the official Konami arcade hardware.

pmarques-dev commented 1 month ago

don't give up so soon.... don't go the pico-example route as that will be painful, I just mentioned that as a last resort if you needed to include more stuff and ran out of PIO space, but that is not the case yet. Please try what I suggested above and don't use the 2 cores. Just setup everything in the setup() function and do everything in the loop() function and don't even define a setup1() or loop1() functions at all and see if that fixes things