From 41baeb565b7c1c62f7e5df39d197dca8b0ca6a1b Mon Sep 17 00:00:00 2001 From: bezineb5 Date: Sun, 11 Nov 2018 09:58:27 -0500 Subject: [PATCH] ads1x15: fix synchronization (#325) * Fixed race condition * Added unit tests * Do not close "stop" channel --- experimental/devices/ads1x15/ads1x15.go | 24 +++- experimental/devices/ads1x15/ads1x15_test.go | 128 +++++++++++++++++++ 2 files changed, 147 insertions(+), 5 deletions(-) diff --git a/experimental/devices/ads1x15/ads1x15.go b/experimental/devices/ads1x15/ads1x15.go index 49a1b0b..272ff29 100644 --- a/experimental/devices/ads1x15/ads1x15.go +++ b/experimental/devices/ads1x15/ads1x15.go @@ -90,7 +90,7 @@ type ConversionQuality int const ( // SaveEnergy optimizes the power consumption of the ADC, at the expense of - // the quality by converting at the gihest rate possible. + // the quality by converting at the highest possible rate. SaveEnergy ConversionQuality = 0 // BestQuality will use the lowest suitable data rate to reduce the impact of // the noise on the reading. @@ -394,6 +394,7 @@ type analogPin struct { waitTime time.Duration requestedFrequency physic.Frequency stop chan struct{} + mu sync.Mutex } // Range returns the maximum supported range [min, max] of the values. @@ -409,14 +410,22 @@ func (p *analogPin) Read() (analog.Reading, error) { } func (p *analogPin) ReadContinuous() <-chan analog.Reading { - p.Halt() - reading := make(chan analog.Reading) + // We need to lock if there are multiple Halt or ReadContinuous + // calls simultaneously. + p.mu.Lock() + defer p.mu.Unlock() + + // First release the current continuous reading if there is one + if p.stop != nil { + p.stop <- struct{}{} + p.stop = nil + } + reading := make(chan analog.Reading, 16) p.stop = make(chan struct{}) go func() { t := time.NewTicker(p.requestedFrequency.Duration()) defer t.Stop() - defer p.Halt() defer close(reading) for { @@ -450,8 +459,13 @@ func (p *analogPin) Function() string { } func (p *analogPin) Halt() error { + // We need to lock if there are multiple Halt or ReadContinuous + // calls simultaneously. + p.mu.Lock() + defer p.mu.Unlock() + if p.stop != nil { - close(p.stop) + p.stop <- struct{}{} p.stop = nil } return nil diff --git a/experimental/devices/ads1x15/ads1x15_test.go b/experimental/devices/ads1x15/ads1x15_test.go index 551bbf8..9b9dc86 100644 --- a/experimental/devices/ads1x15/ads1x15_test.go +++ b/experimental/devices/ads1x15/ads1x15_test.go @@ -8,6 +8,7 @@ import ( "testing" "periph.io/x/periph/conn/i2c/i2ctest" + "periph.io/x/periph/conn/physic" ) func TestChannel_String(t *testing.T) { @@ -69,3 +70,130 @@ func TestDev_String(t *testing.T) { t.Fatal(err) } } + +func TestPinADC_Read(t *testing.T) { + b := i2ctest.Playback{ + Ops: []i2ctest.IO{ + { + Addr: 0x48, + W: []byte{0x1, 0x91, 0xc3}, + R: []byte{}, + }, + { + Addr: 0x48, + W: []byte{0x0}, + R: []byte{0xff, 0x50}, + }, + }, + } + + d, err := NewADS1015(&b, &DefaultOpts) + if err != nil { + t.Fatal(err) + } + // Obtain an analog pin from the ADC + pin, err := d.PinForChannel(Channel0Minus3, 5*physic.Volt, 1*physic.Hertz, SaveEnergy) + if err != nil { + t.Fatal(err) + } + + // Read values from ADC. + reading, err := pin.Read() + if err != nil { + t.Fatal(err) + } + + if reading.Raw != -176 { + t.Fatalf("Found %d, expected %d", reading.Raw, -176) + } + + if reading.V != -33*physic.MilliVolt { + t.Fatalf("Found %s, expected %s", reading.V, -33*physic.MilliVolt) + } + + if err := pin.Halt(); err != nil { + t.Fatal(err) + } + + if err := d.Halt(); err != nil { + t.Fatal(err) + } +} + +func TestPinADC_ReadContinous(t *testing.T) { + b := i2ctest.Playback{ + Ops: []i2ctest.IO{ + { + Addr: 0x48, + W: []byte{0x1, 0x91, 0x3}, + R: []byte{}, + }, + { + Addr: 0x48, + W: []byte{0x0}, + R: []byte{0x52, 0xd0}, + }, + { + Addr: 0x48, + W: []byte{0x1, 0x91, 0x3}, + R: []byte{}, + }, + { + Addr: 0x48, + W: []byte{0x0}, + R: []byte{0x52, 0xc0}, + }, + // We add one extra exchange, as halting the polling is not instant + { + Addr: 0x48, + W: []byte{0x1, 0x91, 0x3}, + R: []byte{}, + }, + { + Addr: 0x48, + W: []byte{0x0}, + R: []byte{0x52, 0xc0}, + }, + }, + } + + rawValues := []int32{21200, 21184} + voltValues := []physic.ElectricPotential{3975 * physic.MilliVolt, 3972 * physic.MilliVolt} + + d, err := NewADS1015(&b, &DefaultOpts) + if err != nil { + t.Fatal(err) + } + // Obtain an analog pin from the ADC + pin, err := d.PinForChannel(Channel0Minus3, 5*physic.Volt, 100*physic.Hertz, BestQuality) + if err != nil { + t.Fatal(err) + } + + // Read values from ADC. + c := pin.ReadContinuous() + + var i = 0 + for reading := range c { + if reading.Raw != rawValues[i] { + t.Fatalf("Found %d, expected %d", reading.Raw, rawValues[i]) + } + + if reading.V != voltValues[i] { + t.Fatalf("Found %s, expected %s", reading.V, voltValues[i]) + } + + i++ + if i >= len(rawValues) { + break + } + } + + if err := pin.Halt(); err != nil { + t.Fatal(err) + } + + if err := d.Halt(); err != nil { + t.Fatal(err) + } +}