Add tests for BoardsServiceProvider

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

@@ -22,6 +22,9 @@ import { nls } from '@theia/core/lib/common';
 import { Deferred } from '@theia/core/lib/common/promise-util';
 import { FrontendApplicationStateService } from '@theia/core/lib/browser/frontend-application-state';
 
+export const LATEST_VALID_BOARDS_CONFIG = 'latest-valid-boards-config';
+export const LATEST_BOARDS_CONFIG = 'latest-boards-config';
+
 @injectable()
 export class BoardsServiceProvider implements FrontendApplicationContribution {
   @inject(ILogger)
@@ -56,15 +59,20 @@ export class BoardsServiceProvider implements FrontendApplicationContribution {
    * We have to listen on such changes and auto-reconnect the same board on another port.
    * See: https://arduino.slack.com/archives/CJJHJCJSJ/p1568645417013000?thread_ts=1568640504.009400&cid=CJJHJCJSJ
    */
-  protected latestValidBoardsConfig:
+  protected _latestValidBoardsConfig:
     | RecursiveRequired<BoardsConfig.Config>
     | undefined = undefined;
-  protected latestBoardsConfig: BoardsConfig.Config | undefined = undefined;
+  protected _latestBoardsConfig: BoardsConfig.Config | undefined = undefined;
   protected _boardsConfig: BoardsConfig.Config = {};
   protected _attachedBoards: Board[] = []; // This does not contain the `Unknown` boards. They're visible from the available ports only.
   protected _availablePorts: Port[] = [];
   protected _availableBoards: AvailableBoard[] = [];
 
+  private lastItemRemovedForUpload: { board: Board; port: Port } | undefined;
+  // "lastPersistingUploadPort", is a port created during an upload, that persisted after
+  // the upload finished, it's "substituting" the port selected when the user invoked the upload
+  private lastPersistingUploadPort: Port | undefined;
+
   /**
    * Unlike `onAttachedBoardsChanged` this even fires when the user modifies the selected board in the IDE.\
    * This even also fires, when the boards package was not available for the currently selected board,
@@ -111,6 +119,64 @@ export class BoardsServiceProvider implements FrontendApplicationContribution {
     return this._reconciled.promise;
   }
 
+  private checkForItemRemoved(event: AttachedBoardsChangeEvent): void {
+    if (!this.lastItemRemovedForUpload) {
+      const {
+        oldState: { ports: oldPorts, boards: oldBoards },
+        newState: { ports: newPorts },
+      } = event;
+
+      const disappearedPorts = oldPorts.filter((oldPort: Port) =>
+        newPorts.every((newPort: Port) => !Port.sameAs(oldPort, newPort))
+      );
+
+      if (disappearedPorts.length > 0) {
+        this.lastItemRemovedForUpload = {
+          board: oldBoards.find((board: Board) =>
+            Port.sameAs(board.port, disappearedPorts[0])
+          ) as Board,
+          port: disappearedPorts[0],
+        };
+      }
+
+      return;
+    }
+  }
+
+  private checkForPersistingPort(event: AttachedBoardsChangeEvent): void {
+    if (this.lastItemRemovedForUpload) {
+      const {
+        oldState: { ports: oldPorts },
+        newState: { ports: newPorts, boards: newBoards },
+      } = event;
+
+      const disappearedItem = this.lastItemRemovedForUpload;
+      this.lastItemRemovedForUpload = undefined;
+
+      const appearedPorts = newPorts.filter((newPort: Port) =>
+        oldPorts.every((oldPort: Port) => !Port.sameAs(newPort, oldPort))
+      );
+
+      if (appearedPorts.length > 0) {
+        const boardOnAppearedPort = newBoards.find((board: Board) =>
+          Port.sameAs(board.port, appearedPorts[0])
+        );
+
+        if (
+          boardOnAppearedPort &&
+          Board.sameAs(boardOnAppearedPort, disappearedItem.board)
+        ) {
+          this.lastPersistingUploadPort = appearedPorts[0];
+          return;
+        }
+      }
+
+      return;
+    }
+
+    this.lastPersistingUploadPort = undefined;
+  }
+
   protected notifyAttachedBoardsChanged(
     event: AttachedBoardsChangeEvent
   ): void {
@@ -119,10 +185,23 @@ export class BoardsServiceProvider implements FrontendApplicationContribution {
       this.logger.info(AttachedBoardsChangeEvent.toString(event));
       this.logger.info('------------------------------------------');
     }
+
+    const { uploadInProgress } = event;
+
+    if (uploadInProgress) {
+      this.checkForItemRemoved(event);
+    } else {
+      this.checkForPersistingPort(event);
+    }
+
     this._attachedBoards = event.newState.boards;
     this._availablePorts = event.newState.ports;
     this.onAvailablePortsChangedEmitter.fire(this._availablePorts);
-    this.reconcileAvailableBoards().then(() => this.tryReconnect());
+    this.reconcileAvailableBoards().then(() => {
+      if (!uploadInProgress) {
+        this.tryReconnect();
+      }
+    });
   }
 
   protected notifyPlatformInstalled(event: { item: BoardsPackage }): void {
@@ -225,32 +304,47 @@ export class BoardsServiceProvider implements FrontendApplicationContribution {
   }
 
   protected tryReconnect(): boolean {
-    if (this.latestValidBoardsConfig && !this.canUploadTo(this.boardsConfig)) {
+    if (this._latestValidBoardsConfig && !this.canUploadTo(this.boardsConfig)) {
       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 &&
-          Port.sameAs(this.latestValidBoardsConfig.selectedPort, board.port)
+          this._latestValidBoardsConfig.selectedBoard.fqbn === board.fqbn &&
+          this._latestValidBoardsConfig.selectedBoard.name === board.name &&
+          Port.sameAs(this._latestValidBoardsConfig.selectedPort, board.port)
         ) {
-          this.boardsConfig = this.latestValidBoardsConfig;
+          this.boardsConfig = this._latestValidBoardsConfig;
           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`.
+
+      if (!this.lastPersistingUploadPort) return false;
+
+      const lastPersistingUploadPort = this.lastPersistingUploadPort;
+      this.lastPersistingUploadPort = undefined;
+
+      if (
+        !Port.sameAs(
+          lastPersistingUploadPort,
+          this._latestValidBoardsConfig.selectedPort
+        )
+      ) {
+        return false;
+      }
+
       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 ===
+          this._latestValidBoardsConfig.selectedBoard.fqbn === board.fqbn &&
+          this._latestValidBoardsConfig.selectedBoard.name === board.name &&
+          this._latestValidBoardsConfig.selectedPort.protocol ===
             board.port?.protocol
         ) {
           this.boardsConfig = {
-            ...this.latestValidBoardsConfig,
+            ...this._latestValidBoardsConfig,
             selectedPort: board.port,
           };
           return true;
@@ -273,12 +367,22 @@ export class BoardsServiceProvider implements FrontendApplicationContribution {
     return this._boardsConfig;
   }
 
+  get latestValidBoardsConfig():
+    | RecursiveRequired<BoardsConfig.Config>
+    | undefined {
+    return this._latestValidBoardsConfig;
+  }
+
+  get latestBoardsConfig(): BoardsConfig.Config | undefined {
+    return this._latestBoardsConfig;
+  }
+
   protected setBoardsConfig(config: BoardsConfig.Config): void {
     this.logger.debug('Board config changed: ', JSON.stringify(config));
     this._boardsConfig = config;
-    this.latestBoardsConfig = this._boardsConfig;
+    this._latestBoardsConfig = this._boardsConfig;
     if (this.canUploadTo(this._boardsConfig)) {
-      this.latestValidBoardsConfig = this._boardsConfig;
+      this._latestValidBoardsConfig = this._boardsConfig;
     }
   }
 
@@ -458,6 +562,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
       );
@@ -476,7 +585,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 {
@@ -491,7 +602,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.
@@ -529,7 +640,7 @@ export class BoardsServiceProvider implements FrontendApplicationContribution {
   protected async getLastSelectedBoardOnPort(
     port: Port
   ): Promise<Board | undefined> {
-    const key = this.getLastSelectedBoardOnPortKey(port);
+    const key = getLastSelectedBoardOnPortKey(port);
     return this.getData<Board>(key);
   }
 
@@ -540,45 +651,38 @@ export class BoardsServiceProvider implements FrontendApplicationContribution {
     // https://github.com/arduino/arduino-cli/issues/623
     const { selectedBoard, selectedPort } = this.boardsConfig;
     if (selectedBoard && selectedPort) {
-      const key = this.getLastSelectedBoardOnPortKey(selectedPort);
+      const key = getLastSelectedBoardOnPortKey(selectedPort);
       await this.setData(key, selectedBoard);
     }
     await Promise.all([
-      this.setData('latest-valid-boards-config', this.latestValidBoardsConfig),
-      this.setData('latest-boards-config', this.latestBoardsConfig),
+      this.setData(LATEST_VALID_BOARDS_CONFIG, this._latestValidBoardsConfig),
+      this.setData(LATEST_BOARDS_CONFIG, this._latestBoardsConfig),
     ]);
   }
 
-  protected getLastSelectedBoardOnPortKey(port: Port | string): string {
-    // TODO: we lose the port's `protocol` info (`serial`, `network`, etc.) here if the `port` is a `string`.
-    return `last-selected-board-on-port:${
-      typeof port === 'string' ? port : port.address
-    }`;
-  }
-
   protected async loadState(): Promise<void> {
     const storedLatestValidBoardsConfig = await this.getData<
       RecursiveRequired<BoardsConfig.Config>
-    >('latest-valid-boards-config');
+    >(LATEST_VALID_BOARDS_CONFIG);
     if (storedLatestValidBoardsConfig) {
-      this.latestValidBoardsConfig = storedLatestValidBoardsConfig;
-      if (this.canUploadTo(this.latestValidBoardsConfig)) {
-        this.boardsConfig = this.latestValidBoardsConfig;
+      this._latestValidBoardsConfig = storedLatestValidBoardsConfig;
+      if (this.canUploadTo(this._latestValidBoardsConfig)) {
+        this.boardsConfig = this._latestValidBoardsConfig;
       }
     } else {
       // If we could not restore the latest valid config, try to restore something, the board at least.
       let storedLatestBoardsConfig = await this.getData<
         BoardsConfig.Config | undefined
-      >('latest-boards-config');
+      >(LATEST_BOARDS_CONFIG);
       // Try to get from the URL if it was not persisted.
       if (!storedLatestBoardsConfig) {
         storedLatestBoardsConfig = BoardsConfig.Config.getConfig(
           new URL(window.location.href)
         );
       }
       if (storedLatestBoardsConfig) {
-        this.latestBoardsConfig = storedLatestBoardsConfig;
-        this.boardsConfig = this.latestBoardsConfig;
+        this._latestBoardsConfig = storedLatestBoardsConfig;
+        this.boardsConfig = this._latestBoardsConfig;
       }
     }
   }
@@ -673,3 +777,10 @@ export namespace AvailableBoard {
     return naturalCompare(left.port?.address!, right.port?.address!);
   };
 }
+
+export function getLastSelectedBoardOnPortKey(port: Port | string): string {
+  // TODO: we lose the port's `protocol` info (`serial`, `network`, etc.) here if the `port` is a `string`.
+  return `last-selected-board-on-port:${
+    typeof port === 'string' ? port : port.address
+  }`;
+}

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
     });
   }
 
+  public 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)
+  protected 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);
   }