scanSync() - enhancement

[MaBecker]

like to have two parameter to pass own search range

Bus.prototype.scanSync = function (first_addr,last_addr) {
  var scanBus = openSync(this._busNumber, {forceAccess: this._forceAccess}),
    addresses = [],
    addr;

  var firstAddr = first_addr || FIRST_SCAN_ADDR;
  var lastAddr = last_addr || LAST_SCAN_ADDR;

  for (addr = firstAddr; addr <= lastAddr; addr += 1) {
    try {
      scanBus.receiveByteSync(addr);
      addresses.push(addr);
    } catch (ignore) {
    }
  }

  scanBus.closeSync();
  return addresses;
};

What do you think?

Not sure if you like the var names and code style, so I did not create a pull request.

[fivdi]

What do you think?

It looks like a useful enhancement and a PR would be welcome.

Not sure if you like the var names and code style, so I did not create a pull request.

I'd prefer if first_addr and last_addr were renamed to firstAddr and lastAddr respectively.

And if this:

  var firstAddr = first_addr || FIRST_SCAN_ADDR;
  var lastAddr = last_addr || LAST_SCAN_ADDR;

was changed to this:

  firstAddr = firstAddr || FIRST_SCAN_ADDR;
  lastAddr = lastAddr || LAST_SCAN_ADDR;

That being said, there are bugs that need to be fixed here. Calling scanSync(0, 0) would not scan the address range 0 through 0. It would scan the address range FIRST_SCAN_ADDR through LAST_SCAN_ADDR as 0 evaluates to false in JavaScript.

The asynchronous scan method would also need to be updated to provide the same functionality. The documentation would also need to be updated :)

[MaBecker]

https://developer.mozilla.org/de/docs/Web/JavaScript/Reference/Functions/Default_parameters

bugfix:

Bus.prototype.scanSync = function (firstAddr,lastAddr) {
  var scanBus = openSync(this._busNumber, {forceAccess: this._forceAccess}),
    addresses = [],
    addr;

  firstAddr = (typeof firstAddr !== 'undefined') ? firstAddr :  FIRST_SCAN_ADDR;
  lastAddr = (typeof lastAddr !== 'undefined') ? lastAddr :  LAST_SCAN_ADDR;

  for (addr = firstAddr; addr <= lastAddr; addr += 1) {
    try {
      scanBus.receiveByteSync(addr);
      addresses.push(addr);
    } catch (ignore) {
    }
  }

  scanBus.closeSync();
  return addresses;
};

[fivdi]

That looks better.

[MaBecker]

Can you please test, because I have no i2c async device on my desk

Bus.prototype.scan = function (firstAddr,lastAddr,cb) {
  var scanBus,
    addresses = [];

  firstAddr = (typeof firstAddr !== 'undefined') ? firstAddr :  FIRST_SCAN_ADDR;
  lastAddr = (typeof lastAddr !== 'undefined') ? lastAddr :  LAST_SCAN_ADDR;

  scanBus = open(this._busNumber, {forceAccess: this._forceAccess}, function (err) {
    if (err) {
      return cb(err);
    }

    (function next(addr) {
      if (addr > lastAddr) {
        return scanBus.close(function (err) {
          if (err) {
            return cb(err);
          }
          cb(null, addresses);
        });
      }

      scanBus.receiveByte(addr, function (err) {
        if (!err) {
          addresses.push(addr);
        }
        next(addr + 1);
      });
    }(firstAddr));
  });
};

[fivdi]

Synchronous and asynchronous methods are related to how Node.js works rather than how the I2C devices work. The synchronous and asynchronous methods will therefore work with any type of I2C device.

If you try the following code which uses the asynchronous scan method on your system it should print the addresses of all devices attached to I2C bus 1. The code will need to be modified if you're using a different bus.

'use strict';

var i2c = require('i2c-bus'),
  i2cBus = i2c.openSync(1);

