Initial work on full flashing speed (#397)

Almost all the impact is from the connection library option but the other tweaks make it easier to rule things out. We're still spending seconds of full flashing time updating the UI only so there's more that can be done here. To reproduce the original issue flash a Python hex before either the data collection or the MakeCode hexes.
microbit-foundation · Oct 17, 2024 · ef39410 · ef39410
1 parent f40354a
commit ef39410
Show file tree

Hide file tree

Showing 9 changed files with 100 additions and 102 deletions.
diff --git a/package-lock.json b/package-lock.json
diff --git a/package.json b/package.json
@@ -60,7 +60,7 @@
     "@emotion/react": "^11.11.4",
     "@emotion/styled": "^11.11.5",
     "@microbit/makecode-embed": "^0.0.0-alpha.7",
-    "@microbit/microbit-connection": "^0.0.0-alpha.21",
+    "@microbit/microbit-connection": "^0.0.0-alpha.25",
     "@microbit/ml-header-generator": "^0.4.3",
     "@tensorflow/tfjs": "^4.20.0",
     "@types/w3c-web-serial": "^1.0.6",

diff --git a/src/components/ConnectionFlowDialogs.tsx b/src/components/ConnectionFlowDialogs.tsx
@@ -1,12 +1,12 @@
-import { useDisclosure } from "@chakra-ui/react";
-import { useCallback, useEffect, useState } from "react";
+import { useCallback, useState } from "react";
 import { bluetoothUniversalHex } from "../connection-stage-actions";
 import {
   ConnectionFlowStep,
   ConnectionFlowType,
   ConnectionStage,
   useConnectionStage,
 } from "../connection-stage-hooks";
+import { useLogging } from "../logging/logging-hooks";
 import BrokenFirmwareDialog from "./BrokenFirmwareDialog";
 import ConnectBatteryDialog from "./ConnectBatteryDialog";
 import ConnectCableDialog, {
@@ -23,29 +23,19 @@ import TryAgainDialog from "./TryAgainDialog";
 import UnsupportedMicrobitDialog from "./UnsupportedMicrobitDialog";
 import WebUsbBluetoothUnsupportedDialog from "./WebUsbBluetoothUnsupportedDialog";
 import WhatYouWillNeedDialog from "./WhatYouWillNeedDialog";
-import { useLogging } from "../logging/logging-hooks";
 
 const ConnectionDialogs = () => {
   const { stage, actions } = useConnectionStage();
   const logging = useLogging();
   const [flashProgress, setFlashProgress] = useState<number>(0);
-  const { isOpen, onClose: onCloseDialog, onOpen } = useDisclosure();
   const [microbitName, setMicrobitName] = useState<string | undefined>(
     stage.bluetoothMicrobitName
   );
   const onClose = useCallback(() => {
     actions.setFlowStep(ConnectionFlowStep.None);
-    onCloseDialog();
-  }, [actions, onCloseDialog]);
+  }, [actions]);
 
-  useEffect(() => {
-    if (stage.flowStep !== ConnectionFlowStep.None && !isOpen) {
-      onOpen();
-    }
-    if (stage.flowStep === ConnectionFlowStep.None && isOpen) {
-      onClose();
-    }
-  }, [isOpen, onClose, onOpen, stage]);
+  const isOpen = stage.flowStep !== ConnectionFlowStep.None;
 
   const progressCallback = useCallback(
     (progress: number) => {
@@ -57,51 +47,8 @@ const ConnectionDialogs = () => {
     [actions, stage.flowStep]
   );
 
-  const onChangeMicrobitName = useCallback(
-    (name: string) => {
-      actions.onChangeMicrobitName(name);
-      setMicrobitName(name);
-    },
-    [actions]
-  );
-
-  const onFlashSuccess = useCallback((newStage: ConnectionStage) => {
-    // Inferring microbit name saves the user from entering the pattern
-    // for bluetooth connection flow
-    if (newStage.bluetoothMicrobitName) {
-      setMicrobitName(newStage.bluetoothMicrobitName);
-    }
-  }, []);
-
-  const connectAndFlash = useCallback(async () => {
-    if (stage.flowType === ConnectionFlowType.ConnectRadioBridge) {
-      logging.event({
-        type: "connect-user",
-        message: "radio-bridge",
-      });
-    }
-    await actions.connectAndflashMicrobit(progressCallback, onFlashSuccess);
-  }, [actions, logging, onFlashSuccess, progressCallback, stage.flowType]);
-
-  const onSkip = useCallback(
-    () => actions.setFlowStep(ConnectionFlowStep.ConnectBattery),
-    [actions]
-  );
-  const onInstructManualFlashing = useCallback(
-    () => actions.setFlowStep(ConnectionFlowStep.ManualFlashingTutorial),
-    [actions]
-  );
-
   const dialogCommonProps = { isOpen, onClose };
 
-  const handleConnectBluetooth = useCallback(() => {
-    logging.event({
-      type: "connect-user",
-      message: "bluetooth",
-    });
-    void actions.connectBluetooth();
-  }, [actions, logging]);
-
   switch (stage.flowStep) {
     case ConnectionFlowStep.ReconnectFailedTwice:
     case ConnectionFlowStep.Start: {
@@ -134,12 +81,29 @@ const ConnectionDialogs = () => {
           onBackClick={actions.onBackClick}
           onNextClick={actions.onNextClick}
           config={config}
-          onSkip={onSkip}
+          onSkip={() => actions.setFlowStep(ConnectionFlowStep.ConnectBattery)}
           onSwitch={actions.switchFlowType}
         />
       );
     }
     case ConnectionFlowStep.WebUsbFlashingTutorial: {
+      const connectAndFlash = async () => {
+        const onFlashSuccess = (newStage: ConnectionStage) => {
+          // Inferring microbit name saves the user from entering the pattern
+          // for bluetooth connection flow
+          if (newStage.bluetoothMicrobitName) {
+            setMicrobitName(newStage.bluetoothMicrobitName);
+          }
+        };
+
+        if (stage.flowType === ConnectionFlowType.ConnectRadioBridge) {
+          logging.event({
+            type: "connect-user",
+            message: "radio-bridge",
+          });
+        }
+        await actions.connectAndflashMicrobit(progressCallback, onFlashSuccess);
+      };
       return (
         <SelectMicrobitUsbDialog
           {...dialogCommonProps}
@@ -175,11 +139,21 @@ const ConnectionDialogs = () => {
           onBackClick={actions.onBackClick}
           onNextClick={actions.onNextClick}
           microbitName={microbitName}
-          onChangeMicrobitName={onChangeMicrobitName}
+          onChangeMicrobitName={(name: string) => {
+            actions.onChangeMicrobitName(name);
+            setMicrobitName(name);
+          }}
         />
       );
     }
     case ConnectionFlowStep.ConnectBluetoothTutorial: {
+      const handleConnectBluetooth = () => {
+        logging.event({
+          type: "connect-user",
+          message: "bluetooth",
+        });
+        void actions.connectBluetooth();
+      };
       return (
         <SelectMicrobitBluetoothDialog
           {...dialogCommonProps}
@@ -227,7 +201,9 @@ const ConnectionDialogs = () => {
       return (
         <BrokenFirmwareDialog
           {...dialogCommonProps}
-          onSkip={onInstructManualFlashing}
+          onSkip={() =>
+            actions.setFlowStep(ConnectionFlowStep.ManualFlashingTutorial)
+          }
           onTryAgain={actions.onTryAgain}
         />
       );

diff --git a/src/components/DownloadDialogs.tsx b/src/components/DownloadDialogs.tsx
@@ -1,34 +1,24 @@
-import { useCallback } from "react";
+import { useDownloadActions } from "../hooks/download-hooks";
+import { useLogging } from "../logging/logging-hooks";
+import { DownloadStep } from "../model";
+import { useStore } from "../store";
+import { getTotalNumSamples } from "../utils/gestures";
 import ConnectCableDialog from "./ConnectCableDialog";
-import DownloadProgressDialog from "./DownloadProgressDialog";
+import ConnectRadioDataCollectionMicrobitDialog from "./ConnectRadioDataCollectionMicrobitDialog";
 import DownloadChooseMicrobitDialog from "./DownloadChooseMicrobitDialog";
 import DownloadHelpDialog from "./DownloadHelpDialog";
+import DownloadProgressDialog from "./DownloadProgressDialog";
+import IncompatibleEditorDevice from "./IncompatibleEditorDevice";
 import ManualFlashingDialog from "./ManualFlashingDialog";
 import SelectMicrobitUsbDialog from "./SelectMicrobitUsbDialog";
-import { DownloadStep as DownloadStep } from "../model";
-import { useDownloadActions } from "../hooks/download-hooks";
-import { useStore } from "../store";
 import UnplugRadioLinkMicrobitDialog from "./UnplugRadioLinkMicrobitDialog";
-import ConnectRadioDataCollectionMicrobitDialog from "./ConnectRadioDataCollectionMicrobitDialog";
-import IncompatibleEditorDevice from "./IncompatibleEditorDevice";
-import { useLogging } from "../logging/logging-hooks";
-import { getTotalNumSamples } from "../utils/gestures";
 
 const DownloadDialogs = () => {
   const actions = useDownloadActions();
   const stage = useStore((s) => s.download);
+  const flashingProgress = useStore((s) => s.downloadFlashingProgress);
   const gestures = useStore((s) => s.gestures);
   const logging = useLogging();
-  const handleDownloadProject = useCallback(async () => {
-    logging.event({
-      type: "hex-download",
-      detail: {
-        actions: gestures.length,
-        samples: getTotalNumSamples(gestures),
-      },
-    });
-    await actions.connectAndFlashMicrobit(stage);
-  }, [actions, gestures, logging, stage]);
 
   switch (stage.step) {
     case DownloadStep.Help:
@@ -81,7 +71,18 @@ const DownloadDialogs = () => {
           onNextClick={actions.getOnNext()}
         />
       );
-    case DownloadStep.WebUsbFlashingTutorial:
+    case DownloadStep.WebUsbFlashingTutorial: {
+      const handleDownloadProject = async () => {
+        logging.event({
+          type: "hex-download",
+          detail: {
+            actions: gestures.length,
+            samples: getTotalNumSamples(gestures),
+          },
+        });
+        await actions.connectAndFlashMicrobit(stage);
+      };
+
       return (
         <SelectMicrobitUsbDialog
           isOpen
@@ -90,12 +91,13 @@ const DownloadDialogs = () => {
           onNextClick={handleDownloadProject}
         />
       );
+    }
     case DownloadStep.FlashingInProgress:
       return (
         <DownloadProgressDialog
           isOpen
           headingId="downloading-header"
-          progress={stage.flashProgress * 100}
+          progress={flashingProgress * 100}
         />
       );
     case DownloadStep.ManualFlashingTutorial:

diff --git a/src/components/DownloadProgressDialog.tsx b/src/components/DownloadProgressDialog.tsx
@@ -29,6 +29,8 @@ export const getHeadingId = (flowType: ConnectionFlowType) => {
   }
 };
 
+const noop = () => {};
+
 const DownloadProgressDialog = ({
   isOpen,
   headingId,
@@ -39,7 +41,7 @@ const DownloadProgressDialog = ({
       closeOnOverlayClick={false}
       motionPreset="none"
       isOpen={isOpen}
-      onClose={() => {}}
+      onClose={noop}
       size="3xl"
       isCentered
     >

diff --git a/src/connect-actions.ts b/src/connect-actions.ts
@@ -122,7 +122,10 @@ export class ConnectActions {
     try {
       await usb.flash(data, {
         partial: true,
-        progress: (v: number | undefined) => progress(v ?? 100),
+        // If we could improve the re-rendering due to progress further we can remove this and accept the
+        // default which updates 4x as often.
+        minimumProgressIncrement: 0.01,
+        progress: (v: number | undefined) => progress(v ?? 1),
       });
       return ConnectResult.Success;
     } catch (e) {

diff --git a/src/hooks/download-hooks.tsx b/src/hooks/download-hooks.tsx
@@ -23,6 +23,7 @@ import { useSettings, useStore } from "../store";
 import { downloadHex } from "../utils/fs-util";
 
 export class DownloadProjectActions {
+  private flashingProgressCallback: (value: number) => void;
   constructor(
     private state: DownloadState,
     private setState: (stage: DownloadState) => void,
@@ -31,8 +32,16 @@ export class DownloadProjectActions {
     private connectActions: ConnectActions,
     private connectionStage: ConnectionStage,
     private connectionStageActions: ConnectionStageActions,
-    private connectionStatus: ConnectionStatus
-  ) {}
+    private connectionStatus: ConnectionStatus,
+    flashingProgressCallback: (value: number) => void
+  ) {
+    this.flashingProgressCallback = (value: number) => {
+      if (state.step !== DownloadStep.FlashingInProgress) {
+        setState({ ...state, step: DownloadStep.FlashingInProgress });
+      }
+      flashingProgressCallback(value);
+    };
+  }
 
   clearMakeCodeUsbDevice = () => {
     this.setState({ ...this.state, usbDevice: undefined });
@@ -196,14 +205,6 @@ export class DownloadProjectActions {
     }
   };
 
-  private flashingProgressCallback = (progress: number) => {
-    this.setState({
-      ...this.state,
-      step: DownloadStep.FlashingInProgress,
-      flashProgress: progress,
-    });
-  };
-
   getOnNext = () => {
     const nextStep = this.getNextStep();
     return nextStep ? () => this.setStep(nextStep) : undefined;
@@ -270,6 +271,9 @@ export class DownloadProjectActions {
 
 export const useDownloadActions = (): DownloadProjectActions => {
   const stage = useStore((s) => s.download);
+  const setDownloadFlashingProgress = useStore(
+    (s) => s.setDownloadFlashingProgress
+  );
   const setStage = useStore((s) => s.setDownload);
   const [settings, setSettings] = useSettings();
   const connectActions = useConnectActions();
@@ -288,13 +292,15 @@ export const useDownloadActions = (): DownloadProjectActions => {
         connectActions,
         connectionStage,
         connectionStageActions,
-        connectionStatus
+        connectionStatus,
+        setDownloadFlashingProgress
       ),
     [
       connectActions,
       connectionStage,
       connectionStageActions,
       connectionStatus,
+      setDownloadFlashingProgress,
       setSettings,
       setStage,
       settings,

diff --git a/src/model.ts b/src/model.ts
@@ -130,7 +130,6 @@ export enum MicrobitToFlash {
 export interface DownloadState {
   step: DownloadStep;
   microbitToFlash: MicrobitToFlash;
-  flashProgress: number;
   hex?: HexData;
   // The micro:bit used to flash the hex.  We remember your choice for easy code
   // iteration for as long as the editor is open.