BUGFIX: I2C connection-bus caching and multiple device usage

adaptor.go

@@ -93,10 +93,67 @@ type PWMPinnerProvider interface {
 	PWMPin(id string) (PWMPinner, error)
 }
 
-// I2cSystemDevicer is the interface to a i2c bus at system level.
+// I2cSystemDevicer is the interface to a i2c bus at system level, according to I2C/SMBus specification.
+// Some functions are not in the interface yet:
+// * Process Call (WriteWordDataReadWordData)
+// * Block Write - Block Read (WriteBlockDataReadBlockData)
+// * Host Notify - WriteWordData() can be used instead
+//
+// see: https://docs.kernel.org/i2c/smbus-protocol.html#key-to-symbols
+//
+// S: Start condition; Sr: Repeated start condition, used to switch from write to read mode.
+// P: Stop condition; Rd/Wr (1 bit): Read/Write bit. Rd equals 1, Wr equals 0.
+// A, NA (1 bit): Acknowledge (ACK) and Not Acknowledge (NACK) bit
+// Addr (7 bits): I2C 7 bit address. (10 bit I2C address not yet supported by gobot).
+// Comm (8 bits): Command byte, a data byte which often selects a register on the device.
+// Data (8 bits): A plain data byte. DataLow and DataHigh represent the low and high byte of a 16 bit word.
+// Count (8 bits): A data byte containing the length of a block operation.
+// [..]: Data sent by I2C device, as opposed to data sent by the host adapter.
+//
 type I2cSystemDevicer interface {
-	I2cOperations
-	SetAddress(int) error
+	// ReadByte must be implemented as the sequence:
+	// "S Addr Rd [A] [Data] NA P"
+	ReadByte(address int) (byte, error)
+
+	// ReadByteData must be implemented as the sequence:
+	// "S Addr Wr [A] Comm [A] Sr Addr Rd [A] [Data] NA P"
+	ReadByteData(address int, reg uint8) (uint8, error)
+
+	// ReadWordData must be implemented as the sequence:
+	// "S Addr Wr [A] Comm [A] Sr Addr Rd [A] [DataLow] A [DataHigh] NA P"
+	ReadWordData(address int, reg uint8) (uint16, error)
+
+	// ReadBlockData must be implemented as the sequence:
+	// "S Addr Wr [A] Comm [A] Sr Addr Rd [A] [Count] A [Data] A [Data] A ... A [Data] NA P"
+	ReadBlockData(address int, reg uint8, data []byte) error
+
+	// WriteByte must be implemented as the sequence:
+	// "S Addr Wr [A] Data [A] P"
+	WriteByte(address int, val byte) error
+
+	// WriteByteData must be implemented as the sequence:
+	// "S Addr Wr [A] Comm [A] Data [A] P"
+	WriteByteData(address int, reg uint8, val uint8) error
+
+	// WriteBlockData must be implemented as the sequence:
+	// "S Addr Wr [A] Comm [A] Count [A] Data [A] Data [A] ... [A] Data [A] P"
+	WriteBlockData(address int, reg uint8, data []byte) error
+
+	// WriteWordData must be implemented as the sequence:
+	// "S Addr Wr [A] Comm [A] DataLow [A] DataHigh [A] P"
+	WriteWordData(address int, reg uint8, val uint16) error
+
+	// WriteBytes writes the given data starting from the current register of bus device.
+	WriteBytes(address int, data []byte) error
+
+	// Read implements direct read operations.
+	Read(address int, b []byte) (int, error)
+
+	// Write implements direct write operations.
+	Write(address int, b []byte) (n int, err error)
+
+	// Close closes the character device file.
+	Close() error
 }
 
 // SpiSystemDevicer is the interface to a SPI bus at system level.
@@ -123,46 +180,14 @@ type BusOperations interface {
 }
 
 // I2cOperations represents the i2c methods according to I2C/SMBus specification.
