#1032 failing upload flag for monitor mgr

arduino-ide-extension/package.json

@@ -155,7 +155,11 @@
   ],
   "arduino": {
     "cli": {
-      "version": "0.23.0"
+      "version": {
+        "owner": "arduino",
+        "repo": "arduino-cli",
+        "commitish": "master"  
+      }
     },
     "fwuploader": {
       "version": "2.2.0"

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

@@ -204,6 +204,7 @@ export class UploadSketch extends SketchContribution {
     // toggle the toolbar button and menu item state.
     // uploadInProgress will be set to false whether the upload fails or not
     this.uploadInProgress = true;
+
     this.onDidChangeEmitter.fire();
     const sketch = await this.sketchServiceClient.currentSketch();
     if (!CurrentSketch.isValid(sketch)) {
@@ -227,7 +228,7 @@ export class UploadSketch extends SketchContribution {
         ...boardsConfig.selectedBoard,
         name: boardsConfig.selectedBoard?.name || '',
         fqbn,
-      }
+      };
       let options: CoreService.Upload.Options | undefined = undefined;
       const sketchUri = sketch.uri;
       const optimizeForDebug = this.editorMode.compileForDebug;
@@ -290,6 +291,7 @@ export class UploadSketch extends SketchContribution {
       this.messageService.error(errorMessage);
     } finally {
       this.uploadInProgress = false;
+
       this.onDidChangeEmitter.fire();
     }
   }

arduino-ide-extension/src/node/arduino-firmware-uploader-impl.ts

@@ -90,7 +90,7 @@ export class ArduinoFirmwareUploaderImpl implements ArduinoFirmwareUploader {
     } catch (e) {
       throw e;
     } finally {
-      this.monitorManager.notifyUploadFinished(board, port);
+      await this.monitorManager.notifyUploadFinished(board, port);
       return output;
     }
   }

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

@@ -112,8 +112,8 @@ export class CoreServiceImpl extends CoreClientAware implements CoreService {
     }
   }
 
