From 1a6f77ece359bd89bdf4324aa88aa6de9ae4eace Mon Sep 17 00:00:00 2001
From: David Lechner <david@pybricks.com>
Date: Sat, 23 Jan 2021 13:46:54 -0600
Subject: [PATCH] remove disconnectMonitor()

This fixes multiple didFailToFinish() actions for a single flash
firmware request in certain cases.

Since 783f7e2 is fixed, waitForDidRequest() will always return with
success or error. So when used alone, waitForDidRequest() will process
the error. When using race(waitForDidRequest(), waitForResponse()),
waitForResponse() will handle the error if it wins the race and cancel
waitForResponse().
---
 src/sagas/flash-firmware.test.ts |  7 +++++--
 src/sagas/flash-firmware.ts      | 29 +++++++----------------------
 2 files changed, 12 insertions(+), 24 deletions(-)

diff --git a/src/sagas/flash-firmware.test.ts b/src/sagas/flash-firmware.test.ts
index eea5b67c..580105d3 100644
--- a/src/sagas/flash-firmware.test.ts
+++ b/src/sagas/flash-firmware.test.ts
@@ -312,8 +312,6 @@ describe('flashFirmware', () => {
             action = await saga.take();
             expect(action).toEqual(infoRequest(0));
 
-            saga.put(didRequest(0));
-
             // hub disconnects before replying
 
             saga.updateState({
@@ -328,6 +326,11 @@ describe('flashFirmware', () => {
                 didFailToFinish(FailToFinishReasonType.Disconnected),
             );
 
+            // On city hub, we can end up in this situation. BLE writeValueWithResponse()
+            // doesn't return until erasing is done, so there is a long window for
+            // this to happen.
+            saga.put(didRequest(0, new Error('failed due to disconnect')));
+
             await saga.end();
         });
 
diff --git a/src/sagas/flash-firmware.ts b/src/sagas/flash-firmware.ts
index a2a57674..2d2484da 100644
--- a/src/sagas/flash-firmware.ts
+++ b/src/sagas/flash-firmware.ts
@@ -11,7 +11,6 @@ import {
     call,
     cancel,
     delay,
-    fork,
     getContext,
     put,
     race,
@@ -107,9 +106,10 @@ function* waitForResponse<T extends BootloaderResponseAction>(
     type: BootloaderResponseActionType,
     timeout = 500,
 ): SagaGenerator<T> {
-    const { response, error, timedOut } = yield* race({
+    const { response, error, disconnected, timedOut } = yield* race({
         response: take<T>(type),
         error: take<BootloaderErrorResponseAction>(BootloaderResponseActionType.Error),
+        disconnected: take(BootloaderConnectionActionType.DidDisconnect),
         timedOut: delay(timeout),
     });
 
@@ -125,6 +125,11 @@ function* waitForResponse<T extends BootloaderResponseAction>(
         yield* disconnectAndCancel();
     }
 
+    if (disconnected) {
+        yield* put(didFailToFinish(FailToFinishReasonType.Disconnected));
+        yield* disconnectAndCancel();
+    }
+
     defined(response);
 
     return response;
@@ -233,23 +238,6 @@ function* loadFirmware(
     return { firmware, deviceId: metadata['device-id'], checksum };
 }
 
-/**
- * Monitors for BLE disconnection event. If disconnection occurs, then a failure
- * action is raised and the task (including the parent task) is canceled.
- */
-function* disconnectMonitor(): SagaGenerator<void> {
-    const { disconnected } = yield* race({
-        disconnected: take(BootloaderConnectionActionType.DidDisconnect),
-        finished: take(FlashFirmwareActionType.DidFinish),
-        failedToFinish: take(FlashFirmwareActionType.DidFailToFinish),
-    });
-
-    if (disconnected) {
-        yield* put(didFailToFinish(FailToFinishReasonType.Disconnected));
-        yield* disconnectAndCancel();
-    }
-}
-
 /**
  * Flashes firmware to a Powered Up device.
  * @param action The action that triggered this saga.
@@ -296,8 +284,6 @@ function* flashFirmware(action: FlashFirmwareFlashAction): Generator {
             return;
         }
 
-        const disconnectMonitorTask = yield* fork(disconnectMonitor);
-
         const nextMessageId = yield* getContext<() => number>('nextMessageId');
 
         const infoAction = yield* put(infoRequest(nextMessageId()));
@@ -438,7 +424,6 @@ function* flashFirmware(action: FlashFirmwareFlashAction): Generator {
 
         // this will cause the remote device to disconnect and reboot
         const rebootAction = yield* put(rebootRequest(nextMessageId()));
-        disconnectMonitorTask.cancel();
         yield* waitForDidRequest(rebootAction.id);
 
         yield* put(didFinish());