ble: add support for functional options, add tests (#1059)

README.md

@@ -174,8 +174,7 @@ import (
 
 func NewSwarmBot(port string) *gobot.Robot {
   spheroAdaptor := serialport.NewAdaptor(port)
-  spheroDriver := serial.NewSpheroDriver(spheroAdaptor)
-  spheroDriver.SetName("Sphero" + port)
+  spheroDriver := serial.NewSpheroDriver(spheroAdaptor, serial.WithName("Sphero" + port))
 
   work := func() {
     spheroDriver.Stop()

adaptor.go

@@ -235,7 +235,7 @@ type BLEConnector interface {
 
 	ReadCharacteristic(cUUID string) ([]byte, error)
 	WriteCharacteristic(cUUID string, data []byte) error
-	Subscribe(cUUID string, f func([]byte, error)) error
+	Subscribe(cUUID string, f func(data []byte)) error
 	WithoutResponses(use bool)
 }
 

doc.go

@@ -85,8 +85,7 @@ Finally, you can use Master Gobot to add the complete Gobot API or control swarm
 
 		func NewSwarmBot(port string) *gobot.Robot {
 		    spheroAdaptor := serialport.NewAdaptor(port)
-		    spheroDriver := serial.NewSpheroDriver(spheroAdaptor)
-		    spheroDriver.SetName("Sphero" + port)
+		    spheroDriver := serial.NewSpheroDriver(spheroAdaptor, serial.WithName("Sphero" + port))
 
 		    work := func() {
 		        spheroDriver.Stop()

driver.go

@@ -5,7 +5,7 @@ type Driver interface {
 	// Name returns the label for the Driver
 	Name() string
 	// SetName sets the label for the Driver.
-	// Please prefer to use options [gpio.WithName or aio.WithName] instead, if possible.
+	// Please use options [aio.WithName, ble.WithName, gpio.WithName or serial.WithName] instead.
 	SetName(s string)
 	// Start initiates the Driver
 	Start() error

drivers/MIGRATION.md

@@ -39,6 +39,44 @@ import(
 ...
 ```
 
+### BLE client adaptor changed signature for Subscribe()
+
+Since introducing the usage of "github.com/muka/go-bluetooth" in 2020, the callback do not support the given error
+parameter anymore. The switch to usage of "tinygo.org/x/bluetooth" has not changed this. Therefore it is removed now
+from the function.
+
+### BLE generic drivers changed signature for Get*() functions
+
+All those functions log an error only or panic, so the caller gets no nice programmatic feedback. The error is now
+returned instead and the log output needs to be done at caller side.
+
+```go
+// old
+...
+  devName := access.GetDeviceName()
+  appearance := access.GetAppearance()
+  modelNo := info.GetModelNumber()
+  fwRev := info.GetFirmwareRevision()
+  hwRev := info.GetHardwareRevision()
+  manuName := info.GetManufacturerName()
+  pid := info.GetPnPId()
+  level := battery.GetBatteryLevel()
+...
+
+// new
+...
+  devName, err := access.GetDeviceName()
+  if err != nil {
+    fmt.Println(err)
+  }
+  appearance, err := access.GetAppearance()
+  if err != nil {
+    fmt.Println(err)
+  }
+  ...
+...
+```
+
 ### Sphero adaptor split off
 
 The Serial Based Sphero adaptor was split off into a generic serial adaptor and the driver part. With this, the imports

drivers/ble/battery_driver.go

@@ -2,7 +2,6 @@ package ble
 
 import (
 	"bytes"
-	"log"
 
 	"gobot.io/x/gobot/v2"
 )
@@ -16,24 +15,23 @@ type BatteryDriver struct {
 }
 
 // NewBatteryDriver creates a new driver
-func NewBatteryDriver(a gobot.BLEConnector) *BatteryDriver {
+func NewBatteryDriver(a gobot.BLEConnector, opts ...OptionApplier) *BatteryDriver {
 	d := &BatteryDriver{
-		Driver:  NewDriver(a, "Battery", nil, nil),
+		Driver:  NewDriver(a, "Battery", nil, nil, opts...),
 		Eventer: gobot.NewEventer(),
 	}
 
 	return d
 }
 
 // GetBatteryLevel reads and returns the current battery level
-func (d *BatteryDriver) GetBatteryLevel() uint8 {
+func (d *BatteryDriver) GetBatteryLevel() (uint8, error) {
 	c, err := d.Adaptor().ReadCharacteristic(batteryCharaShort)
 	if err != nil {
-		log.Println(err)
-		return 0
+		return 0, err
 	}
 	buf := bytes.NewBuffer(c)
 	val, _ := buf.ReadByte()
 	level := val
-	return level
+	return level, nil
 }

drivers/ble/battery_driver_test.go

@@ -5,6 +5,7 @@ import (
 	"testing"
 
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 
 	"gobot.io/x/gobot/v2"
 	"gobot.io/x/gobot/v2/drivers/ble/testutil"
@@ -13,12 +14,27 @@ import (
 var _ gobot.Driver = (*BatteryDriver)(nil)
 
 func TestNewBatteryDriver(t *testing.T) {
+	// arrange
 	d := NewBatteryDriver(testutil.NewBleTestAdaptor())
+	// act & assert
 	assert.True(t, strings.HasPrefix(d.Name(), "Battery"))
 	assert.NotNil(t, d.Eventer)
 }
 
+func TestNewBatteryDriverWithName(t *testing.T) {
+	// This is a general test, that options are applied in constructor by using the common WithName() option.	Further
+	// tests for options can also be done by call of "WithOption(val).apply(cfg)".
+	// arrange
+	const newName = "new name"
+	a := testutil.NewBleTestAdaptor()
+	// act
+	d := NewBatteryDriver(a, WithName(newName))
+	// assert
+	assert.Equal(t, newName, d.Name())
+}
+
 func TestBatteryDriverRead(t *testing.T) {
+	// arrange
 	a := testutil.NewBleTestAdaptor()
 	d := NewBatteryDriver(a)
 	a.SetReadCharacteristicTestFunc(func(cUUID string) ([]byte, error) {
@@ -28,6 +44,9 @@ func TestBatteryDriverRead(t *testing.T) {
 
 		return nil, nil
 	})
-
-	assert.Equal(t, uint8(20), d.GetBatteryLevel())
+	// act
+	level, err := d.GetBatteryLevel()
+	// assert
+	require.NoError(t, err)
+	assert.Equal(t, uint8(20), level)
 }

drivers/ble/ble_driver.go

@@ -7,8 +7,8 @@ import (
 	"gobot.io/x/gobot/v2"
 )
 
-// optionApplier needs to be implemented by each configurable option type
-type optionApplier interface {
+// OptionApplier needs to be implemented by each configurable option type
+type OptionApplier interface {
 	apply(cfg *configuration)
 }
 
@@ -31,7 +31,11 @@ type Driver struct {
 }
 
 // NewDriver creates a new basic BLE gobot driver.
-func NewDriver(a interface{}, name string, afterStart func() error, beforeHalt func() error) *Driver {
+func NewDriver(
+	a interface{}, name string,
+	afterStart func() error, beforeHalt func() error,
+	opts ...OptionApplier,
+) *Driver {
 	if afterStart == nil {
 		afterStart = func() error { return nil }
 	}
@@ -49,11 +53,15 @@ func NewDriver(a interface{}, name string, afterStart func() error, beforeHalt f
 		mutex:      &sync.Mutex{},
 	}
 
+	for _, o := range opts {
+		o.apply(d.driverCfg)
+	}
+
 	return &d
 }
 
 // WithName is used to replace the default name of the driver.
-func WithName(name string) optionApplier {
+func WithName(name string) OptionApplier {
 	return nameOption(name)
 }
 
@@ -63,7 +71,7 @@ func (d *Driver) Name() string {
 }
 
 // SetName sets the name of the driver.
-// Deprecated: Please use option [aio.WithName] instead.
+// Deprecated: Please use option [ble.WithName] instead.
 func (d *Driver) SetName(name string) {
 	WithName(name).apply(d.driverCfg)
 }

drivers/ble/ble_driver_test.go

@@ -37,6 +37,21 @@ func TestNewDriver(t *testing.T) {
 	assert.NotNil(t, d.mutex)
 }
 
+func TestNewDriverWithName(t *testing.T) {
+	// This is a general test, that options are applied in constructor by using the common WithName() option.	Further
+	// tests for options can also be done by call of "WithOption(val).apply(cfg)".
+	// arrange
+	const (
+		name    = "mybot"
+		newName = "overwrite mybot"
+	)
+	a := testutil.NewBleTestAdaptor()
+	// act
+	d := NewDriver(a, name, nil, nil, WithName(newName))
+	// assert
+	assert.Equal(t, newName, d.Name())
+}
+
 func Test_applyWithName(t *testing.T) {
 	// arrange
 	const name = "mybot"
@@ -68,3 +83,18 @@ func TestHalt(t *testing.T) {
 	// act, assert
 	require.EqualError(t, d.Halt(), "before halt error")
 }
+
+func TestAdaptor(t *testing.T) {
+	wrongConnectorType := struct {
+		a uint32
+	}{}
+	// arrange
+	a := testutil.NewBleTestAdaptor()
+	d := NewDriver(a, "BLE_BASIC", nil, nil)
+	// act, assert
+	assert.Equal(t, a, d.Adaptor())
+	// arrange wrong connector type
+	d.connection = wrongConnectorType
+	// act, assert
+	assert.Nil(t, d.Adaptor())
+}

drivers/ble/device_information_driver.go

@@ -2,7 +2,6 @@ package ble
 
 import (
 	"bytes"
-	"log"
 
 	"gobot.io/x/gobot/v2"
 )
@@ -22,71 +21,66 @@ type DeviceInformationDriver struct {
 }
 
 // NewDeviceInformationDriver creates a new driver
-func NewDeviceInformationDriver(a gobot.BLEConnector) *DeviceInformationDriver {
+func NewDeviceInformationDriver(a gobot.BLEConnector, opts ...OptionApplier) *DeviceInformationDriver {
 	n := &DeviceInformationDriver{
-		Driver:  NewDriver(a, "DeviceInformation", nil, nil),
+		Driver:  NewDriver(a, "DeviceInformation", nil, nil, opts...),
 		Eventer: gobot.NewEventer(),
 	}
 
 	return n
 }
 
 // GetModelNumber returns the model number for the BLE Peripheral
-func (d *DeviceInformationDriver) GetModelNumber() string {
+func (d *DeviceInformationDriver) GetModelNumber() (string, error) {
 	c, err := d.Adaptor().ReadCharacteristic(deviceInformationModelNumberCharaShort)
 	if err != nil {
-		log.Println(err)
-		return ""
+		return "", err
 	}
 	buf := bytes.NewBuffer(c)
 	model := buf.String()
-	return model
+	return model, nil
 }
 
 // GetFirmwareRevision returns the firmware revision for the BLE Peripheral
-func (d *DeviceInformationDriver) GetFirmwareRevision() string {
+func (d *DeviceInformationDriver) GetFirmwareRevision() (string, error) {
 	c, err := d.Adaptor().ReadCharacteristic(deviceInformationFirmwareRevisionCharaShort)
 	if err != nil {
-		log.Println(err)
-		return ""
+		return "", err
 	}
 	buf := bytes.NewBuffer(c)
 	val := buf.String()
-	return val
+	return val, nil
 }
 
 // GetHardwareRevision returns the hardware revision for the BLE Peripheral
-func (d *DeviceInformationDriver) GetHardwareRevision() string {
+func (d *DeviceInformationDriver) GetHardwareRevision() (string, error) {
 	c, err := d.Adaptor().ReadCharacteristic(deviceInformationHardwareRevisionCharaShort)
 	if err != nil {
-		log.Println(err)
-		return ""
+		return "", err
 	}
 	buf := bytes.NewBuffer(c)
 	val := buf.String()
-	return val
+	return val, nil
 }
 
 // GetManufacturerName returns the manufacturer name for the BLE Peripheral
-func (d *DeviceInformationDriver) GetManufacturerName() string {
+func (d *DeviceInformationDriver) GetManufacturerName() (string, error) {
 	c, err := d.Adaptor().ReadCharacteristic(deviceInformationManufacturerNameCharaShort)
 	if err != nil {
-		log.Println(err)
-		return ""
+		return "", err
 	}
 	buf := bytes.NewBuffer(c)
 	val := buf.String()
-	return val
+	return val, nil
 }
 
 // GetPnPId returns the PnP ID for the BLE Peripheral
-func (d *DeviceInformationDriver) GetPnPId() string {
+func (d *DeviceInformationDriver) GetPnPId() (string, error) {
 	c, err := d.Adaptor().ReadCharacteristic(deviceInformationPnPIdCharaShort)
 	if err != nil {
-		log.Println(err)
-		return ""
+		return "", err
 	}
 	buf := bytes.NewBuffer(c)
 	val := buf.String()
-	return val
+	return val, nil
 }