inky: Fix Impression Driver (#106)

*Changed the SPI CS pin to work under automatic or manual control. This allows it to work when CS is controlled by SPI, or when it is controlled manually. We don't want it to fail in the configuration the Pimoroni python driver requires.
*Changed the wait() routine to use a 10ms debounce on WaitForEdge() as the python driver does.
*If the BUSY line is high on entry to wait to go directly to a timed wait as the python driver does.
*Fixed errors in the palette definitions. Removed extra memory allocation.

Author: gsexton

go.mod

@@ -12,7 +12,7 @@ require (
 	github.com/mattn/go-colorable v0.1.13
 	golang.org/x/image v0.23.0
 	periph.io/x/conn/v3 v3.7.2
-	periph.io/x/host/v3 v3.8.4
+	periph.io/x/host/v3 v3.8.5
 )
 
 require (

go.sum

@@ -17,5 +17,5 @@ golang.org/x/sys v0.29.0 h1:TPYlXGxvx1MGTn2GiZDhnjPA9wZzZeGKHHmKhHYvgaU=
 golang.org/x/sys v0.29.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA=
 periph.io/x/conn/v3 v3.7.2 h1:qt9dE6XGP5ljbFnCKRJ9OOCoiOyBGlw7JZgoi72zZ1s=
 periph.io/x/conn/v3 v3.7.2/go.mod h1:Ao0b4sFRo4QOx6c1tROJU1fLJN1hUIYggjOrkIVnpGg=
-periph.io/x/host/v3 v3.8.4 h1:QNleTythDd0k6Chu0n+ISrJFlf3LFig9oNbtOIkxoCc=
-periph.io/x/host/v3 v3.8.4/go.mod h1:hPq8dISZIc+UNfWoRj+bPH3XEBQqJPdFdx218W92mdc=
+periph.io/x/host/v3 v3.8.5 h1:g4g5xE1XZtDiGl1UAJaUur1aT7uNiFLMkyMEiZ7IHII=
+periph.io/x/host/v3 v3.8.5/go.mod h1:hPq8dISZIc+UNfWoRj+bPH3XEBQqJPdFdx218W92mdc=

inky/impression.go

@@ -17,6 +17,7 @@ import (
 	"periph.io/x/conn/v3"
 	"periph.io/x/conn/v3/display"
 	"periph.io/x/conn/v3/gpio"
+	"periph.io/x/conn/v3/gpio/gpioreg"
 	"periph.io/x/conn/v3/physic"
 	"periph.io/x/conn/v3/spi"
 )
@@ -28,36 +29,36 @@ var _ draw.Image = &DevImpression{}
 var (
 	// For more: https://github.com/pimoroni/inky/issues/115#issuecomment-887453065
 	dsc = []color.NRGBA{
-		{0, 0, 0, 0},         // Black
+		{0, 0, 0, 255},       // Black
 		{255, 255, 255, 255}, // White
 		{0, 255, 0, 255},     // Green
 		{0, 0, 255, 255},     // Blue
 		{255, 0, 0, 255},     // Red
 		{255, 255, 0, 255},   // Yellow
 		{255, 140, 0, 255},   // Orange
-		{255, 255, 255, 255},
+		{255, 255, 255, 0},   // Clear
 	}
 
 	sc = []color.NRGBA{
-		{57, 48, 57, 0},      // Black
+		{57, 48, 57, 255},    // Black
 		{255, 255, 255, 255}, // White
 		{58, 91, 70, 255},    // Green
 		{61, 59, 94, 255},    // Blue
 		{156, 72, 75, 255},   // Red
 		{208, 190, 71, 255},  // Yellow
 		{177, 106, 73, 255},  // Orange
-		{255, 255, 255, 255},
+		{255, 255, 255, 0},   // Clear
 	}
 
 	sc7 = []color.NRGBA{
-		{0, 0, 0, 0},         // Black
+		{0, 0, 0, 255},       // Black
 		{217, 242, 255, 255}, // White
 		{3, 124, 76, 255},    // Green
 		{27, 46, 198, 255},   // Blue
 		{245, 80, 34, 255},   // Red
 		{255, 255, 68, 255},  // Yellow
 		{239, 121, 44, 255},  // Orange
-		{255, 255, 255, 255},
+		{255, 255, 255, 0},   // Clear
 	}
 )
 
@@ -155,7 +156,7 @@ func NewImpression(p spi.Port, dc gpio.PinOut, reset gpio.PinOut, busy gpio.PinI
 	if o.Model == IMPRESSION73 {
 		cSpeed = acSpeed
 	}
-	c, err := p.Connect(cSpeed, spi.Mode0, cs0Pin)
+	c, err := p.Connect(cSpeed, spi.Mode0, 8)
 	if err != nil {
 		return nil, fmt.Errorf("failed to connect to inky over spi: %v", err)
 	}
@@ -173,6 +174,11 @@ func NewImpression(p spi.Port, dc gpio.PinOut, reset gpio.PinOut, busy gpio.PinI
 			maxTxSize = 4096 // Use a conservative default.
 		}
 	}
+	// If possible, grab the CS pin.
+	cs := gpioreg.ByName(cs0Pin)
+	if cs != nil && cs.Out(csDisabled) != nil {
+		cs = nil
+	}
 
 	d := &DevImpression{
 		Dev: Dev{
@@ -186,6 +192,7 @@ func NewImpression(p spi.Port, dc gpio.PinOut, reset gpio.PinOut, busy gpio.PinI
 			model:      o.Model,
 			variant:    o.DisplayVariant,
 			pcbVariant: o.PCBVariant,
+			cs:         cs,
 		},
 		saturation: 50, // Looks good enough for most of the images.
 	}
@@ -217,10 +224,10 @@ func NewImpression(p spi.Port, dc gpio.PinOut, reset gpio.PinOut, busy gpio.PinI
 }
 
 // blend recalculates the palette based on the saturation level.
-func (d *DevImpression) blend() []color.Color {
-	sat := float64(d.saturation / 100)
+func (d *DevImpression) blend() color.Palette {
+	sat := float64(d.saturation) / 100.0
 
-	pr := []color.Color{}
+	pr := make([]color.Color, 0)
 	for i := 0; i < 7; i++ {
 		var rs, gs, bs uint8
 		if d.Dev.model == IMPRESSION73 {
@@ -288,7 +295,7 @@ func (d *DevImpression) Render() error {
 
 	merged := make([]uint8, len(d.Pix)/2)
 	for i, offset := 0, 0; i < len(d.Pix)-1; i, offset = i+2, offset+1 {
-		merged[offset] = (d.Pix[i]<<4)&0xF0 | d.Pix[i+1]&0x0F
+		merged[offset] = ((d.Pix[i] << 4) & 0xF0) | (d.Pix[i+1] & 0x0F)
 	}
 
 	return d.update(merged)
@@ -519,17 +526,16 @@ func (d *DevImpression) updateAC(pix []uint8) error {
 	}
 
 	// TODO there has to be a better way to force the white colour to be used instead of clear...
-	buf := make([]byte, len(pix))
 	for i := range pix {
 		if pix[i]&0xF == 7 {
-			buf[i] = (pix[i] & 0xF0) + 1
+			pix[i] = (pix[i] & 0xF0) + 1
 		}
 		if pix[i]&0xF0 == 0x70 {
-			buf[i] = (pix[i] & 0xF) + 0x10
+			pix[i] = (pix[i] & 0xF) + 0x10
 		}
 	}
 
-	if err := d.sendCommand(ac073TC1DTM, buf); err != nil {
+	if err := d.sendCommand(ac073TC1DTM, pix); err != nil {
 		return err
 	}
 
@@ -558,8 +564,28 @@ func (d *DevImpression) wait(dur time.Duration) {
 		log.Printf("Err: %s", err)
 		return
 	}
+	if d.busy.Read() == gpio.High {
+		time.Sleep(dur)
+		return
+	}
 	// Wait for rising edges (Low -> High) or the timeout.
-	d.busy.WaitForEdge(dur)
+	tEnd := time.Now().Add(dur)
+	edgeDur := dur
+	for tEnd.After(time.Now()) {
+		// Debounce the edge
+		edge := d.busy.WaitForEdge(edgeDur)
+		if edge {
+			// The python driver is using 10ms debounce period
+			time.Sleep(10 * time.Millisecond)
+			l := d.busy.Read()
+			if l {
+				// It's still high. Return
+				return
+			}
+			// It was a bounce. Recalculate the duration to wait for the edge.
+			edgeDur = time.Until(tEnd)
+		}
+	}
 }
 
 // ColorModel returns the device native color model.
@@ -589,7 +615,7 @@ func (d *DevImpression) Set(x, y int, c color.Color) {
 // Draw updates the display with the image.
 func (d *DevImpression) Draw(r image.Rectangle, src image.Image, sp image.Point) error {
 	if r != d.Bounds() {
-		return fmt.Errorf("partial updates are not supported")
+		return fmt.Errorf("partial updates are not supported r=%#v bounds=%#v", r, d.Bounds())
 	}
 
 	if src.Bounds() != d.Bounds() {
@@ -598,6 +624,7 @@ func (d *DevImpression) Draw(r image.Rectangle, src image.Image, sp image.Point)
 
 	// Dither the image using Floyd–Steinberg dithering algorithm otherwise it won't look as good on the screen.
 	draw.FloydSteinberg.Draw(d, r, src, image.Point{})
+
 	return d.Render()
 }
 

inky/inky.go

@@ -14,24 +14,27 @@ import (
 	"periph.io/x/conn/v3"
 	"periph.io/x/conn/v3/display"
 	"periph.io/x/conn/v3/gpio"
+	"periph.io/x/conn/v3/gpio/gpioreg"
 	"periph.io/x/conn/v3/physic"
 	"periph.io/x/conn/v3/spi"
 )
 
 var _ display.Drawer = &Dev{}
 var _ conn.Resource = &Dev{}
 
-const (
-	cs0Pin = 8
-)
-
 var borderColor = map[Color]byte{
 	Black:  0x00,
 	Red:    0x73,
 	Yellow: 0x33,
 	White:  0x31,
 }
 
+const (
+	cs0Pin     = "GPIO8"
+	csEnabled  = gpio.Low
+	csDisabled = gpio.High
+)
+
 // Dev is a handle to an Inky.
 type Dev struct {
 	c conn.Conn
@@ -65,6 +68,8 @@ type Dev struct {
 	variant uint
 	// PCB Variant of the panel. Represents a version string as a number (12 -> 1.2).
 	pcbVariant uint
+	// cs is the chip-select pin for SPI. Refer to setCSPin() for information.
+	cs gpio.PinOut
 }
 
 // New opens a handle to an Inky pHAT or wHAT.
@@ -73,7 +78,7 @@ func New(p spi.Port, dc gpio.PinOut, reset gpio.PinOut, busy gpio.PinIn, o *Opts
 		return nil, fmt.Errorf("unsupported color: %v", o.ModelColor)
 	}
 
-	c, err := p.Connect(488*physic.KiloHertz, spi.Mode0, cs0Pin)
+	c, err := p.Connect(488*physic.KiloHertz, spi.Mode0, 8)
 	if err != nil {
 		return nil, fmt.Errorf("failed to connect to inky over spi: %v", err)
 	}
@@ -87,7 +92,11 @@ func New(p spi.Port, dc gpio.PinOut, reset gpio.PinOut, busy gpio.PinIn, o *Opts
 	if maxTxSize == 0 {
 		maxTxSize = 4096 // Use a conservative default.
 	}
-
+	// If possible, grab the CS pin.
+	cs := gpioreg.ByName(cs0Pin)
+	if cs != nil && cs.Out(csDisabled) != nil {
+		cs = nil
+	}
 	d := &Dev{
 		c:          c,
 		maxTxSize:  maxTxSize,
@@ -99,6 +108,7 @@ func New(p spi.Port, dc gpio.PinOut, reset gpio.PinOut, busy gpio.PinIn, o *Opts
 		model:      o.Model,
 		variant:    o.DisplayVariant,
 		pcbVariant: o.PCBVariant,
+		cs:         cs,
 	}
 
 	switch o.Model {
@@ -330,43 +340,79 @@ func (d *Dev) reset() (err error) {
 		}
 	}()
 	if err := d.sendCommand(0x12, nil); err != nil { // Soft Reset
-		return fmt.Errorf("failed to reset inky: %v", err)
+		return fmt.Errorf("inky: failed to reset inky: %v", err)
 	}
 	d.busy.WaitForEdge(-1)
 	return
 }
 
-func (d *Dev) sendCommand(command byte, data []byte) error {
-	if err := d.dc.Out(gpio.Low); err != nil {
-		return err
+// setCSPin sets the ChipSelect pin to the desired mode. The Pimoroni driver
+// uses manual control over the CS pin. To do this, they require the
+// Raspberry Pi /boot/firmware/config.txt to have dtloverlay=spi0-0cs set.
+//
+// So, if we run with automatic CS handling, we won't be compatible with the
+// pimoroni samples. If we run with manual control required, we then require
+// the dtoverlay setting. We really don't want to be incompatible with the
+// Pimoroni driver because that will confuse people. If the CS Pin is
+// not in use, use manual control, and if it is used by the SPI driver, let
+// it handle it.
+func (d *Dev) setCSPin(mode gpio.Level) error {
+	if d.cs != nil {
+		return d.cs.Out(mode)
+	}
+	return nil
+}
+
+func (d *Dev) sendCommand(command byte, data []byte) (err error) {
+	err = d.setCSPin(csEnabled)
+	if err != nil {
+		return
+	}
+
+	if err = d.dc.Out(gpio.Low); err != nil {
+		return
 	}
-	if err := d.c.Tx([]byte{command}, nil); err != nil {
-		return fmt.Errorf("failed to send command %x to inky: %v", command, err)
+	if err = d.c.Tx([]byte{command}, nil); err != nil {
+		err = fmt.Errorf("inky: failed to send command %x to inky: %v", command, err)
+		return
 	}
+	err = d.setCSPin(csDisabled)
+	if err != nil {
+		return
+	}
+
 	if data != nil {
-		if err := d.sendData(data); err != nil {
-			return fmt.Errorf("failed to send data for command %x to inky: %v", command, err)
+		if err = d.sendData(data); err != nil {
+			err = fmt.Errorf("inky: failed to send data for command %x to inky: %v", command, err)
+			return
 		}
 	}
-	return nil
+	return
 }
 
-func (d *Dev) sendData(data []byte) error {
-	if err := d.dc.Out(gpio.High); err != nil {
+func (d *Dev) sendData(data []byte) (err error) {
+	err = d.setCSPin(csEnabled)
+	if err != nil {
+		return
+	}
+	if err = d.dc.Out(gpio.High); err != nil {
 		return err
 	}
+
 	for len(data) != 0 {
 		var chunk []byte
 		if len(data) > d.maxTxSize {
 			chunk, data = data[:d.maxTxSize], data[d.maxTxSize:]
 		} else {
 			chunk, data = data, nil
 		}
-		if err := d.c.Tx(chunk, nil); err != nil {
-			return fmt.Errorf("failed to send data to inky: %v", err)
+		if err = d.c.Tx(chunk, nil); err != nil {
+			err = fmt.Errorf("inky: failed to send data to inky: %v", err)
+			return
 		}
 	}
-	return nil
+	err = d.setCSPin(csDisabled)
+	return
 }
 
 func pack(bits []bool) ([]byte, error) {

inky/types.go

@@ -69,6 +69,8 @@ func (c *Color) Set(s string) error {
 		*c = Yellow
 	case "white":
 		*c = White
+	case "multi":
+		*c = Multi
 	default:
 		return fmt.Errorf("unknown color %q: expected either black, red, yellow or white", s)
 	}