-  async upload(options: CoreService.Upload.Options): Promise<void> {
-    await this.doUpload(
+  upload(options: CoreService.Upload.Options): Promise<void> {
+    return this.doUpload(
       options,
       () => new UploadRequest(),
       (client, req) => client.upload(req)
@@ -123,7 +123,7 @@ export class CoreServiceImpl extends CoreClientAware implements CoreService {
   async uploadUsingProgrammer(
     options: CoreService.Upload.Options
   ): Promise<void> {
-    await this.doUpload(
+    return this.doUpload(
       options,
       () => new UploadUsingProgrammerRequest(),
       (client, req) => client.uploadUsingProgrammer(req),
@@ -149,7 +149,6 @@ export class CoreServiceImpl extends CoreClientAware implements CoreService {
 
     this.uploading = true;
     const { sketchUri, board, port, programmer } = options;
-    await this.monitorManager.notifyUploadStarted(board, port);
 
     const sketchPath = FileUri.fsPath(sketchUri);
 
@@ -181,13 +180,16 @@ export class CoreServiceImpl extends CoreClientAware implements CoreService {
       req.getUserFieldsMap().set(e.name, e.value);
     });
 
-    const result = responseHandler(client, req);
-
-    const uploadBuffer = new SimpleBuffer(
-      this.flushOutputPanelMessages.bind(this),
-      FLUSH_OUTPUT_MESSAGES_TIMEOUT_MS
-    );
     try {
+      await this.monitorManager.notifyUploadStarted(board, port);
+
+      const result = responseHandler(client, req);
+
+      const uploadBuffer = new SimpleBuffer(
+        this.flushOutputPanelMessages.bind(this),
+        FLUSH_OUTPUT_MESSAGES_TIMEOUT_MS
+      );
+
       await new Promise<void>((resolve, reject) => {
         result.on('data', (resp: UploadResponse) => {
           uploadBuffer.addChunk(resp.getOutStream_asU8());
@@ -222,14 +224,13 @@ export class CoreServiceImpl extends CoreClientAware implements CoreService {
       throw new Error(errorMessage);
     } finally {
       this.uploading = false;
-      this.monitorManager.notifyUploadFinished(board, port);
+      await this.monitorManager.notifyUploadFinished(board, port);
     }
   }
 
   async burnBootloader(options: CoreService.Bootloader.Options): Promise<void> {
     this.uploading = true;
     const { board, port, programmer } = options;
-    await this.monitorManager.notifyUploadStarted(board, port);
 
     await this.coreClientProvider.initialized;
     const coreClient = await this.coreClient();
@@ -252,13 +253,14 @@ export class CoreServiceImpl extends CoreClientAware implements CoreService {
     }
     burnReq.setVerify(options.verify);
     burnReq.setVerbose(options.verbose);
-    const result = client.burnBootloader(burnReq);
-
-    const bootloaderBuffer = new SimpleBuffer(
-      this.flushOutputPanelMessages.bind(this),
-      FLUSH_OUTPUT_MESSAGES_TIMEOUT_MS
-    );
     try {
+      await this.monitorManager.notifyUploadStarted(board, port);
+      const result = client.burnBootloader(burnReq);
+
+      const bootloaderBuffer = new SimpleBuffer(
+        this.flushOutputPanelMessages.bind(this),
+        FLUSH_OUTPUT_MESSAGES_TIMEOUT_MS
+      );
       await new Promise<void>((resolve, reject) => {
         result.on('data', (resp: BurnBootloaderResponse) => {
           bootloaderBuffer.addChunk(resp.getOutStream_asU8());

arduino-ide-extension/src/node/monitor-manager-proxy-impl.ts

@@ -40,11 +40,14 @@ export class MonitorManagerProxyImpl implements MonitorManagerProxy {
     if (settings) {
       await this.changeMonitorSettings(board, port, settings);
     }
-    const status = await this.manager.startMonitor(board, port);
-    if (status === Status.ALREADY_CONNECTED || status === Status.OK) {
-      // Monitor started correctly, connect it with the frontend
-      this.client.connect(this.manager.getWebsocketAddressPort(board, port));
-    }
+
+    const onFinish = (status: Status) => {
+      if (status === Status.ALREADY_CONNECTED || status === Status.OK) {
+        // Monitor started correctly, connect it with the frontend
+        this.client.connect(this.manager.getWebsocketAddressPort(board, port));
+      }
+    };
+    return this.manager.startMonitor(board, port, onFinish);
   }
 
   /**

arduino-ide-extension/src/node/monitor-manager.ts

@@ -1,6 +1,6 @@
 import { ILogger } from '@theia/core';
 import { inject, injectable, named } from '@theia/core/shared/inversify';
-import { Board, Port, Status } from '../common/protocol';
+import { Board, BoardsService, Port, Status } from '../common/protocol';
 import { CoreClientAware } from './core-client-provider';
 import { MonitorService } from './monitor-service';
 import { MonitorServiceFactory } from './monitor-service-factory';
@@ -11,16 +11,34 @@ import {
 
 type MonitorID = string;
 
+type UploadState = 'uploadInProgress' | 'pausedForUpload' | 'disposedForUpload';
+type MonitorIDsByUploadState = Record<UploadState, MonitorID[]>;
+
 export const MonitorManagerName = 'monitor-manager';
 
 @injectable()
 export class MonitorManager extends CoreClientAware {
+  @inject(BoardsService)
+  protected boardsService: BoardsService;
+
   // Map of monitor services that manage the running pluggable monitors.
   // Each service handles the lifetime of one, and only one, monitor.
   // If either the board or port managed changes, a new service must
   // be started.
   private monitorServices = new Map<MonitorID, MonitorService>();
 
+  private monitorIDsByUploadState: MonitorIDsByUploadState = {
+    uploadInProgress: [],
+    pausedForUpload: [],
+    disposedForUpload: [],
+  };
+
+  private startMonitor_PendingRequests: {
+    monitorID: string;
+    requestFor: [Board, Port];
+    postStartCallback: (status: Status) => void;
+  }[] = [];
+
   @inject(MonitorServiceFactory)
   private monitorServiceFactory: MonitorServiceFactory;
 
@@ -48,6 +66,27 @@ export class MonitorManager extends CoreClientAware {
     return false;
   }
 
+  uploadIsInProgress(): boolean {
+    return this.monitorIDsByUploadState.uploadInProgress.length > 0;
+  }
+
+  addToMonitorIDsByUploadState(state: UploadState, monitorID: string): void {
+    this.monitorIDsByUploadState[state].push(monitorID);
+  }
+
+  removeFromMonitorIDsByUploadState(
+    state: UploadState,
+    monitorID: string
+  ): void {
+    this.monitorIDsByUploadState[state] = this.monitorIDsByUploadState[
+      state
+    ].filter((id) => id !== monitorID);
+  }
+
+  monitorIDIsInUploadState(state: UploadState, monitorID: string): boolean {
+    return this.monitorIDsByUploadState[state].includes(monitorID);
+  }
+
   /**
    * Start a pluggable monitor that receives and sends messages
    * to the specified board and port combination.
@@ -56,13 +95,35 @@ export class MonitorManager extends CoreClientAware {
    * @returns a Status object to know if the process has been
    * started or if there have been errors.
    */
-  async startMonitor(board: Board, port: Port): Promise<Status> {
+  async startMonitor(
+    board: Board,
+    port: Port,
+    postStartCallback: (status: Status) => void
+  ): Promise<void> {
     const monitorID = this.monitorID(board, port);
+
     let monitor = this.monitorServices.get(monitorID);
     if (!monitor) {
       monitor = this.createMonitor(board, port);
     }
-    return await monitor.start();
+
+    if (this.uploadIsInProgress()) {
+      this.startMonitor_PendingRequests =
+        this.startMonitor_PendingRequests.filter(
+          (request) => request.monitorID !== monitorID
+        );
+
+      this.startMonitor_PendingRequests.push({
+        monitorID,
+        requestFor: [board, port],
+        postStartCallback,
+      });
+
+      return;
+    }
+
+    const result = await monitor.start();
+    postStartCallback(result);
   }
 
   /**
@@ -111,14 +172,18 @@ export class MonitorManager extends CoreClientAware {
       // to retrieve if we don't have this information.
       return;
     }
+
     const monitorID = this.monitorID(board, port);
+    this.addToMonitorIDsByUploadState('uploadInProgress', monitorID);
+
     const monitor = this.monitorServices.get(monitorID);
     if (!monitor) {
       // There's no monitor running there, bail
       return;
     }
-    monitor.setUploadInProgress(true);
-    return await monitor.pause();
+
+    this.addToMonitorIDsByUploadState('pausedForUpload', monitorID);
+    return monitor.pause();
   }
 
   /**
@@ -130,19 +195,69 @@ export class MonitorManager extends CoreClientAware {
    * started or if there have been errors.
    */
   async notifyUploadFinished(board?: Board, port?: Port): Promise<Status> {
-    if (!board || !port) {
-      // We have no way of knowing which monitor
-      // to retrieve if we don't have this information.
-      return Status.NOT_CONNECTED;
+    let status: Status = Status.NOT_CONNECTED;
+    let portDidChangeOnUpload = false;
+
+    // We have no way of knowing which monitor
+    // to retrieve if we don't have this information.
+    if (board && port) {
+      const monitorID = this.monitorID(board, port);
+      this.removeFromMonitorIDsByUploadState('uploadInProgress', monitorID);
+
+      const monitor = this.monitorServices.get(monitorID);
+      if (monitor) {
+        status = await monitor.start();
+      }
+
+      // this monitorID will only be present in "disposedForUpload"
+      // if the upload changed the board port
+      portDidChangeOnUpload = this.monitorIDIsInUploadState(
+        'disposedForUpload',
+        monitorID
+      );
+      if (portDidChangeOnUpload) {
+        this.removeFromMonitorIDsByUploadState('disposedForUpload', monitorID);
+      }
+
+      // in case a service was paused but not disposed
+      this.removeFromMonitorIDsByUploadState('pausedForUpload', monitorID);
     }
-    const monitorID = this.monitorID(board, port);
-    const monitor = this.monitorServices.get(monitorID);
-    if (!monitor) {
-      // There's no monitor running there, bail
-      return Status.NOT_CONNECTED;
+
+    await this.startQueuedServices(portDidChangeOnUpload);
+    return status;
+  }
+
+  async startQueuedServices(portDidChangeOnUpload: boolean): Promise<void> {
+    // if the port changed during upload with the monitor open, "startMonitorPendingRequests"
+    // will include a request for our "upload port', most likely at index 0.
+    // We remove it, as this port was to be used exclusively for the upload
+    const queued = portDidChangeOnUpload
+      ? this.startMonitor_PendingRequests.slice(1)
+      : this.startMonitor_PendingRequests;
+    this.startMonitor_PendingRequests = [];
+
+    for (const {
+      monitorID,
+      requestFor: [_, port],
+      postStartCallback: onFinish,
+    } of queued) {
+      const boardsState = await this.boardsService.getState();
+      const boardIsStillOnPort = Object.keys(boardsState)
+        .map((connection: string) => {
+          const portAddress = connection.split('|')[0];
+          return portAddress;
+        })
+        .some((portAddress: string) => port.address === portAddress);
+
+      if (boardIsStillOnPort) {
+        const monitorService = this.monitorServices.get(monitorID);
+
+        if (monitorService) {
+          const result = await monitorService.start();
+          onFinish(result);
+        }
+      }
     }
-    monitor.setUploadInProgress(false);
-    return await monitor.start();
   }
 
   /**
@@ -202,6 +317,18 @@ export class MonitorManager extends CoreClientAware {
     this.monitorServices.set(monitorID, monitor);
     monitor.onDispose(
       (() => {
+        // if a service is disposed during upload and
+        // we paused it beforehand we know it was disposed
+        // of because the upload changed the board port
+        if (
+          this.uploadIsInProgress() &&
+          this.monitorIDIsInUploadState('pausedForUpload', monitorID)
+        ) {
+          this.removeFromMonitorIDsByUploadState('pausedForUpload', monitorID);
+
+          this.addToMonitorIDsByUploadState('disposedForUpload', monitorID);
+        }
+
         this.monitorServices.delete(monitorID);
       }).bind(this)
     );