Use Seek to speed up read/write in sysfs

This maintains `direction` and `value` `File`s for each DigitalPin
implementation. Instead of Open/Read/Close we now only do Seek/Read,
this speeds up Read/Write operations a bit.

A silly benchmark on the mock FS gives:

benchmark                  old ns/op     new ns/op     delta
BenchmarkDigitalRead-8     647           7.36          -98.86%

benchmark                  old allocs     new allocs     delta
BenchmarkDigitalRead-8     5              0              -100.00%

benchmark                  old bytes     new bytes     delta
BenchmarkDigitalRead-8     96            0             -100.00%

sysfs/digital_pin.go

@@ -1,6 +1,7 @@
 package sysfs
 
 import (
+	"errors"
 	"fmt"
 	"os"
 	"strconv"
@@ -37,6 +38,9 @@ type DigitalPin interface {
 type digitalPin struct {
 	pin   string
 	label string
+
+	value     File
+	direction File
 }
 
 // NewDigitalPin returns a DigitalPin given the pin number and an optional sysfs pin label.
@@ -53,62 +57,124 @@ func NewDigitalPin(pin int, v ...string) DigitalPin {
 	return d
 }
 
+var notExportedError = errors.New("pin has not been exported")
+
 func (d *digitalPin) Direction(dir string) error {
-	_, err := writeFile(fmt.Sprintf("%v/%v/direction", GPIOPATH, d.label), []byte(dir))
+	_, err := writeFile(d.direction, []byte(dir))
 	return err
 }
 
 func (d *digitalPin) Write(b int) error {
-	_, err := writeFile(fmt.Sprintf("%v/%v/value", GPIOPATH, d.label), []byte(strconv.Itoa(b)))
+	_, err := writeFile(d.value, []byte(strconv.Itoa(b)))
 	return err
 }
 
 func (d *digitalPin) Read() (n int, err error) {
-	buf, err := readFile(fmt.Sprintf("%v/%v/value", GPIOPATH, d.label))
+	buf, err := readFile(d.value)
 	if err != nil {
 		return 0, err
 	}
 	return strconv.Atoi(string(buf[0]))
 }
 
 func (d *digitalPin) Export() error {
-	if _, err := writeFile(GPIOPATH+"/export", []byte(d.pin)); err != nil {
+	export, err := fs.OpenFile(GPIOPATH+"/export", os.O_WRONLY, 0644)
+	if err != nil {
+		return err
+	}
+	defer export.Close()
+
+	_, err = writeFile(export, []byte(d.pin))
+	if err != nil {
 		// If EBUSY then the pin has already been exported
 		if err.(*os.PathError).Err != syscall.EBUSY {
 			return err
 		}
 	}
-	return nil
+
+	if d.direction != nil {
+		d.direction.Close()
+	}
+
+	d.direction, err = fs.OpenFile(fmt.Sprintf("%v/%v/direction", GPIOPATH, d.label), os.O_RDWR, 0644)
+
+	if d.value != nil {
+		d.value.Close()
+	}
+	if err == nil {
+		d.value, err = fs.OpenFile(fmt.Sprintf("%v/%v/value", GPIOPATH, d.label), os.O_RDWR, 0644)
+	}
+
+	if err != nil {
+		// Should we unexport here?
+		// If we don't unexport we should make sure to close d.direction and d.value here
+		d.Unexport()
+	}
+
+	return err
 }
 
 func (d *digitalPin) Unexport() error {
-	if _, err := writeFile(GPIOPATH+"/unexport", []byte(d.pin)); err != nil {
+	unexport, err := fs.OpenFile(GPIOPATH+"/unexport", os.O_WRONLY, 0644)
+	if err != nil {
+		return err
+	}
+	defer unexport.Close()
+
+	if d.direction != nil {
+		d.direction.Close()
+		d.direction = nil
+	}
+	if d.value != nil {
+		d.value.Close()
+		d.value = nil
+	}
+
+	_, err = writeFile(unexport, []byte(d.pin))
+	if err != nil {
 		// If EINVAL then the pin is reserved in the system and can't be unexported
 		if err.(*os.PathError).Err != syscall.EINVAL {
 			return err
 		}
 	}
+
 	return nil
 }
 
-var writeFile = func(path string, data []byte) (i int, err error) {
-	file, err := OpenFile(path, os.O_WRONLY, 0644)
-	defer file.Close()
-	if err != nil {
-		return
+// Linux sysfs / GPIO specific sysfs docs.
+//  https://www.kernel.org/doc/Documentation/filesystems/sysfs.txt
+//  https://www.kernel.org/doc/Documentation/gpio/sysfs.txt
+
+var writeFile = func(f File, data []byte) (i int, err error) {
+	if f == nil {
+		return 0, notExportedError
 	}
 
-	return file.Write(data)
+	// sysfs docs say:
+	// > When writing sysfs files, userspace processes should first read the
+	// > entire file, modify the values it wishes to change, then write the
+	// > entire buffer back.
+	// however, this seems outdated/inaccurate (docs are from back in the Kernel BitKeeper days).
+
+	i, err = f.Write(data)
+	return i, err
 }
 
-var readFile = func(path string) ([]byte, error) {
-	file, err := OpenFile(path, os.O_RDONLY, 0644)
-	defer file.Close()
-	if err != nil {
-		return make([]byte, 0), err
+var readFile = func(f File) ([]byte, error) {
+	if f == nil {
+		return nil, notExportedError
 	}
 
+	// sysfs docs say:
+	// > If userspace seeks back to zero or does a pread(2) with an offset of '0' the [..] method will
+	// > be called again, rearmed, to fill the buffer.
+
+	// TODO: Examine if seek is needed if full buffer is read from sysfs file.
+
 	buf := make([]byte, 2)
-	_, err = file.Read(buf)
+	_, err := f.Seek(0, os.SEEK_SET)
+	if err == nil {
+		_, err = f.Read(buf)
+	}
 	return buf, err
 }

sysfs/digital_pin_bench_test.go

@@ -0,0 +1,21 @@
+package sysfs
+
+import "testing"
+
+func BenchmarkDigitalRead(b *testing.B) {
+	fs := NewMockFilesystem([]string{
+		"/sys/class/gpio/export",
+		"/sys/class/gpio/unexport",
+		"/sys/class/gpio/gpio10/value",
+		"/sys/class/gpio/gpio10/direction",
+	})
+
+	SetFilesystem(fs)
+	pin := NewDigitalPin(10)
+	pin.Write(1)
+
+	for i := 0; i < b.N; i++ {
+		pin.Read()
+	}
+
+}

sysfs/digital_pin_test.go

@@ -24,53 +24,60 @@ func TestDigitalPin(t *testing.T) {
 	gobot.Assert(t, pin.label, "custom")
 
 	pin = NewDigitalPin(10).(*digitalPin)
+	gobot.Assert(t, pin.pin, "10")
 	gobot.Assert(t, pin.label, "gpio10")
+	gobot.Assert(t, pin.value, nil)
 
-	pin.Unexport()
+	err := pin.Unexport()
+	gobot.Assert(t, err, nil)
 	gobot.Assert(t, fs.Files["/sys/class/gpio/unexport"].Contents, "10")
 
-	pin.Export()
-	gobot.Assert(t, fs.Files["/sys/class/gpio/unexport"].Contents, "10")
+	err = pin.Export()
+	gobot.Assert(t, err, nil)
+	gobot.Assert(t, fs.Files["/sys/class/gpio/export"].Contents, "10")
+	gobot.Refute(t, pin.value, nil)
 
-	pin.Write(1)
+	err = pin.Write(1)
+	gobot.Assert(t, err, nil)
 	gobot.Assert(t, fs.Files["/sys/class/gpio/gpio10/value"].Contents, "1")
 
-	pin.Direction(IN)
+	err = pin.Direction(IN)
+	gobot.Assert(t, err, nil)
 	gobot.Assert(t, fs.Files["/sys/class/gpio/gpio10/direction"].Contents, "in")
 
 	data, _ := pin.Read()
 	gobot.Assert(t, 1, data)
 
 	pin2 := NewDigitalPin(30, "custom")
-	err := pin2.Write(1)
+	err = pin2.Write(1)
 	gobot.Refute(t, err, nil)
 
 	data, err = pin2.Read()
 	gobot.Refute(t, err, nil)
 	gobot.Assert(t, data, 0)
 
-	writeFile = func(string, []byte) (int, error) {
+	writeFile = func(File, []byte) (int, error) {
 		return 0, &os.PathError{Err: syscall.EINVAL}
 	}
 
 	err = pin.Unexport()
 	gobot.Assert(t, err, nil)
 
-	writeFile = func(string, []byte) (int, error) {
+	writeFile = func(File, []byte) (int, error) {
 		return 0, &os.PathError{Err: errors.New("write error")}
 	}
 
 	err = pin.Unexport()
 	gobot.Assert(t, err.(*os.PathError).Err, errors.New("write error"))
 
-	writeFile = func(string, []byte) (int, error) {
+	writeFile = func(File, []byte) (int, error) {
 		return 0, &os.PathError{Err: syscall.EBUSY}
 	}
 
 	err = pin.Export()
 	gobot.Assert(t, err, nil)
 
-	writeFile = func(string, []byte) (int, error) {
+	writeFile = func(File, []byte) (int, error) {
 		return 0, &os.PathError{Err: errors.New("write error")}
 	}
 

sysfs/fs.go

@@ -11,6 +11,7 @@ type File interface {
 	Sync() (err error)
 	Read(b []byte) (n int, err error)
 	ReadAt(b []byte, off int64) (n int, err error)
+	Seek(offset int64, whence int) (ret int64, err error)
 	Fd() uintptr
 	Close() error
 }

sysfs/fs_mock.go

@@ -32,6 +32,10 @@ func (f *MockFile) Write(b []byte) (n int, err error) {
 	return f.WriteString(string(b))
 }
 
+func (f *MockFile) Seek(offset int64, whence int) (ret int64, err error) {
+	return offset, nil
+}
+
 // WriteString writes s to f.Contents
 func (f *MockFile) WriteString(s string) (ret int, err error) {
 	f.Contents = s