pimoroni / pimoroni-pico

Libraries and examples to support Pimoroni Pico add-ons in C++ and MicroPython.
https://shop.pimoroni.com/collections/pico
MIT License
1.32k stars 496 forks source link

Using two optical flow sensors simultaneously #228

Closed roaldarbol closed 2 years ago

roaldarbol commented 2 years ago

I am really excited that the optical flow sensors (PMW3901 & PAA5100) are now supported on the Pico. I was wondering if it is possible to use two sensors simultaneously? The "normal" PMW3901 library lets you set arguments including slots and ports, which opens the option of having 2 sensors recording simultaneously which I've used before, but it's not clear from the Pico example how to achieve this. I'm not a wiz' in .cpp, but it seems that it might be possible from the source code, but I'm not sure how. Any ideas?

Gadgetoid commented 2 years ago

Looks like we're lacking on the documentation front here.

The line:

flo = FlowSensor()

Can take arguments to specify the SPI bus. You can - in theory - connect multiple sensors to the same SPI bus and nominate a different chip-select pin for each.

If you keep the default SPI bus, that's as simple as (the below are the pins for the Front/Back slots on Breakout Garden):

flo1 = FlowSensor(cs=17)
flo2 = FlowSensor(cs=22)

You can specify different SPI pins altogether if you're wiring directly to a different bus:

flo = FlowSensor()

Is equivalent to:

flo = FlowSensor(
    cs=17,
    sck=18,
    mosi=19,
    miso=16,
    interrupt=20
)
roaldarbol commented 2 years ago

Awesome, thanks! I'll look forward to testing it out and let you know how it pans out!

roaldarbol commented 2 years ago

Alright, now I've finally gotten around to testing this (finally!) out and have 2 issues/questions you can maybe help me figure out.

1. Running the example script (and modified versions) twice

Running the example without altering anything works - at first. After stopping and restarting the script, it throws:

Traceback (most recent call last):
  File "<stdin>", line 22, in <module>
TypeError: can't convert

...line 22 being delta = flo.get_motion().

I've been running this both in Thonny and VSCode with Pico-Go, so I'm not assuming it to be a REPL issue.

2. Specifying sensor pins

First, just specifying cs=17 throws an error which was a bit tricky to debug:

Traceback (most recent call last):
  File "<stdin>", line 11, in <module>
TypeError: 'slot' argument required

Specifying all pins like above, however, resolves the issue:

flo = FlowSensor(
    cs=17,
    sck=18,
    mosi=19,
    miso=16,
    interrupt=20
)

The error message might need some adjustment, as it didn't actually need a 'slot' argument (or maybe there's something I'm missing here?).

In continuation of this, I made a second sensor with only the cspin modified:

flo2 = FlowSensor(
    cs=22,
    sck=18,
    mosi=19,
    miso=16,
    interrupt=20
)

I have a feeling there's something I'm just super rusty on - should all the SPI pins be different to get independent measurements from both sensors? My case is measuring the rotation of a floating ball (see picture).

Thanks!

Gadgetoid commented 2 years ago

As long as the chip-select pins are different you should be getting results from each sensor in turn, with no need to provide different SPI pins. This assumes the chip-select behavior is implemented correctly in our library, of course :grimacing:

What results are you seeing running the two sensors?

Is there anything else to your "TypeError: can't convert" error, it looks cut short? It might be related to this line: https://github.com/pimoroni/pimoroni-pico/blob/665ed08123402d93f5a2e3fe6c9d2f036203bd5d/micropython/modules/breakout_paa5100/breakout_paa5100.cpp#L193

I've ran into problems recently using an inline mp_obj_new_float in arguments. Very strange things happened. Some minimal code to reproduce this might help me fix it.

The "slot" argument required error is a side-effect of the slightly weird implementation here: https://github.com/pimoroni/pimoroni-pico/blob/665ed08123402d93f5a2e3fe6c9d2f036203bd5d/micropython/modules/breakout_paa5100/breakout_paa5100.cpp#L52-L126

If there's one argument it assumes you're supplying a "slot", ie: BG_SPI_FRONT or BG_SPI_BACK otherwise it expects a full complement of pins.

roaldarbol commented 2 years ago

