RFC: Hardware API: finalising machine.I2C

[deshipu]

What has the experience been with the peripheral ID? I know some ports don't have it and it doesn't seem very portable to me. Seems like it'd be better to let the port's implementation match up a hardware peripheral to the given pins where its a different MCU on a board with a standard form factor like Arduino Uno or Adafruit Feather.

What about platforms, that can use several different peripherals on the same pin, like the ESP32 with pretty much anything, or like the PyBoard with timers? How should the peripheral be picked in such cases?

Also, if you want to allow that, I think the software I2C should be then moved to a separate class (SoftI2C), so that you don't get a software peripheral just because you picked the wrong pin.

In addition to close() why not add Context Manager support to match how files and network sockets are used in Python? ContextManagers are super nice because they deinit even when a KeyboardInterrupt happens.

I'm not sure that would work well, because unlike files, i2c peripherals will be often used throughout the whole lifetime of your program. That would lead to some quite mess code, like:

Code: Select all

with machine.I2C(Pin(5), Pin(4)) as i2c:
    # All the code of your application

Instead of:

Code: Select all

init_all_peripherals()
try:
    # All the code of your application
finally:
    deinit_all_peripherals()

Of course I see no reason why it couldn't be possible, I just don't see it as very convenient. The access pattern is just so different from files, which you usually open, write to, and immediately close.

[deshipu]

So, according to https://github.com/micropython/micropython/issues/1948 we will need to add one more (optional?) parameter for the (soft) I2C peripheral -- the clock stretching timeout.

[mianos]

It basically means that if you want to do DMA transfers you have to "fake" the low level API. You need to buffer everything until the end and then DMA out the whole thing. Or not do DMA if somebody chooses to use the low level API, and only do DMA if the higher-level APIs are used. Which means that the high level APIs will no longer just be the low-level APIs concatenated together.

You have the same issues if you write fully interrupt driven drivers.
I think the low level interface is pretty good. When I did my first I2C driver for the ESP it pretty much looked like this suggestion. A couple of layers, one to basically write bytes, words and commands and a high level I2C.
How would things work with an asynchronous api for dma and interrupts?
I'd like a generic queue to shift the data from interrupt/dma time collection to user asynchronous python.

[tannewt]

What has the experience been with the peripheral ID? I know some ports don't have it and it doesn't seem very portable to me. Seems like it'd be better to let the port's implementation match up a hardware peripheral to the given pins where its a different MCU on a board with a standard form factor like Arduino Uno or Adafruit Feather.

What about platforms, that can use several different peripherals on the same pin, like the ESP32 with pretty much anything, or like the PyBoard with timers? How should the peripheral be picked in such cases?

Also, if you want to allow that, I think the software I2C should be then moved to a separate class (SoftI2C), so that you don't get a software peripheral just because you picked the wrong pin.

In addition to close() why not add Context Manager support to match how files and network sockets are used in Python? ContextManagers are super nice because they deinit even when a KeyboardInterrupt happens.

I'm not sure that would work well, because unlike files, i2c peripherals will be often used throughout the whole lifetime of your program. That would lead to some quite mess code, like:

Code: Select all

with machine.I2C(Pin(5), Pin(4)) as i2c:
    # All the code of your application

Instead of:

Code: Select all

init_all_peripherals()
try:
    # All the code of your application
finally:
    deinit_all_peripherals()

Of course I see no reason why it couldn't be possible, I just don't see it as very convenient. The access pattern is just so different from files, which you usually open, write to, and immediately close.

For picking the peripheral, the port code should know all available peripherals that can output on the given pins and iterate through them until one is free. On the ESP32 this is super easy, if you initialize an I2C object on pins 3 and 5 you get I2C1. If you do another set of pins you get I2C2. Of course with timers this can get more complicated because of the common settings between channels. However, I would guess that in most cases you aren't using all of them at once so being specific isn't critical. If it is, you could have the peripheral ID as a kwarg that defaults to None to mean AUTO and could be passed constants from a port specific class to identify them.

