feat(sdk): Implement continuous scanning (#1814)

This implements an SDK.Scanner which handles any given
adapters and manages the scans. This change enables continuous
scanning without the need to `.scan()` scheduling in other places.

Change-Type: minor

lib/gui/app.js

@@ -203,7 +203,7 @@ app.run(() => {
 })
 
 app.run(($timeout) => {
-  driveScanner.on('drives', (drives) => {
+  driveScanner.on('devices', (drives) => {
     // Safely trigger a digest cycle.
     // In some cases, AngularJS doesn't acknowledge that the
     // available drives list has changed, and incorrectly

lib/gui/modules/drive-scanner.js

@@ -16,46 +16,12 @@
 
 'use strict'
 
-const EventEmitter = require('events').EventEmitter
 const _ = require('lodash')
 const Bluebird = require('bluebird')
 const fs = Bluebird.promisifyAll(require('fs'))
 const path = require('path')
 const settings = require('../models/settings')
-const sdk = require('../../shared/sdk')
-
-/**
- * @summary Time to wait between scans
- * @type {Number}
- * @constant
- */
-const DRIVE_SCANNER_INTERVAL_MS = 2000
-
-/**
- * @summary Scanner event emitter singleton instance
- * @type {Object}
- * @constant
- */
-const emitter = new EventEmitter()
-
-/*
- * This service emits the following events:
- *
- * - `drives (Object[])`
- * - `error (Error)`
- *
- * For example:
- *
- * ```
- * driveScanner.on('drives', (drives) => {
- *   console.log(drives);
- * });
- *
- * driveScanner.on('error', (error) => {
- *   throw error;
- * });
- * ```
- */
+const SDK = require('../../shared/sdk')
 
 /**
  * @summary The Etcher "blobs" directory path
@@ -64,87 +30,23 @@ const emitter = new EventEmitter()
  */
 const BLOBS_DIRECTORY = path.join(__dirname, '..', '..', 'blobs')
 
-/**
- * @summary Flag to control scanning status
- * @type {Boolean}
- */
-let scanning = false
+const scanner = SDK.createScanner({
+  standard: {
+    includeSystemDrives: settings.get('unsafeMode')
+  },
+  usbboot: {
+    readFile: (name) => {
+      const isRaspberryPi = _.includes([
+        'bootcode.bin',
+        'start_cd.elf',
+        'fixup_cd.dat'
+      ], name)
 
-/**
- * @summary Start the scanning loop
- * @function
- * @private
- *
- * @description
- * This function emits `drives` or `error` events
- * using the event emitter singleton instance.
- *
- * @returns {Promise}
- *
- * @example
- * scanning = true
- * scan()
- */
-const scan = () => {
-  if (!scanning) {
-    return Bluebird.resolve()
-  }
+      const blobPath = isRaspberryPi ? path.join('raspberrypi', name) : name
 
-  return sdk.scan({
-    standard: {
-      includeSystemDrives: settings.get('unsafeMode')
-    },
-    usbboot: {
-      readFile: (name) => {
-        const blobPath = _.includes([
-          'bootcode.bin',
-          'start_cd.elf',
-          'fixup_cd.dat'
-        ], name) ? path.join('raspberrypi', name) : name
-
-        return fs.readFileAsync(path.join(BLOBS_DIRECTORY, 'usbboot', blobPath))
-      }
+      return fs.readFileAsync(path.join(BLOBS_DIRECTORY, 'usbboot', blobPath))
     }
-  }).then((drives) => {
-    emitter.emit('drives', drives)
-  }).catch((error) => {
-    emitter.emit('error', error)
-  }).finally(() => {
-    if (!scanning) {
-      return Bluebird.resolve()
-    }
-
-    return Bluebird
-      .delay(DRIVE_SCANNER_INTERVAL_MS)
-      .then(scan)
-  })
-}
-
-/**
- * @summary Start scanning drives
- * @function
- * @public
- *
- * @example
- * driveScanner.start();
- */
-emitter.start = () => {
-  if (!scanning) {
-    scanning = true
-    scan()
   }
-}
+})
 
-/**
- * @summary Stop scanning drives
- * @function
- * @public
- *
- * @example
- * driveScanner.stop();
- */
-emitter.stop = () => {
-  scanning = false
-}
-
-module.exports = emitter
+module.exports = scanner

lib/shared/sdk/index.js

@@ -16,9 +16,12 @@
 
 'use strict'
 
-const Bluebird = require('bluebird')
+const EventEmitter = require('events')
 const _ = require('lodash')
-const sdk = module.exports
+const SDK = module.exports
+const debug = require('debug')('sdk')
+
+debug.enabled = true
 
 /**
  * @summary The list of loaded adapters
@@ -35,44 +38,223 @@ const ADAPTERS = [
  * @type {Object<String,Adapter>}
  * @constant
  */
-sdk.adapters = _.reduce(ADAPTERS, (adapters, Adapter) => {
-  adapters[Adapter.name] = new Adapter()
+SDK.adapters = _.reduce(ADAPTERS, (adapters, Adapter) => {
+  adapters[Adapter.id] = new Adapter()
   return adapters
 }, {})
 
+/* eslint-disable lodash/prefer-lodash-method */
+
 /**
- * @summary Scan for drives using all registered adapters
- * @function
- * @public
- *
- * @description
- * The options object contains options for all the registered
- * adapters. For the `standard` adapter, for example, place
- * options in `options.standard`.
- *
- * @param {Object} options - options
- * @fulfil {Object[]} - drives
- * @returns {Promise}
- *
+ * Adapter Scanner
+ * @class Scanner
+ */
+SDK.Scanner = class Scanner extends EventEmitter {
+  /**
+   * @summary Adapter Scanner constructor
+   * @param {Object<String,Object>} [options] - device adapter options
+   * @param {Object} [options.adapters] - map of external device adapters
+   * @example
+   * new SDK.Scanner({
+   *   standard: { ... },
+   *   usbboot: { ... }
+   * })
+   */
+  constructor (options = {}) {
+    // Inherit from EventEmitter
+    super()
+
+    this.options = options
+    this.isScanning = false
+    this.adapters = new Map()
+
+    // Bind event handlers to own context to facilitate
+    // removing listeners by reference
+    this.onDevices = this.onDevices.bind(this)
+    this.onError = this.onError.bind(this)
+
+    this.init()
+  }
+
+  /**
+   * @summary Initialize adapters
+   * @private
+   * @example
+   * // Only to be used internally
+   * this.init()
+   */
+  init () {
+    debug('scanner:init', this)
+    _.each(_.keys(this.options), (adapterId) => {
+      const adapter = SDK.adapters[adapterId] ||
+        _.get(this.options, [ 'adapters', adapterId ])
+
+      if (_.isNil(adapter)) {
+        throw new Error(`Unknown adapter "${adapterId}"`)
+      }
+
+      this.subsribe(adapter)
+    })
+  }
+
+  /**
+   * @summary Event handler for adapter's "device" events
+   * @private
+   * @example
+   * adapter.on('devices', this.onDevices)
+   */
+  onDevices () {
+    const devices = []
+    this.adapters.forEach((adapter) => {
+      devices.push(...adapter.devices)
+    })
+    this.emit('devices', devices)
+  }
+
+  /**
+   * @summary Event handler for adapter's "error" events
+   * @param {Error} error - error
+   * @private
+   * @example
+   * adapter.on('error', this.onError)
+   */
+  onError (error) {
+    this.emit('error', error)
+  }
+
+  /**
+   * @summary Start scanning for devices
+   * @public
+   * @returns {SDK.Scanner}
+   * @example
+   * scanner.start()
+   */
+  start () {
+    debug('start', !this.isScanning)
+    if (this.isScanning) {
+      return this
+    }
+
+    this.adapters.forEach((adapter) => {
+      const options = this.options[adapter.id]
+
+      /**
+       * @summary Run a scan with an adapter
+       * @function
+       * @private
+       * @example
+       * runScan()
+       */
+      const runScan = () => {
+        adapter.scan(options, () => {
+          if (this.isScanning) {
+            setTimeout(runScan, SDK.Scanner.MIN_SCAN_DELAY)
+          }
+        })
+      }
+
+      adapter
+        .on('devices', this.onDevices)
+        .on('error', this.onError)
+
+      runScan()
+    })
+
+    this.emit('start')
+    this.isScanning = true
+
+    return this
+  }
+
+  /**
+   * @summary Stop scanning for devices
+   * @public
+   * @returns {SDK.Scanner}
+   * @example
+   * scanner.stop()
+   */
+  stop () {
+    debug('stop', this.isScanning)
+    if (!this.isScanning) {
+      return this
+    }
+
+    this.adapters.forEach((adapter) => {
+      // Adapter.stopScan()
+      adapter.removeListener('devices', this.onDevices)
+      adapter.removeListener('error', this.onError)
+    })
+
+    this.isScanning = false
+    this.emit('stop')
+
+    return this
+  }
+
+  /**
+   * @summary Subsribe to an adapter
+   * @public
+   * @param {Adapter} adapter - device adapter
+   * @returns {SDK.Scanner}
+   * @example
+   * scanner.subscribe(adapter)
+   */
+  subsribe (adapter) {
+    debug('subsribe', adapter)
+
+    if (this.adapters.get(adapter.id)) {
+      throw new Error(`Scanner: Already subsribed to ${adapter.id}`)
+    }
+
+    this.adapters.set(adapter.id, adapter)
+    this.emit('subsribe', adapter)
+
+    return this
+  }
+
+  /**
+   * @summary Unsubsribe from an adapter
+   * @public
+   * @param {Adapter} adapter - device adapter
+   * @returns {SDK.Scanner}
+   * @example
+   * scanner.unsubscribe(adapter)
+   * // OR
+   * scanner.unsubscribe('adapterName')
+   */
+  unsubscribe (adapter) {
+    debug('unsubsribe', adapter)
+    const instance = _.isString(adapter) ? this.adapters.get(adapter) : this.adapters.get(adapter.id)
+
+    if (_.isNil(instance)) {
+      // Not subscribed
+      return this
+    }
+
+    this.adapters.delete(instance.name)
+    this.emit('unsubsribe', adapter)
+
+    return this
+  }
+}
+
+/**
+ * @summary Minimum delay between scans in ms
+ * @const
+ * @type {Number}
+ */
+SDK.Scanner.MIN_SCAN_DELAY = 500
+
+/**
+ * @summary Create a new Scanner
+ * @param {Object} [options] - options
+ * @returns {SDK.Scanner}
  * @example
- * sdk.scan({
- *   standard: {
- *     includeSystemDrives: true
- *   }
- * }).then((drives) => {
- *   console.log(drives)
+ * SDK.createScanner({
+ *   standard: { ... },
+ *   usbboot: { ... }
  * })
  */
-sdk.scan = (options) => {
-  return Bluebird.all(_.map(sdk.adapters, (adapter) => {
-    return new Bluebird((resolve, reject) => {
-      adapter.scan(_.get(options, [ adapter.id ], {}), (error, devices) => {
-        if (error) {
-          reject(error)
-        } else {
-          resolve(devices)
-        }
-      })
-    })
-  })).then(_.flatten)
+SDK.createScanner = (options) => {
+  return new SDK.Scanner(options)
 }

lib/shared/sdk/standard/index.js

@@ -37,6 +37,11 @@ class StandardAdapter extends EventEmitter {
 
     /** @type {String} Adapter name */
     this.id = this.constructor.id
+
+    this.devices = []
+    this.on('devices', (devices) => {
+      this.devices = devices
+    })
   }
 
   /**
@@ -45,7 +50,7 @@ class StandardAdapter extends EventEmitter {
    *
    * @param {Object} [options] - options
    * @param {Object} [options.includeSystemDrives=false] - include system drives
-   * @param {Function} callback - callback
+   * @param {Function} [callback] - optional callback
    * @returns {StandardAdapter}
    *
    * @example
@@ -71,12 +76,13 @@ class StandardAdapter extends EventEmitter {
 
       return drive
     }).catch((error) => {
-      callback(error)
+      this.emit('error', error)
+      callback && callback(error)
     }).filter((drive) => {
       return options.includeSystemDrives || !drive.system
     }).then((drives) => {
       this.emit('devices', drives)
-      callback(null, drives)
+      callback && callback(null, drives)
     })
 
     return this

lib/shared/sdk/usbboot/index.js

@@ -192,6 +192,11 @@ class USBBootAdapter extends EventEmitter {
 
     /** @type {Object} Progress hash */
     this.progress = {}
+
+    this.devices = []
+    this.on('devices', (devices) => {
+      this.devices = devices
+    })
   }
 
   /**
@@ -230,7 +235,7 @@ class USBBootAdapter extends EventEmitter {
    *
    * @param {Object} options - options
    * @param {Object} options.files - files buffers
-   * @param {Function} callback - callback
+   * @param {Function} [callback] - optional callback
    * @returns {USBBootAdapter}
    *
    * @example
@@ -268,9 +273,10 @@ class USBBootAdapter extends EventEmitter {
       }
 
       if (_.isNil(this.progress[result.raw])) {
-        // TODO: Emit an error event if this fails
         this.prepare(device, {
           readFile: options.readFile
+        }).catch((error) => {
+          this.emit('error', error)
         })
       }
 
@@ -286,10 +292,11 @@ class USBBootAdapter extends EventEmitter {
     }, {
       concurrency: 5
     }).catch((error) => {
-      callback(error)
+      this.emit('error', error)
+      callback && callback(error)
     }).then((devices) => {
       this.emit('devices', devices)
-      callback(null, devices)
+      callback && callback(null, devices)
     })
 
     return this

tests/gui/modules/drive-scanner.spec.js

@@ -18,27 +18,16 @@
 
 const m = require('mochainon')
 const os = require('os')
-const Bluebird = require('bluebird')
 const drivelist = require('drivelist')
 const driveScanner = require('../../../lib/gui/modules/drive-scanner')
-const sdk = require('../../../lib/shared/sdk')
 
 describe('Browser: driveScanner', function () {
-  describe('given no available drives', function () {
-    beforeEach(function () {
-      this.sdkScanStub = m.sinon.stub(sdk, 'scan')
-      this.sdkScanStub.returns(Bluebird.resolve([]))
-    })
-
-    afterEach(function () {
-      this.sdkScanStub.restore()
-    })
-
+  describe('detected devices should be an array', function () {
     it('should emit an empty array', function (done) {
       const spy = m.sinon.spy()
 
-      driveScanner.once('drives', function (drives) {
-        m.chai.expect(drives).to.deep.equal([])
+      driveScanner.once('devices', function (drives) {
+        m.chai.expect(drives).to.be.an.instanceof(Array)
         m.chai.expect(spy).to.not.have.been.called
         driveScanner.removeListener('error', spy)
         driveScanner.stop()
@@ -75,7 +64,7 @@ describe('Browser: driveScanner', function () {
     it('should emit an empty array', function (done) {
       const spy = m.sinon.spy()
 
-      driveScanner.once('drives', function (drives) {
+      driveScanner.once('devices', function (drives) {
         m.chai.expect(drives).to.deep.equal([])
         m.chai.expect(spy).to.not.have.been.called
         driveScanner.removeListener('error', spy)
@@ -148,7 +137,7 @@ describe('Browser: driveScanner', function () {
       it('should emit the non removable drives', function (done) {
         const spy = m.sinon.spy()
 
-        driveScanner.once('drives', function (drives) {
+        driveScanner.once('devices', function (drives) {
           m.chai.expect(drives).to.deep.equal([
             {
               device: '/dev/sdb',
@@ -246,7 +235,7 @@ describe('Browser: driveScanner', function () {
       it('should emit the non removable drives', function (done) {
         const spy = m.sinon.spy()
 
-        driveScanner.once('drives', function (drives) {
+        driveScanner.once('devices', function (drives) {
           m.chai.expect(drives).to.deep.equal([
             {
               device: '\\\\.\\PHYSICALDRIVE2',
@@ -309,7 +298,7 @@ describe('Browser: driveScanner', function () {
       it('should use the drive letter as the name', function (done) {
         const spy = m.sinon.spy()
 
-        driveScanner.once('drives', function (drives) {
+        driveScanner.once('devices', function (drives) {
           m.chai.expect(drives).to.have.length(1)
           m.chai.expect(drives[0].displayName).to.equal('F:')
           m.chai.expect(spy).to.not.have.been.called
@@ -355,7 +344,7 @@ describe('Browser: driveScanner', function () {
       it('should join all the mountpoints in `name`', function (done) {
         const spy = m.sinon.spy()
 
-        driveScanner.once('drives', function (drives) {
+        driveScanner.once('devices', function (drives) {
           m.chai.expect(drives).to.have.length(1)
           m.chai.expect(drives[0].displayName).to.equal('F:, G:, H:')
           m.chai.expect(spy).to.not.have.been.called