#854 fix platform installation only offered if port is selected (#1130)

* ensure desired prompts shown + refactor

* pr review changes

arduino-ide-extension/src/browser/boards/boards-auto-installer.ts

@@ -5,19 +5,37 @@ import {
   BoardsService,
   BoardsPackage,
   Board,
+  Port,
 } from '../../common/protocol/boards-service';
 import { BoardsServiceProvider } from './boards-service-provider';
-import { BoardsConfig } from './boards-config';
 import { Installable, ResponseServiceArduino } from '../../common/protocol';
 import { BoardsListWidgetFrontendContribution } from './boards-widget-frontend-contribution';
 import { nls } from '@theia/core/lib/common';
+import { NotificationCenter } from '../notification-center';
+
+interface AutoInstallPromptAction {
+  // isAcceptance, whether or not the action indicates acceptance of auto-install proposal
+  isAcceptance?: boolean;
+  key: string;
+  handler: (...args: unknown[]) => unknown;
+}
+
+type AutoInstallPromptActions = AutoInstallPromptAction[];
 
 /**
  * Listens on `BoardsConfig.Config` changes, if a board is selected which does not
  * have the corresponding core installed, it proposes the user to install the core.
  */
+
+// * Cases in which we do not show the auto-install prompt:
+// 1. When a related platform is already installed
+// 2. When a prompt is already showing in the UI
+// 3. When a board is unplugged
 @injectable()
 export class BoardsAutoInstaller implements FrontendApplicationContribution {
+  @inject(NotificationCenter)
+  private readonly notificationCenter: NotificationCenter;
+
   @inject(MessageService)
   protected readonly messageService: MessageService;
 
@@ -36,97 +54,228 @@ export class BoardsAutoInstaller implements FrontendApplicationContribution {
   // Workaround for https://github.com/eclipse-theia/theia/issues/9349
   protected notifications: Board[] = [];
 
+  // * "refusal" meaning a "prompt action" not accepting the auto-install offer ("X" or "install manually")
+  // we can use "portSelectedOnLastRefusal" to deduce when a board is unplugged after a user has "refused"
+  // an auto-install prompt. Important to know as we do not want "an unplug" to trigger a "refused" prompt
+  // showing again
+  private portSelectedOnLastRefusal: Port | undefined;
+  private lastRefusedPackageId: string | undefined;
+
   onStart(): void {
-    this.boardsServiceClient.onBoardsConfigChanged(
+    const setEventListeners = () => {
-      this.ensureCoreExists.bind(this)
+      this.boardsServiceClient.onBoardsConfigChanged((config) => {
-    );
+        const { selectedBoard, selectedPort } = config;
-    this.ensureCoreExists(this.boardsServiceClient.boardsConfig);
-  }
 
-  protected ensureCoreExists(config: BoardsConfig.Config): void {
+        const boardWasUnplugged =
-    const { selectedBoard, selectedPort } = config;
+          !selectedPort && this.portSelectedOnLastRefusal;
-    if (
+
-      selectedBoard &&
+        this.clearLastRefusedPromptInfo();
-      selectedPort &&
-      !this.notifications.find((board) => Board.sameAs(board, selectedBoard))
-    ) {
-      this.notifications.push(selectedBoard);
-      this.boardsService.search({}).then((packages) => {
-        // filter packagesForBoard selecting matches from the cli (installed packages)
-        // and matches based on the board name
-        // NOTE: this ensures the Deprecated & new packages are all in the array
-        // so that we can check if any of the valid packages is already installed
-        const packagesForBoard = packages.filter(
-          (pkg) =>
-            BoardsPackage.contains(selectedBoard, pkg) ||
-            pkg.boards.some((board) => board.name === selectedBoard.name)
-        );
 
-        // check if one of the packages for the board is already installed. if so, no hint
         if (
-          packagesForBoard.some(({ installedVersion }) => !!installedVersion)
+          boardWasUnplugged ||
+          !selectedBoard ||
+          this.promptAlreadyShowingForBoard(selectedBoard)
         ) {
           return;
         }
 
-        // filter the installable (not installed) packages,
+        this.ensureCoreExists(selectedBoard, selectedPort);
-        // CLI returns the packages already sorted with the deprecated ones at the end of the list
+      });
-        // in order to ensure the new ones are preferred
-        const candidates = packagesForBoard.filter(
-          ({ installable, installedVersion }) =>
-            installable && !installedVersion
-        );
 
-        const candidate = candidates[0];
+      // we "clearRefusedPackageInfo" if a "refused" package is eventually
-        if (candidate) {
+      // installed, though this is not strictly necessary. It's more of a
-          const version = candidate.availableVersions[0]
+      // cleanup, to ensure the related variables are representative of
-            ? `[v ${candidate.availableVersions[0]}]`
+      // current state.
-            : '';
+      this.notificationCenter.onPlatformInstalled((installed) => {
-          const yes = nls.localize('vscode/extensionsUtils/yes', 'Yes');
+        if (this.lastRefusedPackageId === installed.item.id) {
-          const manualInstall = nls.localize(
+          this.clearLastRefusedPromptInfo();
-            'arduino/board/installManually',
-            'Install Manually'
-          );
-          // tslint:disable-next-line:max-line-length
-          this.messageService
-            .info(
-              nls.localize(
-                'arduino/board/installNow',
-                'The "{0} {1}" core has to be installed for the currently selected "{2}" board. Do you want to install it now?',
-                candidate.name,
-                version,
-                selectedBoard.name
-              ),
-              manualInstall,
-              yes
-            )
-            .then(async (answer) => {
-              const index = this.notifications.findIndex((board) =>
-                Board.sameAs(board, selectedBoard)
-              );
-              if (index !== -1) {
-                this.notifications.splice(index, 1);
-              }
-              if (answer === yes) {
-                await Installable.installWithProgress({
-                  installable: this.boardsService,
-                  item: candidate,
-                  messageService: this.messageService,
-                  responseService: this.responseService,
-                  version: candidate.availableVersions[0],
-                });
-                return;
-              }
-              if (answer === manualInstall) {
-                this.boardsManagerFrontendContribution
-                  .openView({ reveal: true })
-                  .then((widget) =>
-                    widget.refresh(candidate.name.toLocaleLowerCase())
-                  );
-              }
-            });
         }
       });
+    };
+
+    // we should invoke this.ensureCoreExists only once we're sure
+    // everything has been reconciled
+    this.boardsServiceClient.reconciled.then(() => {
+      const { selectedBoard, selectedPort } =
+        this.boardsServiceClient.boardsConfig;
+
+      if (selectedBoard) {
+        this.ensureCoreExists(selectedBoard, selectedPort);
+      }
+
+      setEventListeners();
+    });
+  }
+
+  private removeNotificationByBoard(selectedBoard: Board): void {
+    const index = this.notifications.findIndex((notification) =>
+      Board.sameAs(notification, selectedBoard)
+    );
+    if (index !== -1) {
+      this.notifications.splice(index, 1);
     }
   }
+
+  private clearLastRefusedPromptInfo(): void {
+    this.lastRefusedPackageId = undefined;
+    this.portSelectedOnLastRefusal = undefined;
+  }
+
+  private setLastRefusedPromptInfo(
+    packageId: string,
+    selectedPort?: Port
+  ): void {
+    this.lastRefusedPackageId = packageId;
+    this.portSelectedOnLastRefusal = selectedPort;
+  }
+
+  private promptAlreadyShowingForBoard(board: Board): boolean {
+    return Boolean(
+      this.notifications.find((notification) =>
+        Board.sameAs(notification, board)
+      )
+    );
+  }
+
+  protected ensureCoreExists(selectedBoard: Board, selectedPort?: Port): void {
+    this.notifications.push(selectedBoard);
+    this.boardsService.search({}).then((packages) => {
+      const candidate = this.getInstallCandidate(packages, selectedBoard);
+
+      if (candidate) {
+        this.showAutoInstallPrompt(candidate, selectedBoard, selectedPort);
+      } else {
+        this.removeNotificationByBoard(selectedBoard);
+      }
+    });
+  }
+
+  private getInstallCandidate(
+    packages: BoardsPackage[],
+    selectedBoard: Board
+  ): BoardsPackage | undefined {
+    // filter packagesForBoard selecting matches from the cli (installed packages)
+    // and matches based on the board name
+    // NOTE: this ensures the Deprecated & new packages are all in the array
+    // so that we can check if any of the valid packages is already installed
+    const packagesForBoard = packages.filter(
+      (pkg) =>
+        BoardsPackage.contains(selectedBoard, pkg) ||
+        pkg.boards.some((board) => board.name === selectedBoard.name)
+    );
+
+    // check if one of the packages for the board is already installed. if so, no hint
+    if (packagesForBoard.some(({ installedVersion }) => !!installedVersion)) {
+      return;
+    }
+
+    // filter the installable (not installed) packages,
+    // CLI returns the packages already sorted with the deprecated ones at the end of the list
+    // in order to ensure the new ones are preferred
+    const candidates = packagesForBoard.filter(
+      ({ installable, installedVersion }) => installable && !installedVersion
+    );
+
+    return candidates[0];
+  }
+
+  private showAutoInstallPrompt(
+    candidate: BoardsPackage,
+    selectedBoard: Board,
+    selectedPort?: Port
+  ): void {
+    const candidateName = candidate.name;
+    const version = candidate.availableVersions[0]
+      ? `[v ${candidate.availableVersions[0]}]`
+      : '';
+
+    const info = this.generatePromptInfoText(
+      candidateName,
+      version,
+      selectedBoard.name
+    );
+
+    const actions = this.createPromptActions(candidate);
+
+    const onRefuse = () => {
+      this.setLastRefusedPromptInfo(candidate.id, selectedPort);
+    };
+    const handleAction = this.createOnAnswerHandler(actions, onRefuse);
+
+    const onAnswer = (answer: string) => {
+      this.removeNotificationByBoard(selectedBoard);
+
+      handleAction(answer);
+    };
+
+    this.messageService
+      .info(info, ...actions.map((action) => action.key))
+      .then(onAnswer);
+  }
+
+  private generatePromptInfoText(
+    candidateName: string,
+    version: string,
+    boardName: string
+  ): string {
+    return nls.localize(
+      'arduino/board/installNow',
+      'The "{0} {1}" core has to be installed for the currently selected "{2}" board. Do you want to install it now?',
+      candidateName,
+      version,
+      boardName
+    );
+  }
+
+  private createPromptActions(
+    candidate: BoardsPackage
+  ): AutoInstallPromptActions {
+    const yes = nls.localize('vscode/extensionsUtils/yes', 'Yes');
+    const manualInstall = nls.localize(
+      'arduino/board/installManually',
+      'Install Manually'
+    );
+
+    const actions: AutoInstallPromptActions = [
+      {
+        isAcceptance: true,
+        key: yes,
+        handler: () => {
+          return Installable.installWithProgress({
+            installable: this.boardsService,
+            item: candidate,
+            messageService: this.messageService,
+            responseService: this.responseService,
+            version: candidate.availableVersions[0],
+          });
+        },
+      },
+      {
+        key: manualInstall,
+        handler: () => {
+          this.boardsManagerFrontendContribution
+            .openView({ reveal: true })
+            .then((widget) =>
+              widget.refresh(candidate.name.toLocaleLowerCase())
+            );
+        },
+      },
+    ];
+
+    return actions;
+  }
+
+  private createOnAnswerHandler(
+    actions: AutoInstallPromptActions,
+    onRefuse?: () => void
+  ): (answer: string) => void {
+    return (answer) => {
+      const actionToHandle = actions.find((action) => action.key === answer);
+      actionToHandle?.handler();
+
+      if (!actionToHandle?.isAcceptance && onRefuse) {
+        onRefuse();
+      }
+    };
+  }
 }

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

@@ -20,6 +20,7 @@ import { NotificationCenter } from '../notification-center';
 import { ArduinoCommands } from '../arduino-commands';
 import { StorageWrapper } from '../storage-wrapper';
 import { nls } from '@theia/core/lib/common';
+import { Deferred } from '@theia/core/lib/common/promise-util';
 
 @injectable()
 export class BoardsServiceProvider implements FrontendApplicationContribution {
@@ -73,6 +74,8 @@ export class BoardsServiceProvider implements FrontendApplicationContribution {
     this.onAvailableBoardsChangedEmitter.event;
   readonly onAvailablePortsChanged = this.onAvailablePortsChangedEmitter.event;
 
+  private readonly _reconciled = new Deferred<void>();
+
   onStart(): void {
     this.notificationCenter.onAttachedBoardsChanged(
       this.notifyAttachedBoardsChanged.bind(this)
@@ -88,14 +91,22 @@ export class BoardsServiceProvider implements FrontendApplicationContribution {
       this.boardsService.getAttachedBoards(),
       this.boardsService.getAvailablePorts(),
       this.loadState(),
-    ]).then(([attachedBoards, availablePorts]) => {
+    ]).then(async ([attachedBoards, availablePorts]) => {
       this._attachedBoards = attachedBoards;
       this._availablePorts = availablePorts;
       this.onAvailablePortsChangedEmitter.fire(this._availablePorts);
-      this.reconcileAvailableBoards().then(() => this.tryReconnect());
+
+      await this.reconcileAvailableBoards();
+
+      this.tryReconnect();
+      this._reconciled.resolve();
     });
   }
 
+  get reconciled(): Promise<void> {
+    return this._reconciled.promise;
+  }
+
   protected notifyAttachedBoardsChanged(
     event: AttachedBoardsChangeEvent
   ): void {
@@ -185,8 +196,8 @@ export class BoardsServiceProvider implements FrontendApplicationContribution {
         const selectedAvailableBoard = AvailableBoard.is(selectedBoard)
           ? selectedBoard
           : this._availableBoards.find((availableBoard) =>
-            Board.sameAs(availableBoard, selectedBoard)
+              Board.sameAs(availableBoard, selectedBoard)
-          );
+            );
         if (
           selectedAvailableBoard &&
           selectedAvailableBoard.selected &&
@@ -209,7 +220,7 @@ export class BoardsServiceProvider implements FrontendApplicationContribution {
     }
   }
 
-  protected async tryReconnect(): Promise<boolean> {
+  protected tryReconnect(): boolean {
     if (this.latestValidBoardsConfig && !this.canUploadTo(this.boardsConfig)) {
       for (const board of this.availableBoards.filter(
         ({ state }) => state !== AvailableBoard.State.incomplete
@@ -231,7 +242,8 @@ export class BoardsServiceProvider implements FrontendApplicationContribution {
         if (
           this.latestValidBoardsConfig.selectedBoard.fqbn === board.fqbn &&
           this.latestValidBoardsConfig.selectedBoard.name === board.name &&
-          this.latestValidBoardsConfig.selectedPort.protocol === board.port?.protocol
+          this.latestValidBoardsConfig.selectedPort.protocol ===
+            board.port?.protocol
         ) {
           this.boardsConfig = {
             ...this.latestValidBoardsConfig,
@@ -376,14 +388,14 @@ export class BoardsServiceProvider implements FrontendApplicationContribution {
     const timeoutTask =
       !!timeout && timeout > 0
         ? new Promise<void>((_, reject) =>
-          setTimeout(
+            setTimeout(
-            () => reject(new Error(`Timeout after ${timeout} ms.`)),
+              () => reject(new Error(`Timeout after ${timeout} ms.`)),
-            timeout
+              timeout
+            )
           )
-        )
         : new Promise<void>(() => {
-          /* never */
+            /* never */
-        });
+          });
     const waitUntilTask = new Promise<void>((resolve) => {
       let candidate = find(what, this.availableBoards);
       if (candidate) {
@@ -534,8 +546,9 @@ export class BoardsServiceProvider implements FrontendApplicationContribution {
 
   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
+    return `last-selected-board-on-port:${
-      }`;
+      typeof port === 'string' ? port : port.address
+    }`;
   }
 
   protected async loadState(): Promise<void> {