From 966027164235d0b87fe8e79306ce7560657094ce Mon Sep 17 00:00:00 2001
From: Marc-Antoine Ruel <maruel@chromium.org>
Date: Tue, 22 May 2018 08:50:00 -0400
Subject: [PATCH] lepton: add NextFrame, explicit Halt, deprecation warnings

Both New() cs argument and ReadImg() will be removed, add warnings.

Done as part of issue #217 and #218.
---
 devices/lepton/lepton.go      | 29 +++++++++++++++++---
 devices/lepton/lepton_test.go | 51 ++++++++++++++++++++++++++++++-----
 2 files changed, 69 insertions(+), 11 deletions(-)

diff --git a/devices/lepton/lepton.go b/devices/lepton/lepton.go
index 980106c..d51ffdd 100644
--- a/devices/lepton/lepton.go
+++ b/devices/lepton/lepton.go
@@ -77,6 +77,8 @@ type Frame struct {
 // Maximum I²C speed is 1Mhz.
 //
 // MOSI is not used and should be grounded.
+//
+// Deprecated: Argument cs will be removed in v3.
 func New(p spi.Port, i i2c.Bus, cs gpio.PinOut) (*Dev, error) {
 	// Sadly the Lepton will unconditionally send 27fps, even if the effective
 	// rate is 9fps.
@@ -158,20 +160,28 @@ func (d *Dev) String() string {
 	return fmt.Sprintf("Lepton(%s/%s/%s)", d.Dev, d.s, d.cs)
 }
 
+// Halt implements conn.Resource.
+func (d *Dev) Halt() error {
+	// TODO(maruel): Stop the read loop.
+	return d.Dev.Halt()
+}
+
 // Bounds returns the device frame size.
 func (d *Dev) Bounds() image.Rectangle {
 	return image.Rect(0, 0, d.w, d.h)
 }
 
-// ReadImg reads an image.
+// NextFrame blocks and returns the next frame from the camera.
 //
 // It is ok to call other functions concurrently to send commands to the
 // camera.
-func (d *Dev) ReadImg() (*Frame, error) {
-	f := &Frame{Gray16: image.NewGray16(d.prevImg.Bounds())}
+func (d *Dev) NextFrame(f *Frame) error {
+	if f.Bounds() != d.Bounds() {
+		return errors.New("lepton: invalid frame size")
+	}
 	for {
 		if err := d.readFrame(f); err != nil {
-			return nil, err
+			return err
 		}
 		if f.Metadata.FFCDesired {
 			// TODO(maruel): Automatically trigger FFC when applicable, only do if
@@ -184,6 +194,17 @@ func (d *Dev) ReadImg() (*Frame, error) {
 		// It also happen if the image is 100% static without noise.
 	}
 	copy(d.prevImg.Pix, f.Pix)
+	return nil
+}
+
+// ReadImg reads an image.
+//
+// Deprecated: Use NextFrame() instead.
+func (d *Dev) ReadImg() (*Frame, error) {
+	f := &Frame{Gray16: image.NewGray16(d.prevImg.Bounds())}
+	if err := d.NextFrame(f); err != nil {
+		return nil, err
+	}
 	return f, nil
 }
 
diff --git a/devices/lepton/lepton_test.go b/devices/lepton/lepton_test.go
index 5f02f7d..423346e 100644
--- a/devices/lepton/lepton_test.go
+++ b/devices/lepton/lepton_test.go
@@ -181,15 +181,15 @@ func TestBounds(t *testing.T) {
 	}
 }
 
-func TestReadImg(t *testing.T) {
+func TestNextFrame(t *testing.T) {
 	i := i2ctest.Playback{Ops: initSequence()}
 	s := spiStream{data: prepareFrame(t)}
 	d, err := New(&s, &i, &gpiotest.Pin{N: "CS"})
 	if err != nil {
 		t.Fatal(err)
 	}
-	f, err := d.ReadImg()
-	if err != nil {
+	f := Frame{Gray16: image.NewGray16(d.Bounds())}
+	if err := d.NextFrame(&f); err != nil {
 		t.Fatal(err)
 	}
 	if f.Metadata.TempHousing != devices.Celsius(2000) {
@@ -212,14 +212,29 @@ func TestReadImg(t *testing.T) {
 	}
 }
 
-func TestReadImg_fail_Tx(t *testing.T) {
+func TestNextFrame_invalid_bounds(t *testing.T) {
+	i := i2ctest.Playback{Ops: initSequence()}
+	s := spiStream{data: prepareFrame(t)}
+	d, err := New(&s, &i, &gpiotest.Pin{N: "CS"})
+	if err != nil {
+		t.Fatal(err)
+	}
+	if err := d.NextFrame(&Frame{Gray16: image.NewGray16(image.Rect(0, 0, 1, 1))}); err == nil {
+		t.Fatal("invalid bounds")
+	}
+	if err := i.Close(); err != nil {
+		t.Fatal(err)
+	}
+}
+
+func TestNextFrame_fail_Tx(t *testing.T) {
 	i := i2ctest.Playback{Ops: initSequence()}
 	s := spitest.Playback{Playback: conntest.Playback{DontPanic: true}}
 	d, err := New(&s, &i, &gpiotest.Pin{N: "CS"})
 	if err != nil {
 		t.Fatal(err)
 	}
-	if _, err := d.ReadImg(); err == nil {
+	if err := d.NextFrame(&Frame{Gray16: image.NewGray16(d.Bounds())}); err == nil {
 		t.Fatal("spi port Tx failed")
 	}
 	if err := i.Close(); err != nil {
@@ -227,14 +242,14 @@ func TestReadImg_fail_Tx(t *testing.T) {
 	}
 }
 
-func TestReadImg_fail_OUt(t *testing.T) {
+func TestNextFrame_fail_Out(t *testing.T) {
 	i := i2ctest.Playback{Ops: initSequence()}
 	s := spitest.Playback{Playback: conntest.Playback{DontPanic: true}}
 	d, err := New(&s, &i, &failPin{})
 	if err != nil {
 		t.Fatal(err)
 	}
-	if _, err := d.ReadImg(); err == nil {
+	if err := d.NextFrame(&Frame{Gray16: image.NewGray16(d.Bounds())}); err == nil {
 		t.Fatal("spi port Tx failed")
 	}
 	if err := i.Close(); err != nil {
@@ -242,6 +257,28 @@ func TestReadImg_fail_OUt(t *testing.T) {
 	}
 }
 
+func TestReadImg(t *testing.T) {
+	i := i2ctest.Playback{Ops: initSequence()}
+	s := spiStream{data: prepareFrame(t)}
+	d, err := New(&s, &i, &gpiotest.Pin{N: "CS"})
+	if err != nil {
+		t.Fatal(err)
+	}
+	f, err := d.ReadImg()
+	if err != nil {
+		t.Fatal(err)
+	}
+	if f.Metadata.TempHousing != devices.Celsius(2000) {
+		t.Fatal(f.Metadata.TempHousing)
+	}
+	if err := i.Close(); err != nil {
+		t.Fatal(err)
+	}
+	if _, err = d.ReadImg(); err == nil {
+		t.Fatal(err)
+	}
+}
+
 func TestParseTelemetry_fail(t *testing.T) {
 	l := telemetryLine(t)
 	m := Metadata{}