EV3/Microbit critical fixes for code freeze (#1354)

* Resolves - BLESession and BTSession should emit PERIPHERAL_SCAN_TIMEOUT #1348.

* Resolves - BLESession should handle 'could not find service' error #1350.

* Resolves - BTSession should handle 'no peripheral connected' error #1351.

* Fixing a typo that caused device scan timeout bugs.

* Resolves - Add casting and clamping throughout the EV3 extension #1352.

* Fixing a linting error.

* Further fixes for issue #1351.

src/engine/runtime.js

@@ -428,6 +428,14 @@ class Runtime extends EventEmitter {
         return 'PERIPHERAL_ERROR';
     }
 
+    /**
+     * Event name for reporting that a peripheral has not been discovered.
+     * @const {string}
+     */
+    static get PERIPHERAL_SCAN_TIMEOUT () {
+        return 'PERIPHERAL_SCAN_TIMEOUT';
+    }
+
     /**
      * Event name for reporting that blocksInfo was updated.
      * @const {string}

src/extensions/scratch3_ev3/index.js

@@ -1,9 +1,10 @@
 const ArgumentType = require('../../extension-support/argument-type');
 const BlockType = require('../../extension-support/block-type');
 const Cast = require('../../util/cast');
-const log = require('../../util/log');
+// const log = require('../../util/log');
 const Base64Util = require('../../util/base64-util');
 const BTSession = require('../../io/btSession');
+const MathUtil = require('../../util/math-util');
 
 // TODO: Refactor/rename all these high level primitives to be clearer/match
 
@@ -53,6 +54,8 @@ const MOTOR_PORTS = [
     }
 ];
 
+const VALID_MOTOR_PORTS = [0, 1, 2, 3];
+
 /**
  * Array of accepted sensor ports.
  * @note These should not be translated as they correspond to labels on
@@ -78,6 +81,8 @@ const SENSOR_PORTS = [
     }
 ];
 
+const VALID_SENSOR_PORTS = [0, 1, 2, 3];
+
 // firmware pdf page 100
 const EV_DEVICE_TYPES = {
     29: 'color',
@@ -120,7 +125,6 @@ class EV3 {
         /**
          * State
          */
-        this.connected = false;
         this._sensorPorts = [];
         this._motorPorts = [];
         this._sensors = {
@@ -201,7 +205,7 @@ class EV3 {
     }
 
     get distance () {
-        if (!this.connected) return 0;
+        if (!this.getPeripheralIsConnected()) return 0;
 
         // https://shop.lego.com/en-US/EV3-Ultrasonic-Sensor-45504
         // Measures distances between one and 250 cm (one to 100 in.)
@@ -214,13 +218,13 @@ class EV3 {
     }
 
     get brightness () {
-        if (!this.connected) return 0;
+        if (!this.getPeripheralIsConnected()) return 0;
 
         return this._sensors.brightness;
     }
 
     getMotorPosition (port) {
-        if (!this.connected) return;
+        if (!this.getPeripheralIsConnected()) return;
 
         let value = this._motors.positions[port];
         value = value % 360;
@@ -230,15 +234,13 @@ class EV3 {
     }
 
     isButtonPressed (port) {
-        if (!this.connected) return;
+        if (!this.getPeripheralIsConnected()) return;
 
         return this._sensors.buttons[port];
     }
 
     beep (freq, time) {
-        if (!this.connected) return;
+        if (!this.getPeripheralIsConnected()) return;
-
-        log.info('should be beeping');
 
         const cmd = [];
         cmd[0] = 15; // length
@@ -274,7 +276,7 @@ class EV3 {
     }
 
     motorTurnClockwise (port, time) {
-        if (!this.connected) return;
+        if (!this.getPeripheralIsConnected()) return;
 
         // Build up motor command
         const cmd = this._applyPrefix(0, this._motorCommand(
@@ -309,7 +311,7 @@ class EV3 {
     }
 
     motorTurnCounterClockwise (port, time) {
-        if (!this.connected) return;
+        if (!this.getPeripheralIsConnected()) return;
 
         // Build up motor command
         const cmd = this._applyPrefix(0, this._motorCommand(
@@ -365,7 +367,7 @@ class EV3 {
     }
 
     motorRotate (port, degrees) {
-        if (!this.connected) return;
+        if (!this.getPeripheralIsConnected()) return;
 
         // Build up motor command
         const cmd = this._applyPrefix(0, this._motorCommand(
@@ -397,7 +399,7 @@ class EV3 {
     }
 
     motorSetPosition (port, degrees) {
-        if (!this.connected) return;
+        if (!this.getPeripheralIsConnected()) return;
 
         // Calculate degrees to turn
         let previousPos = this._motors.positions[port];
@@ -443,7 +445,7 @@ class EV3 {
     }
 
     motorSetPower (port, power) {
-        if (!this.connected) return;
+        if (!this.getPeripheralIsConnected()) return;
 
         this._motors.speeds[port] = power;
     }
@@ -453,7 +455,6 @@ class EV3 {
     // *******
 
     _stopAllMotors () {
-        log.info('stop all motors');
         for (let i = 0; i < this._motorPorts.length; i++) {
             if (this._motorPorts[i] !== 'none') {
                 this.motorCoast(i);
@@ -555,8 +556,6 @@ class EV3 {
 
     // TODO: keep here? / refactor
     _onSessionConnect () {
-        this.connected = true;
-
         // start polling
         // TODO: window?
         this._pollingIntervalID = window.setInterval(this._getSessionData.bind(this), 150);
@@ -564,7 +563,7 @@ class EV3 {
 
     // TODO: keep here? / refactor
     _getSessionData () {
-        if (!this.connected) {
+        if (!this.getPeripheralIsConnected()) {
             window.clearInterval(this._pollingIntervalID);
             return;
         }
@@ -595,7 +594,6 @@ class EV3 {
             cmd[0] = cmd.length - 2;
             cmd[5] = 33;
 
-            // log.info(`REQUEST DEVICE LIST: ${compoundCommand}`);
             // Clear sensor data
             this._updateDevices = true;
             this._sensorPorts = [];
@@ -682,8 +680,8 @@ class EV3 {
             this._motorPorts[1] = EV_DEVICE_TYPES[array[22]] ? EV_DEVICE_TYPES[array[22]] : 'none';
             this._motorPorts[2] = EV_DEVICE_TYPES[array[23]] ? EV_DEVICE_TYPES[array[23]] : 'none';
             this._motorPorts[3] = EV_DEVICE_TYPES[array[24]] ? EV_DEVICE_TYPES[array[24]] : 'none';
-            log.info(`sensor ports: ${this._sensorPorts}`);
+            // log.info(`sensor ports: ${this._sensorPorts}`);
-            log.info(`motor ports: ${this._motorPorts}`);
+            // log.info(`motor ports: ${this._motorPorts}`);
             this._updateDevices = false;
             // eslint-disable-next-line no-undefined
         } else if (!this._sensorPorts.includes(undefined) && !this._motorPorts.includes(undefined)) {
@@ -703,7 +701,7 @@ class EV3 {
                     // Read brightness / distance values and set to 0 if null
                     this._sensors[EV_DEVICE_LABELS[this._sensorPorts[i]]] = value ? value : 0;
                 }
-                log.info(`${JSON.stringify(this._sensors)}`);
+                // log.info(`${JSON.stringify(this._sensors)}`);
                 offset += 4;
             }
             // READ MOTOR POSITION VALUES
@@ -720,7 +718,7 @@ class EV3 {
                 if (value) {
                     this._motors.positions[i] = value;
                 }
-                log.info(`motor positions: ${this._motors.positions}`);
+                // log.info(`motor positions: ${this._motors.positions}`);
                 offset += 4;
             }
         }
@@ -1002,22 +1000,37 @@ class Scratch3Ev3Blocks {
 
     motorTurnClockwise (args) {
         const port = Cast.toNumber(args.PORT);
-        const time = Cast.toNumber(args.TIME) * 1000;
+        let time = Cast.toNumber(args.TIME) * 1000;
+        time = MathUtil.clamp(time, 0, 15000);
+
+        if (!VALID_MOTOR_PORTS.includes(port)) {
+            return;
+        }
 
         return this._device.motorTurnClockwise(port, time);
     }
 
     motorTurnCounterClockwise (args) {
         const port = Cast.toNumber(args.PORT);
-        const time = Cast.toNumber(args.TIME) * 1000;
+        let time = Cast.toNumber(args.TIME) * 1000;
+        time = MathUtil.clamp(time, 0, 15000);
+
+        if (!VALID_MOTOR_PORTS.includes(port)) {
+            return;
+        }
 
         return this._device.motorTurnCounterClockwise(port, time);
     }
 
+    /*
     motorRotate (args) {
         const port = Cast.toNumber(args.PORT);
         const degrees = Cast.toNumber(args.DEGREES);
 
+        if (!VALID_MOTOR_PORTS.includes(port)) {
+            return;
+        }
+
         this._device.motorRotate(port, degrees);
         return;
     }
@@ -1026,40 +1039,55 @@ class Scratch3Ev3Blocks {
         const port = Cast.toNumber(args.PORT);
         const degrees = Cast.toNumber(args.DEGREES);
 
+        if (!VALID_MOTOR_PORTS.includes(port)) {
+            return;
+        }
+
         this._device.motorSetPosition(port, degrees);
         return;
     }
+    */
 
     motorSetPower (args) {
         const port = Cast.toNumber(args.PORT);
-        const power = Cast.toNumber(args.POWER);
+        const power = MathUtil.clamp(Cast.toNumber(args.POWER), 0, 100);
 
-        const value = Math.max(-100, Math.min(power, 100));
+        if (!VALID_MOTOR_PORTS.includes(port)) {
+            return;
+        }
 
-        this._device.motorSetPower(port, value);
+        this._device.motorSetPower(port, power);
         return;
     }
 
     getMotorPosition (args) {
         const port = Cast.toNumber(args.PORT);
 
+        if (!VALID_MOTOR_PORTS.includes(port)) {
+            return;
+        }
+
         return this._device.getMotorPosition(port);
     }
 
     whenButtonPressed (args) {
         const port = Cast.toNumber(args.PORT);
 
+        if (!VALID_SENSOR_PORTS.includes(port)) {
+            return;
+        }
+
         return this._device.isButtonPressed(port);
     }
 
     whenDistanceLessThan (args) {
-        const distance = Cast.toNumber(args.DISTANCE);
+        const distance = MathUtil.clamp(Cast.toNumber(args.DISTANCE), 0, 100);
 
         return this._device.distance < distance;
     }
 
     whenBrightnessLessThan (args) {
-        const brightness = Cast.toNumber(args.DISTANCE);
+        const brightness = MathUtil.clamp(Cast.toNumber(args.DISTANCE), 0, 100);
 
         return this._device.brightness < brightness;
     }
@@ -1067,6 +1095,10 @@ class Scratch3Ev3Blocks {
     buttonPressed (args) {
         const port = Cast.toNumber(args.PORT);
 
+        if (!VALID_SENSOR_PORTS.includes(port)) {
+            return;
+        }
+
         return this._device.isButtonPressed(port);
     }
 
@@ -1079,8 +1111,13 @@ class Scratch3Ev3Blocks {
     }
 
     beep (args) {
-        const note = Cast.toNumber(args.NOTE);
+        const note = MathUtil.clamp(Cast.toNumber(args.NOTE), 47, 99); // valid EV3 sounds
-        const time = Cast.toNumber(args.TIME * 1000);
+        let time = Cast.toNumber(args.TIME) * 1000;
+        time = MathUtil.clamp(time, 0, 3000);
+
+        if (time === 0) {
+            return; // don't send a beep time of 0
+        }
 
         // https://en.wikipedia.org/wiki/MIDI_tuning_standard#Frequency_values
         const freq = Math.pow(2, ((note - 69 + 12) / 12)) * 440;

src/extensions/scratch3_microbit/index.js

@@ -105,7 +105,6 @@ class MicroBit {
      * Called by the runtime when user wants to scan for a device.
      */
     startDeviceScan () {
-        log.info('making a new BLE session');
         this._ble = new BLESession(this._runtime, {
             filters: [
                 {services: [BLEUUID.service]}

src/io/bleSession.js

@@ -22,11 +22,11 @@ class BLESession extends JSONRPCWebSocket {
 
         this._availablePeripherals = {};
         this._connectCallback = connectCallback;
+        this._connected = false;
         this._characteristicDidChangeCallback = null;
         this._deviceOptions = deviceOptions;
+        this._discoverTimeoutID = null;
         this._runtime = runtime;
-
-        this._connected = false;
     }
 
     /**
@@ -35,7 +35,8 @@ class BLESession extends JSONRPCWebSocket {
      */
     requestDevice () {
         if (this._ws.readyState === 1) { // is this needed since it's only called on ws.onopen?
-            // TODO: start a 'discover' timeout
+            this._availablePeripherals = {};
+            this._discoverTimeoutID = window.setTimeout(this._sendDiscoverTimeout.bind(this), 15000);
             this.sendRemoteRequest('discover', this._deviceOptions)
                 .catch(e => this._sendError(e)); // never reached?
         }
@@ -88,7 +89,10 @@ class BLESession extends JSONRPCWebSocket {
                 this._runtime.constructor.PERIPHERAL_LIST_UPDATE,
                 this._availablePeripherals
             );
-            // TODO: cancel a discover timeout if one is active
+            if (this._discoverTimeoutID) {
+                // TODO: window?
+                window.clearTimeout(this._discoverTimeoutID);
+            }
             break;
         case 'characteristicDidChange':
             this._characteristicDidChangeCallback(params.message);
@@ -115,8 +119,10 @@ class BLESession extends JSONRPCWebSocket {
             params.startNotifications = true;
         }
         this._characteristicDidChangeCallback = onCharacteristicChanged;
-        return this.sendRemoteRequest('read', params);
+        return this.sendRemoteRequest('read', params)
-        // TODO: handle error here
+            .catch(e => {
+                this._sendError(e);
+            });
     }
 
     /**
@@ -132,7 +138,10 @@ class BLESession extends JSONRPCWebSocket {
         if (encoding) {
             params.encoding = encoding;
         }
-        return this.sendRemoteRequest('write', params);
+        return this.sendRemoteRequest('write', params)
+            .catch(e => {
+                this._sendError(e);
+            });
     }
 
     _sendError (e) {
@@ -140,6 +149,10 @@ class BLESession extends JSONRPCWebSocket {
         log.error(`BLESession error: ${JSON.stringify(e)}`);
         this._runtime.emit(this._runtime.constructor.PERIPHERAL_ERROR);
     }
+
+    _sendDiscoverTimeout () {
+        this._runtime.emit(this._runtime.constructor.PERIPHERAL_SCAN_TIMEOUT);
+    }
 }
 
 module.exports = BLESession;

src/io/btSession.js

@@ -23,12 +23,12 @@ class BTSession extends JSONRPCWebSocket {
 
         this._availablePeripherals = {};
         this._connectCallback = connectCallback;
+        this._connected = false;
         this._characteristicDidChangeCallback = null;
         this._deviceOptions = deviceOptions;
+        this._discoverTimeoutID = null;
         this._messageCallback = messageCallback;
         this._runtime = runtime;
-
-        this._connected = false;
     }
 
     /**
@@ -37,7 +37,8 @@ class BTSession extends JSONRPCWebSocket {
      */
     requestDevice () {
         if (this._ws.readyState === 1) { // is this needed since it's only called on ws.onopen?
-            // TODO: start a 'discover' timeout
+            this._availablePeripherals = {};
+            this._discoverTimeoutID = window.setTimeout(this._sendDiscoverTimeout.bind(this), 15000);
             this.sendRemoteRequest('discover', this._deviceOptions)
                 .catch(e => this._sendError(e)); // never reached?
         }
@@ -76,9 +77,11 @@ class BTSession extends JSONRPCWebSocket {
         return this._connected;
     }
 
-
     sendMessage (options) {
-        return this.sendRemoteRequest('send', options);
+        return this.sendRemoteRequest('send', options)
+            .catch(e => {
+                this._sendError(e);
+            });
     }
 
     /**
@@ -96,7 +99,10 @@ class BTSession extends JSONRPCWebSocket {
                 this._runtime.constructor.PERIPHERAL_LIST_UPDATE,
                 this._availablePeripherals
             );
-            // TODO: cancel a discover timeout if one is active
+            if (this._discoverTimeoutID) {
+                // TODO: window?
+                window.clearTimeout(this._discoverTimeoutID);
+            }
             break;
         case 'didReceiveMessage':
             this._messageCallback(params); // TODO: refine?
@@ -107,9 +113,14 @@ class BTSession extends JSONRPCWebSocket {
     }
 
     _sendError (e) {
+        this._connected = false;
         log.error(`BTSession error: ${JSON.stringify(e)}`);
         this._runtime.emit(this._runtime.constructor.PERIPHERAL_ERROR);
     }
+
+    _sendDiscoverTimeout () {
+        this._runtime.emit(this._runtime.constructor.PERIPHERAL_SCAN_TIMEOUT);
+    }
 }
 
 module.exports = BTSession;

src/virtual-machine.js

@@ -115,6 +115,9 @@ class VirtualMachine extends EventEmitter {
         this.runtime.on(Runtime.PERIPHERAL_ERROR, () =>
             this.emit(Runtime.PERIPHERAL_ERROR)
         );
+        this.runtime.on(Runtime.PERIPHERAL_SCAN_TIMEOUT, () =>
+            this.emit(Runtime.PERIPHERAL_SCAN_TIMEOUT)
+        );
 
         this.extensionManager = new ExtensionManager(this.runtime);