refactor: add drivers.Pin interface

[deadprogram]

This PR is to add a drivers.Pin interface and use it everywhere that is easy to do so.

Related to work in PR #742 this is intended to help disconnect the drivers repo even more from the machine package.

The remaining drivers that use machine.Pin directly have various complications such as changing the pin config from input to output and back, which will have to be addressed in some different way.

[sago35]

I think defining drivers.Pin is a very good idea.
Except for the examples/ directory, machine.Pin should be completely replaceable.

[soypat]

I prefer a leaner and clearer API. I arrived at an API which is functionally equivalent but splits input from outputs:

// PinInput is hardware abstraction for a pin which receives a
// digital signal and reads it (high or low voltage).
type PinInput func() (level bool)

// PinOutput is hardware abstraction for a pin which outputs a
// digital signal (high or low voltage).
type PinOutput func(level bool)

Cons:

• Not API compatible with machine.Pin

Pros:

• Clearer API. Pins either function as inputs or outputs and that is left clear as water in the API for users and for developer
• More performant API. No virtual method indirection layer and is probably easier for compiler to inline since LLVM already has optimizations built around function pointers. This is probably very important for something like a pin which is an instantaneous call unlike most other HALs in tinygo. Will enable users to do more things with API
• Simpler
• Much easier for users to implement HAL for specialised applications. Below is an example of implementation with function pointer HAL. The equivalent interface API code would be much longer and more error prone in my opinion. There is also the requirement of implementing methods you don't really need. If you are using an output pin you'd still need to implement method(s) relating to input even if it has absolutely nothing to do with your use case

pin := func(level bool) {
    reg := (*volatile.Register32)(unsafe.Pointer(rp.RP_BANK_SIO))
    reg.Set(1<<level)
}
dev := pca1x.New(pin)

Note on compatibility of change: We are breaking API compatibility with any change we perform since lots of drivers call Configure method anyhow, so we'd break users who don't call configure beforehand regardless.

[deadprogram]

Not API compatible with machine.Pin

Hmm, not too sure that sounds like a very good idea....

[soypat]

good idea

Help me out Ron. The more I think about this the more this is outweighed by the benefits of function pointer API.

We are very lucky to be using a statically typed language. It will notify users of API breakage and where and if we add context, we can also let users know how to fix it. I'd go as far as to say the risk is less with breaking users at compile time. If we preserve API compatibility of machine.Pin we are breaking our users silently. This is perhaps the worst breakage. Programs will stop working. Over the coming years users will drop in to slack noting their programs stopped working in the strangest ways. Peripherals stopped responding after running go mod tidy. Maybe they didn't notice the peripheral stopped working (we are lacking in the error handling department in many drivers) until something physical breaks. I can't imagine how this is better than breaking the API and giving users context in the API on how to fix it... I mean, they have to fix it one way or another, best let them know from the get go!

Unless I am missing something big API breakage in this case is better than preserving API compatibility since we break our users regardless due to the missing Configuration call.

[ysoldak]

I think the idea with introducing "backwards compatible" interface was exactly not to break things for people?
How will it break, @soypat? I must be missing something.

I see typical (yet unconventional in world of classic embedded tinygo) use case to be people write their code for specific platform and implement abstractions, like working with pins. I did it for my RPi project.
They then can just pass these abstractions directly to drivers, w/o need to implement function handlers in each and every case.

For example, take st7735 driver (I use it now).
It accepts 4 (four) additional pins in constructor. I'd rather pass 4 structs there, instead of 4 function handlers.

[ysoldak]

I like this proposal better than function handlers. Also consistent with other interfaces.

[soypat]

How will it break, @soypat? I must be missing something.

(See code below if this is not clear) Before this change drivers were initialized with machine.Pin type and as such many drivers called machine.Pin.Configure internally. Look at the changes in this PR to get a better idea of what I mean. This PR removes machine.Pin.Configure calls. This is a breaking change. It will break our users silently. They will not be aware of the breaking change until they either run into unexpected behaviour or come across some other user who is wondering out loud in tinygo slack why their project is not working.

