i2c(all): ensure halt is idempotent (#1171)

Author: gen2thomas

drivers/i2c/adafruit1109_driver.go

@@ -26,22 +26,23 @@ type Adafruit1109Driver struct {
 	*MCP23017Driver
 	*gpio.HD44780Driver
 
-	name      string
-	redPin    adafruit1109PortPin
-	greenPin  adafruit1109PortPin
-	bluePin   adafruit1109PortPin
-	selectPin adafruit1109PortPin
-	upPin     adafruit1109PortPin
-	downPin   adafruit1109PortPin
-	leftPin   adafruit1109PortPin
-	rightPin  adafruit1109PortPin
-	rwPin     adafruit1109PortPin
-	rsPin     adafruit1109PortPin
-	enPin     adafruit1109PortPin
-	dataPinD4 adafruit1109PortPin
-	dataPinD5 adafruit1109PortPin
-	dataPinD6 adafruit1109PortPin
-	dataPinD7 adafruit1109PortPin
+	name       string
+	redPin     adafruit1109PortPin
+	greenPin   adafruit1109PortPin
+	bluePin    adafruit1109PortPin
+	selectPin  adafruit1109PortPin
+	upPin      adafruit1109PortPin
+	downPin    adafruit1109PortPin
+	leftPin    adafruit1109PortPin
+	rightPin   adafruit1109PortPin
+	rwPin      adafruit1109PortPin
+	rsPin      adafruit1109PortPin
+	enPin      adafruit1109PortPin
+	dataPinD4  adafruit1109PortPin
+	dataPinD5  adafruit1109PortPin
+	dataPinD6  adafruit1109PortPin
+	dataPinD7  adafruit1109PortPin
+	mcpStarted bool
 }
 
 // NewAdafruit1109Driver creates is a new driver for the 2x16 LCD display with RGB backlit and 5 keys.
@@ -120,6 +121,8 @@ func (d *Adafruit1109Driver) Start() error {
 		return err
 	}
 
+	d.mcpStarted = true
+
 	// set all to output (inputs will be set by initButton)
 	for pin := uint8(0); pin <= 7; pin++ {
 		if err := d.SetPinMode(pin, "A", 0); err != nil {
@@ -169,15 +172,20 @@ func (d *Adafruit1109Driver) Halt() error {
 	if err := d.HD44780Driver.Halt(); err != nil {
 		errors = append(errors, err.Error())
 	}
-	// switch off the background light
-	if err := d.SetRGB(false, false, false); err != nil {
-		errors = append(errors, err.Error())
-	}
-	// must be after HD44780Driver
-	if err := d.MCP23017Driver.Halt(); err != nil {
-		errors = append(errors, err.Error())
+
+	if d.mcpStarted {
+		// switch off the background light
+		if err := d.SetRGB(false, false, false); err != nil {
+			errors = append(errors, err.Error())
+		}
+		// must be after HD44780Driver
+		if err := d.MCP23017Driver.Halt(); err != nil {
+			errors = append(errors, err.Error())
+		}
 	}
 
+	d.mcpStarted = false
+
 	if len(errors) > 0 {
 		return fmt.Errorf("'Halt' the driver %s", strings.Join(errors, ", "))
 	}

drivers/i2c/adafruit1109_driver_test.go

@@ -88,7 +88,8 @@ func TestAdafruit1109StartReadErr(t *testing.T) {
 
 func TestAdafruit1109Halt(t *testing.T) {
 	d, _ := initTestAdafruit1109WithStubbedAdaptor()
-	_ = d.Start()
+	require.NoError(t, d.Halt()) // must be idempotent
+	require.NoError(t, d.Start())
 	require.NoError(t, d.Halt())
 }
 

drivers/i2c/adafruit2327_driver_test.go

@@ -32,6 +32,15 @@ func TestNewAdafruit2327Driver(t *testing.T) {
 	assert.Equal(t, 0x40, d.defaultAddress)
 }
 
+func TestAdafruit2327Halt(t *testing.T) {
+	// arrange
+	d := NewAdafruit2327Driver(newI2cTestAdaptor())
+	// act, assert
+	require.NoError(t, d.Halt()) // must be idempotent
+	require.NoError(t, d.Start())
+	require.NoError(t, d.Halt())
+}
+
 func TestAdafruit2327Options(t *testing.T) {
 	// This is a general test, that options are applied in constructor by using the common WithBus() option and
 	// least one of this driver. Further tests for options can also be done by call of "WithOption(val)(d)".

drivers/i2c/adafruit2348_driver.go

@@ -125,54 +125,54 @@ func NewAdafruit2348Driver(c Connector, options ...func(Config)) *Adafruit2348Dr
 }
 
 // SetDCMotorSpeed will set the appropriate pins to run the specified DC motor for the given speed.
-func (a *Adafruit2348Driver) SetDCMotorSpeed(dcMotor int, speed int32) error {
-	return a.SetPWM(int(a.dcMotors[dcMotor].pwmPin), 0, uint16(speed*16)) //nolint:gosec // TODO: fix later
+func (d *Adafruit2348Driver) SetDCMotorSpeed(dcMotor int, speed int32) error {
+	return d.SetPWM(int(d.dcMotors[dcMotor].pwmPin), 0, uint16(speed*16)) //nolint:gosec // TODO: fix later
 }
 
 // RunDCMotor will set the appropriate pins to run the specified DC motor for the given direction.
-func (a *Adafruit2348Driver) RunDCMotor(dcMotor int, dir Adafruit2348Direction) error {
+func (d *Adafruit2348Driver) RunDCMotor(dcMotor int, dir Adafruit2348Direction) error {
 	switch dir {
 	case Adafruit2348Forward:
-		if err := a.setPin(a.dcMotors[dcMotor].in2Pin, 0); err != nil {
+		if err := d.setPin(d.dcMotors[dcMotor].in2Pin, 0); err != nil {
 			return err
 		}
-		if err := a.setPin(a.dcMotors[dcMotor].in1Pin, 1); err != nil {
+		if err := d.setPin(d.dcMotors[dcMotor].in1Pin, 1); err != nil {
 			return err
 		}
 	case Adafruit2348Backward:
-		if err := a.setPin(a.dcMotors[dcMotor].in1Pin, 0); err != nil {
+		if err := d.setPin(d.dcMotors[dcMotor].in1Pin, 0); err != nil {
 			return err
 		}
-		if err := a.setPin(a.dcMotors[dcMotor].in2Pin, 1); err != nil {
+		if err := d.setPin(d.dcMotors[dcMotor].in2Pin, 1); err != nil {
 			return err
 		}
 	case Adafruit2348Release:
-		if err := a.setPin(a.dcMotors[dcMotor].in1Pin, 0); err != nil {
+		if err := d.setPin(d.dcMotors[dcMotor].in1Pin, 0); err != nil {
 			return err
 		}
-		if err := a.setPin(a.dcMotors[dcMotor].in2Pin, 0); err != nil {
+		if err := d.setPin(d.dcMotors[dcMotor].in2Pin, 0); err != nil {
 			return err
 		}
 	}
 	return nil
 }
 
 // SetStepperMotorSpeed sets the seconds-per-step for the given stepper motor. It is applied in the next cycle.
-func (a *Adafruit2348Driver) SetStepperMotorSpeed(stepperMotor int, rpm int) error {
-	a.stepperSpeedMutex.Lock()
-	defer a.stepperSpeedMutex.Unlock()
+func (d *Adafruit2348Driver) SetStepperMotorSpeed(stepperMotor int, rpm int) error {
+	d.stepperSpeedMutex.Lock()
+	defer d.stepperSpeedMutex.Unlock()
 
-	revSteps := a.stepperMotors[stepperMotor].revSteps
-	a.stepperMotors[stepperMotor].secPerStep = 60.0 / float64(revSteps*rpm)
+	revSteps := d.stepperMotors[stepperMotor].revSteps
+	d.stepperMotors[stepperMotor].secPerStep = 60.0 / float64(revSteps*rpm)
 	return nil
 }
 
 // Step will rotate the stepper motor the given number of steps, in the given direction and step style.
-func (a *Adafruit2348Driver) Step(motor, steps int, dir Adafruit2348Direction, style Adafruit2348StepStyle) error {
-	a.stepperSpeedMutex.Lock()
-	defer a.stepperSpeedMutex.Unlock()
+func (d *Adafruit2348Driver) Step(motor, steps int, dir Adafruit2348Direction, style Adafruit2348StepStyle) error {
+	d.stepperSpeedMutex.Lock()
+	defer d.stepperSpeedMutex.Unlock()
 
-	secPerStep := a.stepperMotors[motor].secPerStep
+	secPerStep := d.stepperMotors[motor].secPerStep
 	var latestStep int
 	var err error
 	if style == Adafruit2348Interleave {
@@ -186,7 +186,7 @@ func (a *Adafruit2348Driver) Step(motor, steps int, dir Adafruit2348Direction, s
 		log.Printf("[adafruit2348_driver] %f seconds per step", secPerStep)
 	}
 	for i := 0; i < steps; i++ {
-		if latestStep, err = a.oneStep(motor, dir, style); err != nil {
+		if latestStep, err = d.oneStep(motor, dir, style); err != nil {
 			return err
 		}
 		time.Sleep(time.Duration(secPerStep) * time.Second)
@@ -195,7 +195,7 @@ func (a *Adafruit2348Driver) Step(motor, steps int, dir Adafruit2348Direction, s
 	// This is an edge case, if we are in between full steps, keep going to end on a full step
 	if style == Adafruit2348Microstep {
 		for latestStep != 0 && latestStep != adafruit2348StepperMicrosteps {
-			if latestStep, err = a.oneStep(motor, dir, style); err != nil {
+			if latestStep, err = d.oneStep(motor, dir, style); err != nil {
 				return err
 			}
 			time.Sleep(time.Duration(secPerStep) * time.Second)
@@ -204,94 +204,94 @@ func (a *Adafruit2348Driver) Step(motor, steps int, dir Adafruit2348Direction, s
 	return nil
 }
 
-func (a *Adafruit2348Driver) oneStep(motor int, dir Adafruit2348Direction, style Adafruit2348StepStyle) (int, error) {
+func (d *Adafruit2348Driver) oneStep(motor int, dir Adafruit2348Direction, style Adafruit2348StepStyle) (int, error) {
 	pwmA := 255
 	pwmB := 255
 
 	// Determine the stepping procedure
 	switch style {
 	case Adafruit2348Single:
-		if (a.stepperMotors[motor].currentStep / (adafruit2348StepperMicrosteps / 2) % 2) != 0 {
+		if (d.stepperMotors[motor].currentStep / (adafruit2348StepperMicrosteps / 2) % 2) != 0 {
 			// we're at an odd step
 			if dir == Adafruit2348Forward {
-				a.stepperMotors[motor].currentStep += adafruit2348StepperMicrosteps / 2
+				d.stepperMotors[motor].currentStep += adafruit2348StepperMicrosteps / 2
 			} else {
-				a.stepperMotors[motor].currentStep -= adafruit2348StepperMicrosteps / 2
+				d.stepperMotors[motor].currentStep -= adafruit2348StepperMicrosteps / 2
 			}
 		} else {
 			// go to next even step
 			if dir == Adafruit2348Forward {
-				a.stepperMotors[motor].currentStep += adafruit2348StepperMicrosteps
+				d.stepperMotors[motor].currentStep += adafruit2348StepperMicrosteps
 			} else {
-				a.stepperMotors[motor].currentStep -= adafruit2348StepperMicrosteps
+				d.stepperMotors[motor].currentStep -= adafruit2348StepperMicrosteps
 			}
 		}
 	case Adafruit2348Double:
-		if (a.stepperMotors[motor].currentStep / (adafruit2348StepperMicrosteps / 2) % 2) == 0 {
+		if (d.stepperMotors[motor].currentStep / (adafruit2348StepperMicrosteps / 2) % 2) == 0 {
 			// we're at an even step, weird
 			if dir == Adafruit2348Forward {
-				a.stepperMotors[motor].currentStep += adafruit2348StepperMicrosteps / 2
+				d.stepperMotors[motor].currentStep += adafruit2348StepperMicrosteps / 2
 			} else {
-				a.stepperMotors[motor].currentStep -= adafruit2348StepperMicrosteps / 2
+				d.stepperMotors[motor].currentStep -= adafruit2348StepperMicrosteps / 2
 			}
 		} else {
 			// go to next odd step
 			if dir == Adafruit2348Forward {
-				a.stepperMotors[motor].currentStep += adafruit2348StepperMicrosteps
+				d.stepperMotors[motor].currentStep += adafruit2348StepperMicrosteps
 			} else {
-				a.stepperMotors[motor].currentStep -= adafruit2348StepperMicrosteps
+				d.stepperMotors[motor].currentStep -= adafruit2348StepperMicrosteps
 			}
 		}
 	case Adafruit2348Interleave:
 		if dir == Adafruit2348Forward {
-			a.stepperMotors[motor].currentStep += adafruit2348StepperMicrosteps / 2
+			d.stepperMotors[motor].currentStep += adafruit2348StepperMicrosteps / 2
 		} else {
-			a.stepperMotors[motor].currentStep -= adafruit2348StepperMicrosteps / 2
+			d.stepperMotors[motor].currentStep -= adafruit2348StepperMicrosteps / 2
 		}
 	case Adafruit2348Microstep:
 		if dir == Adafruit2348Forward {
-			a.stepperMotors[motor].currentStep++
+			d.stepperMotors[motor].currentStep++
 		} else {
-			a.stepperMotors[motor].currentStep--
+			d.stepperMotors[motor].currentStep--
 		}
 		// go to next step and wrap around
-		a.stepperMotors[motor].currentStep += adafruit2348StepperMicrosteps * 4
-		a.stepperMotors[motor].currentStep %= adafruit2348StepperMicrosteps * 4
+		d.stepperMotors[motor].currentStep += adafruit2348StepperMicrosteps * 4
+		d.stepperMotors[motor].currentStep %= adafruit2348StepperMicrosteps * 4
 
 		pwmA = 0
 		pwmB = 0
-		currStep := a.stepperMotors[motor].currentStep
+		currStep := d.stepperMotors[motor].currentStep
 		switch {
 		case currStep >= 0 && currStep < adafruit2348StepperMicrosteps:
-			pwmA = a.stepperMicrostepCurve[adafruit2348StepperMicrosteps-currStep]
-			pwmB = a.stepperMicrostepCurve[currStep]
+			pwmA = d.stepperMicrostepCurve[adafruit2348StepperMicrosteps-currStep]
+			pwmB = d.stepperMicrostepCurve[currStep]
 		case currStep >= adafruit2348StepperMicrosteps && currStep < adafruit2348StepperMicrosteps*2:
-			pwmA = a.stepperMicrostepCurve[currStep-adafruit2348StepperMicrosteps]
-			pwmB = a.stepperMicrostepCurve[adafruit2348StepperMicrosteps*2-currStep]
+			pwmA = d.stepperMicrostepCurve[currStep-adafruit2348StepperMicrosteps]
+			pwmB = d.stepperMicrostepCurve[adafruit2348StepperMicrosteps*2-currStep]
 		case currStep >= adafruit2348StepperMicrosteps*2 && currStep < adafruit2348StepperMicrosteps*3:
-			pwmA = a.stepperMicrostepCurve[adafruit2348StepperMicrosteps*3-currStep]
-			pwmB = a.stepperMicrostepCurve[currStep-adafruit2348StepperMicrosteps*2]
+			pwmA = d.stepperMicrostepCurve[adafruit2348StepperMicrosteps*3-currStep]
+			pwmB = d.stepperMicrostepCurve[currStep-adafruit2348StepperMicrosteps*2]
 		case currStep >= adafruit2348StepperMicrosteps*3 && currStep < adafruit2348StepperMicrosteps*4:
-			pwmA = a.stepperMicrostepCurve[currStep-adafruit2348StepperMicrosteps*3]
-			pwmB = a.stepperMicrostepCurve[adafruit2348StepperMicrosteps*4-currStep]
+			pwmA = d.stepperMicrostepCurve[currStep-adafruit2348StepperMicrosteps*3]
+			pwmB = d.stepperMicrostepCurve[adafruit2348StepperMicrosteps*4-currStep]
 		}
 	} // switch
 
 	// go to next 'step' and wrap around
-	a.stepperMotors[motor].currentStep += adafruit2348StepperMicrosteps * 4
-	a.stepperMotors[motor].currentStep %= adafruit2348StepperMicrosteps * 4
+	d.stepperMotors[motor].currentStep += adafruit2348StepperMicrosteps * 4
+	d.stepperMotors[motor].currentStep %= adafruit2348StepperMicrosteps * 4
 
 	// only really used for microstepping, otherwise always on!
 	//nolint:gosec // TODO: fix later
-	if err := a.SetPWM(int(a.stepperMotors[motor].pwmPinA), 0, uint16(pwmA*16)); err != nil {
+	if err := d.SetPWM(int(d.stepperMotors[motor].pwmPinA), 0, uint16(pwmA*16)); err != nil {
 		return 0, err
 	}
 	//nolint:gosec // TODO: fix later
-	if err := a.SetPWM(int(a.stepperMotors[motor].pwmPinB), 0, uint16(pwmB*16)); err != nil {
+	if err := d.SetPWM(int(d.stepperMotors[motor].pwmPinB), 0, uint16(pwmB*16)); err != nil {
 		return 0, err
 	}
 	var coils []int32
-	currStep := a.stepperMotors[motor].currentStep
+	currStep := d.stepperMotors[motor].currentStep
 	if style == Adafruit2348Microstep {
 		switch {
 		case currStep >= 0 && currStep < adafruit2348StepperMicrosteps:
@@ -305,34 +305,34 @@ func (a *Adafruit2348Driver) oneStep(motor int, dir Adafruit2348Direction, style
 		}
 	} else {
 		// step-2-coils is initialized in init()
-		coils = a.step2coils[(currStep / (adafruit2348StepperMicrosteps / 2))]
+		coils = d.step2coils[(currStep / (adafruit2348StepperMicrosteps / 2))]
 	}
 	if adafruit2348Debug {
 		log.Printf("[adafruit2348_driver] currStep: %d, index into step2coils: %d\n",
 			currStep, (currStep / (adafruit2348StepperMicrosteps / 2)))
 		log.Printf("[adafruit2348_driver] coils state = %v", coils)
 	}
-	if err := a.setPin(a.stepperMotors[motor].ain2, coils[0]); err != nil {
+	if err := d.setPin(d.stepperMotors[motor].ain2, coils[0]); err != nil {
 		return 0, err
 	}
-	if err := a.setPin(a.stepperMotors[motor].bin1, coils[1]); err != nil {
+	if err := d.setPin(d.stepperMotors[motor].bin1, coils[1]); err != nil {
 		return 0, err
 	}
-	if err := a.setPin(a.stepperMotors[motor].ain1, coils[2]); err != nil {
+	if err := d.setPin(d.stepperMotors[motor].ain1, coils[2]); err != nil {
 		return 0, err
 	}
-	if err := a.setPin(a.stepperMotors[motor].bin2, coils[3]); err != nil {
+	if err := d.setPin(d.stepperMotors[motor].bin2, coils[3]); err != nil {
 		return 0, err
 	}
-	return a.stepperMotors[motor].currentStep, nil
+	return d.stepperMotors[motor].currentStep, nil
 }
 
-func (a *Adafruit2348Driver) setPin(pin byte, value int32) error {
+func (d *Adafruit2348Driver) setPin(pin byte, value int32) error {
 	if value == 0 {
-		return a.SetPWM(int(pin), 0, 4096)
+		return d.SetPWM(int(pin), 0, 4096)
 	}
 	if value == 1 {
-		return a.SetPWM(int(pin), 4096, 0)
+		return d.SetPWM(int(pin), 4096, 0)
 	}
 	return errors.New("invalid pin")
 }

drivers/i2c/adafruit2348_driver_test.go

@@ -33,6 +33,15 @@ func TestNewAdafruit2348Driver(t *testing.T) {
 	assert.Equal(t, 0x60, d.GetAddressOrDefault(d.defaultAddress)) // the really used address
 }
 
+func TestAdafruit2348Halt(t *testing.T) {
+	// arrange
+	d := NewAdafruit2348Driver(newI2cTestAdaptor())
+	// act, assert
+	require.NoError(t, d.Halt()) // must be idempotent
+	require.NoError(t, d.Start())
+	require.NoError(t, d.Halt())
+}
+
 func TestAdafruit2348Options(t *testing.T) {
 	// This is a general test, that options are applied in constructor by using the common WithBus() option and
 	// least one of this driver. Further tests for options can also be done by call of "WithOption(val)(d)".

drivers/i2c/ads1x15_driver.go

@@ -476,11 +476,11 @@ func (d *ADS1x15Driver) waitForConversionFinished(delay time.Duration) error {
 }
 
 func (d *ADS1x15Driver) writeWordBigEndian(reg uint8, val uint16) error {
-	return d.connection.WriteWordData(reg, swapBytes(val))
+	return d.writeWordData(reg, swapBytes(val))
 }
 
 func (d *ADS1x15Driver) readWordBigEndian(reg uint8) (uint16, error) {
-	data, err := d.connection.ReadWordData(reg)
+	data, err := d.readWordData(reg)
 	if err != nil {
 		return 0, err
 	}