pimoroni-pico icon indicating copy to clipboard operation
pimoroni-pico copied to clipboard

Encoder-pio not working with more than one encoder

Open guilhermebene opened this issue 3 years ago • 2 comments

Hello, I have been testing the code available in the branch encoder-pio and I have run into a few issues. I am not sure it is still under development, since the branch has been untouched for a while, but I'd like to address them anyway.

It seems to me that the code was written to allow more than one encoder to be defined (for example in the case of a robot with two wheels), but I believe there are some issues in that case. In my case I am specifically working with two encoders.

In the following line there is an if condition when the first encoder is defined: https://github.com/pimoroni/pimoroni-pico/blob/a15646d922b185622036ce81423f30c256e5645d/drivers/encoder-pio/encoder.cpp#L87

However, I believe the two following lines inside this if should be called for each encoder, which is taken care by a single state machine: https://github.com/pimoroni/pimoroni-pico/blob/a15646d922b185622036ce81423f30c256e5645d/drivers/encoder-pio/encoder.cpp#L90 https://github.com/pimoroni/pimoroni-pico/blob/a15646d922b185622036ce81423f30c256e5645d/drivers/encoder-pio/encoder.cpp#L91

To solve that, I put those two lines outside of the if condition to force them to be executed for each encoder's initialization. But it still did not solve my problem... I tried to check if the calls for the hw_set_bits(&enc_pio->inte0, 1u << enc_sm) function where ok and it seems to work fine, which means both state machines should generate an interrupt that would call the interrupt handler, but that is not the case. For some reason, it seems that setting a second state machine sharing the same code pio code defined in encoder.pio as the first state machine will cause the first one to not generate interruptions, or maybe not work at all.. I still haven't been able to figure out which is the case.

Has anyone ever tried this implementation with more than one encoder?

guilhermebene avatar Dec 31 '21 22:12 guilhermebene

Hi, thanks for raising this! You are correct that this branch hasn't received attention in a long while, and I honestly cannot remember if I ever tested it with multiple encoders.

I am planning to revisit it now we're in the new year, as we may have a product on the horizon that requires it, so knowing that this is broken is a good thing for me to look at first.

ZodiusInfuser avatar Jan 04 '22 15:01 ZodiusInfuser

Hey @guilhermebene. Sorry it has taken so long, but I have finally had chance to look into this.

There were a bunch of issues with the code, but the main reason why only a single encoder would run is because hw_set_bits(&enc_pio->inte0, 1u << enc_sm) was being called within the if(pio_claimed_sms[pio_idx] == 0) { statement, meaning it only got registered for the first SM (whichever that may have been). There were also thing wrong with setting up both PIOs as well as tearing them down at the end. These "should" be fixed now.

I am giving the rest of the encoder code a thorough look over as I polish it up for release, so class and function names and behaviour may change going forward, but if you want to have a play around over the weekend, I would welcome your feedback.

The latest code is within the encoder-pio branch. I also rebased it from main so it has all our other libraries and improvements, if that's useful to you.

ZodiusInfuser avatar Apr 13 '22 19:04 ZodiusInfuser

Hi @guilhermebene. I forgot to reply to this, but the encoder support was rolled into our libraries back in the summer to coincide with the launch of Motor 2040. This fixed the issues with running multiple encoder instances at once, so worth you having a look at if it's still relevant to your project.

ZodiusInfuser avatar Dec 01 '22 16:12 ZodiusInfuser