I see typical (yet unconventional in world of classic embedded tinygo) use case to be people write their code for specific platform and implement abstractions, like working with pins. I did it for my RPi project. They then can just pass these abstractions directly to drivers, w/o need to implement function handlers in each and every case.

Yes, this is not an issue for users who are working with the machine.Pin abstraction. This is an issue for the following cases:

• Users of upstream Go in embedded since they usually have a different GPIO pin abstraction
  • go-rpio's read pin method signature is different from TinyGo's
  • The widely used periph.io has a different interface for GPIOs
• Users of drivers which emulate pins, like GPIO peripherals or shift registers who then want to use the emulated pins in other drivers
  From where I stand it seems we are doing this change to benefit these users yet we are proposing a method interface which matches with none of them requiring them to implement the interface themselves while also breaking our users (silently!).

For example, take st7735 driver (I use it now).
It accepts 4 (four) additional pins in constructor. I'd rather pass 4 structs there, instead of 4 function handlers.

The change required for your project is as follows:

Before

dev := st7735.New(spi, rst, dc, cs, bl)

After

// By making pin HAL users are now required to initialize their pins. This is the breaking change.
bl.Configure(machine.PinConfig{Mode: machine.PinOutput})
dc.Configure(machine.PinConfig{Mode: machine.PinOutput})
cs.Configure(machine.PinConfig{Mode: machine.PinOutput})
rst.Configure(machine.PinConfig{Mode: machine.PinOutput})
dev := st7735.New(spi, rst.Set, dc.Set, cs.Set, bl.Set) // Set implements function interface.

Let's sit down and ask ourselves the cost of this change before pondering the benefits (which are great!). Can we mitigate these costs? I've proposed one way of doing so with a different API which would notify users of a breakage that would otherwise go unnoticed, which is to say the only Con I've written down above is doing its part as a benefit to our users.

[ofauchon]

I think we should always keep TinyGO as simple as possible.

If the goal is to isolate drivers from machine, drivers.Pin interface nicely does the job.

Although function handlers approach may be more powerful, I'm not sure this justifies the extra complexity.

[ysoldak]

My main problem with function handlers approach is it is not consistent with other abstractions that we have already, like SPI, I2C, Displayer interfaces, et al. Having two types of abstractions brings much more confusion in long run, imho.

Yes, I hear argument about breaking users silently, it's an important one indeed.
Yet, in my eyes, it does not overweight importance of keeping API consistent.
After all, we still not at v1.0.0, so feels like we can kind of can afford it.

[ysoldak]

Another thought.
Interface is a contract: the driver going to call this, this and this method.

For pins it's usually just one, a write or a read, of course.
But I'm thinking in general, how we want our API space look like.

[soypat]

I've gained another worry in the last few hours regarding this PR. I've recently been looking at lots of C drivers to write up my driver design document. When looking for a C driver for a device it is common to find something in the order of 3-4 drivers for a single peripheral. More often than not you'l find the quality of these drivers and the abstractions they use vary greatly.

Luckily today the Go community has rallied around the tinygo/drivers repository as a central source of drivers it seems. The HAL designed by TinyGo is second to none, it really got it right in many aspects, all tracing the roots back to:

• Simplify HAL to core utility. All TinyGo interfaces do no setup, and are single method interfaces like I2C, Sensor and SPI (Transfer does not count!). This is to the benefit of host driver developers in great part and also peripheral driver developers to some extent
  • And this is something I know we value a lot because we will break our users (at compile time!) to uphold this simplicity: I2C: discard WriteRegister and ReadRegister methods in interface #578
• Minimize the cost. TinyGo interfaces have a good cost-to-abstraction ratio.
  • Interfaces: You may be aware interfaces are "costly", but their cost might be slightly obfuscated. An interface structure is allocated on the stack, but it's object is almost always on the heap. When one calls an interface method... what happens? Really I don't know for sure. I think last I heard the compiler inserts a for loop with all possible types that could batch the interface underlying type and matches on the first one, but I'm really not sure. In the context of I/O this really is negligible, after all your SPI or I2C will be polling a register during the actual method call to check if the transaction is done. Interfaces do not prevent users from doing things with the SPI, I2C and Sensor interfaces. They have a excellent cost-to-abstraction ratio. This is almost certainly not the case with a machine.Pin interface. Pins toggles are almost always a couple instructions to perform. All of a sudden the virtual method call on the interface becomes the larger part of the work to be done on a pin.High() call. This in my opinion represents a bad cost-to-abstraction ratio. All of a sudden, with one design decision we have now limited what our users can do with the API. Maybe it is the difference between a stepper motor driver max speed being 800RPM and 200RPM. In fact, this question made me write a benchmark to investigate the cost of this abstraction, find it below.