This approach of taking the next free one doesn't work unless peripherals know whether they have been claimed or not. I think this is a good thing because it prevents you from accidentally switching the function of a peripheral or pin accidentally.

I agree that Software I2C should be a separate class, and perhaps a separate module.

ContextManagers do make it a little more difficult in the REPL for sure because the code within a block doesn't run line by line. However, when used in file code, you can (and should) use methods to wrap large chunks of code. So your example should be:

Code: Select all

with machine.I2C(Pin(5), Pin(4)) as i2c:
    # A few method calls

Handling many concurrent ContextManagers can become a pain so you could introduce a wrapper context manager to init and deinit them all at once. Similar to your second example you could have a board specific context manager that inits all peripherals at once. (Thought that costs lots of ram). Or, you can create a parent context manager that you add all of your peripheral context managers that simply exits them all when it itself is exited. Given all this, I think the preferred method should just be to use methods to make sure all of your code isn't actually in the with block.

[deshipu]

For picking the peripheral, the port code should know all available peripherals that can output on the given pins and iterate through them until one is free. On the ESP32 this is super easy, if you initialize an I2C object on pins 3 and 5 you get I2C1. If you do another set of pins you get I2C2. Of course with timers this can get more complicated because of the common settings between channels. However, I would guess that in most cases you aren't using all of them at once so being specific isn't critical. If it is, you could have the peripheral ID as a kwarg that defaults to None to mean AUTO and could be passed constants from a port specific class to identify them.

This approach of taking the next free one doesn't work unless peripherals know whether they have been claimed or not. I think this is a good thing because it prevents you from accidentally switching the function of a peripheral or pin accidentally.

This introduces another problem: the behavior becomes unpredictable, changing depending on the order in which the peripherals are initialized, and (especially in the REPL) depending on what other commands have been issued. For instance, if you have a peripheral P1 that can work on pins 1 and 2, and P2 that can work on pins 1 and 3, then one of the two programs, P(1),P(2) or P(1),P(3) will not work, for no apparent reason, while the other will work fine. Add to that your context managers and only initializing the peripherals right before you use them (as the context managers encourage you to do), and you have pure chaos, with random failures.

[tannewt]

For picking the peripheral, the port code should know all available peripherals that can output on the given pins and iterate through them until one is free. On the ESP32 this is super easy, if you initialize an I2C object on pins 3 and 5 you get I2C1. If you do another set of pins you get I2C2. Of course with timers this can get more complicated because of the common settings between channels. However, I would guess that in most cases you aren't using all of them at once so being specific isn't critical. If it is, you could have the peripheral ID as a kwarg that defaults to None to mean AUTO and could be passed constants from a port specific class to identify them.

This approach of taking the next free one doesn't work unless peripherals know whether they have been claimed or not. I think this is a good thing because it prevents you from accidentally switching the function of a peripheral or pin accidentally.

This introduces another problem: the behavior becomes unpredictable, changing depending on the order in which the peripherals are initialized, and (especially in the REPL) depending on what other commands have been issued. For instance, if you have a peripheral P1 that can work on pins 1 and 2, and P2 that can work on pins 1 and 3, then one of the two programs, P(1),P(2) or P(1),P(3) will not work, for no apparent reason, while the other will work fine. Add to that your context managers and only initializing the peripherals right before you use them (as the context managers encourage you to do), and you have pure chaos, with random failures.

Yes, the underlying peripheral use would change depending on initialization order. However, it wouldn't be for no apparent reason. The exception you throw can explain the issue just like if you provided a peripheral ID that can't output on the pins you give it.

I'm not sure how you can extrapolate to chaos and random failures. Its a deterministic scheme that simplifies the initialization in most cases.

[pfalcon]

I think that the low-level API is fine for bit-banging but annoying for supporting real peripherals. I think that it also makes supporting things like DMA much more difficult.

