Add PinInput and PinOutput HAL

[soypat]

machine.Pin is not compileable by upstream go. To promote development of drivers which are usable by upstream Go I propose we add the following API.

// 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)

[deadprogram]

What about #749 ?

[soypat]

I missed that @deadprogram. Will comment

[soypat]

@aykevl @ysoldak I've started work on the PR. Most of the infra is in place and the drivers I've worked on are good demonstrations of the proposal in action. I'll be on vacations for a few days so I might not be available for a week or so :)

[soypat]

I think around half of the work is done. On vacation this weekend so I'll get around sometime next week

[soypat]

OK! I just finished the brunt of the work. @aykevl @ysoldak @deadprogram @sago35 Tagging ready for review of the work done!

[deadprogram]

@soypat just tried on the following:

• uc8151 on badger-2040w
• st7735 on pybadge

Both worked as expected.

It is kind of a big PR, and I do not have the hardware to test all of the changed devices, but seems legit.

I would suggest squash the commits and rebase against the latest dev. In the meantime, can we get comments from others please?

[ysoldak]

I don’t agree with implied obsolescence of structural API (stuffing it into legacy package) and moving functional API forward.

Structural API has no performance drawbacks (drivers can store pointers to functions internally and use them) while keeping overall drivers API consistent and machine.Pin can be used conveniently too.

In the end, it seems to be boiling down to personal preferences. Just voicing my opinion as asked by @deadprogram

[soypat]

The current reliance on machine.Pin, although convenient, has become a limiting factor in the broader adoption of the TinyGo drivers package. While it has served well for initial development, it restricts portability and prevents drivers from being easily reused across platforms. This PR proposes a path forward that maintains compatibility while addressing these limitations.

This change introduces a new internal model for pins that achieves cross-platform support without breaking the existing user-facing API:

• Stable API surface – From the user’s perspective, nothing changes. The public API continues to accept pins as before, ensuring no disruption for existing applications.

• Cross-platform flexibility – Internally, pins are now backed by function pointers instead of machine.Pin. This allows drivers to run on any platform, fulfilling the promise of true cross-platform drivers.

• Performance considerations – Function pointers do introduce a small overhead compared to direct machine.Pin usage, but remain well within acceptable limits. For example, generating a 25 kHz signal on a Raspberry Pi Pico poses no issue. Importantly, this design avoids the potentially unbounded overhead of interface-based calls in TinyGo, where virtualization cost is not guaranteed.

Driver authors will need to work with function pointers. While this is less familiar to the average Go developer, it is straightforward and actually conceptually simpler. In fact, driver developers are already required to navigate advanced topics such as memory allocation control, so this additional requirement should not be a barrier.

An interface-based pin HAL is a valid alternative and would feel more idiomatic to Go developers. However, the performance overhead of interface method dispatch in TinyGo is difficult to bound, making it unsuitable for drivers that need predictable timing. The function pointer approach, while less conventional, provides the necessary guarantees.

By adopting this design now, TinyGo avoids locking itself into a HAL abstraction that may later prove too slow for advanced use cases (e.g., stepper motor control, pin multiplexing, or debounce handling). Function pointers ensure consistent, predictable performance without requiring a redesign of the HAL in the future.

Can we please put this to rest Yurii?

[ysoldak]

However, the performance overhead of interface method dispatch in TinyGo is difficult to bound, making it unsuitable for drivers that need predictable timing.

But no, why? Inside a driver its author can do what they please. This includes taking and storing function pointer to Set() or Get() function, for performance. Exactly as you already do in this PR bridging legacy.PinOutput to drivers.PinOutput.

Can we please put this to rest Yurii?

@deadprogram asked for comments and opinions, I obey.

Seems like no-one else is interested in the topic or afraid to speak up? This is not healthy.

[ysoldak]

Before we roll with any API, I'd like us understand how we handle cases like this:

// Perform initialization of the communication protocol.
// Device lowers the voltage on pin for startingLow=20ms and starts listening for response
// Section 5.2 in [1]
func initiateCommunication(p machine.Pin) {
	// Send low signal to the device
	p.Configure(machine.PinConfig{Mode: machine.PinOutput})
	p.Low()
	time.Sleep(startingLow)
	// Set pin to high and wait for reply
	p.High()
	p.Configure(machine.PinConfig{Mode: machine.PinInput})
}