-// Some functions are not in the interface yet:
-// * Process Call (WriteWordDataReadWordData)
-// * Block Write - Block Read (WriteBlockDataReadBlockData)
-// * Host Notify - WriteWordData() can be used instead
-//
-// see: https://docs.kernel.org/i2c/smbus-protocol.html#key-to-symbols
-//
-// S: Start condition; Sr: Repeated start condition, used to switch from write to read mode.
-// P: Stop condition; Rd/Wr (1 bit): Read/Write bit. Rd equals 1, Wr equals 0.
-// A, NA (1 bit): Acknowledge (ACK) and Not Acknowledge (NACK) bit
-// Addr (7 bits): I2C 7 bit address. (10 bit I2C address not yet supported by gobot).
-// Comm (8 bits): Command byte, a data byte which often selects a register on the device.
-// Data (8 bits): A plain data byte. DataLow and DataHigh represent the low and high byte of a 16 bit word.
-// Count (8 bits): A data byte containing the length of a block operation.
-// [..]: Data sent by I2C device, as opposed to data sent by the host adapter.
-//
-// ReadByteData must be implemented as the sequence:
-// "S Addr Wr [A] Comm [A] Sr Addr Rd [A] [Data] NA P"
-// ReadBlockData must be implemented as the sequence:
-// "S Addr Wr [A] Comm [A] Sr Addr Rd [A] [Count] A [Data] A [Data] A ... A [Data] NA P"
-// WriteByte must be implemented as the sequence:
-// "S Addr Wr [A] Data [A] P"
-// WriteByteData must be implemented as the sequence:
-// "S Addr Wr [A] Comm [A] Data [A] P"
-// WriteBlockData must be implemented as the sequence:
-// "S Addr Wr [A] Comm [A] Count [A] Data [A] Data [A] ... [A] Data [A] P"
 type I2cOperations interface {
 	io.ReadWriteCloser
 	BusOperations
-
-	// ReadByte must be implemented as the sequence:
-	// "S Addr Rd [A] [Data] NA P"
+	// ReadByte reads a byte from the current register of an i2c device.
 	ReadByte() (byte, error)
-
-	// ReadWordData must be implemented as the sequence:
-	// "S Addr Wr [A] Comm [A] Sr Addr Rd [A] [DataLow] A [DataHigh] NA P"
+	// ReadWordData reads a 16 bit value starting from the given register of an i2c device.
 	ReadWordData(reg uint8) (uint16, error)
-
-	// WriteWordData must be implemented as the sequence:
-	// "S Addr Wr [A] Comm [A] DataLow [A] DataHigh [A] P"
+	// WriteWordData writes the given 16 bit value starting from the given register of an i2c device.
 	WriteWordData(reg uint8, val uint16) error
 }
 

drivers/i2c/adafruit1109_driver.go

