1-wire(all): ensure halt is idempotent (#1174)

Author: gen2thomas

drivers/onewire/ds18b20_driver.go

@@ -108,7 +108,7 @@ func (d *DS18B20Driver) Temperature() (float32, error) {
 	d.mutex.Lock()
 	defer d.mutex.Unlock()
 
-	val, err := d.connection.ReadInteger(temperatureCommand)
+	val, err := d.readInteger(temperatureCommand)
 	if err != nil {
 		return 0, err
 	}
@@ -121,7 +121,7 @@ func (d *DS18B20Driver) Resolution() (uint8, error) {
 	d.mutex.Lock()
 	defer d.mutex.Unlock()
 
-	val, err := d.connection.ReadInteger(resolutionCommand)
+	val, err := d.readInteger(resolutionCommand)
 	if err != nil {
 		return 0, err
 	}
@@ -138,7 +138,7 @@ func (d *DS18B20Driver) IsExternalPowered() (bool, error) {
 	d.mutex.Lock()
 	defer d.mutex.Unlock()
 
-	val, err := d.connection.ReadInteger(extPowerCommand)
+	val, err := d.readInteger(extPowerCommand)
 	if err != nil {
 		return false, err
 	}
@@ -151,7 +151,7 @@ func (d *DS18B20Driver) ConversionTime() (uint16, error) {
 	d.mutex.Lock()
 	defer d.mutex.Unlock()
 
-	val, err := d.connection.ReadInteger(convTimeCommand)
+	val, err := d.readInteger(convTimeCommand)
 	if err != nil {
 		return 0, err
 	}
@@ -166,13 +166,18 @@ func (d *DS18B20Driver) ConversionTime() (uint16, error) {
 // DebugConversionTime try to set the conversion time and compare with real time to read temperature.
 func (d *DS18B20Driver) DebugConversionTime(start, end uint16, stepwide uint16, skipInvalid bool) {
 	r, _ := d.Resolution()
+	id, err := d.id()
+	if err != nil {
+		fmt.Println(err.Error())
+		return
+	}
 	fmt.Printf("\n---- Conversion time check for '%s'@%dbit %d..%d +%d ----\n",
-		d.connection.ID(), r, start, end, stepwide)
+		id, r, start, end, stepwide)
 	fmt.Println("|r1(err)\t|w(err)\t\t|r2(err)\t|T(err)\t\t|real\t\t|diff\t\t|")
 	fmt.Println("--------------------------------------------------------------------------------")
 	for ct := start; ct < end; ct += stepwide {
 		r1, e1 := d.ConversionTime()
-		ew := d.connection.WriteInteger(convTimeCommand, int(ct))
+		ew := d.writeInteger(convTimeCommand, int(ct))
 		r2, e2 := d.ConversionTime()
 		time.Sleep(100 * time.Millisecond) // relax the system
 		start := time.Now()
@@ -189,27 +194,31 @@ func (d *DS18B20Driver) DebugConversionTime(start, end uint16, stepwide uint16,
 
 func (d *DS18B20Driver) initialize() error {
 	if d.ds18b20Cfg.resolution != ds18b20DefaultResolution {
-		if err := d.connection.WriteInteger(resolutionCommand, int(d.ds18b20Cfg.resolution)); err != nil {
+		if err := d.writeInteger(resolutionCommand, int(d.ds18b20Cfg.resolution)); err != nil {
 			return err
 		}
 	}
 
 	if d.ds18b20Cfg.conversionTime != ds18b20DefaultConversionTime {
-		return d.connection.WriteInteger(convTimeCommand, int(d.ds18b20Cfg.conversionTime))
+		return d.writeInteger(convTimeCommand, int(d.ds18b20Cfg.conversionTime))
 	}
 
 	return nil
 }
 
 func (d *DS18B20Driver) shutdown() error {
+	if d.connection == nil {
+		return nil
+	}
+
 	if d.ds18b20Cfg.resolution != ds18b20DefaultResolution {
-		if err := d.connection.WriteInteger(resolutionCommand, ds18b20DefaultResolution); err != nil {
+		if err := d.writeInteger(resolutionCommand, ds18b20DefaultResolution); err != nil {
 			return err
 		}
 	}
 
 	if d.ds18b20Cfg.conversionTime != ds18b20DefaultConversionTime {
-		return d.connection.WriteInteger(convTimeCommand, int(ds18b20DefaultConversionTime))
+		return d.writeInteger(convTimeCommand, int(ds18b20DefaultConversionTime))
 	}
 
 	return nil

drivers/onewire/ds18b20_driver_test.go

@@ -129,6 +129,18 @@ func TestDS18B20Halt(t *testing.T) {
 	}
 }
 
+func TestDS18B20HaltIdempotent(t *testing.T) {
+	// arrange
+	d := NewDS18B20Driver(newOneWireTestAdaptor(), 2345)
+	// to trigger write calls:
+	d.ds18b20Cfg.resolution = ds18b20DefaultResolution + 1
+	d.ds18b20Cfg.conversionTime = ds18b20DefaultConversionTime + 2
+	// act, assert
+	require.NoError(t, d.Halt()) // must be idempotent
+	require.NoError(t, d.Start())
+	require.NoError(t, d.Halt())
+}
+
 func TestDS18B20Temperature(t *testing.T) {
 	const readValue = 24500
 	tests := map[string]struct {

drivers/onewire/onewire_driver.go

@@ -87,8 +87,12 @@ func (d *driver) SetName(name string) {
 	d.driverCfg.name = name
 }
 
-// Connection returns the connection of the device.
+// Connection returns the gobot connection of the device.
 func (d *driver) Connection() gobot.Connection {
+	if d.connection == nil {
+		log.Printf("1-wire driver not started for %s\n", d.driverCfg.name)
+	}
+
 	if conn, ok := d.connection.(gobot.Connection); ok {
 		return conn
 	}
@@ -116,10 +120,52 @@ func (d *driver) Halt() error {
 	d.mutex.Lock()
 	defer d.mutex.Unlock()
 
-	// currently there is nothing to do here for the driver, the connection is cached on adaptor side
-	// and will be closed on adaptor Finalize()
+	if err := d.beforeHalt(); err != nil {
+		return err
+	}
+
+	// the connection is also cached on adaptor side and will be closed on adaptor Finalize()
+	d.connection = nil
+
+	return nil
+}
+
+func (d *driver) id() (string, error) {
+	if d.connection == nil {
+		return "", fmt.Errorf("1-wire driver not started for %s", d.driverCfg.name)
+	}
+
+	return d.connection.ID(), nil
+}
+
+//nolint:unused // ok for now
+func (d *driver) readData(command string, data []byte) error {
+	if d.connection == nil {
+		return fmt.Errorf("1-wire driver not started for %s", d.driverCfg.name)
+	}
+	return d.connection.ReadData(command, data)
+}
 
-	return d.beforeHalt()
+//nolint:unused // ok for now
+func (d *driver) writeData(command string, data []byte) error {
+	if d.connection == nil {
+		return fmt.Errorf("1-wire driver not started for %s", d.driverCfg.name)
+	}
+	return d.connection.WriteData(command, data)
+}
+
+func (d *driver) readInteger(command string) (int, error) {
+	if d.connection == nil {
+		return 0, fmt.Errorf("1-wire driver not started for %s", d.driverCfg.name)
+	}
+	return d.connection.ReadInteger(command)
+}
+
+func (d *driver) writeInteger(command string, val int) error {
+	if d.connection == nil {
+		return fmt.Errorf("1-wire driver not started for %s", d.driverCfg.name)
+	}
+	return d.connection.WriteInteger(command, val)
 }
 
 func (o nameOption) String() string {

drivers/onewire/onewire_driver_test.go

@@ -62,6 +62,8 @@ func TestHalt(t *testing.T) {
 	// arrange
 	d := initTestDriver()
 	// act, assert
+	require.NoError(t, d.Halt()) // must be idempotent
+	require.NoError(t, d.Start())
 	require.NoError(t, d.Halt())
 }