Skip to content

i2c(all): ensure halt is idempotent #1171

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
gen2thomas merged 1 commit into from
Oct 4, 2025
Merged

i2c(all): ensure halt is idempotent #1171

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 31 additions & 23 deletions drivers/i2c/adafruit1109_driver.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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, ", "))
}
Expand Down
3 changes: 2 additions & 1 deletion drivers/i2c/adafruit1109_driver_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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())
}

Expand Down
9 changes: 9 additions & 0 deletions drivers/i2c/adafruit2327_driver_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)".
Expand Down
120 changes: 60 additions & 60 deletions drivers/i2c/adafruit2348_driver.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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)
Expand All @@ -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)
Expand All @@ -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:
Expand All @@ -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")
}
9 changes: 9 additions & 0 deletions drivers/i2c/adafruit2348_driver_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)".
Expand Down
4 changes: 2 additions & 2 deletions drivers/i2c/ads1x15_driver.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
Loading