I worry that the drivers.Pin interface is conducive to splitting the TinyGo ecosystem between those who seek good cost-to-abstraction drivers and those who seek the most used drivers, much like the C ecosystem. I think whether or not we decide to include the drivers.Pin interface eventually users will come to need an abstraction that does not prevent our users from doing the most they can with the hardware they have. None of us want a future where TinyGo maintainers suggest that driver users "unimplement interfaces" to get better results out of their pin driven drivers.

And if we really do go the way of having both drivers.Pin and drivers.PinfuncOutput, will it have been worth it to avoid setting a pin HAL with pin.Set? I've yet to hear anyone address the issue of the cost of the complexity of a function pointer HAL. I've mentioned that implementing a function pointer HAL is much simpler than implementing an interface yet we say it is more complex. Are we maybe not giving our users enough credit?

We seem to be worried about users having to correctly set the function pointer which means basically "replace pinName with pinName.Set" when maybe we should be even more worried about users finding out they have to configure their pins before use...

Interface vs function pointer benchmark

package main

import (
	"machine"
	"time"
)

func main() {
	// Register more types with drivers.Pin interface to measure impact of virtual method call with many implementing types.
	impl1 := &pinimpl[uint8]{}
	d2 := driver{ipin: impl1}
	d2.doIface(10)
	impl2 := &pinimpl[uint16]{}
	d3 := driver{ipin: impl2}
	d3.doIface(10)

	p := machine.LED
	p.Configure(machine.PinConfig{Mode: machine.PinOutput})
	d := driver{ipin: p, fpin: p.Set}

	const N = 10000
	fstart := time.Now()
	d.doFunc(N)
	felapse := time.Since(fstart)

	istart := time.Now()
	d.doIface(N)
	ielapse := time.Since(istart)

	for {
		println("per func call", (felapse / N).String())
		println("per interface call", (ielapse / N).String())
		time.Sleep(time.Second)

	}
}

type driver struct {
	ipin ipin
	fpin fpin
}

type ipin interface {
	Set(b bool)
	Get() bool
	High()
	Low()
}

type fpin func(bool)

func (d *driver) doIface(n int) {
	k := true
	for i := 0; i < n; i++ {
		d.ipin.Set(k)
		k = !k
	}
}

func (d *driver) doFunc(n int) {
	k := true
	for i := 0; i < n; i++ {
		d.fpin(k)
		k = !k
	}
}

type pinimpl[T ~uint8 | ~uint16 | ~uint32 | ~uint64] struct {
	k T
}

func (p *pinimpl[T]) Get() bool { return p.k != T(0) }
func (p *pinimpl[T]) Set(b bool) {
	if b {
		p.k = 1
	} else {
		p.k = 0
	}
}
func (p *pinimpl[T]) High() { p.Set(true) }
func (p *pinimpl[T]) Low()  { p.Set(false) }

Results:

1 ipin implementation:
per func call 82ns
per interface call 60ns

2 ipin implementations:
per func call 72ns
per interface call 196ns

3 ipin implementations:
per func call 72ns
per interface call 281ns

It seems like when there is only one interface implementation the compiler can optimize to a static function call which is even faster than the dynamic function call. As soon as another interface implementation is added the speed drops dramatically, over x3 slowdown and linear scaling slowdown for every implementation added.

[ysoldak]

Great write up, @soypat !

And thanks for actual benchmark, super!

I don't see anyone having multiple Pin interface implementations for any real use case?
Like one has a device, a platform, one implements Pin interaction driver and just uses it in their program.
A codebase can have different Pin drivers, but only one will remain, selected on compile time (assuming cross-compiling for multiple platforms).

