From 5395f9c3218cabe613da54c59b5ea1fd8ebbba9d Mon Sep 17 00:00:00 2001
From: Evelyn Eastmond <evhan55@gmail.com>
Date: Sun, 19 Aug 2018 22:09:57 -0400
Subject: [PATCH] WeDo2 changes for upcoming smoke test (#1493)

* Starting on #1491: use 'startNotifications' instead of 'read'.  Still need to make backwards compatible.

* Fixing #1489: WeDo2 set light color block should wrap values to 0-100.

* Fixing #1488: WeDo2 should not clear LED light on stop button.

* Fixing to support negative LED light color inputs.

* Removing a binding.

* Using wrapClamp for inputHue for LED color block.
---
 src/extensions/scratch3_wedo2/index.js | 21 +++++++++++++++------
 src/io/bleSession.js                   | 22 +++++++++++++++++++---
 2 files changed, 34 insertions(+), 9 deletions(-)

diff --git a/src/extensions/scratch3_wedo2/index.js b/src/extensions/scratch3_wedo2/index.js
index 8ff0578b..5b6f0d16 100644
--- a/src/extensions/scratch3_wedo2/index.js
+++ b/src/extensions/scratch3_wedo2/index.js
@@ -509,7 +509,11 @@ class WeDo2 {
             })
             .then(() => {
                 // register for attached io notifications
-                this._ble.read(UUID.DEVICE_SERVICE, UUID.ATTACHED_IO, true, this._onMessage);
+                // TODO: make backwards compatible with 'read':
+                // - try 'startNotifications'
+                // - then try 'read' with 'startNotifications' flag
+                // - then catch OSX and Windows errors
+                this._ble.startNotifications(UUID.DEVICE_SERVICE, UUID.ATTACHED_IO, this._onMessage);
             });
     }
 
@@ -618,7 +622,11 @@ class WeDo2 {
 
             this._send(UUID.INPUT_COMMAND, Base64Util.uint8ArrayToBase64(cmd))
                 .then(() => {
-                    this._ble.read(UUID.IO_SERVICE, UUID.INPUT_VALUES, true, this._onMessage.bind(this));
+                    // TODO: make backwards compatible with 'read':
+                    // - try 'startNotifications'
+                    // - then try 'read' with 'startNotifications' flag
+                    // - then catch OSX and Windows errors
+                    this._ble.startNotifications(UUID.IO_SERVICE, UUID.INPUT_VALUES, this._onMessage);
                 });
         }
     }
@@ -646,12 +654,12 @@ class WeDo2 {
     }
 
     /**
-     * Stop the tone playing, LED output and motors on the WeDo 2.0 hub.
+     * Stop the tone playing and motors on the WeDo 2.0 hub.
      */
     _stopAll () {
         this.stopTone();
         this.stopAllMotors();
-        this.stopLED();
+        // this.stopLED();
     }
 }
 
@@ -1064,8 +1072,9 @@ class Scratch3WeDo2Blocks {
      */
     setLightHue (args) {
         // Convert from [0,100] to [0,360]
-        const inputHue = Cast.toNumber(args.HUE);
-        const hue = MathUtil.clamp(inputHue, 0, 100) * 360 / 100;
+        let inputHue = Cast.toNumber(args.HUE);
+        inputHue = MathUtil.wrapClamp(inputHue, 0, 100);
+        const hue = inputHue * 360 / 100;
 
         const rgbObject = color.hsvToRgb({h: hue, s: 1, v: 1});
 
diff --git a/src/io/bleSession.js b/src/io/bleSession.js
index b543fdce..b44c314f 100644
--- a/src/io/bleSession.js
+++ b/src/io/bleSession.js
@@ -103,14 +103,30 @@ class BLESession extends JSONRPCWebSocket {
     }
 
     /**
-     * Start reading from the specified ble service.
+     * Start receiving notifications from the specified ble service.
+     * @param {number} serviceId - the ble service to read.
+     * @param {number} characteristicId - the ble characteristic to get notifications from.
+     * @param {object} onCharacteristicChanged - callback for characteristic change notifications.
+     * @return {Promise} - a promise from the remote startNotifications request.
+     */
+    startNotifications (serviceId, characteristicId, onCharacteristicChanged = null) {
+        const params = {
+            serviceId,
+            characteristicId
+        };
+        this._characteristicDidChangeCallback = onCharacteristicChanged;
+        return this.sendRemoteRequest('startNotifications', params);
+    }
+
+    /**
+     * Read from the specified ble service.
      * @param {number} serviceId - the ble service to read.
      * @param {number} characteristicId - the ble characteristic to read.
      * @param {boolean} optStartNotifications - whether to start receiving characteristic change notifications.
      * @param {object} onCharacteristicChanged - callback for characteristic change notifications.
      * @return {Promise} - a promise from the remote read request.
      */
-    read (serviceId, characteristicId, optStartNotifications = false, onCharacteristicChanged) {
+    read (serviceId, characteristicId, optStartNotifications = false, onCharacteristicChanged = null) {
         const params = {
             serviceId,
             characteristicId
@@ -121,7 +137,7 @@ class BLESession extends JSONRPCWebSocket {
         this._characteristicDidChangeCallback = onCharacteristicChanged;
         return this.sendRemoteRequest('read', params)
             .catch(e => {
-                if (e.data !== 'Reading is not permitted.') { // TODO: workaround til notify-only supported
+                if (e.data !== 'Reading is not permitted.') { // TODO: move this error check to extension
                     this._sendError(e);
                 }
             });