fix 180: prevent erroneous "auto-reconnect"(s) in board selector (#1328)

* ensure auto-select doesn't select wrong port

prevent auto-select of unattached boards Co-authored-by: Francesco Spissu <[email protected]>

* clean up

* add "uploadInProgress" prop to boards change event

* remove unused methods and deps

* [WIP]: leverage upload event to derived new port

* remove timeout

* refine port selection logic

* clean up naming

* refine port selection logic & add delayed clean up

* renaming & refactoring

* method syntax & remove unnecessary methods

Author: davegarthsimpson

arduino-ide-extension/src/browser/boards/boards-service-provider.ts

@@ -65,6 +65,10 @@ export class BoardsServiceProvider implements FrontendApplicationContribution {
   protected _availablePorts: Port[] = [];
   protected _availableBoards: AvailableBoard[] = [];
 
+  private lastBoardsConfigOnUpload: BoardsConfig.Config | undefined;
+  private lastAvailablePortsOnUpload: Port[] | undefined;
+  private boardConfigToAutoSelect: BoardsConfig.Config | undefined;
+
   /**
    * Unlike `onAttachedBoardsChanged` this event fires when the user modifies the selected board in the IDE.\
    * This event also fires, when the boards package was not available for the currently selected board,
@@ -112,6 +116,84 @@ export class BoardsServiceProvider implements FrontendApplicationContribution {
     return this._reconciled.promise;
   }
 
+  snapshotBoardDiscoveryOnUpload(): void {
+    this.lastBoardsConfigOnUpload = this._boardsConfig;
+    this.lastAvailablePortsOnUpload = this._availablePorts;
+  }
+
+  clearBoardDiscoverySnapshot(): void {
+    this.lastBoardsConfigOnUpload = undefined;
+    this.lastAvailablePortsOnUpload = undefined;
+  }
+
+  private portToAutoSelectCanBeDerived(): boolean {
+    return Boolean(
+      this.lastBoardsConfigOnUpload && this.lastAvailablePortsOnUpload
+    );
+  }
+
+  attemptPostUploadAutoSelect(): void {
+    setTimeout(() => {
+      if (this.portToAutoSelectCanBeDerived()) {
+        this.attemptAutoSelect({
+          ports: this._availablePorts,
+          boards: this._availableBoards,
+        });
+      }
+    }, 2000); // 2 second delay same as IDE 1.8
+  }
+
+  private attemptAutoSelect(
+    newState: AttachedBoardsChangeEvent['newState']
+  ): void {
+    this.deriveBoardConfigToAutoSelect(newState);
+    this.tryReconnect();
+  }
+
+  private deriveBoardConfigToAutoSelect(
+    newState: AttachedBoardsChangeEvent['newState']
+  ): void {
+    if (!this.portToAutoSelectCanBeDerived()) {
+      this.boardConfigToAutoSelect = undefined;
+      return;
+    }
+
+    const oldPorts = this.lastAvailablePortsOnUpload!;
+    const { ports: newPorts, boards: newBoards } = newState;
+
+    const appearedPorts =
+      oldPorts.length > 0
+        ? newPorts.filter((newPort: Port) =>
+            oldPorts.every((oldPort: Port) => !Port.sameAs(newPort, oldPort))
+          )
+        : newPorts;
+
+    for (const port of appearedPorts) {
+      const boardOnAppearedPort = newBoards.find((board: Board) =>
+        Port.sameAs(board.port, port)
+      );
+
+      const lastBoardsConfigOnUpload = this.lastBoardsConfigOnUpload!;
+
+      if (
+        boardOnAppearedPort &&
+        lastBoardsConfigOnUpload.selectedBoard &&
+        Board.sameAs(
+          boardOnAppearedPort,
+          lastBoardsConfigOnUpload.selectedBoard
+        )
+      ) {
+        this.clearBoardDiscoverySnapshot();
+
+        this.boardConfigToAutoSelect = {
+          selectedBoard: boardOnAppearedPort,
+          selectedPort: port,
+        };
+        return;
+      }
+    }
+  }
+
   protected notifyAttachedBoardsChanged(
     event: AttachedBoardsChangeEvent
   ): void {
@@ -120,10 +202,18 @@ export class BoardsServiceProvider implements FrontendApplicationContribution {
       this.logger.info(AttachedBoardsChangeEvent.toString(event));
       this.logger.info('------------------------------------------');
     }
+
     this._attachedBoards = event.newState.boards;
     this._availablePorts = event.newState.ports;
     this.onAvailablePortsChangedEmitter.fire(this._availablePorts);
-    this.reconcileAvailableBoards().then(() => this.tryReconnect());
+    this.reconcileAvailableBoards().then(() => {
+      const { uploadInProgress } = event;
+      // avoid attempting "auto-selection" while an
+      // upload is in progress
+      if (!uploadInProgress) {
+        this.attemptAutoSelect(event.newState);
+      }
+    });
   }
 
   protected notifyPlatformInstalled(event: { item: BoardsPackage }): void {
@@ -239,24 +329,12 @@ export class BoardsServiceProvider implements FrontendApplicationContribution {
           return true;
         }
       }
-      // If we could not find an exact match, we compare the board FQBN-name pairs and ignore the port, as it might have changed.
-      // See documentation on `latestValidBoardsConfig`.
-      for (const board of this.availableBoards.filter(
-        ({ state }) => state !== AvailableBoard.State.incomplete
-      )) {
-        if (
-          this.latestValidBoardsConfig.selectedBoard.fqbn === board.fqbn &&
-          this.latestValidBoardsConfig.selectedBoard.name === board.name &&
-          this.latestValidBoardsConfig.selectedPort.protocol ===
-            board.port?.protocol
-        ) {
-          this.boardsConfig = {
-            ...this.latestValidBoardsConfig,
-            selectedPort: board.port,
-          };
-          return true;
-        }
-      }
+
+      if (!this.boardConfigToAutoSelect) return false;
+
+      this.boardsConfig = this.boardConfigToAutoSelect;
+      this.boardConfigToAutoSelect = undefined;
+      return true;
     }
     return false;
   }
@@ -450,6 +528,11 @@ export class BoardsServiceProvider implements FrontendApplicationContribution {
       const board = attachedBoards.find(({ port }) =>
         Port.sameAs(boardPort, port)
       );
+      // "board" will always be falsey for
+      // port that was originally mapped
+      // to unknown board and then selected
+      // manually by user
+
       const lastSelectedBoard = await this.getLastSelectedBoardOnPort(
         boardPort
       );
@@ -468,7 +551,9 @@ export class BoardsServiceProvider implements FrontendApplicationContribution {
         availableBoard = {
           ...lastSelectedBoard,
           state: AvailableBoard.State.guessed,
-          selected: BoardsConfig.Config.sameAs(boardsConfig, lastSelectedBoard),
+          selected:
+            BoardsConfig.Config.sameAs(boardsConfig, lastSelectedBoard) &&
+            Port.sameAs(boardPort, boardsConfig.selectedPort), // to avoid double selection
           port: boardPort,
         };
       } else {
@@ -483,7 +568,7 @@ export class BoardsServiceProvider implements FrontendApplicationContribution {
 
     if (
       boardsConfig.selectedBoard &&
-      !availableBoards.some(({ selected }) => selected)
+      availableBoards.every(({ selected }) => !selected)
     ) {
       // If the selected board has the same port of an unknown board
       // that is already in availableBoards we might get a duplicate port.

arduino-ide-extension/src/browser/contributions/upload-sketch.ts

@@ -190,6 +190,7 @@ export class UploadSketch extends CoreServiceContribution {
       // toggle the toolbar button and menu item state.
       // uploadInProgress will be set to false whether the upload fails or not
       this.uploadInProgress = true;
+      this.boardsServiceProvider.snapshotBoardDiscoveryOnUpload();
       this.onDidChangeEmitter.fire();
       this.clearVisibleNotification();
 
@@ -243,6 +244,7 @@ export class UploadSketch extends CoreServiceContribution {
       this.handleError(e);
     } finally {
       this.uploadInProgress = false;
+      this.boardsServiceProvider.attemptPostUploadAutoSelect();
       this.onDidChangeEmitter.fire();
     }
   }

arduino-ide-extension/src/common/protocol/boards-service.ts

@@ -25,6 +25,7 @@ export namespace AvailablePorts {
 export interface AttachedBoardsChangeEvent {
   readonly oldState: Readonly<{ boards: Board[]; ports: Port[] }>;
   readonly newState: Readonly<{ boards: Board[]; ports: Port[] }>;
+  readonly uploadInProgress: boolean;
 }
 export namespace AttachedBoardsChangeEvent {
   export function isEmpty(event: AttachedBoardsChangeEvent): boolean {

arduino-ide-extension/src/node/board-discovery.ts

@@ -54,6 +54,8 @@ export class BoardDiscovery
   private readonly onStreamDidCancelEmitter = new Emitter<void>(); // when the watcher is canceled by the IDE2
   private readonly toDisposeOnStopWatch = new DisposableCollection();
 
+  private uploadInProgress = false;
+
   /**
    * Keys are the `address` of the ports.
    *
@@ -123,6 +125,10 @@ export class BoardDiscovery
     });
   }
 
+  setUploadInProgress(uploadAttemptInProgress: boolean): void {
+    this.uploadInProgress = uploadAttemptInProgress;
+  }
+
   private createTimeout(
     after: number,
     onTimeout: (error: Error) => void
@@ -318,6 +324,7 @@ export class BoardDiscovery
             ports: newAvailablePorts,
             boards: newAttachedBoards,
           },
+          uploadInProgress: this.uploadInProgress,
         };
 
         this._availablePorts = newState;

arduino-ide-extension/src/node/core-service-impl.ts

@@ -34,6 +34,7 @@ import { Instance } from './cli-protocol/cc/arduino/cli/commands/v1/common_pb';
 import { firstToUpperCase, notEmpty } from '../common/utils';
 import { ServiceError } from './service-error';
 import { ExecuteWithProgress, ProgressResponse } from './grpc-progressible';
+import { BoardDiscovery } from './board-discovery';
 
 namespace Uploadable {
   export type Request = UploadRequest | UploadUsingProgrammerRequest;
@@ -51,6 +52,9 @@ export class CoreServiceImpl extends CoreClientAware implements CoreService {
   @inject(CommandService)
   private readonly commandService: CommandService;
 
+  @inject(BoardDiscovery)
+  private readonly boardDiscovery: BoardDiscovery;
+
   async compile(options: CoreService.Options.Compile): Promise<void> {
     const coreClient = await this.coreClient;
     const { client, instance } = coreClient;
@@ -378,6 +382,7 @@ export class CoreServiceImpl extends CoreClientAware implements CoreService {
     fqbn?: string | undefined;
     port?: Port | undefined;
   }): Promise<void> {
+    this.boardDiscovery.setUploadInProgress(true);
     return this.monitorManager.notifyUploadStarted(fqbn, port);
   }
 
@@ -388,6 +393,7 @@ export class CoreServiceImpl extends CoreClientAware implements CoreService {
     fqbn?: string | undefined;
     port?: Port | undefined;
   }): Promise<Status> {
+    this.boardDiscovery.setUploadInProgress(false);
     return this.monitorManager.notifyUploadFinished(fqbn, port);
   }