SO FAST! Thanks, that's amazing support!

As long as the chip-select pins are different you should be getting results from each sensor in turn, with no need to provide different SPI pins. This assumes the chip-select behavior is implemented correctly in our library, of course 😬 It might just be me being overly suspicious of the output when turning the ball manually... could maybe be light conditions, dunno. I.e. I do get some actual output - although it tended to crash after a bit.

The TypeError wasn't cut short, that was the whole error. Occasionally it showed (and yes, the question mark symbol is what it actually shows):

Traceback (most recent call last):
  File "<stdin>", line 37, in <module>
TypeError: can't convert � to float

I also tried supplying BG_SPI_FRONT to slot as

flo = FlowSensor(
    cs=17,
    slot=BG_SPI_FRONT
)

...but in return I get:

Traceback (most recent call last):
  File "<stdin>", line 11, in <module>
NameError: name 'BG_SPI_FRONT' isn't defined
roaldarbol commented 2 years ago

Also, brief side question; is there any way of turning off the PAA LEDs when not running the script in the REPL? They remain on after stopping, and when I try restarting the script, they blink briefly and then remain on.

Gadgetoid commented 2 years ago

There seems to be a maybe possibly undocumented way to control the LEDs but it's unimplemented in this library at the moment. See: https://github.com/pimoroni/pmw3901-python/issues/4

Looks like we don't define BG_SPI_FRONT or BG_SPI_BACK for MicroPython. That should probably be fixed!

You can add them at the top of your code like so for now:

BG_SPI_FRONT = 0
BG_SPI_BACK = 1

Note that they include the chip-select pin, so I think the init is just:

flo = FlowSensor(
    slot=BG_SPI_FRONT
)

That cryptic TypeError definitely feels like the one I ran into. I'll have to try the same fix. Good grief, it's only through blind luck with a recent library that I have the slightest clue what it might be. Good timing, I suppose!

roaldarbol commented 2 years ago

Haha, brilliant! The slot solution works - great! Just ran a bit more, and when it crashes on me, the error is VERY similar looking (line 37 again being get_motion):

Traceback (most recent call last):
  File "<stdin>", line 37, in <module>
TypeError: can't convert tuple to float

I think the reason I might have a bad feeling about the readings comes down to implementation - and they are definitely off for the back sensor. Currently I'm testing with:

while(True):
    delta = flo1.get_motion()
    delta2 = flo2.get_motion()
    if delta is not None and delta2 is not None:
        x = (delta[0] + delta2[0]) // 2
        y = delta[1]
        z = delta2[1]
        tx += x
        ty += y
        tz += z
        print("A Relative, rel: x {}, y {}, z {} | Absolute: tx {}, ty {}, tz {}".format(x, y, z, tx, ty, tz))
    time.sleep(0.05)

The front sensor seems much more sensitive - I guess because I run them sequentially and it's first in line? Do you happen to have an idea of how it could be achieved to get_motion synchronously from them?

roaldarbol commented 2 years ago

Hi @Gadgetoid - just a quick follow-up, do you know when you'll have time to look at this? Just trying to figure out if I should be looking for an alternative hardware solution. :-)

Gadgetoid commented 2 years ago

Oof, the last couple of weeks have been chaos and I'd lost the plot here sorry. I'll try and expedite this!

Gadgetoid commented 2 years ago

If you're logged into GitHub you can download a possibly fixed build here - https://github.com/pimoroni/pimoroni-pico/actions/runs/1994051353

Grab the version applicable to your board, or just "pimoroni-pico-7d39788b73a244077495793c27f5673aae281ff1-micropython.uf2" if in doubt.

roaldarbol commented 2 years ago

Was about to comment no worries - but wow, that was proper expediting! Really appreciate it! I'll give it a spin tomorrow and report back🤞🏼

roaldarbol commented 2 years ago

Tested out, and it seems to have solved the issues. 👍
As for my differences in readings, it seems it's due to a faulty or damaged sensor rather than code. Guess I'll just have to stock up a few. ;-) Thanks for the help!

Gadgetoid commented 2 years ago

Whew truly a lucky coincidence I’d encountered this issue! I’ll get it merged and released post haste.