i2cBus.scan(0, 0x7f, function (err, addresses) {
  addresses.forEach(function (address) {
    console.log('0x' + address.toString(16));
  });
});

Please note that firstAddr and lastAddr are optional and need not be specified. The following code should also function correctly and print the addresses of all devices attached to I2C bus 1.

'use strict';

var i2c = require('i2c-bus'),
  i2cBus = i2c.openSync(1);

i2cBus.scan(function (err, addresses) {
  addresses.forEach(function (address) {
    console.log('0x' + address.toString(16));
  });
});

[fivdi]

This extension to i2c-bus shouldn't be a breaking change. In other words, code that works today without the extensions should continue to work after the extension is added.

Given that firstAddr and lastAddr will both be optional, it will be valid to specify only one of them like this:

i2cBus.scanSync(5);

or this:

i2cBus.scan(5, function () {});

In these cases 5 should interpreted as firstAddr.

[MaBecker]

Hmm, what about make firstAddr and lastAddr public so user can change them to their own values or leave them as they are?

[fivdi]

Hmm, what about make firstAddr and lastAddr public so user can change them to their own values or leave them as they are?

I don't think this is a good idea. It's possible to use more than one I2C bus at the same time and the values may be different for each bus. The values may also be context dependent (and different in each context) for the same bus.

[MaBecker]

So what about a optional option section with fist and last?

i2cBus.scan(function(){}, {firstAddr:0, lastAddr:10});

[fivdi]

I prefer two optional parameters like this:

i2cBus.scan([firstAddr,] [lastAddr,] function () {});

[MaBecker]

OK, so thats it

[fivdi]

Excellent :smile:

[MaBecker]

running some test, changing the position of callback will end in a nightmare of parameter checking!

const FIRST_SCAN_ADDR = 3;
const LAST_SCAN_ADDR = 77;

var scan = function(cb, firstAddr, lastAddr){

  firstAddr = (typeof firstAddr !== 'undefined') ? firstAddr :  FIRST_SCAN_ADDR;
  lastAddr = (typeof lastAddr !== 'undefined') ? lastAddr :  LAST_SCAN_ADDR;

  console.log("scan:",firstAddr,lastAddr);

}

var scanSync = function(firstAddr,lastAddr){

  firstAddr = (typeof firstAddr !== 'undefined') ? firstAddr :  FIRST_SCAN_ADDR;
  lastAddr = (typeof lastAddr !== 'undefined') ? lastAddr :  LAST_SCAN_ADDR;

  console.log("scanSync:",firstAddr,lastAddr);

}

scan(function() { console.log("callback");});
scan(function() { console.log("callback");},0);
scan(function() { console.log("callback");},0,50);

scanSync();
scanSync(0);
scanSync(0,50);

output

scan: 3 77
scan: 0 77
scan: 0 50
scanSync: 3 77
scanSync: 0 77
scanSync: 0 50

[fivdi]

running some test, changing the position of callback will end in a nightmare of parameter checking!

Yes, it will make parameter checking more difficult. However, it's important for the callback to be the last parameter. If you look closely at the API you'll see that there is a pattern. The sync and async variants of a method have the same parameters except for the addition of the callback in the async variant and the callback is always the last parameter. Sometimes it's also the first parameter (if there is only one parameter) but it's always the last parameter and things should stay this way to keep things consistent (Wiedererkennungswert.)

[MaBecker]

please double check, any optimization hints are welcome.

'use strict';

const FIRST_SCAN_ADDR = 3;
const LAST_SCAN_ADDR = 77;

