Skip to content

wiichuck: fix go routine leaking and concurrency problem #1172

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

wiichuck: fix go routine leaking and concurrency problem #1172

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
175 changes: 103 additions & 72 deletions drivers/i2c/wiichuck_driver.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,18 +19,23 @@ const (
Z = "z"
)

const wiichuckDefaultAddress = 0x52
const (
wiichuckDefaultAddress = 0x52
wiichuckStopTimeout = 2 * time.Second
)

// WiichuckDriver contains the attributes for the i2c driver
type WiichuckDriver struct {
*Driver
gobot.Eventer

interval time.Duration
pauseTime time.Duration
mtx sync.Mutex
joystick map[string]float64
data map[string]float64
interval time.Duration
pauseTime time.Duration
mtx sync.Mutex
doneChan chan struct{}
reallyDoneChan chan struct{}
joystick map[string]float64
data map[string]float64
}

// NewWiichuckDriver creates a WiichuckDriver with specified i2c interface.
Expand All @@ -44,7 +49,7 @@ type WiichuckDriver struct {
// i2c.WithBus(int): bus to use with this driver
// i2c.WithAddress(int): address to use with this driver
func NewWiichuckDriver(c Connector, options ...func(Config)) *WiichuckDriver {
w := &WiichuckDriver{
d := &WiichuckDriver{
Driver: NewDriver(c, "Wiichuck", wiichuckDefaultAddress),
interval: 10 * time.Millisecond,
pauseTime: 1 * time.Millisecond,
Expand All @@ -60,132 +65,158 @@ func NewWiichuckDriver(c Connector, options ...func(Config)) *WiichuckDriver {
"c": 0,
},
}
w.afterStart = w.initialize
d.afterStart = d.initialize
d.beforeHalt = d.shutdown

for _, option := range options {
option(w)
option(d)
}

w.AddEvent(Z)
w.AddEvent(C)
w.AddEvent(Joystick)
w.AddEvent(Error)
d.AddEvent(Z)
d.AddEvent(C)
d.AddEvent(Joystick)
d.AddEvent(Error)

return w
return d
}

// Joystick returns the current value for the joystick
func (w *WiichuckDriver) Joystick() map[string]float64 {
func (d *WiichuckDriver) Joystick() map[string]float64 {
val := make(map[string]float64)
w.mtx.Lock()
defer w.mtx.Unlock()
val["sx_origin"] = w.joystick["sx_origin"]
val["sy_origin"] = w.joystick["sy_origin"]
d.mtx.Lock()
defer d.mtx.Unlock()
val["sx_origin"] = d.joystick["sx_origin"]
val["sy_origin"] = d.joystick["sy_origin"]
return val
}

// update parses value to update buttons and joystick.
// If value is encrypted, warning message is printed
func (w *WiichuckDriver) update(value []byte) error {
if w.isEncrypted(value) {
func (d *WiichuckDriver) update(value []byte) error {
if d.isEncrypted(value) {
return fmt.Errorf("encrypted bytes")
}

w.parse(value)
w.adjustOrigins()
w.updateButtons()
w.updateJoystick()
d.parse(value)
d.adjustOrigins()
d.updateButtons()
d.updateJoystick()
return nil
}

// setJoystickDefaultValue sets default value if value is -1
func (w *WiichuckDriver) setJoystickDefaultValue(joystickAxis string, defaultValue float64) {
w.mtx.Lock()
defer w.mtx.Unlock()
if w.joystick[joystickAxis] == -1 {
w.joystick[joystickAxis] = defaultValue
func (d *WiichuckDriver) setJoystickDefaultValue(joystickAxis string, defaultValue float64) {
d.mtx.Lock()
defer d.mtx.Unlock()
if d.joystick[joystickAxis] == -1 {
d.joystick[joystickAxis] = defaultValue
}
}

// calculateJoystickValue returns distance between axis and origin
func (w *WiichuckDriver) calculateJoystickValue(axis float64, origin float64) float64 {
func (d *WiichuckDriver) calculateJoystickValue(axis float64, origin float64) float64 {
return axis - origin
}

// isEncrypted returns true if value is encrypted
func (w *WiichuckDriver) isEncrypted(value []byte) bool {
func (d *WiichuckDriver) isEncrypted(value []byte) bool {
if value[0] == value[1] && value[2] == value[3] && value[4] == value[5] {
return true
}
return false
}

// decode removes encoding from `x` byte
func (w *WiichuckDriver) decode(x byte) float64 {
func (d *WiichuckDriver) decode(x byte) float64 {
return float64((x ^ 0x17) + 0x17)
}

// adjustOrigins sets sy_origin and sx_origin with values from data
func (w *WiichuckDriver) adjustOrigins() {
w.setJoystickDefaultValue("sy_origin", w.data["sy"])
w.setJoystickDefaultValue("sx_origin", w.data["sx"])
func (d *WiichuckDriver) adjustOrigins() {
d.setJoystickDefaultValue("sy_origin", d.data["sy"])
d.setJoystickDefaultValue("sx_origin", d.data["sx"])
}

// updateButtons publishes "c" and "x" events if present in data
func (w *WiichuckDriver) updateButtons() {
if w.data["c"] == 0 {
w.Publish(w.Event(C), true)
func (d *WiichuckDriver) updateButtons() {
if d.data["c"] == 0 {
d.Publish(d.Event(C), true)
}
if w.data["z"] == 0 {
w.Publish(w.Event(Z), true)
if d.data["z"] == 0 {
d.Publish(d.Event(Z), true)
}
}

// updateJoystick publishes event with current x and y values for joystick
func (w *WiichuckDriver) updateJoystick() {
joy := w.Joystick()
w.Publish(w.Event(Joystick), map[string]float64{
"x": w.calculateJoystickValue(w.data["sx"], joy["sx_origin"]),
"y": w.calculateJoystickValue(w.data["sy"], joy["sy_origin"]),
func (d *WiichuckDriver) updateJoystick() {
joy := d.Joystick()
d.Publish(d.Event(Joystick), map[string]float64{
"x": d.calculateJoystickValue(d.data["sx"], joy["sx_origin"]),
"y": d.calculateJoystickValue(d.data["sy"], joy["sy_origin"]),
})
}

// parse sets driver values based on parsed value
func (w *WiichuckDriver) parse(value []byte) {
w.data["sx"] = w.decode(value[0])
w.data["sy"] = w.decode(value[1])
w.data["z"] = float64(uint8(w.decode(value[5])) & 0x01)
w.data["c"] = float64(uint8(w.decode(value[5])) & 0x02)
func (d *WiichuckDriver) parse(value []byte) {
d.data["sx"] = d.decode(value[0])
d.data["sy"] = d.decode(value[1])
d.data["z"] = float64(uint8(d.decode(value[5])) & 0x01)
d.data["c"] = float64(uint8(d.decode(value[5])) & 0x02)
}

// reads from adaptor using specified interval to update with new value
func (w *WiichuckDriver) initialize() error {
func (d *WiichuckDriver) initialize() error {
d.doneChan = make(chan struct{})
d.reallyDoneChan = make(chan struct{})

go func() {
for {
if _, err := w.connection.Write([]byte{0x40, 0x00}); err != nil {
w.Publish(w.Event(Error), err)
continue
}
time.Sleep(w.pauseTime)
if _, err := w.connection.Write([]byte{0x00}); err != nil {
w.Publish(w.Event(Error), err)
continue
}
time.Sleep(w.pauseTime)
newValue := make([]byte, 6)
bytesRead, err := w.connection.Read(newValue)
if err != nil {
w.Publish(w.Event(Error), err)
continue
}
if bytesRead == 6 {
if err = w.update(newValue); err != nil {
w.Publish(w.Event(Error), err)
select {
case <-d.doneChan:
if d.reallyDoneChan != nil {
close(d.reallyDoneChan)
}
return
default:
if _, err := d.connection.Write([]byte{0x40, 0x00}); err != nil {
d.Publish(d.Event(Error), err)
continue
}
time.Sleep(d.pauseTime)
if _, err := d.connection.Write([]byte{0x00}); err != nil {
d.Publish(d.Event(Error), err)
continue
}
time.Sleep(d.pauseTime)
newValue := make([]byte, 6)
bytesRead, err := d.connection.Read(newValue)
if err != nil {
d.Publish(d.Event(Error), err)
continue
}
if bytesRead == 6 {
if err = d.update(newValue); err != nil {
d.Publish(d.Event(Error), err)
continue
}
}
time.Sleep(d.interval)
}
time.Sleep(w.interval)
}
}()
return nil
}

func (d *WiichuckDriver) shutdown() error {
if d.doneChan != nil {
close(d.doneChan)
// wait until go routine is really finished, so connection can be set to nil safely
select {
case <-time.After(wiichuckStopTimeout):
return fmt.Errorf("go routine not finished in %s", wiichuckStopTimeout)
case <-d.reallyDoneChan:
}
}

return nil
}
6 changes: 5 additions & 1 deletion drivers/i2c/wiichuck_driver_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,11 @@ func TestWiichuckDriverStart(t *testing.T) {
}

func TestWiichuckDriverHalt(t *testing.T) {
d := initTestWiichuckDriverWithStubbedAdaptor()
// arrange
d := NewWiichuckDriver(newI2cTestAdaptor())
// act, assert
require.NoError(t, d.Halt()) // must be idempotent
require.NoError(t, d.Start())
require.NoError(t, d.Halt())
}

Expand Down