https://github.com/tinygo-org/drivers/blob/dev/dht/thermometer.go#L98-L103

[soypat]

Inside a driver its author can do what they please.

Absolutely, though we can all agree it would be less confusing to newcomers and existing driver developers if there was only one way to do things. Of course anyone can develop a driver as they very well please. Seeing this is a tinygo-org repo we should establish a baseline way of doing things that is "the best" we can do. At the time being all evidence points me to function pointers as being the best compromise.

Before we roll with any API, I'd like us understand how we handle cases like this:

The DHT interface is not a pin interface but rather a single wire protocol interface similar to SPI and I2C that could be implemented via pin bitbanging or maybe a pico PIO.

But, if we were to entertain the idea and somehow use pins where they shouldn't be used, it might look something like this:

pin := machine.GPIO12
pinIn := func() bool {
   pin.Configure(inputCfg)
   return pin.Get()
}
pinOut := func(b bool) {
   pin.Configure(outputCfg)
   pin.Set(b)
}
therm := dht.NewThermometer(pinIn, pinOut)

@deadprogram asked for comments and opinions, I obey.
Seems like no-one else is interested in the topic or afraid to speak up? This is not healthy.

I appreciate your comment regarding the DHT interface. I appreciate the conversation we've had back and forth over the past few months. It really feels like we've reached a good compromise. It just felt like your previous comment was a backpedal without expressing the underlying reason on why you are opposed. I find it hard to discuss about a "Structural API" if there is no example of what you mean by it. It also confused me you mentioned machine.Pin as convenient- this PR reason for being is to eliminate machine.Pin since it poses a great inconvenience to those wishing to use drivers in the greater embedded Go ecosystem.

[ysoldak]

IMHO, it is convenient (a friendly API) if people could directly use variables of machine.Pin type in driver constructors / configuration routines. To satisfy that, we need drivers.Pin interface be compatible with machine.Pin type.

I'd argue passing machine.Pin.Get() and machine.Pin.Set() function pointers to constructors is less intuitive than passing a variable of machine.Pin type. But that can be just me. I'd really like community to speak up.

Given above, I feel it's reasonable to optimize for arguably default and intuitive use, while making it possible to bring other pin implementations (breaking hard dependency from machine.Pin).

[soypat]

Yurii, all APIs in this PR receive machine.Pin types -.-

[gen2thomas]

I'm currently implement #790. Until now I'm not aware of this PR but read the driver-design guide. My intension is to use TinyGo-drivers more and more for gobot to prevent reinventing the wheel and reduce duplicate maintenance effort. Many thanks to @soypat for driving this possibility forward.

I will take the time to review this PR before continuing with the hx711 driver.

[gen2thomas]

Suggested change

	SCK drivers.PinOutput
	SetSCK drivers.PinOutput

picked this as a first example, but applies for all occurrences in this PR

reasoning:

• in go it is best-practice that get-functions not start with "get"
• the functions "Low()", "High()" are not named quite correct ("SetLow()", "SetHigh()" would be better), but are still understandable by its context e.g. "pin.Low()"
• with this in mind, for "SCK(low)" I read it as "get-state-of-SCK-for-false" and was confused, why the return value is not used

[gen2thomas]

We should also have in mind, that the "SCK" is not an attribute anymore, but a function now - maybe the name "drivers.PinOutput" does not make that clear enough?

[soypat]

I'm still not convinced of this change. In Go we typically avoid setters and getters. I understand where the confusion stems from and am open to the change though yet skeptical

[gen2thomas]

In Go we typically avoid setters and getters

this is not quite correct: https://go.dev/doc/effective_go#Getters

[gen2thomas]