@@ -187,15 +187,15 @@ func (d *Adafruit1109Driver) Halt() error {
 // This is called by HD44780 driver to set one gpio output. We redirect the call to the i2c driver MCP23017.
 // The given id is the same as defined in dataPins and has the syntax "<port>_<pin>".
 func (d *Adafruit1109Driver) DigitalWrite(id string, val byte) error {
-	portio := adafruit1109ParseId(id)
+	portio := adafruit1109ParseID(id)
 	return d.writePin(portio, val)
 }
 
 // DigitalRead implements the DigitalReader interface
 // This is called by HD44780 driver to read one gpio input. We redirect the call to the i2c driver MCP23017.
 // The given id is the same as defined in dataPins and has the syntax "<port>_<pin>".
 func (d *Adafruit1109Driver) DigitalRead(id string) (int, error) {
-	portio := adafruit1109ParseId(id)
+	portio := adafruit1109ParseID(id)
 	uval, err := d.readPin(portio)
 	if err != nil {
 		return 0, err
@@ -276,7 +276,7 @@ func (ap *adafruit1109PortPin) String() string {
 	return fmt.Sprintf("%s_%d", ap.port, ap.pin)
 }
 
-func adafruit1109ParseId(id string) adafruit1109PortPin {
+func adafruit1109ParseID(id string) adafruit1109PortPin {
 	items := strings.Split(id, "_")
 	io := uint8(0)
 	if io64, err := strconv.ParseUint(items[1], 10, 32); err == nil {

drivers/i2c/adafruit1109_driver_test.go

@@ -81,7 +81,7 @@ func TestAdafruit1109StartReadErr(t *testing.T) {
 	adaptor.i2cReadImpl = func([]byte) (int, error) {
 		return 0, errors.New("read error")
 	}
-	gobottest.Assert(t, d.Start(), errors.New("read error"))
+	gobottest.Assert(t, d.Start(), errors.New("MCP write-read: MCP write-ReadByteData(reg=0): read error"))
 }
 
 func TestAdafruit1109Halt(t *testing.T) {
@@ -287,15 +287,15 @@ func TestAdafruit1109RightButton(t *testing.T) {
 	}
 }
 
-func TestAdafruit1109_parseId(t *testing.T) {
+func TestAdafruit1109_parseID(t *testing.T) {
 	// arrange
 	ports := []string{"A", "B"}
 	for _, port := range ports {
 		for pin := uint8(0); pin <= 7; pin++ {
 			id := fmt.Sprintf("%s_%d", port, pin)
 			t.Run(id, func(t *testing.T) {
 				// act
-				got := adafruit1109ParseId(id)
+				got := adafruit1109ParseID(id)
 				// assert
 				gobottest.Assert(t, got, adafruit1109PortPin{port, pin})
 			})

drivers/i2c/i2c_connection.go

@@ -1,8 +1,7 @@
 package i2c
 
 import (
-	"errors"
-	"sync"
+	"fmt"
 
 	"gobot.io/x/gobot"
 )
@@ -21,10 +20,10 @@ const (
 )
 
 var (
-	ErrEncryptedBytes  = errors.New("Encrypted bytes")
-	ErrNotEnoughBytes  = errors.New("Not enough bytes read")
-	ErrNotReady        = errors.New("Device is not ready")
-	ErrInvalidPosition = errors.New("Invalid position value")
+	// ErrNotEnoughBytes is used when the count of read bytes was too small
+	ErrNotEnoughBytes = fmt.Errorf("Not enough bytes read")
+	// ErrNotReady is used when the device is not ready
+	ErrNotReady = fmt.Errorf("Device is not ready")
 )
 
 type bitState uint8
@@ -44,144 +43,71 @@ type Connection gobot.I2cOperations
 type i2cConnection struct {
 	bus     gobot.I2cSystemDevicer
 	address int
-	mutex   sync.Mutex
 }
 
-// NewConnection creates and returns a new connection to a specific
-// i2c device on a bus and address.
+// NewConnection creates and returns a new connection to a specific i2c device on a bus and address.
 func NewConnection(bus gobot.I2cSystemDevicer, address int) (connection *i2cConnection) {
 	return &i2cConnection{bus: bus, address: address}
 }
 
 // Read data from an i2c device.
 func (c *i2cConnection) Read(data []byte) (read int, err error) {
-	c.mutex.Lock()
-	defer c.mutex.Unlock()
-
-	if err = c.bus.SetAddress(c.address); err != nil {
-		return 0, err
-	}
-	read, err = c.bus.Read(data)
-	return
+	return c.bus.Read(c.address, data)
 }
 
 // Write data to an i2c device.
 func (c *i2cConnection) Write(data []byte) (written int, err error) {
-	c.mutex.Lock()
-	defer c.mutex.Unlock()
-
-	if err = c.bus.SetAddress(c.address); err != nil {
-		return 0, err
-	}
-	written, err = c.bus.Write(data)
-	return
+	return c.bus.Write(c.address, data)
 }
 
-// Close connection to i2c device.
+// Close connection to i2c device. The bus was created by adaptor and will be closed there.
 func (c *i2cConnection) Close() error {
-	c.mutex.Lock()
-	defer c.mutex.Unlock()
-
-	return c.bus.Close()
+	return nil
 }
 
 // ReadByte reads a single byte from the i2c device.
-func (c *i2cConnection) ReadByte() (val byte, err error) {
-	c.mutex.Lock()
-	defer c.mutex.Unlock()
-
-	if err := c.bus.SetAddress(c.address); err != nil {
-		return 0, err
-	}
-	return c.bus.ReadByte()
+func (c *i2cConnection) ReadByte() (byte, error) {
+	return c.bus.ReadByte(c.address)
 }
 
 // ReadByteData reads a byte value for a register on the i2c device.
-func (c *i2cConnection) ReadByteData(reg uint8) (val uint8, err error) {
-	c.mutex.Lock()
-	defer c.mutex.Unlock()
-
-	if err := c.bus.SetAddress(c.address); err != nil {
-		return 0, err
-	}
-	return c.bus.ReadByteData(reg)
+func (c *i2cConnection) ReadByteData(reg uint8) (uint8, error) {
+	return c.bus.ReadByteData(c.address, reg)
 }
 
 // ReadWordData reads a word value for a register on the i2c device.
-func (c *i2cConnection) ReadWordData(reg uint8) (val uint16, err error) {
-	c.mutex.Lock()
-	defer c.mutex.Unlock()
-
-	if err := c.bus.SetAddress(c.address); err != nil {
-		return 0, err
-	}
-	return c.bus.ReadWordData(reg)
+func (c *i2cConnection) ReadWordData(reg uint8) (uint16, error) {
+	return c.bus.ReadWordData(c.address, reg)
 }
 
 // ReadBlockData reads a block of bytes from a register on the i2c device.
-func (c *i2cConnection) ReadBlockData(reg uint8, b []byte) (err error) {
-	c.mutex.Lock()
-	defer c.mutex.Unlock()
-
-	if err := c.bus.SetAddress(c.address); err != nil {
-		return err
-	}
-	return c.bus.ReadBlockData(reg, b)
+func (c *i2cConnection) ReadBlockData(reg uint8, b []byte) error {
+	return c.bus.ReadBlockData(c.address, reg, b)
 }
 
 // WriteByte writes a single byte to the i2c device.
-func (c *i2cConnection) WriteByte(val byte) (err error) {
-	c.mutex.Lock()
-	defer c.mutex.Unlock()
-
-	if err := c.bus.SetAddress(c.address); err != nil {
-		return err
-	}
-	return c.bus.WriteByte(val)
+func (c *i2cConnection) WriteByte(val byte) error {
+	return c.bus.WriteByte(c.address, val)
 }
 
 // WriteByteData writes a byte value to a register on the i2c device.
-func (c *i2cConnection) WriteByteData(reg uint8, val uint8) (err error) {
-	c.mutex.Lock()
-	defer c.mutex.Unlock()
-
-	if err := c.bus.SetAddress(c.address); err != nil {
-		return err
-	}
-	return c.bus.WriteByteData(reg, val)
+func (c *i2cConnection) WriteByteData(reg uint8, val uint8) error {
+	return c.bus.WriteByteData(c.address, reg, val)
 }
 
 // WriteWordData writes a word value to a register on the i2c device.
 func (c *i2cConnection) WriteWordData(reg uint8, val uint16) (err error) {
-	c.mutex.Lock()
-	defer c.mutex.Unlock()
-
-	if err := c.bus.SetAddress(c.address); err != nil {
-		return err
-	}
-	return c.bus.WriteWordData(reg, val)
+	return c.bus.WriteWordData(c.address, reg, val)
 }
 
 // WriteBlockData writes a block of bytes to a register on the i2c device.
 func (c *i2cConnection) WriteBlockData(reg uint8, b []byte) (err error) {
-	c.mutex.Lock()
-	defer c.mutex.Unlock()
-
-	if err := c.bus.SetAddress(c.address); err != nil {
-		return err
-	}
-	return c.bus.WriteBlockData(reg, b)
+	return c.bus.WriteBlockData(c.address, reg, b)
 }
 
 // WriteBytes writes a block of bytes to the current register on the i2c device.
 func (c *i2cConnection) WriteBytes(b []byte) (err error) {
-	c.mutex.Lock()
-	defer c.mutex.Unlock()
-
-	if err := c.bus.SetAddress(c.address); err != nil {
-		return err
-	}
-	return c.bus.WriteBytes(b)
+	return c.bus.WriteBytes(c.address, b)
 }
 
 // setBit is used to set a bit at a given position to 1.