Using es6 super sugar for a method/function fails

[dkebler]

Never had this issue before when extending using ES6 and I have a whole library where I use "super" methods.

but extending onoff Gpio if I create a method "read" and then call super.read within it calls my new read method instead of onoff's Gpio read function. I assume it does this cause creating the read method is somehow overwriting or masking the parent function/method. Is there something in the onoff prototype that precludes using the es6 super sugar?

BTW I am using a latest node in this case 12.4. Do you always run your tests against latest node? If not is this the issue maybe???

For now I just changed my method name so as not to superceed the read method but this is an issue I've never had extending any other object using es6 class syntax.

here is the the way I had it coded

import DeadJim from 'death'
import { Gpio } from 'onoff'
import to from 'await-to-js'


import logger from '@uci-utils/logger'

let log = logger({package:'@uci/gpio', file:'/src/gpio.js'})

class Pin extends Gpio {
  constructor(pin, opts = {}) {
    if (typeof pin !=='number') pin = parseInt(pin)  // make sure pin is a number!
    // TODO check against a set of known acceptable pin numbers based on SBC
    log.debug({pin:pin, msg:'construtor - creating gpio pin'})
    if (isNaN(pin)) throw new Error('not a valid gpio pin number')
    super(pin,opts.direction || 'out',opts)
    this.id = (opts.id || 'gpio') + ':' + pin
    log.debug({ pin: pin, opts: opts, method:'constructor', line:15, msg:'created gpio with these opts'})
    this.number = pin
    this.direction = opts.direction || 'out'
    this.value = opts.init || 0
  } // end constructor

  async init() {
    await this.set(this.value)
    DeadJim((signal,err) => {
      log.warn({signal:signal, method:'init', line:56, error:err, msg:'gpio process was killed, unexporting pin'})
      this.unexport() // kill the kernel entry
    })
  }

  async on() { return await this.set(1) }
  async off() { return await this.set(0) }
  async toggle() { return await this.set(!this.value) }

  async set(value) {
    console.log('starting set NEW', value, this.number)
    // this.write(parseInt(value) || 0).then(res => console.log('returning from write'))
    let [err] = await to(this.write(parseInt(value) || 0))
    console.log('returned from write')
    if (err) {
      console.log('write error')
      log.warn({error:err, pin:this.number, msg:'error reading gpio pin'})
      throw err
    }
    console.log('doing positive read')
    let curValue = await this.read()
    console.log('returning from get', curValue)
    if (value !== curValue) throw new Error(`pin ${this.number} was not set correctly: ${value}:${curValue}`)
    return curValue
  }

  async read() {
    console.log('doing an actual read', this.number)
// !!!!  super.read here just calls this.read making an infinite loop and blowing the stack, should call onoff's Gpio read (as the parent)
    let [err, res] = await to(super.read())  
    if (err) {
      log.warn({error:err, pin:this.pin, msg:'error reading gpio pin'})
      return err
    }
    this.value=res
    this.state = res ? 'on' : 'off'
    return this.value
  }

  async get() {
    return this.value
  }

} // end Class

export default Pin

Here I just chose a different name that then calls onoff read. It works fine but I'm scratching my head why something that's always worked for me (super method calls) is not in this case.

import DeadJim from 'death'
import { Gpio } from 'onoff'
import to from 'await-to-js'


import logger from '@uci-utils/logger'

let log = logger({package:'@uci/gpio', file:'/src/gpio.js'})

class Pin extends Gpio {
  constructor(pin, opts = {}) {
    if (typeof pin !=='number') pin = parseInt(pin)  // make sure pin is a number!
    // TODO check against a set of known acceptable pin numbers based on SBC
    log.debug({pin:pin, msg:'construtor - creating gpio pin'})
    if (isNaN(pin)) throw new Error('not a valid gpio pin number')
    super(pin,opts.direction || 'out',opts)
    this.id = (opts.id || 'gpio') + ':' + pin
    log.debug({ pin: pin, opts: opts, method:'constructor', line:15, msg:'created gpio with these opts'})
    this.number = pin
    this.direction = opts.direction || 'out'
    this.value = opts.init || 0
  } // end constructor

  async init() {
    await this.set(this.value)
    DeadJim((signal,err) => {
      log.warn({signal:signal, method:'init', line:56, error:err, msg:'gpio process was killed, unexporting pin'})
      this.unexport() // kill the kernel entry
    })
  }

  async on() { return await this.set(1) }
  async off() { return await this.set(0) }
  async toggle() { return await this.set(!this.value) }

  async set(value) {
    console.log('starting set NEW', value, this.number)
    // this.write(parseInt(value) || 0).then(res => console.log('returning from write'))
    let [err] = await to(this.write(parseInt(value) || 0))
    console.log('returned from write')
    if (err) {
      console.log('write error')
      log.warn({error:err, pin:this.number, msg:'error reading gpio pin'})
      throw err
    }
    console.log('doing positive read')
    let curValue = await this.get(true)
    console.log('returning from get', curValue)
    if (value !== curValue) throw new Error(`pin ${this.number} was not set correctly: ${value}:${curValue}`)
    return curValue
  }

  async get(read) {
    if (read) {
      console.log('doing an actual read', this.number)
      let [err, res] = await to(this.read())
      if (err) {
        log.warn({error:err, pin:this.pin, msg:'error reading gpio pin'})
        return err
      }
      this.value=res
      this.state = res ? 'on' : 'off'
    }
    return this.value
  }

} // end Class

export default Pin

[fivdi]

@dkebler Welcome back and thank you for reporting this issue.

Do you always run your tests against latest node?

Yes, currently tests are run against Node.js 4, 6, 8, 10 and 12. See here in travis.yml.

The issue can also be reproduced with the following test program:

const Gpio = require('onoff').Gpio;

class InputPin extends Gpio {
  constructor(pin) {
    super(pin, 'in');
  }

  read() {
    console.log('InputPin#read');
    return super.read();
  }
}

let button = new InputPin(4);

setInterval(() => {
  button.read().then(value => {
    if (value === 1) {
      console.log('button pressed');
    } else {
      console.log('button released');
    }
  });
}, 1000);

The issue occurs due to a recent optimization. Part of the optimization is that Gpio#read now invokes Gpio#read recursively, i.e., it invokes itself recursively here. If Gpio#read is overridden, Gpio#read ends up invoking the override rather than Gpio#read. This results in infinite recursion.

I'll fix this asap.

If you would like to workaround the issue on your system until a fix is published change this line in onoff.js:

        this.read((err, value) => {

to:

        Gpio.prototype.read.call(this, (err, value) => {

[dkebler]

Phew... just couldn't grok why my code was blowing the stack. Took me awhile to track this down but glad it was something in onoff not my code. For now I am fine. When you push a fix and close this issue I'll know I can try out the fix. Thx

[fivdi]

fixed with b6f3ce3 in [email protected]

@dkebler once again, thank you for reporting this issue.

[dkebler]

finally getting back this...just confirming that a super.read() now works! with 4.1.3. Thx