apply suggestions from team

Author: soypat

apa102/apa102.go

@@ -5,9 +5,9 @@ package apa102 // import "tinygo.org/x/drivers/apa102"
 
 import (
 	"image/color"
-	"machine"
 
 	"tinygo.org/x/drivers"
+	"tinygo.org/x/drivers/internal/legacy"
 )
 
 const (
@@ -37,8 +37,11 @@ func New(b drivers.SPI) *Device {
 
 // NewSoftwareSPI returns a new APA102 driver that will use a software based
 // implementation of the SPI protocol.
-func NewSoftwareSPI(sckPin, sdoPin machine.Pin, delay uint32) *Device {
-	return New(&bbSPI{SCK: sckPin, SDO: sdoPin, Delay: delay})
+func NewSoftwareSPI(sckPin, sdoPin legacy.PinOutput, delay uint32) *Device {
+	return New(&bbSPI{SCK: sckPin.Set, SDO: sdoPin.Set, Delay: delay, config: func() {
+		legacy.ConfigurePinOut(sckPin)
+		legacy.ConfigurePinOut(sdoPin)
+	}})
 }
 
 // WriteColors writes the given RGBA color slice out using the APA102 protocol.

apa102/softspi.go

@@ -1,24 +1,26 @@
 package apa102
 
-import "machine"
+import (
+	"tinygo.org/x/drivers"
+)
 
 // bbSPI is a dumb bit-bang implementation of SPI protocol that is hardcoded
 // to mode 0 and ignores trying to receive data. Just enough for the APA102.
 // Note: making this unexported for now because it is probable not suitable
 // most purposes other than the APA102 package. It might be desirable to make
 // this more generic and include it in the TinyGo "machine" package instead.
 type bbSPI struct {
-	SCK   machine.Pin
-	SDO   machine.Pin
-	Delay uint32
+	SCK    drivers.PinOutput
+	SDO    drivers.PinOutput
+	Delay  uint32
+	config func()
 }
 
 // Configure sets up the SCK and SDO pins as outputs and sets them low
 func (s *bbSPI) Configure() {
-	s.SCK.Configure(machine.PinConfig{Mode: machine.PinOutput})
-	s.SDO.Configure(machine.PinConfig{Mode: machine.PinOutput})
-	s.SCK.Low()
-	s.SDO.Low()
+	s.config()
+	s.SCK(false)
+	s.SDO(false)
 	if s.Delay == 0 {
 		s.Delay = 1
 	}
@@ -47,19 +49,19 @@ func (s *bbSPI) Transfer(b byte) (byte, error) {
 	for i := uint8(0); i < 8; i++ {
 
 		// half clock cycle high to start
-		s.SCK.High()
+		s.SCK(true)
 		s.delay()
 
 		// write the value to SDO (MSB first)
 		if b&(1<<(7-i)) == 0 {
-			s.SDO.Low()
+			s.SDO(false)
 		} else {
-			s.SDO.High()
+			s.SDO(true)
 		}
 		s.delay()
 
 		// half clock cycle low
-		s.SCK.Low()
+		s.SCK(false)
 		s.delay()
 
 		// for actual SPI would try to read the SDI value here

bmi160/bmi160.go

@@ -1,40 +1,43 @@
 package bmi160
 
 import (
-	"machine"
 	"time"
 
 	"tinygo.org/x/drivers"
+	"tinygo.org/x/drivers/internal/legacy"
 )
 
 // DeviceSPI is the SPI interface to a BMI160 accelerometer/gyroscope. There is
 // also an I2C interface, but it is not yet supported.
 type DeviceSPI struct {
 	// Chip select pin
-	CSB machine.Pin
+	CSB drivers.PinOutput
 
 	buf [7]byte
 
 	// SPI bus (requires chip select to be usable).
-	Bus drivers.SPI
+	Bus    drivers.SPI
+	config func()
 }
 
 // NewSPI returns a new device driver. The pin and SPI interface are not
 // touched, provide a fully configured SPI object and call Configure to start
 // using this device.
-func NewSPI(csb machine.Pin, spi drivers.SPI) *DeviceSPI {
+func NewSPI(csb legacy.PinOutput, spi drivers.SPI) *DeviceSPI {
 	return &DeviceSPI{
-		CSB: csb, // chip select
+		CSB: csb.Set, // chip select
 		Bus: spi,
+		config: func() {
+			legacy.ConfigurePinOut(csb)
+		},
 	}
 }
 
 // Configure configures the BMI160 for use. It configures the CSB pin and
 // configures the BMI160, but it does not configure the SPI interface (it is
 // assumed to be up and running).
 func (d *DeviceSPI) Configure() error {
-	d.CSB.Configure(machine.PinConfig{Mode: machine.PinOutput})
-	d.CSB.High()
+	d.CSB(true)
 
 	// The datasheet recommends doing a register read from address 0x7F to get
 	// SPI communication going:
@@ -86,9 +89,9 @@ func (d *DeviceSPI) ReadTemperature() (temperature int32, err error) {
 	data[0] = 0x80 | reg_TEMPERATURE_0
 	data[1] = 0
 	data[2] = 0
-	d.CSB.Low()
+	d.CSB(false)
 	err = d.Bus.Tx(data, data)
-	d.CSB.High()
+	d.CSB(true)
 	if err != nil {
 		return
 	}
@@ -123,9 +126,9 @@ func (d *DeviceSPI) ReadAcceleration() (x int32, y int32, z int32, err error) {
 	for i := 1; i < len(data); i++ {
 		data[i] = 0
 	}
-	d.CSB.Low()
+	d.CSB(false)
 	err = d.Bus.Tx(data, data)
-	d.CSB.High()
+	d.CSB(true)
 	if err != nil {
 		return
 	}
@@ -153,9 +156,9 @@ func (d *DeviceSPI) ReadRotation() (x int32, y int32, z int32, err error) {
 	for i := 1; i < len(data); i++ {
 		data[i] = 0
 	}
-	d.CSB.Low()
+	d.CSB(false)
 	err = d.Bus.Tx(data, data)
-	d.CSB.High()
+	d.CSB(true)
 	if err != nil {
 		return
 	}
@@ -201,9 +204,9 @@ func (d *DeviceSPI) readRegister(address uint8) uint8 {
 	data := d.buf[:2]
 	data[0] = 0x80 | address
 	data[1] = 0
-	d.CSB.Low()
+	d.CSB(false)
 	d.Bus.Tx(data, data)
-	d.CSB.High()
+	d.CSB(true)
 	return data[1]
 }
 
@@ -217,7 +220,7 @@ func (d *DeviceSPI) writeRegister(address, data uint8) {
 	buf[0] = address
 	buf[1] = data
 
-	d.CSB.Low()
+	d.CSB(false)
 	d.Bus.Tx(buf, buf)
-	d.CSB.High()
+	d.CSB(true)
 }

buzzer/buzzer.go

@@ -2,37 +2,38 @@
 package buzzer // import "tinygo.org/x/drivers/buzzer"
 
 import (
-	"machine"
-
 	"time"
+
+	"tinygo.org/x/drivers"
+	"tinygo.org/x/drivers/internal/legacy"
 )
 
 // Device wraps a GPIO connection to a buzzer.
 type Device struct {
-	pin  machine.Pin
+	pin  drivers.PinOutput
 	High bool
 	BPM  float64
 }
 
 // New returns a new buzzer driver given which pin to use
-func New(pin machine.Pin) Device {
+func New(pin legacy.PinOutput) Device {
 	return Device{
-		pin:  pin,
+		pin:  pin.Set,
 		High: false,
 		BPM:  96.0,
 	}
 }
 
 // On sets the buzzer to a high state.
 func (l *Device) On() (err error) {
-	l.pin.Set(true)
+	l.pin(true)
 	l.High = true
 	return
 }
 
 // Off sets the buzzer to a low state.
 func (l *Device) Off() (err error) {
-	l.pin.Set(false)
+	l.pin(false)
 	l.High = false
 	return
 }

internal/legacy/pinhal.go

@@ -0,0 +1,29 @@
+package legacy
+
+// PinOutput represents a pin hardware abstraction layer for a pin that can output a digital signal.
+// This is an alternative to drivers.PinOutput abstraction which is a function type. Pros and cons
+// of both approaches have been discussed in the [relevant issue]. PinOutput should only be used
+// to expose an initialization function of a driver that receives pins of this type. Ideally
+// driver developers should also expose the initialization with drivers.Pin type:
+//
+//	func New(p1, p2, p3 legacy.PinOutput) *Device {
+//		return NewWithPinfuncs(p1.Set, p2.Set, p3.Set)
+//	}
+//
+//	func NewWithPinfuncs(p1, p2, p3 drivers.PinOutput) *Device {
+//		return &Device{p1:p1, p2:p2, p3:p3}
+//	}
+//
+// [relevant issue]: https://github.com/tinygo-org/drivers/pull/749/files
+type PinOutput interface {
+	Set(level bool)
+}
+
+// ConfigurePinOut is a legacy function used to configure pins as outputs.
+//
+// Deprecated: Do not configure pins in drivers.
+// This is a legacy feature and should only be used by drivers that
+// previously configured pins in initialization to avoid breaking users.
+func ConfigurePinOut(p PinOutput) {
+	configurePinOut(p)
+}

internal/legacy/pinhal_baremetal.go

@@ -0,0 +1,12 @@
+//go:build baremetal
+
+package legacy
+
+import "machine"
+
+func configurePinOut(p PinOutput) {
+	machinePin, ok := p.(machine.Pin)
+	if ok {
+		machinePin.Configure(machine.PinConfig{Mode: machine.PinOutput})
+	}
+}

internal/legacy/pinhal_os.go

@@ -0,0 +1,5 @@
+//go:build !baremetal
+
+package legacy
+
+func configurePinOut(p PinOutput) {}

pins.go

@@ -1,9 +1,9 @@
 package drivers
 
-// 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)
+
+// PinInput is hardware abstraction for a pin which receives a
+// digital signal and reads it (high or low voltage).
+type PinInput func() (level bool)