var scan = function(firstAddr, lastAddr,cb){

  if ( typeof firstAddr === 'number') {
    console.log("1. is a number");
  } else if (typeof firstAddr !== 'undefined') {
    console.log("1. is a function");
    cb = firstAddr;
    firstAddr = FIRST_SCAN_ADDR;
  }
  if ( typeof lastAddr === 'number') {
    console.log("2. is a number");
  } else if (typeof lastAddr !== 'undefined') {
    console.log("2. is a function");
    cb = lastAddr;
    lastAddr = LAST_SCAN_ADDR;
  } else {
    console.log("2. is undefined");
    lastAddr = LAST_SCAN_ADDR;
  }

  console.log("scan:",'['+firstAddr+'],['+lastAddr+'],['+cb+']');

}

var scanSync = function(firstAddr,lastAddr){

  firstAddr = (typeof firstAddr === 'number') ? firstAddr :  FIRST_SCAN_ADDR;
  lastAddr = (typeof lastAddr === 'number') ? lastAddr :  LAST_SCAN_ADDR;

  console.log("scanSync:",'['+firstAddr+'],['+lastAddr+']');

}

console.log('call scan(function() { console.log("callback");})');
scan(function() { console.log("callback");});
console.log('call scan(0,function() {console.log("callback");})');
scan(0,function() { console.log("callback");});
console.log('call scan(function() { 0,50,console.log("callback");})');
scan(0,50,function() { console.log("callback");});

console.log('call scanSync()');
scanSync();
console.log('cal scanSync(0)');
scanSync(0);
console.log('call scanSync(0,50)');
scanSync(0,50);

output

call scan(function() { console.log("callback");})
1. is a function
2. is undefined
scan: [3],[77],[function () { console.log("callback");}]
call scan(0,function() {console.log("callback");})
1. is a number
2. is a function
scan: [0],[77],[function () { console.log("callback");}]
call scan(function() { 0,50,console.log("callback");})
1. is a number
2. is a number
scan: [0],[50],[function () { console.log("callback");}]
call scanSync()
scanSync: [3],[77]
cal scanSync(0)
scanSync: [0],[77]
call scanSync(0,50)
scanSync: [0],[50]

[fivdi]

I modified the console output and added a few more test.

Here's the code:

'use strict';

const FIRST_SCAN_ADDR = 3;
const LAST_SCAN_ADDR = 77;

var scan = function(firstAddr, lastAddr,cb){
  var message = "[" + firstAddr + ", " + lastAddr + ", " + cb + "]";

  if ( typeof firstAddr === 'number') {
  } else if (typeof firstAddr !== 'undefined') {
    cb = firstAddr;
    firstAddr = FIRST_SCAN_ADDR;
  }
  if ( typeof lastAddr === 'number') {
  } else if (typeof lastAddr !== 'undefined') {
    cb = lastAddr;
    lastAddr = LAST_SCAN_ADDR;
  } else {
    lastAddr = LAST_SCAN_ADDR;
  }

  message += " -> [" + firstAddr + ", " + lastAddr + ", " + cb + "]";
  console.log(message);
}

var scanSync = function(firstAddr,lastAddr){
  var message = "[" + firstAddr + ", " + lastAddr + "]";

  firstAddr = (typeof firstAddr === 'number') ? firstAddr :  FIRST_SCAN_ADDR;
  lastAddr = (typeof lastAddr === 'number') ? lastAddr :  LAST_SCAN_ADDR;

  message += " -> [" + firstAddr + ", " + lastAddr + "]"
  console.log(message);
}

console.log('scan');
scan(function() {});
scan(0,function() {});
scan(0,50,function() {});
scan(50, 0, function() {});
scan(50, undefined, function() {});
scan(undefined, 50, function() {});
scan(undefined, undefined, function() {});
scan(null, null, function() {});
scan([], [], function() {});
scan({}, {}, function() {});
scan(false, false, function() {});
scan(true, true, function() {});
scan(NaN, NaN, function() {});
scan(-Infinity, Infinity, function() {});
scan(3.142, 1000, function() {});
scan(-2000, -1000, function() {});
scan(1000, 2000, function() {});
scan("hello", "world", function() {});
scan(0, 0x7f, function() {});

