fix reset loop

Author: Alberto Iannaccone

Diff for: arduino-ide-extension/src/browser/monitor-manager-proxy-client-impl.ts

@@ -1,4 +1,9 @@
-import { CommandRegistry, Emitter, MessageService } from '@theia/core';
+import {
+  CommandRegistry,
+  Disposable,
+  Emitter,
+  MessageService,
+} from '@theia/core';
 import { inject, injectable } from '@theia/core/shared/inversify';
 import { Board, Port } from '../common/protocol';
 import {
@@ -11,8 +16,7 @@ import {
   MonitorSettings,
 } from '../node/monitor-settings/monitor-settings-provider';
 import { BoardsConfig } from './boards/boards-config';
-import { MonitorViewContribution } from './serial/monitor/monitor-view-contribution';
-import { SerialPlotterContribution } from './serial/plotter/plotter-frontend-contribution';
+import { BoardsServiceProvider } from './boards/boards-service-provider';
 
 @injectable()
 export class MonitorManagerProxyClientImpl
@@ -32,11 +36,15 @@ export class MonitorManagerProxyClientImpl
   readonly onMonitorSettingsDidChange =
     this.onMonitorSettingsDidChangeEmitter.event;
 
+  protected readonly onMonitorShouldResetEmitter = new Emitter();
+  readonly onMonitorShouldReset = this.onMonitorShouldResetEmitter.event;
+
   // WebSocket used to handle pluggable monitor communication between
   // frontend and backend.
   private webSocket?: WebSocket;
   private wsPort?: number;
   private lastConnectedBoard: BoardsConfig.Config;
+  private onBoardsConfigChanged: Disposable | undefined;
 
   getWebSocketPort(): number | undefined {
     return this.wsPort;
@@ -51,7 +59,10 @@ export class MonitorManagerProxyClientImpl
     protected server: MonitorManagerProxyFactory,
 
     @inject(CommandRegistry)
-    protected readonly commandRegistry: CommandRegistry
+    protected readonly commandRegistry: CommandRegistry,
+
+    @inject(BoardsServiceProvider)
+    protected readonly boardsServiceProvider: BoardsServiceProvider
   ) {}
 
   /**
@@ -89,6 +100,8 @@ export class MonitorManagerProxyClientImpl
    */
   disconnect(): void {
     if (!this.webSocket) return;
+    this.onBoardsConfigChanged?.dispose();
+    this.onBoardsConfigChanged = undefined;
     try {
       this.webSocket?.close();
       this.webSocket = undefined;
@@ -101,27 +114,46 @@ export class MonitorManagerProxyClientImpl
     return !!this.webSocket;
   }
 
-  async startMonitor(
-    board: Board,
-    port: Port,
-    settings?: PluggableMonitorSettings
-  ): Promise<void> {
-    await this.server().startMonitor(board, port, settings);
-    if (
-      board.fqbn !== this.lastConnectedBoard?.selectedBoard?.fqbn ||
-      port.id !== this.lastConnectedBoard?.selectedPort?.id
-    ) {
-      await this.commandRegistry.executeCommand(
-        MonitorViewContribution.RESET_SERIAL_MONITOR
-      );
-      await this.commandRegistry.executeCommand(
-        SerialPlotterContribution.Commands.RESET.id
-      );
-    }
+  async startMonitor(settings?: PluggableMonitorSettings): Promise<void> {
     this.lastConnectedBoard = {
-      selectedBoard: board,
-      selectedPort: port,
+      selectedBoard: this.boardsServiceProvider.boardsConfig.selectedBoard,
+      selectedPort: this.boardsServiceProvider.boardsConfig.selectedPort,
     };
+
+    if (!this.onBoardsConfigChanged) {
+      this.onBoardsConfigChanged =
+        this.boardsServiceProvider.onBoardsConfigChanged(
+          async ({ selectedBoard, selectedPort }) => {
+            if (
+              typeof selectedBoard === 'undefined' ||
+              typeof selectedPort === 'undefined'
+            )
+              return;
+
+            // a board is plugged and it's different from the old connected board
+            if (
+              selectedBoard?.fqbn !==
+                this.lastConnectedBoard?.selectedBoard?.fqbn ||
+              selectedPort?.id !== this.lastConnectedBoard?.selectedPort?.id
+            ) {
+              this.onMonitorShouldResetEmitter.fire(null);
+              this.lastConnectedBoard = {
+                selectedBoard: selectedBoard,
+                selectedPort: selectedPort,
+              };
+            } else {
+              // a board is plugged and it's the same as prev, rerun "this.startMonitor" to
+              // recreate the listener callback
+              this.startMonitor();
+            }
+          }
+        );
+    }
+
+    const { selectedBoard, selectedPort } =
+      this.boardsServiceProvider.boardsConfig;
+    if (!selectedBoard || !selectedBoard.fqbn || !selectedPort) return;
+    await this.server().startMonitor(selectedBoard, selectedPort, settings);
   }
 
   getCurrentSettings(board: Board, port: Port): Promise<MonitorSettings> {

Diff for: arduino-ide-extension/src/browser/serial/monitor/monitor-view-contribution.tsx

@@ -11,6 +11,7 @@ import { ArduinoToolbar } from '../../toolbar/arduino-toolbar';
 import { ArduinoMenus } from '../../menu/arduino-menus';
 import { nls } from '@theia/core/lib/common';
 import { MonitorModel } from '../../monitor-model';
+import { MonitorManagerProxyClient } from '../../../common/protocol';
 
 export namespace SerialMonitor {
   export namespace Commands {
@@ -49,10 +50,13 @@ export class MonitorViewContribution
     MonitorWidget.ID + ':toggle-toolbar';
   static readonly RESET_SERIAL_MONITOR = MonitorWidget.ID + ':reset';
 
-  @inject(MonitorModel)
-  protected readonly model: MonitorModel;
+  constructor(
+    @inject(MonitorModel)
+    protected readonly model: MonitorModel,
 
-  constructor() {
+    @inject(MonitorManagerProxyClient)
+    protected readonly monitorManagerProxy: MonitorManagerProxyClient
+  ) {
     super({
       widgetId: MonitorWidget.ID,
       widgetName: MonitorWidget.LABEL,
@@ -62,6 +66,7 @@ export class MonitorViewContribution
       toggleCommandId: MonitorViewContribution.TOGGLE_SERIAL_MONITOR,
       toggleKeybinding: 'CtrlCmd+Shift+M',
     });
+    this.monitorManagerProxy.onMonitorShouldReset(() => this.reset());
   }
 
   registerMenus(menus: MenuModelRegistry): void {

Diff for: arduino-ide-extension/src/browser/serial/monitor/monitor-widget.tsx

@@ -61,28 +61,6 @@ export class MonitorWidget extends ReactWidget {
     this.toDispose.push(
       Disposable.create(() => this.monitorManagerProxy.disconnect())
     );
-
-    // Start monitor right away if there is already a board/port combination selected
-    const { selectedBoard, selectedPort } =
-      this.boardsServiceProvider.boardsConfig;
-    if (selectedBoard && selectedBoard.fqbn && selectedPort) {
-      this.monitorManagerProxy.startMonitor(selectedBoard, selectedPort);
-    }
-
-    this.toDispose.push(
-      this.boardsServiceProvider.onBoardsConfigChanged(
-        async ({ selectedBoard, selectedPort }) => {
-          if (selectedBoard && selectedBoard.fqbn && selectedPort) {
-            await this.monitorManagerProxy.startMonitor(
-              selectedBoard,
-              selectedPort
-            );
-
-            this.update();
-          }
-        }
-      )
-    );
   }
 
   protected onBeforeAttach(msg: Message): void {
@@ -92,6 +70,8 @@ export class MonitorWidget extends ReactWidget {
     this.monitorManagerProxy.onMonitorSettingsDidChange(
       this.onMonitorSettingsDidChange.bind(this)
     );
+
+    this.monitorManagerProxy.startMonitor();
   }
 
   onMonitorSettingsDidChange(settings: MonitorSettings): void {
@@ -207,7 +187,6 @@ export class MonitorWidget extends ReactWidget {
           <div className="send">
             <SerialMonitorSendInput
               boardsServiceProvider={this.boardsServiceProvider}
-              monitorManagerProxy={this.monitorManagerProxy}
               monitorModel={this.monitorModel}
               resolveFocus={this.onFocusResolved}
               onSend={this.onSend}

Diff for: arduino-ide-extension/src/browser/serial/monitor/serial-monitor-send-input.tsx

@@ -3,14 +3,12 @@ import { Key, KeyCode } from '@theia/core/lib/browser/keys';
 import { Board } from '../../../common/protocol/boards-service';
 import { isOSX } from '@theia/core/lib/common/os';
 import { DisposableCollection, nls } from '@theia/core/lib/common';
-import { MonitorManagerProxyClient } from '../../../common/protocol';
 import { BoardsServiceProvider } from '../../boards/boards-service-provider';
 import { MonitorModel } from '../../monitor-model';
 
 export namespace SerialMonitorSendInput {
   export interface Props {
     readonly boardsServiceProvider: BoardsServiceProvider;
-    readonly monitorManagerProxy: MonitorManagerProxyClient;
     readonly monitorModel: MonitorModel;
     readonly onSend: (text: string) => void;
     readonly resolveFocus: (element: HTMLElement | undefined) => void;

Diff for: arduino-ide-extension/src/browser/serial/plotter/plotter-frontend-contribution.ts

@@ -14,6 +14,7 @@ import { MonitorManagerProxyClient } from '../../../common/protocol';
 import { SerialPlotter } from './protocol';
 import { BoardsServiceProvider } from '../../boards/boards-service-provider';
 import { MonitorModel } from '../../monitor-model';
+
 const queryString = require('query-string');
 
 export namespace SerialPlotterContribution {
@@ -57,6 +58,8 @@ export class PlotterFrontendContribution extends Contribution {
         this.window = null;
       }
     });
+    this.monitorManagerProxy.onMonitorShouldReset(() => this.reset());
+
     return super.onStart(app);
   }
 
@@ -78,19 +81,7 @@ export class PlotterFrontendContribution extends Contribution {
   }
 
   async startPlotter(): Promise<void> {
-    if (
-      !this.boardsServiceProvider.boardsConfig.selectedBoard ||
-      !this.boardsServiceProvider.boardsConfig.selectedPort
-    ) {
-      this.messageService.error(
-        `You need to select a connected board to start the serial plotter`
-      );
-      return;
-    }
-    await this.monitorManagerProxy.startMonitor(
-      this.boardsServiceProvider.boardsConfig.selectedBoard,
-      this.boardsServiceProvider.boardsConfig.selectedPort
-    );
+    await this.monitorManagerProxy.startMonitor();
     if (!!this.window) {
       this.window.focus();
       return;

Diff for: arduino-ide-extension/src/common/protocol/monitor-service.ts

@@ -30,15 +30,12 @@ export const MonitorManagerProxyClient = Symbol('MonitorManagerProxyClient');
 export interface MonitorManagerProxyClient {
   onMessagesReceived: Event<{ messages: string[] }>;
   onMonitorSettingsDidChange: Event<MonitorSettings>;
+  onMonitorShouldReset: Event<void>;
   connect(addressPort: number): void;
   disconnect(): void;
   getWebSocketPort(): number | undefined;
   isWSConnected(): Promise<boolean>;
-  startMonitor(
-    board: Board,
-    port: Port,
-    settings?: PluggableMonitorSettings
-  ): Promise<void>;
+  startMonitor(settings?: PluggableMonitorSettings): Promise<void>;
   getCurrentSettings(board: Board, port: Port): Promise<MonitorSettings>;
   send(message: string): void;
   changeSettings(settings: MonitorSettings): void;

Diff for: arduino-ide-extension/src/node/monitor-service.ts

@@ -62,6 +62,7 @@ export class MonitorService extends CoreClientAware implements Disposable {
 
   protected uploadInProgress = false;
   protected _initialized = new Deferred<void>();
+  protected creating: Deferred<Status>;
 
   constructor(
     @inject(ILogger)
@@ -121,6 +122,7 @@ export class MonitorService extends CoreClientAware implements Disposable {
   dispose(): void {
     this.stop();
     this.onDisposeEmitter.fire();
+    this.onWSClientsNumberChanged?.dispose();
   }
 
   /**
@@ -148,24 +150,30 @@ export class MonitorService extends CoreClientAware implements Disposable {
    * @returns a status to verify connection has been established.
    */
   async start(): Promise<Status> {
+    if (this.creating?.state === 'unresolved') return this.creating.promise;
+    this.creating = new Deferred();
     if (this.duplex) {
       this.updateClientsSettings({
         monitorUISettings: { connected: true, serialPort: this.port.address },
       });
-      return Status.ALREADY_CONNECTED;
+      this.creating.resolve(Status.ALREADY_CONNECTED);
+      return this.creating.promise;
     }
 
     if (!this.board?.fqbn || !this.port?.address || !this.port?.protocol) {
       this.updateClientsSettings({ monitorUISettings: { connected: false } });
 
-      return Status.CONFIG_MISSING;
+      this.creating.resolve(Status.CONFIG_MISSING);
+      return this.creating.promise;
     }
 
     if (this.uploadInProgress) {
       this.updateClientsSettings({
         monitorUISettings: { connected: false, serialPort: this.port.address },
       });
-      return Status.UPLOAD_IN_PROGRESS;
+
+      this.creating.resolve(Status.UPLOAD_IN_PROGRESS);
+      return this.creating.promise;
     }
 
     this.logger.info('starting monitor');
@@ -213,7 +221,7 @@ export class MonitorService extends CoreClientAware implements Disposable {
 
     // Promise executor
     const writeToStream = (resolve: (value: boolean) => void) => {
-      this.duplex = this.duplex || coreClient.client.monitor();
+      this.duplex = coreClient.client.monitor();
 
       const duplexHandlers: DuplexHandler[] = [
         {
@@ -257,17 +265,16 @@ export class MonitorService extends CoreClientAware implements Disposable {
               this.logger.error(res.getError());
               return;
             }
+            if (res.getSuccess()) {
+              resolve(true);
+              return;
+            }
             const data = res.getRxData();
             const message =
               typeof data === 'string'
                 ? data
                 : new TextDecoder('utf8').decode(data);
             this.messages.push(...splitLines(message));
-
-            // if (res.getSuccess()) {
-            //   resolve(true);
-            //   return;
-            // }
           },
         },
       ];
@@ -277,27 +284,30 @@ export class MonitorService extends CoreClientAware implements Disposable {
     };
 
     let attemptsRemaining = 10;
-    let wroteToStreamWithoutError = false;
-    do {
-      await new Promise((r) => setTimeout(r, 10000));
-      wroteToStreamWithoutError = await new Promise(writeToStream);
+    let wroteToStreamSuccessfully = false;
+    while (attemptsRemaining > 0) {
+      wroteToStreamSuccessfully = await new Promise(writeToStream);
+      if (wroteToStreamSuccessfully) break;
       attemptsRemaining -= 1;
-    } while (!wroteToStreamWithoutError && attemptsRemaining > 0);
+      await new Promise((r) => setTimeout(r, 2000));
+    }
 
-    if (wroteToStreamWithoutError) {
+    if (wroteToStreamSuccessfully) {
       this.startMessagesHandlers();
       this.logger.info(
         `started monitor to ${this.port?.address} using ${this.port?.protocol}`
       );
       this.updateClientsSettings({
         monitorUISettings: { connected: true, serialPort: this.port.address },
       });
-      return Status.OK;
+      this.creating.resolve(Status.OK);
+      return this.creating.promise;
     } else {
       this.logger.warn(
         `failed starting monitor to ${this.port?.address} using ${this.port?.protocol}`
       );
-      return Status.NOT_CONNECTED;
+      this.creating.resolve(Status.NOT_CONNECTED);
+      return this.creating.promise;
     }
   }