Avoid resetting serial terminal when possible.

Follow up to ca1f325

This aims to avoid EVENT_RESET when its due to the workaround we have in
place that disconnects background tabs.

Author: microbit-matt-hillsdon

src/device/device.ts

@@ -129,8 +129,23 @@ export interface FlashDataSource {
   files(): Promise<Record<string, Uint8Array>>;
 }
 
+export const enum SerialOption {
+  /**
+   * Don't read serial data.
+   */
+  None,
+  /**
+   * Emit a reset to clear any listeners, then read serial data.
+   */
+  Reset,
+  /**
+   * Read serial data. No reset is emitted.
+   */
+  NoReset,
+}
+
 export interface ConnectOptions {
-  serial?: boolean;
+  serial: SerialOption;
 }
 
 export interface DeviceConnection extends EventEmitter {

src/device/webusb.ts

@@ -23,6 +23,7 @@ import {
   FlashDataSource,
   HexGenerationError,
   MicrobitWebUSBConnectionOptions,
+  SerialOption,
   WebUSBError,
 } from "./device";
 
@@ -78,7 +79,11 @@ export class MicrobitWebUSBConnection
         this.visibilityReconnect = false;
         if (!this.flashing) {
           this.log("Reconnecting visible tab");
-          this.connect();
+          this.connect({
+            // If any other connection status change occurs then visibilitReconnect is set to false, so
+            // it's likely the same program at this point.
+            serial: SerialOption.NoReset,
+          });
         }
       }
     } else {
@@ -113,7 +118,7 @@ export class MicrobitWebUSBConnection
         setTimeout(() => {
           if (this.status === ConnectionStatus.CONNECTED) {
             this.unloading = false;
-            this.startSerialInternal();
+            this.startSerialInternal(SerialOption.NoReset);
           }
         }, assumePageIsStayingOpenDelay);
       },
@@ -165,7 +170,9 @@ export class MicrobitWebUSBConnection
     }
   }
 
-  async connect(options: ConnectOptions = {}): Promise<ConnectionStatus> {
+  async connect(
+    options: ConnectOptions = { serial: SerialOption.Reset }
+  ): Promise<ConnectionStatus> {
     return this.withEnrichedErrors(async () => {
       await this.connectInternal(options);
       return this.status;
@@ -217,7 +224,7 @@ export class MicrobitWebUSBConnection
     await this.stopSerialInternal();
     this.log("Reconnecting before flash");
     await this.connectInternal({
-      serial: false,
+      serial: SerialOption.None,
     });
     if (!this.connection) {
       throw new Error("Must be connected now");
@@ -249,13 +256,13 @@ export class MicrobitWebUSBConnection
         this.log("Reinstating serial after flash");
         if (this.connection.daplink) {
           await this.connection.daplink.connect();
-          await this.startSerialInternal();
+          await this.startSerialInternal(SerialOption.Reset);
         }
       }
     }
   }
 
-  private async startSerialInternal() {
+  private async startSerialInternal(option: SerialOption) {
     if (!this.connection) {
       // As connecting then starting serial are async we could disconnect between them,
       // so handle this gracefully.
@@ -264,6 +271,12 @@ export class MicrobitWebUSBConnection
     if (this.serialReadInProgress) {
       await this.stopSerialInternal();
     }
+    if (option === SerialOption.None || option === SerialOption.Reset) {
+      this.emit(EVENT_SERIAL_RESET, {});
+    }
+    if (option === SerialOption.None) {
+      return;
+    }
     // This is async but won't return until we stop serial so we error handle with an event.
     this.serialReadInProgress = this.connection
       .startSerial(this.serialListener)
@@ -278,7 +291,6 @@ export class MicrobitWebUSBConnection
       this.connection.stopSerial(this.serialListener);
       await this.serialReadInProgress;
       this.serialReadInProgress = undefined;
-      this.emit(EVENT_SERIAL_RESET, {});
     }
   }
 
@@ -384,9 +396,7 @@ export class MicrobitWebUSBConnection
       this.connection = new DAPWrapper(device, this.logging);
     }
     await withTimeout(this.connection.reconnectAsync(), 10_000);
-    if (options.serial === undefined || options.serial) {
-      this.startSerialInternal();
-    }
+    this.startSerialInternal(options.serial);
     this.setStatus(ConnectionStatus.CONNECTED);
   }
 

src/project/project-actions.tsx

@@ -24,6 +24,7 @@ import {
   DeviceConnection,
   EVENT_END_USB_SELECT,
   HexGenerationError,
+  SerialOption,
   WebUSBError,
   WebUSBErrorCode,
 } from "../device/device";
@@ -131,7 +132,12 @@ export class ProjectActions {
     } else {
       if (await this.showConnectHelp(forceConnectHelp, finalFocusRef)) {
         return this.connectInternal(
-          { serial: userAction !== ConnectionAction.FLASH },
+          {
+            serial:
+              userAction === ConnectionAction.FLASH
+                ? SerialOption.None
+                : SerialOption.Reset,
+          },
           userAction,
           finalFocusRef
         );

src/serial/SerialArea.tsx

@@ -53,31 +53,31 @@ const SerialArea = ({
         position="relative"
         overflow="hidden"
       >
-        {!connected ? null : (
-          <Box
-            alignItems="stretch"
-            backgroundColor={backgroundColorTerm}
-            height="100%"
-          >
-            <SerialBar
-              height={12}
-              compact={compact}
-              onSizeChange={onSizeChange}
-              showSyncStatus={showSyncStatus}
-              expandDirection={expandDirection}
-              hideExpandTextOnTraceback={hideExpandTextOnTraceback}
-              showHintsAndTips={showHintsAndTips}
-            />
-            <XTerm
-              visibility={compact ? "hidden" : undefined}
-              height={`calc(100% - ${SerialArea.compactSize}px)`}
-              ml={1}
-              mr={1}
-              fontSizePt={terminalFontSizePt}
-              tabOutRef={tabOutRef}
-            />
-          </Box>
-        )}
+        <Box
+          // Need to render this when not connected as we need it to maintain scrollback across disconnect/reconnect in some cases.
+          display={connected ? undefined : "none"}
+          alignItems="stretch"
+          backgroundColor={backgroundColorTerm}
+          height="100%"
+        >
+          <SerialBar
+            height={12}
+            compact={compact}
+            onSizeChange={onSizeChange}
+            showSyncStatus={showSyncStatus}
+            expandDirection={expandDirection}
+            hideExpandTextOnTraceback={hideExpandTextOnTraceback}
+            showHintsAndTips={showHintsAndTips}
+          />
+          <XTerm
+            visibility={compact ? "hidden" : undefined}
+            height={`calc(100% - ${SerialArea.compactSize}px)`}
+            ml={1}
+            mr={1}
+            fontSizePt={terminalFontSizePt}
+            tabOutRef={tabOutRef}
+          />
+        </Box>
       </Flex>
     </TerminalContext>
   );