console.log();
console.log('scanSync');
scanSync();
scanSync(0);
scanSync(0,50);
scanSync(50, 0);
scanSync(50, undefined);
scanSync(undefined, 50);
scanSync(undefined, undefined);
scanSync(null, null);
scanSync([], []);
scanSync({}, {});
scanSync(false, false);
scanSync(true, true);
scanSync(NaN, NaN);
scanSync(-Infinity, Infinity);
scanSync(3.142, 1000);
scanSync(-2000, -1000);
scanSync(1000, 2000);
scanSync("hello", "world");
scanSync(0, 0x7f);

Here's the output with a comment at the end of each line indicating whether everything is ok of if there is a bug:

scan
[function () {}, undefined, undefined] -> [3, 77, function () {}] // ok
[0, function () {}, undefined] -> [0, 77, function () {}] // ok
[0, 50, function () {}] -> [0, 50, function () {}] // ok
[50, 0, function () {}] -> [50, 0, function () {}] // bug
[50, undefined, function () {}] -> [50, 77, function () {}] // ok
[undefined, 50, function () {}] -> [undefined, 50, function () {}] // bug
[undefined, undefined, function () {}] -> [undefined, 77, function () {}] // bug
[null, null, function () {}] -> [3, 77, null] // bug
[, , function () {}] -> [3, 77, ] // bug
[[object Object], [object Object], function () {}] -> [3, 77, [object Object]] // bug
[false, false, function () {}] -> [3, 77, false] // bug
[true, true, function () {}] -> [3, 77, true] // bug
[NaN, NaN, function () {}] -> [NaN, NaN, function () {}] // bug
[-Infinity, Infinity, function () {}] -> [-Infinity, Infinity, function () {}] // bug
[3.142, 1000, function () {}] -> [3.142, 1000, function () {}] // bug
[-2000, -1000, function () {}] -> [-2000, -1000, function () {}] // bug
[1000, 2000, function () {}] -> [1000, 2000, function () {}] // bug
[hello, world, function () {}] -> [3, 77, world] // bug
[0, 127, function () {}] -> [0, 127, function () {}] // ok

scanSync
[undefined, undefined] -> [3, 77] // ok
[0, undefined] -> [0, 77] // ok
[0, 50] -> [0, 50] // ok
[50, 0] -> [50, 0] // bug
[50, undefined] -> [50, 77] // ok
[undefined, 50] -> [3, 50] // ok
[undefined, undefined] -> [3, 77] // ok
[null, null] -> [3, 77] // ok
[, ] -> [3, 77] // ok
[[object Object], [object Object]] -> [3, 77] // ok
[false, false] -> [3, 77] // ok
[true, true] -> [3, 77] // ok
[NaN, NaN] -> [NaN, NaN] // bug
[-Infinity, Infinity] -> [-Infinity, Infinity] // bug
[3.142, 1000] -> [3.142, 1000] // bug
[-2000, -1000] -> [-2000, -1000] // bug
[1000, 2000] -> [1000, 2000] // bug
[hello, world] -> [3, 77] // ok
[0, 127] -> [0, 127] // ok

So there are a few bugs.

The following code would fix things:

'use strict';

const FIRST_SCAN_ADDR = 3;
const LAST_SCAN_ADDR = 77;

var scan = function(firstAddr, lastAddr,cb){
  var message = "[" + firstAddr + ", " + lastAddr + ", " + cb + "]";

  if (typeof firstAddr === 'function') {
    cb = firstAddr;
    firstAddr = undefined;
    lastAddr = undefined;
  } else if (typeof lastAddr === 'function') {
    cb = lastAddr;
    lastAddr = undefined;
  }

  // ensure firstAddr and lastAddr are integers
  firstAddr = Number.isInteger(firstAddr) ? firstAddr :  FIRST_SCAN_ADDR;
  lastAddr = Number.isInteger(lastAddr) ? lastAddr :  LAST_SCAN_ADDR;

  // fallback to default if range is invalid
  if (firstAddr > lastAddr ||
      firstAddr < 0 || firstAddr > 0x7f ||
      lastAddr < 0 || lastAddr > 0x7f) {
    firstAddr = FIRST_SCAN_ADDR;
    lastAddr = LAST_SCAN_ADDR;
  }

  message += " -> [" + firstAddr + ", " + lastAddr + ", " + cb + "]";
  console.log(message);
}

