Closed dkebler closed 7 years ago
hmm...another thought is if you would maintain the C source /src as a separate repo. Then one could use it as a git-submodule as they please.
@dkebler Another option would be to create a thin library on top of i2c-bus
. Your library would take the same options as i2c-bus
and pass then along to openSync
(open
does not take advantage of asynchronous behaviour like the other bus operations do). Then you could provide promise returning methods wrapping as many of the asynchronous methods as you'd like.
That method seems like a better option than forking this library or adding the promise library dependency.
Currently open doesn't take advantage of asynchronous behavior like the other bus operations do as it doesn't execute any blocking code. In the past it did execute blocking code and in the future it may again do so. It would be a good idea to document this.
In other words, I'd still recommend people looking for asynchronous behavior to use open rather than openSync.
@dkebler yes, I'm willing to supply input/clarification.
The first important thing is that I want to support older versions of Node.js for as long as possible. Today, this means supporting Node.js v0.10 and higher. This also means that code that only works on v4+ can't be added yet.
A Bus object can be used to access multiple devices on the same I2C bus. For each device accessed a separate file descriptor is used. These file descriptors are stored in _peripherals.
@fivdi thx, that's great. Maybe rather than do that in an issue could you set up a gitter for the repo and we can do that there. Gitter is great for keeping a running viewable log of the conversation. If that sounds good you can close this issue.
For a specific issue like this one here I think a github issue would be better so let's continue the conversation here.
@fivdi ...a week later....I actually have a fully functioning system using this library (promisfied) and your on/off library so "it ain't broke" which makes me want to leave well enough alone.....
Of the all the promisify libraries I tried only bluebird would work and I think it's the only one that will do so to prototype methods and add them as just regular methods(keys) to the object. As I mentioned it promisfied all the sync functions too and on top of that my "bus" class is clunky in that is just wraps your bus class and then references the promisifyied methods
I have come across a few repos where they have written the async methods to return a promise if no callback is supplied. That's a nice way to satisfy your requirement to be backward compatible and yet support promises without creating another method. So about how me using this approach?
See approach 2 of http://loige.co/to-promise-or-to-callback-that-is-the-question/
FYI, below are the two classes that are the basis of my working i2c system and of course depend on i2c-bus
my bus class
const i2c = require('i2c-bus'),
pA = require('bluebird').promisifyAll,
PQ = require('promisqueue')
class Bus {
constructor(busnum) {
// this.bus is actually the entire i2c bus object (e.g. _peripherals) with all the
// promisfied methods as primary keys, not in the prototype" chain.
this.bus = pA(i2c.open(busnum,()=>{}),{suffix: "_p"})
this.q = new PQ({ limit: 1})
}
scan(){ return this.bus.scan_p()}
close(){ return this.bus.close_p()}
// this take promise, in particular a device class read/write as a promise
qAdd(job){this.q.add(() => job)}
} // end of Bus Class
module.exports = Bus;
then my i2c device class that take a bus class as a constructor argument.
class Device {
// bus is i2c-bus bus object
constructor(bus, address, opts) {
this.bus = bus.bus // artefact of adapting ic2-bus to class format and promisfying
this.address = address
if (opts){
this.id = opts.id // must be unique within a bus
this.desc = opts.desc
}
}
readRaw(length,buffer) {
return this.bus.i2cRead_p(this.address, length, buffer)
}
writeRaw(length,buffer) {
return this.bus.i2cWrite_p(this.address, length, buffer)
}
read(cmd) {
return this.bus.readByte_p(this.address, cmd)
}
write(cmd, byte) {
return this.bus.writeByte_p(this.address, cmd, byte)
}
read2(cmd) {
return this.bus.readWord_p(this.address, cmd)
}
write2(cmd, bytes) {
return this.bus.writeWord_p(this.address, cmd, bytes)
}
}
module.exports = Device;
here is some example code using these classes that waits for gpio pin on RPI connected to interrupt pin of a mcp23017, that then figures out which gpio on the mcp23017 changed and then changes a relay on another i2c relay board device.
const i2c = require('./class'),
_u = require('uci-utils')
let bus1 = new i2c.Bus(1)
// mcp extends from device class
let relays = new i2c.MCP.MCP23008(bus1, 0x21, {
name: 'relay 1-8'
})
let dio1_16 = new i2c.MCP.MCP23017(bus1, 0x27, {
desc: 'switches 1 to 16',
// pincfgDefault: 'output'
pincfgDefault: 'toggle_switch'
// pinConfig:{} // pin configuration override
})
relays.init().then(relays.write(9, 0))
bus1.qAdd(
dio1_16.init().then(dio1_16.read(0x08).then(resp => console.log('Initialize, Clear the Interrupt Pins', _u.byteFormat(resp, { in: 'DEC', out: 'STR' }))))
)
var Gpio = require('onoff').Gpio,
switchPortIntr = new Gpio(17, 'in', 'both');
switchPortIntr.watch(function(err, value) {
if (err) {
throw err;
}
console.log('GPIO interupt watch changed with value of ', value);
dio1_16.read(0x08).then(resp => {
console.log('Pins State at Interrupt', _u.byteFormat(resp, { in: 'DEC', out: 'STR' }))
relays.write(9, resp)
})
});
process.on('SIGINT', function() {
switchPortIntr.unexport();
});
process.on('SIGINT', function() {
switchbank.unexport();
});
thanks for all your work on these two repos! With them I was able to fairly simply get a i2c based system working involving multiple boards.
@dkebler good to hear that you've got things working.
I have come across a few repos where they have written the async methods to return a promise if no callback is supplied. That's a nice way to satisfy your requirement to be backward compatible and yet support promises without creating another method. So about how me using this approach?
Let me think about this approach for a while.
Not that I can really read C code but it seems as though you have a C method/function corresponding to each js prototype method. I have looked a few other libraries for I2c and wondering why you didn't just write a couple "raw" read, write functions and bind just those functions, creating all the rest as classes/extensions (and controlling concurrency) within node. Was there some speed benefit? In such an architecture it would be easier for folks like me to contribute and modify and for you the C code maintenance more trivial.
For example In my code I am controlling concurrency with a queue. But as I poke around in your C code it seems it might be already doing that? So why not just do that in node.
it seems as though you have a C method/function corresponding to each js prototype method.
Yes, this is mostly the case. Notable exceptions are the plain or raw I2C methods like i2cRead, i2cReadSync, i2cWrite, and i2cWriteSync which are "pure" JavaScript methods that use fs calls to access the I2C bus.
I have looked a few other libraries for I2c ...
Which ones have you looked at? :)
... and wondering why you didn't just write a couple "raw" read, write functions and bind just those functions, creating all the rest as classes/extensions (and controlling concurrency) within node. Was there some speed benefit?
It would be possible to achieve a lot with raw read and write functions. This is especially true if you are sure that your application is the only application accessing the I2C bus. However, in order to read data from an I2C device, two raw methods need to be called. The performance test here does just that. Because two raw methods are called, the operating system may do a task switch between the two calls. If multiple processes are accessing the device, this will give those other processes a chance to access the device and mess things up for the first process. This is where SMBus which is derived from I2C comes into play. It defines nine bus protocols. There is a short description of these bus protocols in section 2.3 of this document. Linux has good support for the bus protocols. They make things possible that are not possible with raw methods. For example, the “Read byte/word” protocol can be used to access a byte/word as an atomic transaction. Multiple processes can use the “Read byte/word” protocol to access the same device without issues. The concurrency aspects are managed by the operating system. The i2c-bus module implements most of the bus protocols.
For example In my code I am controlling concurrency with a queue.
Which concurrency aspects are you controlling with a queue?
I've looked at this library in particular https://github.com/kelly/node-i2c
In the code I have written all my requests to read/write to the I2C bus are formed as a promise. Then I use a promise queue. In this way I can be sure that each read/write request is finished before another is made. (see below)
So what you are saying is that is not necessary? That your C code already manages this and I can send all my read/write requests concurrently without the queue. I still need the promises on my end for when I need to wait on something from a bus device before further processing but maybe not the queue?
I am a bit further along then when I first opened this issue. Here is my bus class etc. below. I found an alternative to bluebird which made such a mess. Here I use pify to individually wrap each bus function's callback that I need. You can see I add a queue to the class. Then create a device class that uses the bus class to wrap your those that wrap your node bindings. Then I make an instance of the device class. Then use that instance and and instance of the bus class to actually do something.
So. Back to the original question. I am writing a bus class on top of your bus protytope. It works...not elegant and not DRY but it works. So like I said maybe having an alternative to your repo as it stands with a es6 bus class wrapping directly your C code is the way to go. Then you don't have to worry about supporting existing users. I can pull your C code out into it's own repo although then you would not be maintaining that. But here is what I can't do, write new node-gyp bindings (well I don't know how to do without a lot of learning).
So just repeating myself, but....what I have done works as is....it could be better and more forward looking with a promise based es6 bus class binding directly the C library.
const i2c = require('i2c-bus'),
pify = require('pify'),
PQueue = require('p-queue')
class Bus {
constructor(busnum = 1) {
this.busnum = busnum
this.queue = new PQueue({ concurrency: 1 });
this.qAdd = (p) => { this.queue.add(() => p) } // p is a promise
this.bus = i2c.open(this.busnum, () => {})
}
scan() { return pify(this.bus.scan).bind(this.bus)() }
close() { return pify(this.bus.close).bind(this.bus)() }
readRaw(address, length, buffer) {
return pify(this.bus.i2cRead).bind(this.bus)(address, length, buffer)
}
writeRaw(address, length, buffer) {
return pify(this.bus.i2cWrite).bind(this.bus)(address, length, buffer)
}
read(address, cmd) {
// console.log("read: address, cmd", address, cmd)
return pify(this.bus.readByte).bind(this.bus)(address, cmd)
}
write(address, cmd, byte) {
// console.log("write: address, cmd, byte", address, cmd, byte)
return pify(this.bus.writeByte.bind(this.bus))(address, cmd, byte)
}
read2(address, cmd) {
return pify(this.bus.readWord.bind(this.bus))(address, cmd)
}
write2(address, cmd, bytes) {
return pify(this.bus.writeWord.bind(this.bus))(address, cmd, bytes)
}
} // end of Bus Class
module.exports = {
Bus
};
then in a i2c device class which adds that i2c bus class as a constructor key and "encapulates" those promisified i2c-bus functions.
class Device {
// bus is i2c-bus bus object
constructor(bus, address, opts) {
this.bus = bus
this.address = address
if (opts) {
this.id = opts.id // must be unique within a bus
this.desc = opts.desc
}
}
readRaw(length, buffer) {
return this.bus.readRaw(this.address, length, buffer)
}
writeRaw(length, buffer) {
return this.bus.writeRaw(this.address, length, buffer)
}
read(cmd) {
return this.bus.read(this.address, cmd)
}
write(cmd, byte) {
return this.bus.write(this.address, cmd, byte)
}
read2(cmd) {
return this.bus.read2(this.address, cmd)
}
write2(cmd, bytes) {
return this.bus.write2(this.address, cmd, bytes)
}
}
module.exports = {
Device
}
Then use that in an actual device by extending that device class
const Device = require('uci-dev').Device,
Port = require('uci-gpio').Port,
_u = require('uci-utils')
class MCP23008 extends Device {
constructor(busObj, i2cAddress, opts = {}) {
super(busObj, i2cAddress, opts)
// opts could pass in array of custom pin config, or single for all, or anything
// this.registers = registers // load in register database
this.ports = {}
opts.portID = 'A'
opts.pids = opts.pids ? opts.pids : opts.pidsA
this.ports.A = new Port(opts)
this.chip_config = opts.chip_config // TODO allow opts.chip_config to be a byte instead of a string pointer
this.ports.A.interrupt = opts.interruptA ? opts.interruptA : opts.interrupt
if (this.ports.A.interrupt) { this.ports.A.interrupt.hook = 'A' }
} // end constructor
......methods
Finally using those classes to do something
const Bus = require('uci-i2c').Bus,
MCP = require('uci-mcp'),
Interrupt = require('uci-interrupt').Interrupt,
_ = require('uci-utils')
let bus = new Bus()
// mcp inherts from device class
let relays = new MCP.MCP23017(bus, 0x24, {
desc: 'relay 1-16'
})
let sw1_16 = new MCP.MCP23017(bus, 0x23, {
desc: 'switches 1 to 16',
// pin 36
......
})
// initialize boards
// init relay board
bus.qAdd(relays.init()
// .then((resp) => { console.log('relays back from init', resp) })
.catch((err) => console.log('====error====\n', err))
)
// init switch board
bus.qAdd(sw1_16.init()
.then((resp) => {
console.log('initilized\n', resp)
return sw1_16.start()
.then(resp => console.log(resp))
})
.catch((err) => console.log('====error====\n', err))
)
...
Implementing a new promises based module that uses i2c-bus is the best way to go here in my opinion. I don't want to add promises to i2c-bus. The native C functions in i2c-bus are not part of the i2c-bus public API and this is something that I don't want to change either. Because the native C functions are private, the implementation details can be changed at any time, if they were public this would no longer be possible.
The native C functions in i2c-bus don't actually queue requests either but the operating system does. i2c-bus therefore relies on the operating system to queue requests.
I don't understand why it's necessary to have a separate fork of the repo to achieve what you wish to achieve. Haven't you already achieved what you want to achieve with the code posted above?
I'm extremely new to node.js but this issue is on topic with what I'm looking at.
I've gotten the sample async code working on my hardware, a raspberry pi talking to an arduino.
'use strict';
var async = require('async');
var i2c = require('i2c-bus');
var i2cbus;
(function () {
async.series([
function (cb) {
i2cbus = i2c.open(1, cb);
},
function (cb) {
i2cbus.readByte(0x08, 0x90, function (err, rssi) {
if (err) {
throw err;
}
console.log("rssi: " + rssi);
cb(null);
});
},
function (cb) {
i2cbus.close(cb);
},
], function (err) {
if (err) {
throw err;
}
});
}());
This is super simple stuff to you guys but it's a lot to deal with all these call backs when learning for the first time.
In doing more research on this I came across the async await features included with the latest version of node.js.
I messed around with some sample code but calling the async functions, i2cbus.readByte, for example were returning errors. I eventually got this code working.
"use strict";
var request = require('request-promise');
var i2c = require('i2c-bus');
var i2cbus = i2c.openSync(1);
async function main() {
try {
var rssi = i2cbus.readByteSync(0x08, 0x90);
console.log(rssi);
}
catch(error) {
console.error(error);
}
i2cbus.closeSync();
}
main();
Is this code completely wrong? Is it going to cause issues as I start writing the rest of the program? Is it not even actually asynchronous now that I'm calling i2cbus.readByteSync?
The next step I'll be looking at is adding socket.io or koa-socket on top of this to serve the i2c data out to a browser.
@fivdi , yes I pretty much have achieved my goal but what I was hoping for was not to create this extra "wrapper" on top of node callbacks. The C code doesn't have to change just the node code bindings.
So yea it works yes but in the near future callback style functions will be history. In fact with the latest releases of node now include async/await as @howflyquad would like to use and I will probably change my code to use them soon.
If I knew a bit more about node-gyp, writing bindings, and C I'd give it a shot which I might next Fall.
What I can see is three branches (master=callbacks, promise and async) each with it's own style of bindings with the master branch being callback supporting the older versions of node.
In the meantime I have my promise/async wrappers for your callbacks and it will do.
@howflyquad Just my feeling but I would probably not wrap the sync bindings and instead wrap the callback versions. Async/await is new to me as never did the babel thing and only now does node support it. I haven't had time to think about reworking my wrappers of @fivdi 's callbacks into async versions but I'm assuming that one can manually wrap them or there is some package to help like the one I used for promise wrapping.
When I finally get around to switching over to async functions I'll let you know what I did.
fyi, quick search of github found this. https://github.com/rohitprajapati/node-safe-async-wrapper
@howflyquad fyi read this https://github.com/yortus/asyncawait#what-works-with-await
Looks like promisfy or thunkify are the two routes to make a callback style function "awaitable" So I have already promisfied i2c-bus which then could be awaited. I'll publish my uci-i2c which will get you going (see above). I'll probably do an alternate second export choice in uci-dev for aysnc functions instead of promise returning functions. Then you'd be on your way to whatever device you want to hook up. Currently I've only made extended class for mcp chip.
@dkebler I'm real late to the party, here, but I'm wondering if you ever published your uci-i2c? I looked on your github and couldn't find it and would REALLY love to make use of it in my project.
I wanted to use your prototype cb methods as promises and with a bit of fiddling I got bluebird promisifyAll to do that.
then
As you can see I'm kinda writing a Bus class on top of your Bus class to make this work...seems kinda clunky and nonDRY and I'm having to load the entire BB library to do it and promisfy even your sync methods which is silly.
What I can't live without is your bound C library but I would like to fork the main module (i2c-bus.js) and write it as a proper es6 class (given node now supports that directly) with methods all returning promises and probably dropping the sync methods all together.
So I'm willing to write an alternative class/promise module. Are you willing to give me some input/clarification I will need to do that? For example I'm a bit puzzled by the use of the _peripherals key in the constructor. Initially you have it as an empty array it then gets populated later by the prototype methods?? I'm pretty clueless on the whole binding/gyp/nan...etc. thing so I'd be just trying to use the prototype methods in the existing code more or less as is.
Anyway I could do this a separate file/module/class say i2c-pbus.js/pBus. Users could alternatively load this new module (class) that would keep backward compatibility for those using it now. Normally I'd make just make my own personal fork but I'm happy with your C library and not being a C programmer I'm going to want the advantage of your maintenance on that.
Just so you know I looked into wiring-pi node and the other i2c bus library at npm. The first one is a kitchen sink and not well maintained and the second won't even compile on my RPI3 and is also kinda crusty. So ic2-bus is dakind. Thx.