It basically means that if you want to do DMA transfers you have to "fake" the low level API.

There's no need to fake anything: low-level API is optional. Now the problem is that what is "low-level API" got diluted in all these discussions, though it doesn't have to be, and remains the same still: it's start(), stop(), then ability to read raw bytes and write them. The more strange that there're proposal to add some kind of "address" to start(). There're no "addresses" at that level. That's layering violation.

So specific request to @deshipu - can you please provide code examples (plural would be really helpful) for "There should be a standard way to send a device address. It could be either added to the start method as a pair of optional arguments (address and read)." If your driver uses .start(), it's likely non-portable. You write that you used 8 different MicroPython ports. But on how many Linux boards did you try your over 20 I2C drivers with unix MicroPython port?

Going forward, I guess it's now almost decided that we want to have an endpoint class. But note that if we have that, then there's possibly no need to patch something we already had. Final point - while it's almost decided, there's no rush to implement it (as part of builtin Hardware API) - it can be well implemented in Python. Yep, it won't be optimal, and for Hardware API 2.0 or some like version we want to optimize it, but for now, driver writers can just use it.

[pfalcon]

In addition to close() why not add Context Manager support to match how files and network sockets are used in Python? ContextManagers are super nice because they deinit even when a KeyboardInterrupt happens.

For the helper methods and the convenience class why not break it out of machine and ship it separately? That has the advantage of reducing the ROM footprint for uses that aren't I2C. (There can even be multiple friendlier versions that drivers can choose from.)

Ok, so let's define what "helper" is - it's something without which you can write code. That's exactly what context managers, properties and other syntax sugar are. And that's the reason why they're not part of Hardware API and not intended to be. Syntactic sugar on top of Hardware API can be added by another module (in Python). I'm surprised that over so many years, nothing like that surfaced. The only objective explanation is that nobody really needs it.

[deshipu]

Ok, so let's define what "helper" is - it's something without which you can write code. That's exactly what context managers, properties and other syntax sugar are. And that's the reason why they're not part of Hardware API and not intended to be. Syntactic sugar on top of Hardware API can be added by another module (in Python). I'm surprised that over so many years, nothing like that surfaced. The only objective explanation is that nobody really needs it.

It has surfaced many times before. The only objective explanation is that you have been ignoring it all this time. Possibly because you never needed it, not having to actually use the API in an extensive manner, and not being used to the Python idioms. As they say, one can write C in any language.

[deshipu]

So specific request to @deshipu - can you please provide code examples (plural would be really helpful) for "There should be a standard way to send a device address. It could be either added to the start method as a pair of optional arguments (address and read)." If your driver uses .start(), it's likely non-portable. You write that you used 8 different MicroPython ports. But on how many Linux boards did you try your over 20 I2C drivers with unix MicroPython port?

I promised myself to ignore your ad hominems, but since this one can be reworded in a way that actually contains some technical arguments, I will bite.

If you actually look at the specification for the I²C protocol (available at http://www.nxp.com/documents/user_manual/UM10204.pdf), you will notice that the slave address and the R/W bit are integral part of the protocol, defined together in the same section and the same "protocol layer" with the start and stop conditions, clock stretching, acks and naks. There is no "separate layer" there, and thus there can't be a "layering violation". If you stop and think for a moment about how this is implemented in hardware, you will realize that indeed the slave address has to be part of the protocol -- because the slaves have to be watching the bus for it, and they have to stop watching as soon as the address doesn't match.

I have already provided code examples and references to other languages using exactly this design -- in the original issue at https://github.com/micropython/micropython/issues/2073 . You just ignored them then, but please feel free to revisit it and re-read it. I will not keep repeating the same thing over and over.

I have not used any I²C drivers with the Unix port, and neither have you, for the simple reason that the Unix port doesn't have a machine.I2C peripheral. I did use several I²C libraries with regular CPython on a RaspberryPi and BeagleBone for my robots though. Not sure what is the point behind this question, other than making noise and muddling the discussion?