Suggested change

	type PinOutput interface {
	type PinSeter interface {

I have no strong opinion on that, but find it very handy and use it whenever possible, because:

• it seems to be best practice to name interfaces for one functionality the same like the function, but with adding "-er"
• it is immediately clear to the reader of the code, that this is an interface
• no looking for a better name nor any discussion is needed, it is just a rule
• I personally use this rule also for interfaces with more than one function (if applicable), to earn the benefits, so an alternative is "PinOutputer", if the interface is planned to grow

[ysoldak]

@gen2thomas while I have no opinion on the interface name, I just want to point out this PR puts the interface in internal/legacy package, for retirement. So no grow is planned.

[soypat]

I've left a general comment on this #753 (comment)

[gen2thomas]

Suggested change

	type PinInput interface {
	type PinGeter interface {

same reasoning as for the other interface name

[soypat]

Same comment, using main thread for discussion #753 (comment)

[gen2thomas]

Suggested change

	type PinOutput func(level bool)
	type PinOutput func(level bool) error

I know, this partially violates the intention of this function - keep the same syntax like "machine-pin.Set()" to keep the changes as simple as possible, but when using the character device driver on Linux-machines, e.g. in gobot, setting an output can fail - same for the "Get()" function. Maybe there is another possibility to catch this problem?

[soypat]

Today all TinyGo pin HAL does not return an error. We could think of adding a PinOutputFailer or similar in the future, I'd avoid too much scope in one PR for now.

[gen2thomas]

What happens if "!ok" later on in the program? Should we return an error or at least print a message? Because pin configuration itself can fail if not "baremetal" I prefer to return an error. So the error can be handled at caller site, e.g. by retry.

[soypat]

Two things to keep in mind:

• If !ok then the user is using the driver with a pin they configured as per the new API agreement
• If a new driver is developed it will NOT use configurePin. this is legacy and will eventually be phased out over time.

[gen2thomas]

Is this function really needed? Mostly it is better to call a function and return an error, if the condition is not met at this time than check the condition some time before and call the function later if the condition was met in the past. Reason: the state can change between check and call, which leads to sporadic errors.

[soypat]

This serves only to fulfill perfect backwards compatibility for the one or two drivers that use it. New drivers will NOT use it.

[gen2thomas]

Suggested change

	// digital signal (high or low voltage).
	// digital signal (high or low level).

What I have found with the search engine: "Low voltage ranges from 0 to 50 volts, while high voltage ranges from 1,000 to 500,000 volts."

[gen2thomas]

Suggested change

	busy: busyPin.Get,
	isBusy: busyPin.Get,

... as one example for an input-pin function, see also my remarks for the Set-function (apa102/softspi.go), the same applies for the usage of legacy-Set()-function

[soypat]

I do agree isBusy is much better than busy! Changed in latest commit!

[gen2thomas]

Nice work @soypat ! Some of my suggestions are very opinionated, so don't worry too much about. But the most important would be to change all affected examples, in best case together with this PR. Also we should adapt the driver design page as soon as possible.

As a next step I will adapt my new driver for hx711 to the current state and implement the wrapper for gobot to get some practical experience about the usage.

Additionally I will repeat your benchmark with arm32, arm64 (Go) and MCU nrf52840 (TinyGo).

Update:
The new driver for hx711 is updated to the latest state and the wrapper for gobot is implemented: hybridgroup/gobot#1164

For the benchmark results:

• arm32: Add PinInput and PinOutput HAL #753 (comment)
• arm64: not done, IMO will not provide more information
• nRF52840: Add PinInput and PinOutput HAL #753 (comment)

[gen2thomas]

Suggested change

	type PinOutput func(level bool)
	type PinSet func(level bool)

just my recent experience, this would be more intuitive when writing a new driver, same for PinInput() --> PinGet()

[soypat]

At a glance PinSet conceptually sounds further from a HAL and closer to a struct field setting functionality, same for PinGet. Also PinSet sounds like it could be a set data structure.

I'd also feel odd naming them this way in Go. The "Get" and "Set" functionality is encoded in the underlying type func(bool) or func() bool. We don't call our interfaces WriterInterface in Go.

This is just my gut reaction speaking though.

[gen2thomas]

You are right, in English the noun for "data set" is often shorten as "set" and so can be confused with the verb. I mostly use the extension "-Func" in my own code in this case but never made an external interface with a function until now. Maybe someone has a better idea for the name. Also, because the function is located in the drivers package we are forced to repeat the "Pin" in the name. Normally such things would be located in a pin-package and a "pin.Set" would be ok.

I also understand your example with the "WriterInterface", but please have a look to my comment for the legacy interface names. The interface name can have an "-er" extension (to construct an agent noun), but the function names inside not, of course. In our case the interface name and the function name are merged to a single interface function. Maybe this idea helps to find a better name.

I just tried to use it at the opposite side - device driver side - and it felt bumpy when programming and reading at this side. I had to read the type multiple times and also the example in apa102 (construct the softspi) to train my brain to this wording.

[gen2thomas]

What about this names, very obvious inspired by the comment: "PinSetLevel(newLevel bool)", "PinGetLevel() bool"

[soypat]

So if I'm getting this correctly the name "PinOutput" is hard to understand when reading. May I suggest a couple of approaches:

• Use meaningful and semantic identifiers: Types are only part of the story. You write "PinSetLevel(newLevel bool)" as the ideal legibility standard- this is achievable! Use the variable identifier to your advantage! Variable identifiers appear in driver code much more often than the type itself and can make for extremely readable code. I suggest something along the lines of:

type Device struct {
    csPinSet drivers.PinOutput
}

func (d *Device) Send(newLevel bool) {
    d.csPinSet(newLevel)
}

The actual type drivers.PinOutput would appear about twice in a typical driver. In the struct decvlaration and in the New function- and it appears only once per driver in the refactored version in this PR since we are trying to preserve backward compatibility with legacy.Pin* types. This naturally makes it very hard to adapt to an unfamiliar API since you really are not reading it at all when working with code that uses it.

• Maybe try an interface-centric approach? Change types to PinOutputter and PinInputter. See if this helps reading the code. Function handles are very similar to single-method interfaces after all.

• Try using functions as values in other projects. Go code typically does not make use of functions as values so naturally reading code that does this feels off or unexpected when we follow a variable with parentheses. I suspect this might be a big part of the offputting part. If this is the case the name of the type is not going to aid much since the usage is far from the type's declaration in driver development. Like I mentioned earlier, use meaningful identifiers that help you read the code! If Set and Get help you read the code, then name your variables accordingly!

[gen2thomas]

Of course - please also read my other suggestions and you will find some of your ideas. "Rewording" in the Devices struct code is one of my suggestions.

And yes, "drivers.PinOutput"/"drivers.PinInput" would occur in 2 places normally - so it is maybe not worth to discuss a name change to e.g. "PinSetLevel"/"PinGetLevel" - but it is better to discuss it now, than after it is widely used. Don't get me wrong, I just try to show the confusion of a new driver developer - I personally can de-confuse and handle this by some research in the code. And, it seems nobody else had an objection about this - so please don't let my comment slow you down too much.

[gen2thomas]

if I understand this correctly, the related name for "pinhal_baremetal.go" is "pinhal_os.go" - so maybe we should name it accordingly to "easystepper_os.go"

[soypat]

Good point- will rename pinhal_os.go->pinhal_go.go, since there is no actual OS dependence here, what is more this is meant for other embedded go projects which likely don't use an OS.

[soypat]

This PR would be pretty key to a class I'll be giving in 1.5 weeks. Also need it for a couple drivers I have lying around I've yet to create a PR for :)

[gen2thomas]

Interface vs function pointer benchmark for CPU 32bit [email protected] (tinkerboard):

no additional interfaces (scaling_max_freq 1.8GHz):
 range funct call 2.573µs-6.246µs
 range iface call 2.563µs-7.124µs

no additional interfaces (scaling_max_freq 816MHz and doing some file access):
 range funct call 4.576µs-6.315µs
 range iface call 4.554µs-6.209µs

additional interfaces (scaling_max_freq 816MHz and doing some file access):
 range funct call 4.578µs-6.226µs
 range iface call 4.57µs-6.036µs

There is no remarkable difference beyond normal fluctuations of a default-scheduled CPU. I have repeated this tests also without the TinyGo-wrapper, directly with the gobot-Write/Read functions - nothing changes.

Program code

// Measure the difference between gpio-write/set calls when define the call as direct function or as a given interface
// in the driver. Additionally measure the impact of multiple defined interfaces in the driver and reduced clock speed.
package main

import (
	"fmt"
	"sort"
	"time"

	gobot "gobot.io/x/gobot/v2"
	"gobot.io/x/gobot/v2/platforms/asus/tinkerboard"
	"gobot.io/x/gobot/v2/system"
)

const pinID = "24"

func getConfiguredOuputPin(a gobot.DigitalPinnerProvider) gobot.DigitalPinner {
	pin, err := a.DigitalPin(pinID)
	if err != nil {
		panic(fmt.Errorf("error on get pin: %v", err))
	}
	if err := pin.ApplyOptions(system.WithPinDirectionOutput(0)); err != nil {
		panic(fmt.Errorf("error on apply output for pin: %v", err))
	}

	return pin
}

type tinyGoPin struct {
	// use functions here instead of "gobot.DigitalPinner" interface to reduce additional interface definition
	write func(int) error
	read  func() (int, error)
}

func (p *tinyGoPin) Set(b bool) {
	var v int
	if b {
		v = 1
	}
	if err := p.write(v); err != nil {
		panic(err)
	}
}

func (p *tinyGoPin) Get() bool {
	val, err := p.read()

	if err != nil {
		panic(err)
	}

	return val > 0
}

func (p *tinyGoPin) High() { p.Set(true) }
func (p *tinyGoPin) Low()  { p.Set(false) }

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{pin: impl1}
	d2.doIface(10)
	impl2 := &pinimpl[uint16]{}
	d3 := driver{pin: impl2}
	d3.doIface(10)

	a := tinkerboard.NewAdaptor()
	done := make(chan struct{})

	work := func() {
		p := getConfiguredOuputPin(a)
		tp := tinyGoPin{write: p.Write, read: p.Read}
		d := driver{pin: &tp, pinChangeLevel: tp.Set}

		const (
			N         = 10000
			n         = 60
			relaxTime = time.Second
		)

		var felapses [n]time.Duration
		var ielapses [n]time.Duration

		fmt.Printf("please wait %dx%s ...\n", n, relaxTime)
		for i := 0; i < n; i++ {
			fstart := time.Now()
			d.doFunc(N)
			felapses[i] = time.Since(fstart) / N

			istart := time.Now()
			d.doIface(N)
			ielapses[i] = time.Since(istart) / N

			time.Sleep(relaxTime)
		}

		a := felapses[:]
		sort.Slice(a, func(i, j int) bool {
			return a[i] < a[j]
		})

		b := ielapses[:]
		sort.Slice(b, func(i, j int) bool {
			return b[i] < b[j]
		})

		fmt.Printf("range funct call %s-%s\n", a[0], a[n-1])
		fmt.Printf("range iface call %s-%s\n", b[0], b[n-1])

		done <- struct{}{}
	}

	robot := gobot.NewRobot("benchmarkBot",
		[]gobot.Connection{a},
		[]gobot.Device{},
		work,
	)

	if err := robot.Start(false); err != nil {
		panic(err)
	}

	<-done
	if err := robot.Stop(); err != nil {
		panic(err)
	}
}

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

type pinChangeLeveler func(bool)

type driver struct {
	pin            pinner
	pinChangeLevel pinChangeLeveler
}

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

func (d *driver) doFunc(n int) {
	k := true
	for i := 0; i < n; i++ {
		d.pinChangeLevel(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) }

[gen2thomas]

Interface vs function pointer benchmark for nRF52840 64 MHz, 212 EEMBC CoreMark score running from flash memory

no additional interfaces:
	please wait 60x1s ...
	0 : funct call 24ns iface call 27ns
	1 : funct call 21ns iface call 21ns
	2 : funct call 21ns iface call 21ns
	...
	58 : funct call 21ns iface call 21ns
	59 : funct call 21ns iface call 21ns
	range funct call 59 x 21ns - 1 x 24ns
	range iface call 59 x 21ns - 1 x 27ns

with setting list=n+1, function call varies more, maybe caused by "println", iface not affected in all tests:
	range funct call 42 x 21ns - 18 x 24ns
	range iface call 59 x 21ns - 1 x 24ns


additional interfaces:
	please wait 60x1s ...
	0 : funct call 24ns iface call 558ns
	1 : funct call 21ns iface call 558ns
	2 : funct call 21ns iface call 558ns
	...
	58 : funct call 21ns iface call 524ns
	59 : funct call 21ns iface call 558ns
	range funct call 56 x 21ns - 4 x 24ns
	range iface call 54 x 524ns - 5 x 558ns

with setting list=n+1:
	range funct call 38 x 21ns - 22 x 24ns
	range iface call 1 x 531ns - 55 x 558ns

We see the same behavior like already found by @soypat, see #749 (comment)

With nRF52840 the factor is ~25 with my example code, when using multiple interfaces.

Program code

// Measure the difference between gpio-set calls when define the call as direct function or as a given interface in the
// driver. Additionally measure the impact of multiple defined interfaces in the driver.
package main

import (
	"fmt"
	"machine"
	"sort"
	"time"
)

const (
	N         = 10000
	n         = 60
	list      = 3 // use "n+1" to list all
	relaxTime = time.Second
)

var (
	// otherwise: object size 480 exceeds maximum stack allocation size 256
	felapses = [n]time.Duration{}
	ielapses = [n]time.Duration{}
)

func main() {
	time.Sleep(5 * time.Second) // wait so monitoring can start

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

	p := machine.LED_BLUE
	p.Configure(machine.PinConfig{Mode: machine.PinOutput})
	d := driver{pin: p, pinChangeLevel: p.Set}

	fmt.Printf("please wait %dx%s ...\n", n, relaxTime)
	for i := 0; i < n; i++ {
		fstart := time.Now()
		d.doFunc(N)
		felapses[i] = time.Since(fstart) / N

		istart := time.Now()
		d.doIface(N)
		ielapses[i] = time.Since(istart) / N

		if i < list || i > n-list {
			println(i, ": funct call", felapses[i].String(), "iface call", ielapses[i].String())
		}

		if i == list {
			println("...")
		}

		time.Sleep(relaxTime)
	}

	a := felapses[:]
	sort.Slice(a, func(i, j int) bool {
		return a[i] < a[j]
	})

	b := ielapses[:]
	sort.Slice(b, func(i, j int) bool {
		return b[i] < b[j]
	})

	fmin, fcmin := count(a, 0)
	fmax, fcmax := count(a, n-1)
	fmt.Printf("range funct call %d x %s - %d x %s\n", fcmin, fmin, fcmax, fmax)

	imin, icmin := count(b, 0)
	imax, icmax := count(b, n-1)
	fmt.Printf("range iface call %d x %s - %d x %s\n", icmin, imin, icmax, imax)
}

func count(s []time.Duration, idx int) (time.Duration, int) {
	valOfIdx := s[idx]
	var count int
	for _, v := range s {
		if v == valOfIdx {
			count++
		}
	}

	return valOfIdx, count
}

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

type pinChangeLeveler func(bool)

type driver struct {
	pin            pinner
	pinChangeLevel pinChangeLeveler
}

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

func (d *driver) doFunc(n int) {
	k := true
	for i := 0; i < n; i++ {
		d.pinChangeLevel(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) }

[soypat]

@gen2thomas Thanks for taking the time and adding more benchmark results! Will try to get this merged by wednesday for the class on TinyGo I'm giving at university :)

[gen2thomas]

I can follow the comment of @ysoldak regarding the interface is internal. But, how do we deal with my other comments? @soypat do you agree with my comments regarding the examples and for the names in structs? So please let a comment there.

Maybe I can take over the work to apply it (in the next weeks).

[soypat]

On the matter of naming the HAL I feel like the Get/Set sounds like a personal preference. This is why I suggested adapting the code you write to be more legible to you. When one looks at the pinout of a microcontroller one sees I2C, SPI, ADC, PIO, GPIO. These are the most common names to use as a reference to the interface the user will use. These are also the names we've chosen for the HAL. I do not think this is a coincidence, the user will refer to the pinout to choose the HAL to use. GPIO = General Purpose Input Output, so I'd find it to be quite natural to follow suit and name the pin HAL accordingly. There are other languages that chose their own names for their HAL but it seems confusing to me to choose what "sounds best" to one person, which might not sound right to another. By sticking to what the pinout is called we can ensure a consistent naming scheme across all HAL implementations.

As for the other comments I've just noticed them now, they must have slipped past me. Will answer them now

[deadprogram]

Can we please have a voice chat to discuss some thoughts I have about this that are a bit hard for me to write down?

[gen2thomas]

On the matter of naming the HAL I feel like the Get/Set sounds like a personal preference.

No it is not. If it would be a personal preference, my suggestions e.g. for I2C would be the same, but no - this name is absolutely fine.

There is a difference between I2C HAL and the new GPIO-Pin-HAL. I2C is really a pin (object) which has some functionality to call. We make the GPIO-Pin-HAL a function or two functions (for a very important reason) and those are no objects anymore and also do not represent the GPIO-pin itself in the code but "access options" to it. I'm afraid my English is not good enough to put it in the right words.

I think it is acceptable and good to name the GPIO-HAL-API in the same way like the others to avoid confusion at this point, but one should dissolve it at the earliest possible point - my preferred solution is at "NewDriver()".

@soypat what about my suggestion for changing all examples as soon as possible? I think it does not matter that pins will be configured twice than for some drivers, especially for the examples. That's what my offer stood for ("Maybe I can take over the work to apply it (in the next weeks).").

[soypat]

@deadprogram (Ron) has revealed a divine insight into making the pins more object-like and familiar to us as driver developers. Also given me suggestions on a new pin package to not expose "legacy" word where outward facing users might see and feel worried. Will create a new commit soon.

[soypat]

@ysoldak and @gen2thomas PSA! I feel like @deadprogram found a really cool way to make PinOutput look like the interfaces we know and love in TinyGo by adding the High and Low methods to the user type. The internal driver code is now identical to how it looked before! I feel like this very likely addresses the worry @gen2thomas had about PinOutput not being like an interface. Let me know what y'all think!

I've also removed the legacy package name on user facing API since users may frown on the sight of it. I've left the ConfigurePin* API behind the legacy package since we are sure we never want to configure pins ever again on the driver side (bad pattern, as I'm sure we can all agree upon).

what about my suggestion for changing all examples as soon as possible

Examples should work fine! There has been no outward user breaking changes so they should work exactly as they did before!

[deadprogram]

Suggested change

	cs.Set(true)
	rst.Set(true)
	wr.Set(true)
	cs.High()
	rst.High()
	wr.High()

[gen2thomas]

Maybe I have overlooked something, but why not add a Set(bool) function in addition to High() and Low() in our new "pin.go"?

If this is working, all changes in the drivers can be reverted, except the used types in New() and structs.

[soypat]

To avoid recursive definition. Set is identical to the type definition and can be easy to misuse. Lets start out simple :)

High and Low already expose the needed clarity and funcionality.

[gen2thomas]

and of course:

func (pi PinInput) Get() bool {
	return pi()
}

[gen2thomas]

Yes, it is working - tested with my hx711 driver.

@deadprogram are there other functions on a machine.Pin, which should be added, e.g. Toggle()?

[gen2thomas]

@deadprogram just for completeness: Those High() and Low() are based on Set().

Get() and Set() are defined in many machine files, but there is a generic one https://github.com/tinygo-org/tinygo/blob/3869f76887feef6c444308e7e1531b7cac1bbd10/src/machine/machine_generic.go#L36C1-L42C2

Toggle() is only available for 2 at the moment (machine_atsamd51.go, machine_mimxrt1062.go)

[deadprogram]

Toggle() is only available for 2 at the moment (machine_atsamd51.go, machine_mimxrt1062.go)

I personally would rather they not be there, since they are not part of the interface that is on all the other platforms.

[soypat]

Yes, lets avoid Toggle. If we want a toggleable API it is easy enough to define with current API:

type PinToggler struct {
    pin drivers.PinOutput
    current bool
}

func (pt *PinToggler) Toggle() {
    now := !pt.current
    pt.current = now
    pt.pin(now)
}
// and add any other method deemed desirable.

[soypat]

Also, there are several reasons I'd avoid

func (pi PinInput) Get() bool {
	return pi()
}

• We don't do Getters in Go.
• I don't see the readability added in this case, unlike the High and Low methods.
  • d.cs(true)->d.cs.High() is arguably an improvement
  • isBusy := d.isBusy() vs. isBusy := d.isBusy.Get() adds virtually no readability cues. I'd even go as far as to say it is less readable since it adds a needless word
• Is a recursive type definition and brings with it the following potential misuse: var pi drivers.PinInput = pinInputFunc.Get where if not very careful with scaling API design we can end up wrapping our PinInputs within PinInputs and get some very nasty spaghetti code

[deadprogram]

We don't do Getters in Go.

This is to match the existing interface in the machine package. So indeed, we already do Get() 😺

I don't see the readability added in this case, unlike the High and Low methods.

Matching the existing machine package to reduce confusion is good, IMO.

Is a recursive type definition and brings with it the following potential misuse: var pi drivers.PinInput = pinInputFunc.Get where if not very careful with scaling API design we can end up wrapping our PinInputs within PinInputs and get some very nasty spaghetti code

I think a couple more changes can be made to improved this. I will make a different comment to explain what I mean.

[deadprogram]

Thanks for making my proposed changes and for talking thru everything @soypat

@ysoldak and @gen2thomas thanks for all of the feedback to try to help clarify the driver developer's perspective.

Being able to use the same APIs for GPIO inside of the driver itself is certainly important for anyone who is writing a new driver, or helping maintain existing drivers, and the latest version of the PR preserves that.

Also important to note that the API for the consumers of drivers has not changed. Existing code that utilizes drivers does not change. Hence no changes needed to any of the examples.

I think the most current version of this PR achieves what we all want. Great work all around!

[deadprogram]

I have been thinking about this PR, and have an additional commit that I think should clarify it further.

See d224b5d for what I mean.

cc/ @soypat @ysoldak @gen2thomas and anyone else interested in this subject.

[gen2thomas]

https://go.dev/doc/effective_go#Getters

This does not mean there are no getters in Go, just do not prefix the name with "Get". In this example:
If an object has a property owner, do not call the getter "GetOwner" but just "Owner". To apply this rule to our case, we have to identify the underlying object and the property, which we want to get.

IMO the object is the (input-) pin and the property is the "state", the "value", the "level" or a function to ask for the state. With this general rule, the idiomatic name and usage would be something like state := pin.Level() or stateIsActive := pin.IsHigh().

The counterpart for outputs would be in this idiomatic way:
pin.SetLevel(bool), pin.SetToHigh(); pin.SetToLow()
pin.SetValue(bool), pin.SetToTrue(), pin.SetToFalse()
pin.SetState(bool), pin.SetToActive(), pin.SetToInactive()

This is just to provide the Go best practice as a vision. Of course, we are restricted by the existing names and the requirement to keep compatible and consistent.

[soypat]

I'll get to this some time next week. Currently on a trip!

[soypat]

OK. I am back and ready to drive this PR again.

A little note of interest, I did up ending giving the HAL class showing the students the concept of an interface. I was very surprised with how difficult the concept of an interface was to grasp for these students. They had Python and C experience. Although one of them recognized that the interface was similar to a C header there was a struggle to understand how to use them. I had avoided showing them functions as HAL since I was under the impression it would be harder to explain but I am now quite convinced that interfaces are not only more complex as a concept, but also harder to teach, very likely due to the conceptual complexity.

This is another item in my long list of things against the idea of an interface as HAL- a function HAL is functionally the same, is more performant, conceptually simpler, leads to more readable driver code- as we've discussed over the course of the last few months.

Furthermore I've observed activity in the TinyGo slack with lots of questions surrounding how to implement certain Pin HALs with the function HAL API. I feel it is noteworthy how straightforward it has been to communicate different ways of implementing pin HALs such as Pin Toggles and a onewire driver bit bang pin HAL. Doing the same with interface HAL implementations requires more effort due to the additional boilerplate code and since you cant define an interface inline with the code in a function.

Worth noting choosing the function HAL prevents more method signatures from being added, which seems to be something people tend to want from what I've observed. We really don't want to add method signatures to the Pin HAL since they really achieve nothing. If Toggle is required by a driver developer then they should implement a Toggler driver that recieves a PinOutput. Set is equivalent to having both High and Low methods. We don't want a interface that has both Input and Output functionality- we've already seen how we can implement that with a few lines using the function HAL in slack.

So I'll note that I feel like choosing the TinyGo pin function HAL is a service to tinygo users of the future. I'm pretty sure going the interface route is a disservice to users of the future and also a disservice to users of today who may think otherwise currently; but who would be surprised how far you can get with a little function HAL.

If there are still people opposed to this PR I suggest we book a time to meet in a video call. We're rolling back on topics we've discussed at length and agreed upon only to revert opinion. This is preventing work of mine on other important projects in the Go ecosystem, such as advancing the Go userspace networking stack- although I find this to be much more important in securing TinyGo's future as a robust tool to be used to solve embedded system industry problems.