var scanSync = function(firstAddr,lastAddr){
  var message = "[" + firstAddr + ", " + lastAddr + "]";

  // ensure firstAddr and lastAddr are integers
  firstAddr = Number.isInteger(firstAddr) ? firstAddr :  FIRST_SCAN_ADDR;
  lastAddr = Number.isInteger(lastAddr) ? lastAddr :  LAST_SCAN_ADDR;

  // fallback to default if range is invalid
  if (firstAddr > lastAddr ||
      firstAddr < 0 || firstAddr > 0x7f ||
      lastAddr < 0 || lastAddr > 0x7f) {
    firstAddr = FIRST_SCAN_ADDR;
    lastAddr = LAST_SCAN_ADDR;
  }

  message += " -> [" + firstAddr + ", " + lastAddr + "]"
  console.log(message);
}

console.log('scan');
scan(function() {});
scan(0,function() {});
scan(0,50,function() {});
scan(50, 0, function() {});
scan(50, undefined, function() {});
scan(undefined, 50, function() {});
scan(undefined, undefined, function() {});
scan(null, null, function() {});
scan([], [], function() {});
scan({}, {}, function() {});
scan(false, false, function() {});
scan(true, true, function() {});
scan(NaN, NaN, function() {});
scan(-Infinity, Infinity, function() {});
scan(3.142, 1000, function() {});
scan(-2000, -1000, function() {});
scan(1000, 2000, function() {});
scan("hello", "world", function() {});
scan(0, 0x7f, function() {});

console.log();
console.log('scanSync');
scanSync();
scanSync(0);
scanSync(0,50);
scanSync(50, 0);
scanSync(50, undefined);
scanSync(undefined, 50);
scanSync(undefined, undefined);
scanSync(null, null);
scanSync([], []);
scanSync({}, {});
scanSync(false, false);
scanSync(true, true);
scanSync(NaN, NaN);
scanSync(-Infinity, Infinity);
scanSync(3.142, 1000);
scanSync(-2000, -1000);
scanSync(1000, 2000);
scanSync("hello", "world");
scanSync(0, 0x7f);

Here's the output of the fixed code:

scan
[function () {}, undefined, undefined] -> [3, 77, function () {}]
[0, function () {}, undefined] -> [0, 77, function () {}]
[0, 50, function () {}] -> [0, 50, function () {}]
[50, 0, function () {}] -> [3, 77, function () {}]
[50, undefined, function () {}] -> [50, 77, function () {}]
[undefined, 50, function () {}] -> [3, 50, function () {}]
[undefined, undefined, function () {}] -> [3, 77, function () {}]
[null, null, function () {}] -> [3, 77, function () {}]
[, , function () {}] -> [3, 77, function () {}]
[[object Object], [object Object], function () {}] -> [3, 77, function () {}]
[false, false, function () {}] -> [3, 77, function () {}]
[true, true, function () {}] -> [3, 77, function () {}]
[NaN, NaN, function () {}] -> [3, 77, function () {}]
[-Infinity, Infinity, function () {}] -> [3, 77, function () {}]
[3.142, 1000, function () {}] -> [3, 77, function () {}]
[-2000, -1000, function () {}] -> [3, 77, function () {}]
[1000, 2000, function () {}] -> [3, 77, function () {}]
[hello, world, function () {}] -> [3, 77, function () {}]
[0, 127, function () {}] -> [0, 127, function () {}]