[soypat]

I've worked on codebases with 3 pin implementations, and that was a small project by the comapany's standards:

• Typical machine.Pin implementation
• One implementation that required cycle synchronised pulse output on two machine.Pins
• An implementation which served as a abstraction on machine.Pin to debug pin state

The device wasn't even something strange, it was a commonplace industrial machine that is even present in many homes: a 3D printer. All pins controlled motors and an impact on the speed of the Pin.Set function would have been extremely noticeable on some of the motors. We were even thinking of adding a shift register peripheral to better control some of the motors and that would have been a fourth implementation.

I'll note I've been using the pin HAL for the better part of 3 years to solve a great range of company problems in both C and TinyGo. From my viewpoint it feels the pin HAL is in it's infancy in mainstream TinyGo, just look at the drivers, most of them do the same thing, just toggle an peripheral control pin to then do heavy I/O, of course these drivers could do just fine if the pin HAL was excruciatingly slow. Also, saying most people just need a single pin abstraction is jumping the gun a bit, don't you think?

[soypat]

Heck I forgot the board was going to add a fifth pin HAL as well with a GPIO multiplexer. And I can't guarantee it would've stopped there.

[soypat]

Kept thinking and there are several use cases in the TinyGo ecosystem that would be affected. Use cases which would negatively be affected by the interface slowdown :

• Bitbang communication protocol implementations in drivers repo
• Debouncing implementations are practically guaranteed to be slowed down since a debounce algorithm is functionally a wrapper over a Pin interface, so guaranteed to have 2 Pin implementations
• Stepper motor drivers max speed would be slowed down proportionally to the amount of pin implementations
• Peripheral pins like GPIO multiplexers and shift registers, also guaranteed to be slowed down on targets that have host-based pin implementations like all TinyGo MCU targets.
• Coprocessor MCUs like the Pico W which has controllable pins on the CYW43439 to control LED, so slowdown guaranteed on all pico-w and pico2-w programs that use the LED.

There's certainly more cases. These are just the ones I've run into.

[ysoldak]

So... If interface approach can be slow and, worst of all, its speed depends on number of potential matches a compiler can find...

I wonder, how this nice little softspi can reliably work then, for example?.. A problem indeed...
https://github.com/tinygo-org/drivers/blob/dev/apa102/softspi.go

Can we have a cake and eat it have the best of both worlds perhaps?
From drivers.Pin interface we can have straightforward mapping to machine.Pin and consistent API (consistent with all other abstractions in this repository).
From function handlers we can have speed, for the cases when it matters.

How you ask?

Well, what if we let drivers authors decide how they want it; they are better suited to understand needs for their driver.
So we can require every driver provides standard constructor named New that must use drivers.Pin for pin parameters.
But also there can be another constructor available, say, NewWithFunctionHandlers or NewHighPerformance, you name it, that accepts functions for parameters.

Internally then, it's up to driver author on how to implement this -- they may accept drivers.Pin but store links to High(), Low() or Get() function internally and use them directly. Of course, if they also provide NewWithFunctionHandlers constructor, they assign them functions directly.

---

Good news, btw, in at least several cases touched by this PR the driver constructors return objects, the old way, rather than pointers to objects. We could convert these constructors to return pointers and intentionally break client code to alert users about breaking changes (the need to initialize pins).

[soypat]

OK, nice! I feel we are reaching some common ground! 😄

I feel like we are close to a compromise between PinFuncs and drivers.Pin but that there are details being overlooked. I suggest all interested parties jump in a call. Best way to organize I feel is through slack on the #tinygo-dev channel. The TinyGo core meetup is coming up in a few weeks, though it is far off, maybe we can meet before? I'll start a discussion on slack to see if there is quorum for a meet

And juuuuust for completeness...

I wonder, how this nice little softspi can reliably work then, for example?.. A problem indeed...
https://github.com/tinygo-org/drivers/blob/dev/apa102/softspi.go

Well that example is curious indeed. Almost as if it did not use interfaces for pins... which it doesn't 😅
Even if it did use interfaces and was slow, slowness is likely consistent enough for a protocol to work, though slower. (don't quote me on that)