adafruit / circuitpython

CircuitPython - a Python implementation for teaching coding with microcontrollers
https://circuitpython.org
Other
4.09k stars 1.21k forks source link

Add some kind of I/O expander pin to the core #8377

Open jepler opened 1 year ago

jepler commented 1 year ago

Both Adafruit TFT Experiment board and for an unreleased pimoroni board use on-board I2C bus expanders.

Besides this, the Pico W has a special I/O pin that is on the CYW wifi coprocessor.

Usable for what tasks?

It is desirable for these "pins" to be able to be treated consistently by anything that doesn't require fast or accurate timing. This could include:

Some examples of things where this would not work well are:

Design

The design I have in mind is:

Generic I2C Expander

Generic I2C expander:

Locked bus problem

This presents a potential problem that didn't exist before: Maybe a digital pin can't be read or set "right now". For instance, suppose that an I2C bus contains a temperature sensor and a bus expander. The bus expander is the home of the CS pin of a SPI display. During an I2C transaction to read the temperature, a background display refresh occurs. But because of the ongoing transaction, CS can't be asserted and so the refresh can't actually occur. It's not clear how to discover and recover from such a problem within the current architecture.

Honestly that realization is enough to make me want to turn aside from the whole idea.

@zodiusinfuser did you consider the locked I2C bus problem? What kinds of I/O tasks does your board need to do with expander pins? We have a SPI bus, TFT reset pin, some buttons, and I think the LCD back-light. We'd probably do fine with just a custom bitbang i2c implementation for our use case, and make the rest rely on a userspace implementation of a digital pin, like we have already for other expanders.

jepler commented 1 year ago

My WIP code (it's only compile-tested and only on rp2040) is here: https://github.com/jepler/circuitpython/pull/new/ioexpander

it includes the refactoring to create the "abstract pin protocol" and converts the microcontroller pin to implement it, but doesn't implement any other pin types yet.

ZodiusInfuser commented 1 year ago

@jepler I did not consider the locked I2C problem. I was testing with the ST7789 library, I found that having auto_refresh=True was a bad idea, and it was better to explicitly call display.refresh() when a refresh was wanted. As such I never thought about any async or other way of handling the display that may cause the lockup you describe.

One thing I did have to do in my build was release the displays on reset_board() to avoid the REPL rendering to the screen and thus tanking performance with general user interaction.

    // Releasing displays, as if one is set up with the
    // intended IO expander LCD pins then it will carry over
    // into other user program runs, causing poor performance
    // if they don't realise the screen is still being driven
    common_hal_displayio_release_displays();

As for I/O tasks, you can find the library for the product here: https://github.com/pimoroni/yukon-circuitpython

Here's a summary though:

tannewt commented 1 year ago

I think this is mostly a DigitalInOut abstraction rather than a Pin one. The Pin would only be needed if you wanted to have references to them in board. If that's the case, then they could be attributes on a long living object for the expander that is also available like board.SPI(). This is a similar model to the existing PCF8574 driver.

I don't think a protocol is necessarily the right approach because you may want to make the object available to user code. Instead, use the python method lookup mechanics to load the function to call. The API is then defined by object attribute naming. It's as if you were calling a Python objects attributes.

This presents a potential problem that didn't exist before: Maybe a digital pin can't be read or set "right now". For instance, suppose that an I2C bus contains a temperature sensor and a bus expander. The bus expander is the home of the CS pin of a SPI display. During an I2C transaction to read the temperature, a background display refresh occurs. But because of the ongoing transaction, CS can't be asserted and so the refresh can't actually occur. It's not clear how to discover and recover from such a problem within the current architecture.

This problem isn't new to displayio because both SPI and I2C busses can be locked. displayio checks to see if the bus is locked and skips the refresh if it is.

The DigitalInOut API doesn't have locking but it can throw exceptions. displayio can catch these and skip the refresh if needed.

ZodiusInfuser commented 1 year ago

I think this is mostly a DigitalInOut abstraction rather than a Pin one. The Pin would only be needed if you wanted to have references to them in board.

I just want to throw in that in my case I do have the pins referenced in board: https://github.com/ZodiusInfuser/circuitpython/blob/yukon/ports/raspberrypi/boards/pimoroni_yukon/pins.c My rational there is that the IO Expanders are an implementation limitation of the board, due to the RP2040 not having enough native IO pins to perform the functions the product needs.

tannewt commented 1 year ago

It's ok to have them in board but we need to decide if they should work with native classes like digitalio. On the Pico W, the LED is off micro so we probably do want digitalio to work so blink examples "just work".

ZodiusInfuser commented 1 year ago

My vote is yes, given Yukon also has two LEDs off micro (and two buttons).