diff --git a/adaptor.go b/adaptor.go index ef4730bda..5456136f7 100644 --- a/adaptor.go +++ b/adaptor.go @@ -99,6 +99,14 @@ type PWMPinnerProvider interface { PWMPin(id string) (PWMPinner, error) } +// AnalogPinner is the interface for system analog io interactions +type AnalogPinner interface { + // Read reads the current value of the pin + Read() (int, error) + // Write writes to the pin + Write(val int) error +} + // I2cSystemDevicer is the interface to a i2c bus at system level, according to I2C/SMBus specification. // Some functions are not in the interface yet: // * Process Call (WriteWordDataReadWordData) diff --git a/platforms/adaptors/analogpinsadaptor.go b/platforms/adaptors/analogpinsadaptor.go new file mode 100644 index 000000000..59a210d14 --- /dev/null +++ b/platforms/adaptors/analogpinsadaptor.go @@ -0,0 +1,95 @@ +package adaptors + +import ( + "fmt" + "sync" + + "gobot.io/x/gobot/v2" + "gobot.io/x/gobot/v2/system" +) + +type analogPinTranslator func(pin string) (path string, r, w bool, bufLen uint16, err error) + +// AnalogPinsAdaptor is a adaptor for analog pins, normally used for composition in platforms. +// It is also usable for general sysfs access. +type AnalogPinsAdaptor struct { + sys *system.Accesser + translate analogPinTranslator + pins map[string]gobot.AnalogPinner + mutex sync.Mutex +} + +// NewAnalogPinsAdaptor provides the access to analog pins of the board. Usually sysfs system drivers are used. +// The translator is used to adapt the pin header naming, which is given by user, to the internal file name +// nomenclature. This varies by each platform. +func NewAnalogPinsAdaptor(sys *system.Accesser, t analogPinTranslator) *AnalogPinsAdaptor { + a := AnalogPinsAdaptor{ + sys: sys, + translate: t, + } + return &a +} + +// Connect prepare new connection to analog pins. +func (a *AnalogPinsAdaptor) Connect() error { + a.mutex.Lock() + defer a.mutex.Unlock() + + a.pins = make(map[string]gobot.AnalogPinner) + return nil +} + +// Finalize closes connection to analog pins +func (a *AnalogPinsAdaptor) Finalize() error { + a.mutex.Lock() + defer a.mutex.Unlock() + + a.pins = nil + return nil +} + +// AnalogRead returns an analog value from specified pin or identifier, defined by the translation function. +func (a *AnalogPinsAdaptor) AnalogRead(id string) (int, error) { + a.mutex.Lock() + defer a.mutex.Unlock() + + pin, err := a.analogPin(id) + if err != nil { + return 0, err + } + + return pin.Read() +} + +// AnalogWrite writes an analog value to the specified pin or identifier, defined by the translation function. +func (a *AnalogPinsAdaptor) AnalogWrite(id string, val int) error { + a.mutex.Lock() + defer a.mutex.Unlock() + + pin, err := a.analogPin(id) + if err != nil { + return err + } + + return pin.Write(val) +} + +// analogPin initializes the pin for analog access and returns matched pin for specified identifier. +func (a *AnalogPinsAdaptor) analogPin(id string) (gobot.AnalogPinner, error) { + if a.pins == nil { + return nil, fmt.Errorf("not connected for pin %s", id) + } + + pin := a.pins[id] + + if pin == nil { + path, r, w, bufLen, err := a.translate(id) + if err != nil { + return nil, err + } + pin = a.sys.NewAnalogPin(path, r, w, bufLen) + a.pins[id] = pin + } + + return pin, nil +} diff --git a/platforms/adaptors/analogpinsadaptor_test.go b/platforms/adaptors/analogpinsadaptor_test.go new file mode 100644 index 000000000..3a5289ed4 --- /dev/null +++ b/platforms/adaptors/analogpinsadaptor_test.go @@ -0,0 +1,249 @@ +//nolint:nonamedreturns // ok for tests +package adaptors + +import ( + "fmt" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "gobot.io/x/gobot/v2" + "gobot.io/x/gobot/v2/system" +) + +const ( + analogReadPath = "/sys/bus/iio/devices/iio:device0/in_voltage0_raw" + analogWritePath = "/sys/devices/platform/ff680020.pwm/pwm/pwmchip3/export" + analogReadWritePath = "/sys/devices/platform/ff680020.pwm/pwm/pwmchip3/pwm44/period" + analogReadWriteStringPath = "/sys/devices/platform/ff680020.pwm/pwm/pwmchip3/pwm44/polarity" +) + +var analogMockPaths = []string{ + analogReadPath, + analogWritePath, + analogReadWritePath, + analogReadWriteStringPath, +} + +func initTestAnalogPinsAdaptorWithMockedFilesystem(mockPaths []string) (*AnalogPinsAdaptor, *system.MockFilesystem) { + sys := system.NewAccesser() + fs := sys.UseMockFilesystem(mockPaths) + a := NewAnalogPinsAdaptor(sys, testAnalogPinTranslator) + fs.Files[analogReadPath].Contents = "54321" + fs.Files[analogWritePath].Contents = "0" + fs.Files[analogReadWritePath].Contents = "30000" + fs.Files[analogReadWriteStringPath].Contents = "inverted" + if err := a.Connect(); err != nil { + panic(err) + } + return a, fs +} + +func testAnalogPinTranslator(id string) (string, bool, bool, uint16, error) { + switch id { + case "read": + return analogReadPath, true, false, 10, nil + case "write": + return analogWritePath, false, true, 11, nil + case "read/write": + return analogReadWritePath, true, true, 12, nil + case "read/write_string": + return analogReadWriteStringPath, true, true, 13, nil + } + + return "", false, false, 0, fmt.Errorf("'%s' is not a valid id of a analog pin", id) +} + +func TestAnalogPinsConnect(t *testing.T) { + translate := func(id string) (path string, r, w bool, bufLen uint16, err error) { return } + a := NewAnalogPinsAdaptor(system.NewAccesser(), translate) + assert.Equal(t, (map[string]gobot.AnalogPinner)(nil), a.pins) + + err := a.AnalogWrite("write", 1) + require.ErrorContains(t, err, "not connected") + + err = a.Connect() + require.NoError(t, err) + assert.NotEqual(t, (map[string]gobot.AnalogPinner)(nil), a.pins) + assert.Empty(t, a.pins) +} + +func TestAnalogPinsFinalize(t *testing.T) { + // arrange + sys := system.NewAccesser() + fs := sys.UseMockFilesystem(analogMockPaths) + a := NewAnalogPinsAdaptor(sys, testAnalogPinTranslator) + fs.Files[analogReadPath].Contents = "0" + // assert that finalize before connect is working + require.NoError(t, a.Finalize()) + // arrange + require.NoError(t, a.Connect()) + require.NoError(t, a.AnalogWrite("write", 1)) + assert.Len(t, a.pins, 1) + // act + err := a.Finalize() + // assert + require.NoError(t, err) + assert.Empty(t, a.pins) + // assert that finalize after finalize is working + require.NoError(t, a.Finalize()) + // arrange missing file + require.NoError(t, a.Connect()) + require.NoError(t, a.AnalogWrite("write", 2)) + delete(fs.Files, analogWritePath) + err = a.Finalize() + require.NoError(t, err) // because there is currently no access on finalize + // arrange write error + require.NoError(t, a.Connect()) + require.NoError(t, a.AnalogWrite("read/write_string", 5)) + fs.WithWriteError = true + err = a.Finalize() + require.NoError(t, err) // because there is currently no access on finalize +} + +func TestAnalogPinsReConnect(t *testing.T) { + // arrange + a, _ := initTestAnalogPinsAdaptorWithMockedFilesystem(analogMockPaths) + require.NoError(t, a.AnalogWrite("read/write_string", 1)) + assert.Len(t, a.pins, 1) + require.NoError(t, a.Finalize()) + // act + err := a.Connect() + // assert + require.NoError(t, err) + assert.NotNil(t, a.pins) + assert.Empty(t, a.pins) +} + +func TestAnalogWrite(t *testing.T) { + tests := map[string]struct { + pin string + simulateWriteErr bool + simulateReadErr bool + wantValW string + wantValRW string + wantValRWS string + wantErr string + }{ + "write_w_pin": { + pin: "write", + wantValW: "100", + wantValRW: "30000", + wantValRWS: "inverted", + }, + "write_rw_pin": { + pin: "read/write_string", + wantValW: "0", + wantValRW: "30000", + wantValRWS: "100", + }, + "ok_on_read_error": { + pin: "read/write_string", + simulateReadErr: true, + wantValW: "0", + wantValRW: "30000", + wantValRWS: "100", + }, + "error_write_error": { + pin: "read/write_string", + simulateWriteErr: true, + wantValW: "0", + wantValRW: "30000", + wantValRWS: "inverted", + wantErr: "write error", + }, + "error_notexist": { + pin: "notexist", + wantValW: "0", + wantValRW: "30000", + wantValRWS: "inverted", + wantErr: "'notexist' is not a valid id of a analog pin", + }, + "error_write_not_allowed": { + pin: "read", + wantValW: "0", + wantValRW: "30000", + wantValRWS: "inverted", + wantErr: "the pin '/sys/bus/iio/devices/iio:device0/in_voltage0_raw' is not allowed to write (val: 100)", + }, + } + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + // arrange + a, fs := initTestAnalogPinsAdaptorWithMockedFilesystem(analogMockPaths) + fs.WithWriteError = tc.simulateWriteErr + fs.WithReadError = tc.simulateReadErr + // act + err := a.AnalogWrite(tc.pin, 100) + // assert + if tc.wantErr != "" { + require.EqualError(t, err, tc.wantErr) + } else { + require.NoError(t, err) + } + assert.Equal(t, "54321", fs.Files[analogReadPath].Contents) + assert.Equal(t, tc.wantValW, fs.Files[analogWritePath].Contents) + assert.Equal(t, tc.wantValRW, fs.Files[analogReadWritePath].Contents) + assert.Equal(t, tc.wantValRWS, fs.Files[analogReadWriteStringPath].Contents) + }) + } +} + +func TestAnalogRead(t *testing.T) { + tests := map[string]struct { + pin string + simulateReadErr bool + simulateWriteErr bool + wantVal int + wantErr string + }{ + "read_r_pin": { + pin: "read", + wantVal: 54321, + }, + "read_rw_pin": { + pin: "read/write", + wantVal: 30000, + }, + "ok_on_write_error": { + pin: "read", + simulateWriteErr: true, + wantVal: 54321, + }, + "error_read_error": { + pin: "read", + simulateReadErr: true, + wantErr: "read error", + }, + "error_notexist": { + pin: "notexist", + wantErr: "'notexist' is not a valid id of a analog pin", + }, + "error_invalid_syntax": { + pin: "read/write_string", + wantErr: "strconv.Atoi: parsing \"inverted\": invalid syntax", + }, + "error_read_not_allowed": { + pin: "write", + wantErr: "the pin '/sys/devices/platform/ff680020.pwm/pwm/pwmchip3/export' is not allowed to read", + }, + } + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + // arrange + a, fs := initTestAnalogPinsAdaptorWithMockedFilesystem(analogMockPaths) + fs.WithReadError = tc.simulateReadErr + fs.WithWriteError = tc.simulateWriteErr + // act + got, err := a.AnalogRead(tc.pin) + // assert + if tc.wantErr != "" { + require.EqualError(t, err, tc.wantErr) + } else { + require.NoError(t, err) + } + assert.Equal(t, tc.wantVal, got) + }) + } +} diff --git a/platforms/beaglebone/beaglebone_adaptor.go b/platforms/beaglebone/beaglebone_adaptor.go index 7048a3c66..87269eae4 100644 --- a/platforms/beaglebone/beaglebone_adaptor.go +++ b/platforms/beaglebone/beaglebone_adaptor.go @@ -14,12 +14,19 @@ import ( "gobot.io/x/gobot/v2/system" ) -type pwmPinData struct { +type pwmPinDefinition struct { channel int dir string dirRegexp string } +type analogPinDefinition struct { + path string + r bool // readable + w bool // writable + bufLen uint16 +} + const ( pwmPeriodDefault = 500000 // 0.5 ms = 2 kHz @@ -37,15 +44,15 @@ type Adaptor struct { name string sys *system.Accesser mutex sync.Mutex + *adaptors.AnalogPinsAdaptor *adaptors.DigitalPinsAdaptor *adaptors.PWMPinsAdaptor *adaptors.I2cBusAdaptor *adaptors.SpiBusAdaptor usrLed string - analogPath string pinMap map[string]int - pwmPinMap map[string]pwmPinData - analogPinMap map[string]string + pwmPinMap map[string]pwmPinDefinition + analogPinMap map[string]analogPinDefinition } // NewAdaptor returns a new Beaglebone Black/Green Adaptor @@ -63,9 +70,8 @@ func NewAdaptor(opts ...func(adaptors.Optioner)) *Adaptor { pwmPinMap: bbbPwmPinMap, analogPinMap: bbbAnalogPinMap, usrLed: "/sys/class/leds/beaglebone:green:", - analogPath: "/sys/bus/iio/devices/iio:device0", } - + c.AnalogPinsAdaptor = adaptors.NewAnalogPinsAdaptor(sys, c.translateAnalogPin) c.DigitalPinsAdaptor = adaptors.NewDigitalPinsAdaptor(sys, c.translateAndMuxDigitalPin, opts...) c.PWMPinsAdaptor = adaptors.NewPWMPinsAdaptor(sys, c.translateAndMuxPWMPin, adaptors.WithPWMPinDefaultPeriod(pwmPeriodDefault)) @@ -94,6 +100,10 @@ func (c *Adaptor) Connect() error { return err } + if err := c.AnalogPinsAdaptor.Connect(); err != nil { + return err + } + if err := c.PWMPinsAdaptor.Connect(); err != nil { return err } @@ -111,6 +121,10 @@ func (c *Adaptor) Finalize() error { err = multierror.Append(err, e) } + if e := c.AnalogPinsAdaptor.Finalize(); e != nil { + err = multierror.Append(err, e) + } + if e := c.I2cBusAdaptor.Finalize(); e != nil { err = multierror.Append(err, e) } @@ -140,29 +154,6 @@ func (c *Adaptor) DigitalWrite(id string, val byte) error { return c.DigitalPinsAdaptor.DigitalWrite(id, val) } -// AnalogRead returns an analog value from specified pin -func (c *Adaptor) AnalogRead(pin string) (int, error) { - analogPin, err := c.translateAnalogPin(pin) - if err != nil { - return 0, err - } - fi, err := c.sys.OpenFile(fmt.Sprintf("%v/%v", c.analogPath, analogPin), os.O_RDONLY, 0o644) - defer fi.Close() //nolint:staticcheck // for historical reasons - - if err != nil { - return 0, err - } - - buf := make([]byte, 1024) - _, err = fi.Read(buf) - if err != nil { - return 0, err - } - - val, _ := strconv.Atoi(strings.Split(string(buf), "\n")[0]) - return val, nil -} - func (c *Adaptor) validateSpiBusNumber(busNr int) error { // Valid bus numbers are [0,1] which corresponds to /dev/spidev0.x through /dev/spidev1.x. // x is the chip number <255 @@ -181,11 +172,13 @@ func (c *Adaptor) validateI2cBusNumber(busNr int) error { } // translateAnalogPin converts analog pin name to pin position -func (c *Adaptor) translateAnalogPin(pin string) (string, error) { - if val, ok := c.analogPinMap[pin]; ok { - return val, nil +func (c *Adaptor) translateAnalogPin(pin string) (string, bool, bool, uint16, error) { + pinInfo, ok := c.analogPinMap[pin] + if !ok { + return "", false, false, 0, fmt.Errorf("Not a valid analog pin") } - return "", fmt.Errorf("Not a valid analog pin") + + return pinInfo.path, pinInfo.r, pinInfo.w, pinInfo.bufLen, nil } // translatePin converts digital pin name to pin position @@ -219,7 +212,7 @@ func (c *Adaptor) translateAndMuxPWMPin(id string) (string, int, error) { return path, pinInfo.channel, nil } -func (p pwmPinData) findPWMDir(sys *system.Accesser) (string, error) { +func (p pwmPinDefinition) findPWMDir(sys *system.Accesser) (string, error) { items, _ := sys.Find(p.dir, p.dirRegexp) if len(items) == 0 { return "", fmt.Errorf("No path found for PWM directory pattern, '%s' in path '%s'", p.dirRegexp, p.dir) diff --git a/platforms/beaglebone/black_pins.go b/platforms/beaglebone/black_pins.go index 9d8357f46..c8f03ae98 100644 --- a/platforms/beaglebone/black_pins.go +++ b/platforms/beaglebone/black_pins.go @@ -49,7 +49,7 @@ var bbbPinMap = map[string]int{ "P9_31": 110, } -var bbbPwmPinMap = map[string]pwmPinData{ +var bbbPwmPinMap = map[string]pwmPinDefinition{ "P8_13": { dir: "/sys/devices/platform/ocp/48304000.epwmss/48304200.pwm/pwm/", dirRegexp: "pwmchip[0-9]+$", channel: 1, }, @@ -73,12 +73,12 @@ var bbbPwmPinMap = map[string]pwmPinData{ }, } -var bbbAnalogPinMap = map[string]string{ - "P9_39": "in_voltage0_raw", - "P9_40": "in_voltage1_raw", - "P9_37": "in_voltage2_raw", - "P9_38": "in_voltage3_raw", - "P9_33": "in_voltage4_raw", - "P9_36": "in_voltage5_raw", - "P9_35": "in_voltage6_raw", +var bbbAnalogPinMap = map[string]analogPinDefinition{ + "P9_39": {path: "/sys/bus/iio/devices/iio:device0/in_voltage0_raw", r: true, w: false, bufLen: 1024}, + "P9_40": {path: "/sys/bus/iio/devices/iio:device0/in_voltage1_raw", r: true, w: false, bufLen: 1024}, + "P9_37": {path: "/sys/bus/iio/devices/iio:device0/in_voltage2_raw", r: true, w: false, bufLen: 1024}, + "P9_38": {path: "/sys/bus/iio/devices/iio:device0/in_voltage3_raw", r: true, w: false, bufLen: 1024}, + "P9_33": {path: "/sys/bus/iio/devices/iio:device0/in_voltage4_raw", r: true, w: false, bufLen: 1024}, + "P9_36": {path: "/sys/bus/iio/devices/iio:device0/in_voltage5_raw", r: true, w: false, bufLen: 1024}, + "P9_35": {path: "/sys/bus/iio/devices/iio:device0/in_voltage6_raw", r: true, w: false, bufLen: 1024}, } diff --git a/platforms/beaglebone/pocketbeagle_pins.go b/platforms/beaglebone/pocketbeagle_pins.go index b138c3e60..c94dd42ad 100644 --- a/platforms/beaglebone/pocketbeagle_pins.go +++ b/platforms/beaglebone/pocketbeagle_pins.go @@ -76,7 +76,7 @@ var pocketBeaglePinMap = map[string]int{ // P2_36 - AIO7 } -var pocketBeaglePwmPinMap = map[string]pwmPinData{ +var pocketBeaglePwmPinMap = map[string]pwmPinDefinition{ "P1_33": {dir: "/sys/devices/platform/ocp/48300000.epwmss/48300200.pwm/pwm/", dirRegexp: "pwmchip[0-9]+$", channel: 1}, "P1_36": {dir: "/sys/devices/platform/ocp/48300000.epwmss/48300200.pwm/pwm/", dirRegexp: "pwmchip[0-9]+$", channel: 0}, @@ -84,11 +84,11 @@ var pocketBeaglePwmPinMap = map[string]pwmPinData{ "P2_3": {dir: "/sys/devices/platform/ocp/48304000.epwmss/48304200.pwm/pwm/", dirRegexp: "pwmchip[0-9]+$", channel: 1}, } -var pocketBeagleAnalogPinMap = map[string]string{ - "P1_19": "in_voltage0_raw", - "P1_21": "in_voltage1_raw", - "P1_23": "in_voltage2_raw", - "P1_25": "in_voltage3_raw", - "P1_27": "in_voltage4_raw", - "P2_36": "in_voltage7_raw", +var pocketBeagleAnalogPinMap = map[string]analogPinDefinition{ + "P1_19": {path: "/sys/bus/iio/devices/iio:device0/in_voltage0_raw", r: true, w: false, bufLen: 1024}, + "P1_21": {path: "/sys/bus/iio/devices/iio:device0/in_voltage1_raw", r: true, w: false, bufLen: 1024}, + "P1_23": {path: "/sys/bus/iio/devices/iio:device0/in_voltage2_raw", r: true, w: false, bufLen: 1024}, + "P1_25": {path: "/sys/bus/iio/devices/iio:device0/in_voltage3_raw", r: true, w: false, bufLen: 1024}, + "P1_27": {path: "/sys/bus/iio/devices/iio:device0/in_voltage4_raw", r: true, w: false, bufLen: 1024}, + "P2_36": {path: "/sys/bus/iio/devices/iio:device0/in_voltage7_raw", r: true, w: false, bufLen: 1024}, } diff --git a/platforms/intel-iot/edison/edison_adaptor.go b/platforms/intel-iot/edison/edison_adaptor.go index 17a7b24ff..07a1ca76c 100644 --- a/platforms/intel-iot/edison/edison_adaptor.go +++ b/platforms/intel-iot/edison/edison_adaptor.go @@ -40,6 +40,7 @@ type Adaptor struct { pinmap map[string]sysfsPin tristate gobot.DigitalPinner digitalPins map[int]gobot.DigitalPinner + *adaptors.AnalogPinsAdaptor *adaptors.PWMPinsAdaptor *adaptors.I2cBusAdaptor arduinoI2cInitialized bool @@ -58,6 +59,7 @@ func NewAdaptor(boardType ...string) *Adaptor { if len(boardType) > 0 && boardType[0] != "" { c.board = boardType[0] } + c.AnalogPinsAdaptor = adaptors.NewAnalogPinsAdaptor(sys, c.translateAnalogPin) c.PWMPinsAdaptor = adaptors.NewPWMPinsAdaptor(sys, c.translateAndMuxPWMPin, adaptors.WithPWMPinInitializer(pwmPinInitializer)) defI2cBusNr := defaultI2cBusNumber @@ -82,6 +84,10 @@ func (c *Adaptor) Connect() error { return err } + if err := c.AnalogPinsAdaptor.Connect(); err != nil { + return err + } + if err := c.PWMPinsAdaptor.Connect(); err != nil { return err } @@ -127,6 +133,10 @@ func (c *Adaptor) Finalize() error { err = multierror.Append(err, e) } + if e := c.AnalogPinsAdaptor.Finalize(); e != nil { + err = multierror.Append(err, e) + } + if e := c.I2cBusAdaptor.Finalize(); e != nil { err = multierror.Append(err, e) } @@ -165,14 +175,12 @@ func (c *Adaptor) DigitalPin(id string) (gobot.DigitalPinner, error) { // AnalogRead returns value from analog reading of specified pin func (c *Adaptor) AnalogRead(pin string) (int, error) { - buf, err := c.readFile("/sys/bus/iio/devices/iio:device1/in_voltage" + pin + "_raw") + rawRead, err := c.AnalogPinsAdaptor.AnalogRead(pin) if err != nil { return 0, err } - val, err := strconv.Atoi(string(buf[0 : len(buf)-1])) - - return val / 4, err + return rawRead / 4, err } func (c *Adaptor) validateAndSetupI2cBusNumber(busNr int) error { @@ -263,22 +271,6 @@ func (c *Adaptor) arduinoI2CSetup() error { return c.tristate.Write(system.HIGH) } -func (c *Adaptor) readFile(path string) ([]byte, error) { - file, err := c.sys.OpenFile(path, os.O_RDONLY, 0o644) - defer file.Close() //nolint:staticcheck // for historical reasons - if err != nil { - return make([]byte, 0), err - } - - buf := make([]byte, 200) - var i int - i, err = file.Read(buf) - if i == 0 { - return buf, err - } - return buf[:i], err -} - func (c *Adaptor) digitalPin(id string, o ...func(gobot.DigitalPinOptioner) bool) (gobot.DigitalPinner, error) { i := c.pinmap[id] @@ -347,6 +339,16 @@ func pwmPinInitializer(pin gobot.PWMPinner) error { return pin.SetEnabled(true) } +func (c *Adaptor) translateAnalogPin(pin string) (string, bool, bool, uint16, error) { + path := fmt.Sprintf("/sys/bus/iio/devices/iio:device1/in_voltage%s_raw", pin) + const ( + read = true + write = false + readBufLen = 200 + ) + return path, read, write, readBufLen, nil +} + func (c *Adaptor) translateAndMuxPWMPin(id string) (string, int, error) { sysPin, ok := c.pinmap[id] if !ok { diff --git a/platforms/intel-iot/edison/edison_adaptor_test.go b/platforms/intel-iot/edison/edison_adaptor_test.go index 6709d9991..8451d949e 100644 --- a/platforms/intel-iot/edison/edison_adaptor_test.go +++ b/platforms/intel-iot/edison/edison_adaptor_test.go @@ -364,7 +364,7 @@ func TestFinalize(t *testing.T) { a, fs := initTestAdaptorWithMockedFilesystem("arduino") _ = a.DigitalWrite("3", 1) - _ = a.PwmWrite("5", 100) + require.NoError(t, a.PwmWrite("5", 100)) _, _ = a.GetI2cConnection(0xff, 6) require.NoError(t, a.Finalize()) @@ -373,7 +373,7 @@ func TestFinalize(t *testing.T) { require.NoError(t, a.Finalize()) // assert that re-connect is working - _ = a.Connect() + require.NoError(t, a.Connect()) // remove one file to force Finalize error delete(fs.Files, "/sys/class/gpio/unexport") err := a.Finalize() @@ -384,7 +384,7 @@ func TestFinalize(t *testing.T) { func TestFinalizeError(t *testing.T) { a, fs := initTestAdaptorWithMockedFilesystem("arduino") - _ = a.PwmWrite("5", 100) + require.NoError(t, a.PwmWrite("5", 100)) fs.WithWriteError = true err := a.Finalize() @@ -397,10 +397,10 @@ func TestFinalizeError(t *testing.T) { func TestDigitalIO(t *testing.T) { a, fs := initTestAdaptorWithMockedFilesystem("arduino") - _ = a.DigitalWrite("13", 1) + require.NoError(t, a.DigitalWrite("13", 1)) assert.Equal(t, "1", fs.Files["/sys/class/gpio/gpio40/value"].Contents) - _ = a.DigitalWrite("2", 0) + require.NoError(t, a.DigitalWrite("2", 0)) i, err := a.DigitalRead("2") require.NoError(t, err) assert.Equal(t, 0, i) @@ -411,7 +411,7 @@ func TestDigitalPinInFileError(t *testing.T) { fs := a.sys.UseMockFilesystem(pwmMockPathsMux40) delete(fs.Files, "/sys/class/gpio/gpio40/value") delete(fs.Files, "/sys/class/gpio/gpio40/direction") - _ = a.Connect() + _ = a.Connect() // we drop this error _, err := a.DigitalPin("13") require.ErrorContains(t, err, "no such file") @@ -422,7 +422,7 @@ func TestDigitalPinInResistorFileError(t *testing.T) { fs := a.sys.UseMockFilesystem(pwmMockPathsMux40) delete(fs.Files, "/sys/class/gpio/gpio229/value") delete(fs.Files, "/sys/class/gpio/gpio229/direction") - _ = a.Connect() + _ = a.Connect() // we drop this error _, err := a.DigitalPin("13") require.ErrorContains(t, err, "no such file") @@ -433,7 +433,7 @@ func TestDigitalPinInLevelShifterFileError(t *testing.T) { fs := a.sys.UseMockFilesystem(pwmMockPathsMux40) delete(fs.Files, "/sys/class/gpio/gpio261/value") delete(fs.Files, "/sys/class/gpio/gpio261/direction") - _ = a.Connect() + _ = a.Connect() // we drop this error _, err := a.DigitalPin("13") require.ErrorContains(t, err, "no such file") @@ -444,7 +444,7 @@ func TestDigitalPinInMuxFileError(t *testing.T) { fs := a.sys.UseMockFilesystem(pwmMockPathsMux40) delete(fs.Files, "/sys/class/gpio/gpio243/value") delete(fs.Files, "/sys/class/gpio/gpio243/direction") - _ = a.Connect() + _ = a.Connect() // we drop this error _, err := a.DigitalPin("13") require.ErrorContains(t, err, "no such file") @@ -492,7 +492,7 @@ func TestPwmEnableError(t *testing.T) { a := NewAdaptor() fs := a.sys.UseMockFilesystem(pwmMockPathsMux13) delete(fs.Files, "/sys/class/pwm/pwmchip0/pwm1/enable") - _ = a.Connect() + _ = a.Connect() // we drop this error err := a.PwmWrite("5", 100) require.ErrorContains(t, err, "/sys/class/pwm/pwmchip0/pwm1/enable: no such file") @@ -526,7 +526,8 @@ func TestAnalog(t *testing.T) { a, fs := initTestAdaptorWithMockedFilesystem("arduino") fs.Files["/sys/bus/iio/devices/iio:device1/in_voltage0_raw"].Contents = "1000\n" - i, _ := a.AnalogRead("0") + i, err := a.AnalogRead("0") + require.NoError(t, err) assert.Equal(t, 250, i) } diff --git a/system/PWM.md b/system/PWM.md index 949fce02b..971673829 100644 --- a/system/PWM.md +++ b/system/PWM.md @@ -93,3 +93,5 @@ Now we should measure a value of around 2.3V with the meter, which is the differ If we have attached an oscilloscope we can play around with the values for period and duty_cycle and see what happen. ## Links + +* diff --git a/system/analogpin_sysfs.go b/system/analogpin_sysfs.go new file mode 100644 index 000000000..016f8be3c --- /dev/null +++ b/system/analogpin_sysfs.go @@ -0,0 +1,37 @@ +package system + +import "fmt" + +type analogPinSysFs struct { + sysfsPath string + r, w bool + sfa *sysfsFileAccess +} + +func newAnalogPinSysfs(sfa *sysfsFileAccess, path string, r, w bool) *analogPinSysFs { + p := &analogPinSysFs{ + sysfsPath: path, + sfa: sfa, + r: r, + w: w, + } + return p +} + +// Read reads a value from sysf path +func (p *analogPinSysFs) Read() (int, error) { + if !p.r { + return 0, fmt.Errorf("the pin '%s' is not allowed to read", p.sysfsPath) + } + + return p.sfa.readInteger(p.sysfsPath) +} + +// Write writes a value to sysf path +func (p *analogPinSysFs) Write(val int) error { + if !p.w { + return fmt.Errorf("the pin '%s' is not allowed to write (val: %v)", p.sysfsPath, val) + } + + return p.sfa.writeInteger(p.sysfsPath, val) +} diff --git a/system/analogpin_sysfs_test.go b/system/analogpin_sysfs_test.go new file mode 100644 index 000000000..4551ea7ff --- /dev/null +++ b/system/analogpin_sysfs_test.go @@ -0,0 +1,121 @@ +package system + +import ( + "fmt" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func Test_newAnalogPinSysfs(t *testing.T) { + // arrange + m := &MockFilesystem{} + sfa := sysfsFileAccess{fs: m, readBufLen: 2} + const path = "/sys/whatever" + // act + pin := newAnalogPinSysfs(&sfa, path, true, false) + // assert + assert.Equal(t, path, pin.sysfsPath) + assert.Equal(t, &sfa, pin.sfa) + assert.True(t, pin.r) + assert.False(t, pin.w) +} + +func TestRead(t *testing.T) { + const ( + sysfsPath = "/sys/testread" + value = "32" + ) + tests := map[string]struct { + readable bool + simErr bool + wantVal int + wantErr string + }{ + "read_ok": { + readable: true, + wantVal: 32, + }, + "error_not_readable": { + readable: false, + wantErr: "the pin '/sys/testread' is not allowed to read", + }, + "error_on_read": { + readable: true, + simErr: true, + wantErr: "read error", + }, + } + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + // arrange + fs := newMockFilesystem([]string{sysfsPath}) + sfa := sysfsFileAccess{fs: fs, readBufLen: 2} + pin := newAnalogPinSysfs(&sfa, sysfsPath, tc.readable, false) + fs.Files[sysfsPath].Contents = value + if tc.simErr { + fs.Files[sysfsPath].simulateReadError = fmt.Errorf("read error") + } + // act + got, err := pin.Read() + // assert + if tc.wantErr != "" { + require.EqualError(t, err, tc.wantErr) + } else { + require.NoError(t, err) + } + assert.Equal(t, tc.wantVal, got) + }) + } +} + +func TestWrite(t *testing.T) { + const ( + sysfsPath = "/sys/testwrite" + oldVal = "old_value" + ) + tests := map[string]struct { + writeable bool + simErr bool + wantVal string + wantErr string + }{ + "write_ok": { + writeable: true, + wantVal: "23", + }, + "error_not_writeable": { + writeable: false, + wantErr: "the pin '/sys/testwrite' is not allowed to write (val: 23)", + wantVal: oldVal, + }, + "error_on_write": { + writeable: true, + simErr: true, + wantErr: "write error", + wantVal: "23", // the mock is implemented in this way + }, + } + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + // arrange + fs := newMockFilesystem([]string{sysfsPath}) + sfa := sysfsFileAccess{fs: fs, readBufLen: 10} + pin := newAnalogPinSysfs(&sfa, sysfsPath, false, tc.writeable) + fs.Files[sysfsPath].Contents = oldVal + if tc.simErr { + fs.Files[sysfsPath].simulateWriteError = fmt.Errorf("write error") + } + // act + err := pin.Write(23) + // assert + if tc.wantErr != "" { + require.EqualError(t, err, tc.wantErr) + } else { + require.NoError(t, err) + } + assert.Equal(t, tc.wantVal, fs.Files[sysfsPath].Contents) + }) + } +} diff --git a/system/digitalpin_access.go b/system/digitalpin_access.go index 9ba351338..fa2a3c536 100644 --- a/system/digitalpin_access.go +++ b/system/digitalpin_access.go @@ -8,7 +8,7 @@ import ( // sysfsDitalPinHandler represents the sysfs implementation type sysfsDigitalPinAccess struct { - fs filesystem + sfa *sysfsFileAccess } // gpiodDigitalPinAccess represents the character device implementation @@ -25,11 +25,11 @@ func (h *sysfsDigitalPinAccess) isSupported() bool { func (h *sysfsDigitalPinAccess) createPin(chip string, pin int, o ...func(gobot.DigitalPinOptioner) bool, ) gobot.DigitalPinner { - return newDigitalPinSysfs(h.fs, strconv.Itoa(pin), o...) + return newDigitalPinSysfs(h.sfa, strconv.Itoa(pin), o...) } func (h *sysfsDigitalPinAccess) setFs(fs filesystem) { - h.fs = fs + h.sfa = &sysfsFileAccess{fs: fs, readBufLen: 2} } func (h *gpiodDigitalPinAccess) isSupported() bool { diff --git a/system/digitalpin_sysfs.go b/system/digitalpin_sysfs.go index 9c0e89c8c..e7c0230d2 100644 --- a/system/digitalpin_sysfs.go +++ b/system/digitalpin_sysfs.go @@ -3,7 +3,6 @@ package system import ( "errors" "fmt" - "io" "log" "os" "strconv" @@ -24,21 +23,25 @@ var errNotExported = errors.New("pin has not been exported") type digitalPinSysfs struct { pin string *digitalPinConfig - fs filesystem + sfa *sysfsFileAccess - dirFile File - valFile File - activeLowFile File + dirFile *sysfsFile + valFile *sysfsFile + activeLowFile *sysfsFile } // newDigitalPinSysfs returns a digital pin using for the given number. The name of the sysfs file will prepend "gpio" // to the pin number, eg. a pin number of 10 will have a name of "gpio10". The pin is handled by the sysfs Kernel ABI. -func newDigitalPinSysfs(fs filesystem, pin string, options ...func(gobot.DigitalPinOptioner) bool) *digitalPinSysfs { +func newDigitalPinSysfs( + sfa *sysfsFileAccess, + pin string, + options ...func(gobot.DigitalPinOptioner) bool, +) *digitalPinSysfs { cfg := newDigitalPinConfig("gpio"+pin, options...) d := &digitalPinSysfs{ pin: pin, digitalPinConfig: cfg, - fs: fs, + sfa: sfa, } return d } @@ -68,26 +71,26 @@ func (d *digitalPinSysfs) Export() error { // Unexport release the pin func (d *digitalPinSysfs) Unexport() error { - unexport, err := d.fs.openFile(gpioPath+"/unexport", os.O_WRONLY, 0o644) + unexport, err := d.sfa.openWrite(gpioPath + "/unexport") if err != nil { return err } - defer unexport.Close() + defer unexport.close() if d.dirFile != nil { - d.dirFile.Close() + d.dirFile.close() d.dirFile = nil } if d.valFile != nil { - d.valFile.Close() + d.valFile.close() d.valFile = nil } if d.activeLowFile != nil { - d.activeLowFile.Close() + d.activeLowFile.close() d.activeLowFile = nil } - err = writeFile(unexport, []byte(d.pin)) + err = unexport.write([]byte(d.pin)) if err != nil { // If EINVAL then the pin is reserved in the system and can't be unexported, we suppress the error var pathError *os.PathError @@ -101,13 +104,19 @@ func (d *digitalPinSysfs) Unexport() error { // Write writes the given value to the character device func (d *digitalPinSysfs) Write(b int) error { - err := writeFile(d.valFile, []byte(strconv.Itoa(b))) + if d.valFile == nil { + return errNotExported + } + err := d.valFile.write([]byte(strconv.Itoa(b))) return err } -// Read reads the given value from character device +// Read reads a value from character device func (d *digitalPinSysfs) Read() (int, error) { - buf, err := readFile(d.valFile) + if d.valFile == nil { + return 0, errNotExported + } + buf, err := d.valFile.read() if err != nil { return 0, err } @@ -115,13 +124,13 @@ func (d *digitalPinSysfs) Read() (int, error) { } func (d *digitalPinSysfs) reconfigure() error { - exportFile, err := d.fs.openFile(gpioPath+"/export", os.O_WRONLY, 0o644) + exportFile, err := d.sfa.openWrite(gpioPath + "/export") if err != nil { return err } - defer exportFile.Close() + defer exportFile.close() - err = writeFile(exportFile, []byte(d.pin)) + err = exportFile.write([]byte(d.pin)) if err != nil { // If EBUSY then the pin has already been exported, we suppress the error var pathError *os.PathError @@ -131,13 +140,13 @@ func (d *digitalPinSysfs) reconfigure() error { } if d.dirFile != nil { - d.dirFile.Close() + d.dirFile.close() } attempt := 0 for { attempt++ - d.dirFile, err = d.fs.openFile(fmt.Sprintf("%s/%s/direction", gpioPath, d.label), os.O_RDWR, 0o644) + d.dirFile, err = d.sfa.openReadWrite(fmt.Sprintf("%s/%s/direction", gpioPath, d.label)) if err == nil { break } @@ -148,10 +157,10 @@ func (d *digitalPinSysfs) reconfigure() error { } if d.valFile != nil { - d.valFile.Close() + d.valFile.close() } if err == nil { - d.valFile, err = d.fs.openFile(fmt.Sprintf("%s/%s/value", gpioPath, d.label), os.O_RDWR, 0o644) + d.valFile, err = d.sfa.openReadWrite(fmt.Sprintf("%s/%s/value", gpioPath, d.label)) } // configure direction @@ -162,9 +171,9 @@ func (d *digitalPinSysfs) reconfigure() error { // configure inverse logic if err == nil { if d.activeLow { - d.activeLowFile, err = d.fs.openFile(fmt.Sprintf("%s/%s/active_low", gpioPath, d.label), os.O_RDWR, 0o644) + d.activeLowFile, err = d.sfa.openReadWrite(fmt.Sprintf("%s/%s/active_low", gpioPath, d.label)) if err == nil { - err = writeFile(d.activeLowFile, []byte("1")) + err = d.activeLowFile.write([]byte("1")) } } } @@ -211,48 +220,16 @@ func (d *digitalPinSysfs) reconfigure() error { } func (d *digitalPinSysfs) writeDirectionWithInitialOutput() error { - if err := writeFile(d.dirFile, []byte(d.direction)); err != nil || d.direction == IN { + if d.dirFile == nil { + return errNotExported + } + if err := d.dirFile.write([]byte(d.direction)); err != nil || d.direction == IN { return err } - err := writeFile(d.valFile, []byte(strconv.Itoa(d.outInitialState))) - return err -} -// Linux sysfs / GPIO specific sysfs docs. -// https://www.kernel.org/doc/Documentation/filesystems/sysfs.txt -// https://www.kernel.org/doc/Documentation/gpio/sysfs.txt - -var writeFile = func(f File, data []byte) error { - if f == nil { + if d.valFile == nil { return errNotExported } - // sysfs docs say: - // > When writing sysfs files, userspace processes should first read the - // > entire file, modify the values it wishes to change, then write the - // > entire buffer back. - // however, this seems outdated/inaccurate (docs are from back in the Kernel BitKeeper days). - - // Write() returns already a non-nil error when n != len(b). - _, err := f.Write(data) - return err -} - -var readFile = func(f File) ([]byte, error) { - if f == nil { - return nil, errNotExported - } - - // sysfs docs say: - // > If userspace seeks back to zero or does a pread(2) with an offset of '0' the [..] method will - // > be called again, rearmed, to fill the buffer. - - // TODO: Examine if seek is needed if full buffer is read from sysfs file. - - buf := make([]byte, 2) - _, err := f.Seek(0, io.SeekStart) - if err == nil { - _, err = f.Read(buf) - } - return buf, err + return d.valFile.write([]byte(strconv.Itoa(d.outInitialState))) } diff --git a/system/digitalpin_sysfs_test.go b/system/digitalpin_sysfs_test.go index 6f6473ac9..ca617f7be 100644 --- a/system/digitalpin_sysfs_test.go +++ b/system/digitalpin_sysfs_test.go @@ -21,19 +21,21 @@ var ( func initTestDigitalPinSysfsWithMockedFilesystem(mockPaths []string) (*digitalPinSysfs, *MockFilesystem) { fs := newMockFilesystem(mockPaths) - pin := newDigitalPinSysfs(fs, "10") + sfa := sysfsFileAccess{fs: fs, readBufLen: 2} + pin := newDigitalPinSysfs(&sfa, "10") return pin, fs } func Test_newDigitalPinSysfs(t *testing.T) { // arrange m := &MockFilesystem{} + sfa := sysfsFileAccess{fs: m, readBufLen: 2} const pinID = "1" // act - pin := newDigitalPinSysfs(m, pinID, WithPinOpenDrain()) + pin := newDigitalPinSysfs(&sfa, pinID, WithPinOpenDrain()) // assert assert.Equal(t, pinID, pin.pin) - assert.Equal(t, m, pin.fs) + assert.Equal(t, &sfa, pin.sfa) assert.Equal(t, "gpio"+pinID, pin.label) assert.Equal(t, "in", pin.direction) assert.Equal(t, 1, pin.drive) @@ -134,7 +136,7 @@ func TestDigitalPinExportSysfs(t *testing.T) { changeDebouncePeriod time.Duration changeEdge int changePollInterval time.Duration - simEbusyOnWrite int + simEbusyOnPath string wantWrites int wantExport string wantUnexport string @@ -225,11 +227,11 @@ func TestDigitalPinExportSysfs(t *testing.T) { wantValue: "0", }, "ok_already_exported": { - mockPaths: allMockPaths, - wantWrites: 2, - wantExport: "10", - wantDirection: "in", - simEbusyOnWrite: 1, // just means "already exported" + mockPaths: allMockPaths, + wantWrites: 2, + wantExport: "10", + wantDirection: "in", + simEbusyOnPath: exportPath, // just means "already exported" }, "error_no_eventhandler_for_polling": { // this only tests the call of function, all other is tested separately mockPaths: allMockPaths, @@ -250,11 +252,11 @@ func TestDigitalPinExportSysfs(t *testing.T) { wantErr: "gpio10/direction: no such file", }, "error_write_direction_file": { - mockPaths: allMockPaths, - wantWrites: 3, - wantUnexport: "10", - simEbusyOnWrite: 2, - wantErr: "device or resource busy", + mockPaths: allMockPaths, + wantWrites: 3, + wantUnexport: "10", + simEbusyOnPath: dirPath, + wantErr: "device or resource busy", }, "error_no_value_file": { mockPaths: []string{exportPath, dirPath, unexportPath}, @@ -281,7 +283,8 @@ func TestDigitalPinExportSysfs(t *testing.T) { t.Run(name, func(t *testing.T) { // arrange fs := newMockFilesystem(tc.mockPaths) - pin := newDigitalPinSysfs(fs, "10") + sfa := sysfsFileAccess{fs: fs, readBufLen: 2} + pin := newDigitalPinSysfs(&sfa, "10") if tc.changeDirection != "" { pin.direction = tc.changeDirection } @@ -307,17 +310,9 @@ func TestDigitalPinExportSysfs(t *testing.T) { pin.pollInterval = tc.changePollInterval } // arrange write function - oldWriteFunc := writeFile - numCallsWrite := 0 - writeFile = func(f File, data []byte) error { - numCallsWrite++ - require.NoError(t, oldWriteFunc(f, data)) - if numCallsWrite == tc.simEbusyOnWrite { - return &os.PathError{Err: Syscall_EBUSY} - } - return nil + if tc.simEbusyOnPath != "" { + fs.Files[tc.simEbusyOnPath].simulateWriteError = &os.PathError{Err: Syscall_EBUSY} } - defer func() { writeFile = oldWriteFunc }() // act err := pin.Export() // assert @@ -333,7 +328,7 @@ func TestDigitalPinExportSysfs(t *testing.T) { assert.Equal(t, tc.wantInverse, fs.Files[inversePath].Contents) } assert.Equal(t, tc.wantUnexport, fs.Files[unexportPath].Contents) - assert.Equal(t, tc.wantWrites, numCallsWrite) + assert.Equal(t, tc.wantWrites, fs.numCallsWrite) }) } } @@ -368,7 +363,8 @@ func TestDigitalPinSysfs(t *testing.T) { data, _ := pin.Read() assert.Equal(t, 1, data) - pin2 := newDigitalPinSysfs(fs, "30") + sfa := sysfsFileAccess{fs: fs, readBufLen: 2} + pin2 := newDigitalPinSysfs(&sfa, "30") err = pin2.Write(1) require.ErrorContains(t, err, "pin has not been exported") @@ -376,33 +372,47 @@ func TestDigitalPinSysfs(t *testing.T) { require.ErrorContains(t, err, "pin has not been exported") assert.Equal(t, 0, data) - writeFile = func(File, []byte) error { - return &os.PathError{Err: Syscall_EINVAL} - } - - err = pin.Unexport() - require.NoError(t, err) - - writeFile = func(File, []byte) error { - return &os.PathError{Err: errors.New("write error")} - } - + // arrange: unexport general write error, the error is not suppressed + fs.Files["/sys/class/gpio/unexport"].simulateWriteError = &os.PathError{Err: errors.New("write error")} + // act: unexport err = pin.Unexport() + // assert: the error is not suppressed var pathError *os.PathError require.ErrorAs(t, err, &pathError) require.ErrorContains(t, err, "write error") } func TestDigitalPinUnexportErrorSysfs(t *testing.T) { - mockPaths := []string{ - "/sys/class/gpio/unexport", + tests := map[string]struct { + simulateError error + wantErr string + }{ + "reserved_pin": { + // simulation of reserved pin, the internal error is suppressed + simulateError: &os.PathError{Err: Syscall_EINVAL}, + wantErr: "", + }, + "error_busy": { + simulateError: &os.PathError{Err: Syscall_EBUSY}, + wantErr: " : device or resource busy", + }, } - pin, _ := initTestDigitalPinSysfsWithMockedFilesystem(mockPaths) - - writeFile = func(File, []byte) error { - return &os.PathError{Err: Syscall_EBUSY} + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + // arrange + mockPaths := []string{ + "/sys/class/gpio/unexport", + } + pin, fs := initTestDigitalPinSysfsWithMockedFilesystem(mockPaths) + fs.Files["/sys/class/gpio/unexport"].simulateWriteError = tc.simulateError + // act + err := pin.Unexport() + // assert + if tc.wantErr != "" { + require.ErrorContains(t, err, tc.wantErr) + } else { + require.NoError(t, err) + } + }) } - - err := pin.Unexport() - require.ErrorContains(t, err, " : device or resource busy") } diff --git a/system/fs_mock.go b/system/fs_mock.go index d91c4383e..afcb11a3f 100644 --- a/system/fs_mock.go +++ b/system/fs_mock.go @@ -2,6 +2,7 @@ package system import ( "fmt" + "log" "os" "path" "regexp" @@ -21,16 +22,20 @@ type MockFilesystem struct { WithReadError bool WithWriteError bool WithCloseError bool + numCallsWrite int + numCallsRead int } // A MockFile represents a mock file that contains a single string. Any write // overwrites, and any read returns from the start. type MockFile struct { - Contents string - Seq int // When this file was last written or read. - Opened bool - Closed bool - fd uintptr + Contents string + Seq int // When this file was last written or read. + Opened bool + Closed bool + fd uintptr + simulateWriteError error + simulateReadError error fs *MockFilesystem } @@ -46,6 +51,7 @@ func (f *MockFile) Write(b []byte) (int, error) { if f.fs.WithWriteError { return 0, errWrite } + return f.WriteString(string(b)) } @@ -58,7 +64,8 @@ func (f *MockFile) Seek(offset int64, whence int) (int64, error) { func (f *MockFile) WriteString(s string) (int, error) { f.Contents = s f.Seq = f.fs.next() - return len(s), nil + f.fs.numCallsWrite++ + return len(s), f.simulateWriteError } // Sync implements the File interface Sync function @@ -79,7 +86,8 @@ func (f *MockFile) Read(b []byte) (int, error) { copy(b, []byte(f.Contents)[:count]) f.Seq = f.fs.next() - return count, nil + f.fs.numCallsRead++ + return count, f.simulateReadError } // ReadAt calls MockFile.Read @@ -135,9 +143,10 @@ func (fs *MockFilesystem) openFile(name string, _ int, _ os.FileMode) (File, err func (fs *MockFilesystem) stat(name string) (os.FileInfo, error) { _, ok := fs.Files[name] if ok { - // return file based mock FileInfo - tmpFile, err := os.CreateTemp("", name) + // return file based mock FileInfo, CreateTemp don't like "/" in between + tmpFile, err := os.CreateTemp("", strings.ReplaceAll(name, "/", "_")) if err != nil { + log.Println("A") return nil, err } defer os.Remove(tmpFile.Name()) @@ -194,6 +203,8 @@ func (fs *MockFilesystem) readFile(name string) ([]byte, error) { if !ok { return nil, &os.PathError{Err: fmt.Errorf("%s: no such file", name)} } + + f.fs.numCallsRead++ return []byte(f.Contents), nil } diff --git a/system/pwmpin_sysfs.go b/system/pwmpin_sysfs.go index a038e469f..b31a9484a 100644 --- a/system/pwmpin_sysfs.go +++ b/system/pwmpin_sysfs.go @@ -24,30 +24,25 @@ type pwmPinSysFs struct { polarityNormalIdentifier string polarityInvertedIdentifier string - write func(fs filesystem, path string, data []byte) (i int, err error) - read func(fs filesystem, path string) ([]byte, error) - fs filesystem + sfa *sysfsFileAccess } // newPWMPinSysfs returns a new pwmPin, working with sysfs file access. -func newPWMPinSysfs(fs filesystem, path string, pin int, polNormIdent string, polInvIdent string) *pwmPinSysFs { +func newPWMPinSysfs(sfa *sysfsFileAccess, path string, pin int, polNormIdent string, polInvIdent string) *pwmPinSysFs { p := &pwmPinSysFs{ path: path, pin: strconv.Itoa(pin), polarityNormalIdentifier: polNormIdent, polarityInvertedIdentifier: polInvIdent, - read: readPwmFile, - write: writePwmFile, - fs: fs, + sfa: sfa, } return p } // Export exports the pin for use by the operating system by writing the pin to the "Export" path func (p *pwmPinSysFs) Export() error { - _, err := p.write(p.fs, p.pwmExportPath(), []byte(p.pin)) - if err != nil { + if err := p.sfa.write(p.pwmExportPath(), []byte(p.pin)); err != nil { // If EBUSY then the pin has already been exported, we suppress the error var pathError *os.PathError if !(errors.As(err, &pathError) && errors.Is(err, Syscall_EBUSY)) { @@ -65,7 +60,7 @@ func (p *pwmPinSysFs) Export() error { // Unexport releases the pin from the operating system by writing the pin to the "Unexport" path func (p *pwmPinSysFs) Unexport() error { - if _, err := p.write(p.fs, p.pwmUnexportPath(), []byte(p.pin)); err != nil { + if err := p.sfa.write(p.pwmUnexportPath(), []byte(p.pin)); err != nil { return fmt.Errorf(pwmPinErrorPattern, "Unexport", p.pin, err) } return nil @@ -73,16 +68,12 @@ func (p *pwmPinSysFs) Unexport() error { // Enabled reads and returns the enabled state of the pin func (p *pwmPinSysFs) Enabled() (bool, error) { - buf, err := p.read(p.fs, p.pwmEnablePath()) + val, err := p.sfa.readInteger(p.pwmEnablePath()) if err != nil { return false, fmt.Errorf(pwmPinErrorPattern, "Enabled", p.pin, err) } - if len(buf) == 0 { - return false, nil - } - val, e := strconv.Atoi(string(bytes.TrimRight(buf, "\n"))) - return val > 0, e + return val > 0, nil } // SetEnabled writes enable(1) or disable(0) status. For most platforms this is only possible if period was @@ -92,7 +83,7 @@ func (p *pwmPinSysFs) SetEnabled(enable bool) error { if enable { enableVal = 1 } - if _, err := p.write(p.fs, p.pwmEnablePath(), []byte(strconv.Itoa(enableVal))); err != nil { + if err := p.sfa.writeInteger(p.pwmEnablePath(), enableVal); err != nil { if pwmDebug { p.printState() } @@ -104,7 +95,7 @@ func (p *pwmPinSysFs) SetEnabled(enable bool) error { // Polarity reads and returns false if the polarity is inverted, otherwise true func (p *pwmPinSysFs) Polarity() (bool, error) { - buf, err := p.read(p.fs, p.pwmPolarityPath()) + buf, err := p.sfa.read(p.pwmPolarityPath()) if err != nil { return true, fmt.Errorf(pwmPinErrorPattern, "Polarity", p.pin, err) } @@ -133,7 +124,7 @@ func (p *pwmPinSysFs) SetPolarity(normal bool) error { if !normal { value = p.polarityInvertedIdentifier } - if _, err := p.write(p.fs, p.pwmPolarityPath(), []byte(value)); err != nil { + if err := p.sfa.write(p.pwmPolarityPath(), []byte(value)); err != nil { if pwmDebug { p.printState() } @@ -144,23 +135,17 @@ func (p *pwmPinSysFs) SetPolarity(normal bool) error { // Period returns the current period func (p *pwmPinSysFs) Period() (uint32, error) { - buf, err := p.read(p.fs, p.pwmPeriodPath()) + val, err := p.sfa.readInteger(p.pwmPeriodPath()) if err != nil { return 0, fmt.Errorf(pwmPinErrorPattern, "Period", p.pin, err) } - if len(buf) == 0 { - return 0, nil - } - v := bytes.TrimRight(buf, "\n") - val, e := strconv.Atoi(string(v)) - return uint32(val), e + return uint32(val), nil } // SetPeriod writes the current period in nanoseconds func (p *pwmPinSysFs) SetPeriod(period uint32) error { - //nolint:perfsprint // ok here - if _, err := p.write(p.fs, p.pwmPeriodPath(), []byte(fmt.Sprintf("%v", period))); err != nil { + if err := p.sfa.writeInteger(p.pwmPeriodPath(), int(period)); err != nil { if pwmDebug { p.printState() @@ -172,19 +157,17 @@ func (p *pwmPinSysFs) SetPeriod(period uint32) error { // DutyCycle reads and returns the duty cycle in nanoseconds func (p *pwmPinSysFs) DutyCycle() (uint32, error) { - buf, err := p.read(p.fs, p.pwmDutyCyclePath()) + val, err := p.sfa.readInteger(p.pwmDutyCyclePath()) if err != nil { return 0, fmt.Errorf(pwmPinErrorPattern, "DutyCycle", p.pin, err) } - val, err := strconv.Atoi(string(bytes.TrimRight(buf, "\n"))) return uint32(val), err } // SetDutyCycle writes the duty cycle in nanoseconds func (p *pwmPinSysFs) SetDutyCycle(duty uint32) error { - //nolint:perfsprint // ok here - if _, err := p.write(p.fs, p.pwmDutyCyclePath(), []byte(fmt.Sprintf("%v", duty))); err != nil { + if err := p.sfa.writeInteger(p.pwmDutyCyclePath(), int(duty)); err != nil { if pwmDebug { p.printState() } @@ -223,32 +206,6 @@ func (p *pwmPinSysFs) pwmPolarityPath() string { return path.Join(p.path, "pwm"+p.pin, "polarity") } -func writePwmFile(fs filesystem, path string, data []byte) (int, error) { - file, err := fs.openFile(path, os.O_WRONLY, 0o644) - defer file.Close() //nolint:staticcheck // for historical reasons - if err != nil { - return 0, err - } - - return file.Write(data) -} - -func readPwmFile(fs filesystem, path string) ([]byte, error) { - file, err := fs.openFile(path, os.O_RDONLY, 0o644) - defer file.Close() //nolint:staticcheck // for historical reasons - if err != nil { - return make([]byte, 0), err - } - - buf := make([]byte, 200) - var i int - i, err = file.Read(buf) - if i == 0 { - return []byte{}, err - } - return buf[:i], err -} - func (p *pwmPinSysFs) printState() { enabled, _ := p.Enabled() polarity, _ := p.Polarity() diff --git a/system/pwmpin_sysfs_test.go b/system/pwmpin_sysfs_test.go index b4c1f82c9..a9460667f 100644 --- a/system/pwmpin_sysfs_test.go +++ b/system/pwmpin_sysfs_test.go @@ -19,7 +19,8 @@ const ( func initTestPWMPinSysFsWithMockedFilesystem(mockPaths []string) (*pwmPinSysFs, *MockFilesystem) { fs := newMockFilesystem(mockPaths) - pin := newPWMPinSysfs(fs, "/sys/class/pwm/pwmchip0", 10, normal, inverted) + sfa := &sysfsFileAccess{fs: fs, readBufLen: 200} + pin := newPWMPinSysfs(sfa, "/sys/class/pwm/pwmchip0", 10, normal, inverted) return pin, fs } @@ -76,6 +77,7 @@ func TestPwmPin(t *testing.T) { } func TestPwmPinAlreadyExported(t *testing.T) { + // arrange mockedPaths := []string{ "/sys/class/pwm/pwmchip0/export", "/sys/class/pwm/pwmchip0/unexport", @@ -83,17 +85,14 @@ func TestPwmPinAlreadyExported(t *testing.T) { "/sys/class/pwm/pwmchip0/pwm10/period", "/sys/class/pwm/pwmchip0/pwm10/duty_cycle", } - pin, _ := initTestPWMPinSysFsWithMockedFilesystem(mockedPaths) - - pin.write = func(filesystem, string, []byte) (int, error) { - return 0, &os.PathError{Err: Syscall_EBUSY} - } - - // no error indicates that the pin was already exported + pin, fs := initTestPWMPinSysFsWithMockedFilesystem(mockedPaths) + fs.Files["/sys/class/pwm/pwmchip0/export"].simulateWriteError = &os.PathError{Err: Syscall_EBUSY} + // act & assert: no error indicates that the pin was already exported require.NoError(t, pin.Export()) } func TestPwmPinExportError(t *testing.T) { + // arrange mockedPaths := []string{ "/sys/class/pwm/pwmchip0/export", "/sys/class/pwm/pwmchip0/unexport", @@ -101,18 +100,16 @@ func TestPwmPinExportError(t *testing.T) { "/sys/class/pwm/pwmchip0/pwm10/period", "/sys/class/pwm/pwmchip0/pwm10/duty_cycle", } - pin, _ := initTestPWMPinSysFsWithMockedFilesystem(mockedPaths) - - pin.write = func(filesystem, string, []byte) (int, error) { - return 0, &os.PathError{Err: Syscall_EFAULT} - } - - // no error indicates that the pin was already exported + pin, fs := initTestPWMPinSysFsWithMockedFilesystem(mockedPaths) + fs.Files["/sys/class/pwm/pwmchip0/export"].simulateWriteError = &os.PathError{Err: Syscall_EFAULT} + // act err := pin.Export() + // assert require.ErrorContains(t, err, "Export() failed for id 10 with : bad address") } func TestPwmPinUnxportError(t *testing.T) { + // arrange mockedPaths := []string{ "/sys/class/pwm/pwmchip0/export", "/sys/class/pwm/pwmchip0/unexport", @@ -120,17 +117,16 @@ func TestPwmPinUnxportError(t *testing.T) { "/sys/class/pwm/pwmchip0/pwm10/period", "/sys/class/pwm/pwmchip0/pwm10/duty_cycle", } - pin, _ := initTestPWMPinSysFsWithMockedFilesystem(mockedPaths) - - pin.write = func(filesystem, string, []byte) (int, error) { - return 0, &os.PathError{Err: Syscall_EBUSY} - } - + pin, fs := initTestPWMPinSysFsWithMockedFilesystem(mockedPaths) + fs.Files["/sys/class/pwm/pwmchip0/unexport"].simulateWriteError = &os.PathError{Err: Syscall_EBUSY} + // act err := pin.Unexport() + // assert require.ErrorContains(t, err, "Unexport() failed for id 10 with : device or resource busy") } func TestPwmPinPeriodError(t *testing.T) { + // arrange mockedPaths := []string{ "/sys/class/pwm/pwmchip0/export", "/sys/class/pwm/pwmchip0/unexport", @@ -138,35 +134,34 @@ func TestPwmPinPeriodError(t *testing.T) { "/sys/class/pwm/pwmchip0/pwm10/period", "/sys/class/pwm/pwmchip0/pwm10/duty_cycle", } - pin, _ := initTestPWMPinSysFsWithMockedFilesystem(mockedPaths) - - pin.read = func(filesystem, string) ([]byte, error) { - return nil, &os.PathError{Err: Syscall_EBUSY} - } - + pin, fs := initTestPWMPinSysFsWithMockedFilesystem(mockedPaths) + fs.Files["/sys/class/pwm/pwmchip0/pwm10/period"].simulateReadError = &os.PathError{Err: Syscall_EBUSY} + // act _, err := pin.Period() + // assert require.ErrorContains(t, err, "Period() failed for id 10 with : device or resource busy") } func TestPwmPinPolarityError(t *testing.T) { + // arrange mockedPaths := []string{ "/sys/class/pwm/pwmchip0/export", "/sys/class/pwm/pwmchip0/unexport", "/sys/class/pwm/pwmchip0/pwm10/enable", "/sys/class/pwm/pwmchip0/pwm10/period", "/sys/class/pwm/pwmchip0/pwm10/duty_cycle", + "/sys/class/pwm/pwmchip0/pwm10/polarity", } - pin, _ := initTestPWMPinSysFsWithMockedFilesystem(mockedPaths) - - pin.read = func(filesystem, string) ([]byte, error) { - return nil, &os.PathError{Err: Syscall_EBUSY} - } - + pin, fs := initTestPWMPinSysFsWithMockedFilesystem(mockedPaths) + fs.Files["/sys/class/pwm/pwmchip0/pwm10/polarity"].simulateReadError = &os.PathError{Err: Syscall_EBUSY} + // act _, err := pin.Polarity() + // assert require.ErrorContains(t, err, "Polarity() failed for id 10 with : device or resource busy") } func TestPwmPinDutyCycleError(t *testing.T) { + // arrange mockedPaths := []string{ "/sys/class/pwm/pwmchip0/export", "/sys/class/pwm/pwmchip0/unexport", @@ -174,12 +169,10 @@ func TestPwmPinDutyCycleError(t *testing.T) { "/sys/class/pwm/pwmchip0/pwm10/period", "/sys/class/pwm/pwmchip0/pwm10/duty_cycle", } - pin, _ := initTestPWMPinSysFsWithMockedFilesystem(mockedPaths) - - pin.read = func(filesystem, string) ([]byte, error) { - return nil, &os.PathError{Err: Syscall_EBUSY} - } - + pin, fs := initTestPWMPinSysFsWithMockedFilesystem(mockedPaths) + fs.Files["/sys/class/pwm/pwmchip0/pwm10/duty_cycle"].simulateReadError = &os.PathError{Err: Syscall_EBUSY} + // act _, err := pin.DutyCycle() + // assert require.ErrorContains(t, err, "DutyCycle() failed for id 10 with : device or resource busy") } diff --git a/system/sysfsfile_access.go b/system/sysfsfile_access.go new file mode 100644 index 000000000..f65542eb1 --- /dev/null +++ b/system/sysfsfile_access.go @@ -0,0 +1,146 @@ +package system + +import ( + "io" + "os" + "strconv" + "strings" +) + +type sysfsFileAccess struct { + fs filesystem + readBufLen uint16 +} + +// Linux sysfs / GPIO specific sysfs docs. +// +// https://www.kernel.org/doc/Documentation/filesystems/sysfs.txt +// https://www.kernel.org/doc/Documentation/gpio/sysfs.txt +// https://www.kernel.org/doc/Documentation/thermal/sysfs-api.txt +// see also PWM.md +type sysfsFile struct { + sfa sysfsFileAccess + file File + sysfsPath string +} + +func (sfa sysfsFileAccess) readInteger(path string) (int, error) { + sf, err := sfa.openRead(path) + defer func() { _ = sf.close() }() + if err != nil { + return 0, err + } + + return sf.readInteger() +} + +func (sfa sysfsFileAccess) read(path string) ([]byte, error) { + sf, err := sfa.openRead(path) + defer func() { _ = sf.close() }() + if err != nil { + return nil, err + } + + return sf.read() +} + +func (sfa sysfsFileAccess) writeInteger(path string, val int) error { + sf, err := sfa.openWrite(path) + defer func() { _ = sf.close() }() + if err != nil { + return err + } + + return sf.writeInteger(val) +} + +func (sfa sysfsFileAccess) write(path string, data []byte) error { + sf, err := sfa.openWrite(path) + defer func() { _ = sf.close() }() + if err != nil { + return err + } + + return sf.write(data) +} + +func (sfa sysfsFileAccess) openRead(path string) (*sysfsFile, error) { + f, err := sfa.fs.openFile(path, os.O_RDONLY, 0o644) + if err != nil { + return nil, err + } + return &sysfsFile{sfa: sfa, file: f, sysfsPath: path}, nil +} + +func (sfa sysfsFileAccess) openWrite(path string) (*sysfsFile, error) { + f, err := sfa.fs.openFile(path, os.O_WRONLY, 0o644) + if err != nil { + return nil, err + } + return &sysfsFile{sfa: sfa, file: f, sysfsPath: path}, nil +} + +func (sfa sysfsFileAccess) openReadWrite(path string) (*sysfsFile, error) { + f, err := sfa.fs.openFile(path, os.O_RDWR, 0o644) + if err != nil { + return nil, err + } + return &sysfsFile{sfa: sfa, file: f, sysfsPath: path}, nil +} + +func (sf *sysfsFile) close() error { + if sf == nil || sf.file == nil { + return nil + } + return sf.file.Close() +} + +func (sf *sysfsFile) readInteger() (int, error) { + buf, err := sf.read() + if err != nil { + return 0, err + } + + if len(buf) == 0 { + return 0, nil + } + + return strconv.Atoi(strings.Split(string(buf), "\n")[0]) +} + +func (sf *sysfsFile) read() ([]byte, error) { + // sysfs docs say: + // > If userspace seeks back to zero or does a pread(2) with an offset of '0' the [..] method will + // > be called again, rearmed, to fill the buffer. + // > The buffer will always be PAGE_SIZE bytes in length. On i386, this is 4096. + + // TODO: Examine if seek is needed if full buffer is read from sysfs file. + + buf := make([]byte, sf.sfa.readBufLen) + if _, err := sf.file.Seek(0, io.SeekStart); err != nil { + return nil, err + } + + i, err := sf.file.Read(buf) + if i == 0 { + return []byte{}, err + } + + return buf[:i], err +} + +func (sf *sysfsFile) writeInteger(val int) error { + return sf.write([]byte(strconv.Itoa(val))) +} + +func (sf *sysfsFile) write(data []byte) error { + // sysfs docs say: + // > When writing sysfs files, userspace processes should first read the + // > entire file, modify the values it wishes to change, then write the + // > entire buffer back. + // however, this seems outdated/inaccurate (docs are from back in the Kernel BitKeeper days). + + // Write() returns already a non-nil error when n != len(b). + _, err := sf.file.Write(data) + return err +} diff --git a/system/system.go b/system/system.go index 6e53fe144..210b6b001 100644 --- a/system/system.go +++ b/system/system.go @@ -72,7 +72,7 @@ func NewAccesser(options ...func(Optioner)) *Accesser { fs: &nativeFilesystem{}, } s.spiAccess = &periphioSpiAccess{fs: s.fs} - s.digitalPinAccess = &sysfsDigitalPinAccess{fs: s.fs} + s.digitalPinAccess = &sysfsDigitalPinAccess{sfa: &sysfsFileAccess{fs: s.fs, readBufLen: 2}} for _, option := range options { option(s) } @@ -85,7 +85,7 @@ func (a *Accesser) UseDigitalPinAccessWithMockFs(digitalPinAccess string, files var dph digitalPinAccesser switch digitalPinAccess { case "sysfs": - dph = &sysfsDigitalPinAccess{fs: fs} + dph = &sysfsDigitalPinAccess{sfa: &sysfsFileAccess{fs: fs, readBufLen: 2}} case "cdev": dph = &gpiodDigitalPinAccess{fs: fs} default: @@ -135,7 +135,15 @@ func (a *Accesser) IsSysfsDigitalPinAccess() bool { // NewPWMPin returns a new system PWM pin, according to the given pin number. func (a *Accesser) NewPWMPin(path string, pin int, polNormIdent string, polInvIdent string) gobot.PWMPinner { - return newPWMPinSysfs(a.fs, path, pin, polNormIdent, polInvIdent) + sfa := &sysfsFileAccess{fs: a.fs, readBufLen: 200} + return newPWMPinSysfs(sfa, path, pin, polNormIdent, polInvIdent) +} + +func (a *Accesser) NewAnalogPin(path string, r, w bool, readBufLen uint16) gobot.AnalogPinner { + if readBufLen == 0 { + readBufLen = 32 // max. count of characters for int value is 20 + } + return newAnalogPinSysfs(&sysfsFileAccess{fs: a.fs, readBufLen: readBufLen}, path, r, w) } // NewSpiDevice returns a new connection to SPI with the given parameters. diff --git a/system/system_test.go b/system/system_test.go index 0edeab0da..78dadebd9 100644 --- a/system/system_test.go +++ b/system/system_test.go @@ -82,7 +82,7 @@ func TestNewAccesser_IsSysfsDigitalPinAccess(t *testing.T) { assert.True(t, got) dpaSys := a.digitalPinAccess.(*sysfsDigitalPinAccess) assert.NotNil(t, dpaSys) - assert.Equal(t, a.fs.(*nativeFilesystem), dpaSys.fs) + assert.Equal(t, a.fs.(*nativeFilesystem), dpaSys.sfa.fs) } else { assert.False(t, got) dpaGpiod := a.digitalPinAccess.(*gpiodDigitalPinAccess)