scanSync
[undefined, undefined] -> [3, 77]
[0, undefined] -> [0, 77]
[0, 50] -> [0, 50]
[50, 0] -> [3, 77]
[50, undefined] -> [50, 77]
[undefined, 50] -> [3, 50]
[undefined, undefined] -> [3, 77]
[null, null] -> [3, 77]
[, ] -> [3, 77]
[[object Object], [object Object]] -> [3, 77]
[false, false] -> [3, 77]
[true, true] -> [3, 77]
[NaN, NaN] -> [3, 77]
[-Infinity, Infinity] -> [3, 77]
[3.142, 1000] -> [3, 77]
[-2000, -1000] -> [3, 77]
[1000, 2000] -> [3, 77]
[hello, world] -> [3, 77]
[0, 127] -> [0, 127]

[MaBecker]

Cool - So next step could be to optimize code and write functions for multiple used code

eg:

  // fallback to default if range is invalid
  if (firstAddr > lastAddr ||
      firstAddr < 0 || firstAddr > 0x7f ||
      lastAddr < 0 || lastAddr > 0x7f) {
    firstAddr = FIRST_SCAN_ADDR;
    lastAddr = LAST_SCAN_ADDR;
  } 

as a function

'use strict';

const FIRST_SCAN_ADDR = 3;
const LAST_SCAN_ADDR = 77;

function fallBackAddr( firstAddr, lastAddr) {
  var message = "[" + firstAddr + ", " + lastAddr + "]";
  if (firstAddr > lastAddr ||
      firstAddr < 0 || firstAddr > 0x7f ||
      lastAddr < 0 || lastAddr > 0x7f) {
    firstAddr = FIRST_SCAN_ADDR;
    lastAddr = LAST_SCAN_ADDR;
  }
  message += " -> [" + firstAddr + ", " + lastAddr + "]";
  console.log(message);
  return { lastAddr, firstAddr};
}

var { firstAddr,lastAddr } = fallBackAddr( -5, .10 );
console.log("[" + firstAddr + ", " + lastAddr + "]");

[fivdi]

Good idea. A bit more could be factored out:

function addressRange(firstAddr, lastAddr) {
  // ensure firstAddr and lastAddr are integers
  firstAddr = Number.isInteger(firstAddr) ? firstAddr :  FIRST_SCAN_ADDR;
  lastAddr = Number.isInteger(lastAddr) ? lastAddr :  LAST_SCAN_ADDR;

  // fallback to default if range is invalid
  if (firstAddr > lastAddr ||
      firstAddr < 0 || firstAddr > 0x7f ||
      lastAddr < 0 || lastAddr > 0x7f) {
    firstAddr = FIRST_SCAN_ADDR;
    lastAddr = LAST_SCAN_ADDR;
  }

  return {firstAddr, lastAddr};
}

[fivdi]

i2c-bus still supports Node.js v0.10.x so instead of

return {firstAddr, lastAddr};

the following will be needed

return {
  firstAddr: firstAddr,
  lastAddr: lastAddr
};

[MaBecker]

so how does the call look like?

[fivdi]

I'm not sure I understand. Which call?

[MaBecker]

// function
function addressRange(firstAddr, lastAddr) {
    .......
    return { firstAddr: firstAddr, lastAddr: lastAddr};
}

// does this work with node v0.10.x ?
//call
var addrRange = addressRange( -5, .10 );
firstAddr = addrRange.firstAddr;
lasrtAddr = addrRange.lastAddr;

[fivdi]

Yes, that will work fine with Node.js v0.10.x.

[fivdi]

@MaBecker any news on this one? were you able to make any progress?

[MaBecker]

sorry very busy at the moment, but will continue

[fivdi]

Hey @MaBecker, I need this enhancement for a project so I went ahead and implemented it. It's available with i2c-bus@3.1.0. The implementation is a little different than discussed above so please check out the docs for scan and/or sacnSync if you would like to use it.

[MaBecker]

Wow - thanks - will give a try soon