RFC: Hardware API: finalising machine.Pin

[Damien]

Before it gets too late, and before there are too many ports of MicroPython in the wild, we must come to a concensus on the Hardware API, on the machine module. We've already tried to do this a few times. It's difficult. Everyone has different ideas on what the details should be. But not having a finalised machine API is really starting to hurt development and hold things back. And worse, in some cases there are alternative implementations that are starting to diverge.

So, let's have a (friendly!) discussion here on how the final API will look. I suggest we have a differend thread for each separate component, and this one is for the Pin class (got to start somewhere, should be at the beginning). I suggest we come to a 80% decision here on the API, then someone can write formal docs and submit a PR to the main repo. Then we can have final discussion on the PR, then merge the docs. And then it's just implementations that need to be written

[Damien]

Here is my first stab at a revised Pin API, just to get the discussion started. It may be that in the end we change nothing for Pin, but at least we had a good discussion about it.

Constructor:

Code: Select all

Pin(id, mode, pull, alt, value)
- modes: IN, OUT, OPEN_DRAIN, ALT, ALT_OPEN_DRAIN
- pull: None, PULL_UP, PULL_DOWN
- alt: alternate function (port specific values)
- value: initial value to set the pin to

Methods:

Code: Select all

pin.mode([value]) # get or set the mode
pin.alt([value]) # get or set the alt function
pin.pull([value]) # get or set the pull
pin.input() # convenience for pin.mode(Pin.IN)
pin.output() # convenience for pin.mode(Pin.OUTPUT)
pin.high() # same as pin.write(1)
pin.low() # same as pin.write(0)
pin.read()
pin.write(value)

If you construct a pin without a mode (eg Pin(0)) then it stays with the existing mode/pull/alt values.

Pin(-1) creates a dummy software pin.

[dhylands]

I think that we need to address the more fundamental question, should a port be allowed to add features to say machine.Pin?

For example, many ports support the notion of setting drive-strength. Would it be acceptable for such a port to add a drive_strength parameter to init, and perhaps an accessor/setter method?

Or would this need to be done in some other non-machine module?

Weighing in on the Pin API, I'm not sure I like read/write and would prefer value with no args being a read, and value(n) being a write.

Is id allowed to be a string? Or just an int?

What about mode == ANALOG? On the freescale parts, ANALOG mode is an alt-function. On the stmhal, its an actual mode, and not an alternate function.

[pfalcon]

So, let's have a (friendly!) discussion here on how the final API will look.

Friendly discussion: We already have Hardware API specification docs at https://github.com/micropython/micropyt ... rdware-API . Number of people contributed to it over extended period of time. There were no edits or big discussions on it for a while, only more or less contained changes. So, it would be fair to think that *that specification* is what being finalized.

So, why suddenly this topic without any references to prior art? If there is a pull from some side, it would be very friendly to disclose it.

[deshipu]

I agree that we should definitely start with what is already there at https://github.com/micropython/micropyt ... rdware-API and also actually implemented in the various ports. However, judging by the "number of people" who contributed to it (which is pretty much 2, looking at the history), I'm not sure we actually have a complete picture there. We need feedback from people who are actually using this in the wild, writing drivers and libraries, trying to make it work with actual hardware that is available out there. The "everything in one place" format of the current hardware API discussions is not conductive to that. I think that splitting it into smaller sub-discussions concerning individual modules and discussing the problems with the current approach and possible changes has a much better chance of capturing useful feedback.

So, for the Pin API specifically, why don't we take what is there already and point out what is wrong or awkward with it -- with references to particular ports or use cases -- without proposing any actual changes to it, because that tends to divide people into camps and invites bike shedding. Then we can actually think about possible adjustments.

[deshipu]

For my part, I would love to have a way to toggle the pin quickly, without having to do two separate function calls in Python (which slows things down a bit). The `pyb` module had a `toggle` method on it, which is a convenient way to do that.

[Damien]

I think that we need to address the more fundamental question, should a port be allowed to add features to say machine.Pin?

Yes, that's an important question, and would apply to all classes and functions (eg machine.reset_cause and its constants). We need a way to add extensions to the API for a particular port. One way is to define subclasses, eg stm.Pin has machine.Pin as a base. But that will just lead to even more unportable code because there'd be stm.Pin everywhere instead of machine.Pin. Another way is to define functions that act on machine objects, eg nrf.config_drive_strength(pin, nrf.WEAK_DRIVE). IMO that's pretty ugly and awkward. So it seems the best solution is to just add methods, constants and keyword args to existing functions. By having a general ".config(...)" function we can limit the changes to that function (similar to network config).

Is id allowed to be a string? Or just an int?

Just like the existing API, it can int or string.

[Damien]

So, why suddenly this topic without any references to prior art? If there is a pull from some side, it would be very friendly to disclose it.

Sorry, it wasn't explicit, but I was implying all existing discussion, implementations and docs.

There is no specific pull from some side. It's just that from my point of view this is still a big mess, the discussions are deeply buried in the long issue https://github.com/micropython/micropython/issues/1430, the wiki https://github.com/micropython/micropyt ... rdware-API is not consistent with any particular implementation (probably WiPy is the closest), and we still have inconsistency in the docs. Each of these can be independently fixed, but there's no real source of "truth" so I don't know in what direction to fix things.

There's also been a bit of talk lately about changing APIs here and there to make things better, and I wanted to make sure we are "happy" with the particular components of the machine API so we can say it's "done".

[Damien]

So, for the Pin API specifically, why don't we take what is there already and point out what is wrong or awkward with it -- with references to particular ports or use cases -- without proposing any actual changes to it, because that tends to divide people into camps and invites bike shedding. Then we can actually think about possible adjustments.

If we are going to take the "Hardware API wiki" as truth (probably a good idea) then let's try to clean it up. For example:

• There is a remark: "TOD: Do we really need 2+ ways to set the value?" this refers to having pin.value() and pin() to get/set the pin value. I know I advocated for pin() as a fast way of doing get/set, but I accept that it's more Pythonic to have only one way of doing things.
• Do we need pin.out_value() to get the output buffer value of a pin?
• I didn't even realise that pin.toggle() was specified in these specs Do we need it? It's listed in the specs as "optional" but I think it's a bad idea to have it optional because then some ports will have it and others will not.
• Do we want to add pin.high() and pin.low() as convenient ways of doing the most common pin operation? It aids a lot to readability of code. If we have pin.toggle() I think it's warranted to have high/low.
• Similarly, do we want to add pin.input() and pin.output()?

[platforma]

I'd agree with Dave on the idea of pin.value() replacing the read and pin.value(n) replacing the read/write, just like the wiki describes. The name is clear and intuitive for both cases. Setting/getting the value with pin() might be an unnecessary convenience.

Also would agree with a pin.toggle() method, it is frequently used, easy to read and very obvious even to someone who doesn't know that it's equivalent to pin.value(n). Both pin.high() and pin.low() would logically go with pin.toggle(), but I'm more reluctant to those. The argument for this is: if you don't know the current value of the pin somewhere in the code, you have to:

Code: Select all

if pin is high:
    pin is low
else:
    pin is high

which could be simplified to pin.toggle(). Where pin.high() and pin.low() are replaced with pin.value(1) and pin.value(0), not a big trade off. The same goes for pin.intput()/output() both of those are replaces with one line that has 8 extra characters to type. But maybe I'm too biased towards everything bare minimum.