ffi use vs. child_process.exec(command)

jgtaylor commented 7 years ago

This isn't really an issue, but I discovered that module ffi wasn't doing well at one of the older bonescript levels (before updating to 2.0) when updating node to 7.x I fooled around with it and noticed it only gets called to do device tree compilation with dtc, via my.create_dt used in hw_capemgr.js.

I changed it to use child_process.exec(command), and it works ok. (ffi 2.0 also worked fine after updating and rebuilding)

Just out of curiosity, does bonescript really need the additional ffi package to execute the dtc command? Would it be easier to have fewer dependencies? Caveat - I haven't tested what happens if this part of the code runs when called, just verified that bonescript loads and produces output from .getPlatform();

here's the platform and nodejs version output:

  os: 
   { hostname: 'auto-garden',
     type: 'Linux',
     arch: 'arm',
     release: '4.4.52-bone16',
     uptime: 54211,
     loadavg: [ 0.14208984375, 0.0322265625, 0.009765625 ],
     totalmem: 519057408,
     freemem: 167845888,
     networkInterfaces: { lo: [Object], eth0: [Object] } },
  version: '00C0',
  serialNumber: '4215BBBK177E',
  dogtag: 'BeagleBoard.org Debian Image 2016-03-20\n' }
root@auto-garden:/usr/local/lib/node_modules/bonescript# nodejs -v 
v7.7.1

jadonk commented 6 years ago

In reality, with newer kernels and properly installed universal cape overlays with pinmux helpers, it shouldn't ever call the device tree compiler at all.

The system call was used to get real synchronous behavior as we need that routine to complete before moving on. I'll try to find some way to kill these lingering old implementations that likely no one is using.

jgtaylor commented 6 years ago

If async/sync is the issue, it looks like https://nodejs.org/dist/latest-v8.x/docs/api/child_process.html#child_process_synchronous_process_creations might help. These were added in 0.11, just after Bonescript got going :-), so it wasn't available back then. These are all blocking calls which should resolve the async issues, and remove one more dependency. I'll see if I can't get a pull request together for this issue. But like you mentioned - testing may be an issue with the pinmux helpers doing all the heavy lifting.

jadonk commented 6 years ago

With moving to node v4 as a minimum requirement, I'm able to remove ffi.

https://github.com/jadonk/bonescript/commit/772b6ab866bc769a637ec9dfb374306096043da1

jadonk / bonescript

ffi use vs. child_process.exec(command) #149