fix: propagate monitor errors to the frontend

- Handle when the board's platform is not installed (Closes #1974) - UX: Smoother monitor widget reset (Closes #1985) - Fixed monitor <input> readOnly state (Closes #1984) - Set monitor widget header color (Ref #682) Closes #1508 Signed-off-by: Akos Kitta <[email protected]>
arduino · Mar 30, 2023 · ba1d61f · ba1d61f
1 parent 6e72be1
commit ba1d61f
Show file tree

Hide file tree

Showing 15 changed files with 681 additions and 330 deletions.
diff --git a/arduino-ide-extension/src/browser/arduino-ide-frontend-module.ts b/arduino-ide-extension/src/browser/arduino-ide-frontend-module.ts
@@ -496,15 +496,7 @@ export default new ContainerModule((bind, unbind, isBound, rebind) => {
   bind(TabBarToolbarContribution).toService(MonitorViewContribution);
   bind(WidgetFactory).toDynamicValue((context) => ({
     id: MonitorWidget.ID,
-    createWidget: () => {
-      return new MonitorWidget(
-        context.container.get<MonitorModel>(MonitorModel),
-        context.container.get<MonitorManagerProxyClient>(
-          MonitorManagerProxyClient
-        ),
-        context.container.get<BoardsServiceProvider>(BoardsServiceProvider)
-      );
-    },
+    createWidget: () => context.container.get(MonitorWidget),
   }));
 
   bind(MonitorManagerProxyFactory).toFactory(

diff --git a/arduino-ide-extension/src/browser/monitor-manager-proxy-client-impl.ts b/arduino-ide-extension/src/browser/monitor-manager-proxy-client-impl.ts
@@ -1,10 +1,12 @@
 import {
+  ApplicationError,
   CommandRegistry,
   Disposable,
   Emitter,
   MessageService,
   nls,
 } from '@theia/core';
+import { Deferred } from '@theia/core/lib/common/promise-util';
 import { inject, injectable } from '@theia/core/shared/inversify';
 import { Board, Port } from '../common/protocol';
 import {
@@ -37,7 +39,7 @@ export class MonitorManagerProxyClientImpl
   readonly onMonitorSettingsDidChange =
     this.onMonitorSettingsDidChangeEmitter.event;
 
-  protected readonly onMonitorShouldResetEmitter = new Emitter();
+  protected readonly onMonitorShouldResetEmitter = new Emitter<void>();
   readonly onMonitorShouldReset = this.onMonitorShouldResetEmitter.event;
 
   // WebSocket used to handle pluggable monitor communication between
@@ -71,9 +73,11 @@ export class MonitorManagerProxyClientImpl
    * @param addressPort port of the WebSocket
    */
   async connect(addressPort: number): Promise<void> {
-    if (!!this.webSocket) {
-      if (this.wsPort === addressPort) return;
-      else this.disconnect();
+    if (this.webSocket) {
+      if (this.wsPort === addressPort) {
+        return;
+      }
+      this.disconnect();
     }
     try {
       this.webSocket = new WebSocket(`ws://localhost:${addressPort}`);
@@ -87,6 +91,9 @@ export class MonitorManagerProxyClientImpl
       return;
     }
 
+    const opened = new Deferred<void>();
+    this.webSocket.onopen = () => opened.resolve();
+    this.webSocket.onerror = () => opened.reject();
     this.webSocket.onmessage = (message) => {
       const parsedMessage = JSON.parse(message.data);
       if (Array.isArray(parsedMessage))
@@ -99,19 +106,26 @@ export class MonitorManagerProxyClientImpl
       }
     };
     this.wsPort = addressPort;
+    return opened.promise;
   }
 
   /**
    * Disconnects the WebSocket if connected.
    */
   disconnect(): void {
-    if (!this.webSocket) return;
+    if (!this.webSocket) {
+      return;
+    }
     this.onBoardsConfigChanged?.dispose();
     this.onBoardsConfigChanged = undefined;
     try {
-      this.webSocket?.close();
+      this.webSocket.close();
       this.webSocket = undefined;
-    } catch {
+    } catch (err) {
+      console.error(
+        'Could not close the websocket connection for the monitor.',
+        err
+      );
       this.messageService.error(
         nls.localize(
           'arduino/monitor/unableToCloseWebSocket',
@@ -126,6 +140,7 @@ export class MonitorManagerProxyClientImpl
   }
 
   async startMonitor(settings?: PluggableMonitorSettings): Promise<void> {
+    await this.boardsServiceProvider.reconciled;
     this.lastConnectedBoard = {
       selectedBoard: this.boardsServiceProvider.boardsConfig.selectedBoard,
       selectedPort: this.boardsServiceProvider.boardsConfig.selectedPort,
@@ -150,7 +165,7 @@ export class MonitorManagerProxyClientImpl
                   ? Port.keyOf(this.lastConnectedBoard.selectedPort)
                   : undefined)
             ) {
-              this.onMonitorShouldResetEmitter.fire(null);
+              this.onMonitorShouldResetEmitter.fire();
               this.lastConnectedBoard = {
                 selectedBoard: selectedBoard,
                 selectedPort: selectedPort,
@@ -167,7 +182,13 @@ export class MonitorManagerProxyClientImpl
     const { selectedBoard, selectedPort } =
       this.boardsServiceProvider.boardsConfig;
     if (!selectedBoard || !selectedBoard.fqbn || !selectedPort) return;
-    await this.server().startMonitor(selectedBoard, selectedPort, settings);
+    try {
+      await this.server().startMonitor(selectedBoard, selectedPort, settings);
+    } catch (err) {
+      this.messageService.error(
+        ApplicationError.is(err) ? err.message : String(err)
+      );
+    }
   }
 
   getCurrentSettings(board: Board, port: Port): Promise<MonitorSettings> {

diff --git a/arduino-ide-extension/src/browser/monitor-model.ts b/arduino-ide-extension/src/browser/monitor-model.ts
@@ -4,7 +4,14 @@ import {
   LocalStorageService,
 } from '@theia/core/lib/browser';
 import { inject, injectable } from '@theia/core/shared/inversify';
-import { MonitorManagerProxyClient } from '../common/protocol';
+import {
+  isMonitorConnected,
+  MonitorConnectionStatus,
+  monitorConnectionStatusEquals,
+  MonitorEOL,
+  MonitorManagerProxyClient,
+  MonitorState,
+} from '../common/protocol';
 import { isNullOrUndefined } from '../common/utils';
 import { MonitorSettings } from '../node/monitor-settings/monitor-settings-provider';
 
@@ -19,48 +26,48 @@ export class MonitorModel implements FrontendApplicationContribution {
   protected readonly monitorManagerProxy: MonitorManagerProxyClient;
 
   protected readonly onChangeEmitter: Emitter<
-    MonitorModel.State.Change<keyof MonitorModel.State>
+    MonitorState.Change<keyof MonitorState>
   >;
 
   protected _autoscroll: boolean;
   protected _timestamp: boolean;
-  protected _lineEnding: MonitorModel.EOL;
+  protected _lineEnding: MonitorEOL;
   protected _interpolate: boolean;
   protected _darkTheme: boolean;
   protected _wsPort: number;
   protected _serialPort: string;
-  protected _connected: boolean;
+  protected _connectionStatus: MonitorConnectionStatus;
 
   constructor() {
     this._autoscroll = true;
     this._timestamp = false;
     this._interpolate = false;
-    this._lineEnding = MonitorModel.EOL.DEFAULT;
+    this._lineEnding = MonitorEOL.DEFAULT;
     this._darkTheme = false;
     this._wsPort = 0;
     this._serialPort = '';
-    this._connected = true;
+    this._connectionStatus = 'not-connected';
 
     this.onChangeEmitter = new Emitter<
-      MonitorModel.State.Change<keyof MonitorModel.State>
+      MonitorState.Change<keyof MonitorState>
     >();
   }
 
   onStart(): void {
     this.localStorageService
-      .getData<MonitorModel.State>(MonitorModel.STORAGE_ID)
+      .getData<MonitorState>(MonitorModel.STORAGE_ID)
       .then(this.restoreState.bind(this));
 
     this.monitorManagerProxy.onMonitorSettingsDidChange(
       this.onMonitorSettingsDidChange.bind(this)
     );
   }
 
-  get onChange(): Event<MonitorModel.State.Change<keyof MonitorModel.State>> {
+  get onChange(): Event<MonitorState.Change<keyof MonitorState>> {
     return this.onChangeEmitter.event;
   }
 
-  protected restoreState(state: MonitorModel.State): void {
+  protected restoreState(state: MonitorState): void {
     if (!state) {
       return;
     }
@@ -125,11 +132,11 @@ export class MonitorModel implements FrontendApplicationContribution {
     this.timestamp = !this._timestamp;
   }
 
-  get lineEnding(): MonitorModel.EOL {
+  get lineEnding(): MonitorEOL {
     return this._lineEnding;
   }
 
-  set lineEnding(lineEnding: MonitorModel.EOL) {
+  set lineEnding(lineEnding: MonitorEOL) {
     if (lineEnding === this._lineEnding) return;
     this._lineEnding = lineEnding;
     this.monitorManagerProxy.changeSettings({
@@ -211,19 +218,26 @@ export class MonitorModel implements FrontendApplicationContribution {
     );
   }
 
-  get connected(): boolean {
-    return this._connected;
+  get connectionStatus(): MonitorConnectionStatus {
+    return this._connectionStatus;
   }
 
-  set connected(connected: boolean) {
-    if (connected === this._connected) return;
-    this._connected = connected;
+  set connectionStatus(connectionStatus: MonitorConnectionStatus) {
+    if (
+      monitorConnectionStatusEquals(connectionStatus, this.connectionStatus)
+    ) {
+      return;
+    }
+    this._connectionStatus = connectionStatus;
     this.monitorManagerProxy.changeSettings({
-      monitorUISettings: { connected },
+      monitorUISettings: {
+        connectionStatus,
+        connected: isMonitorConnected(connectionStatus),
+      },
     });
     this.onChangeEmitter.fire({
-      property: 'connected',
-      value: this._connected,
+      property: 'connectionStatus',
+      value: this._connectionStatus,
     });
   }
 
@@ -238,7 +252,7 @@ export class MonitorModel implements FrontendApplicationContribution {
       darkTheme,
       wsPort,
       serialPort,
-      connected,
+      connectionStatus,
     } = monitorUISettings;
 
     if (!isNullOrUndefined(autoscroll)) this.autoscroll = autoscroll;
@@ -248,31 +262,7 @@ export class MonitorModel implements FrontendApplicationContribution {
     if (!isNullOrUndefined(darkTheme)) this.darkTheme = darkTheme;
     if (!isNullOrUndefined(wsPort)) this.wsPort = wsPort;
     if (!isNullOrUndefined(serialPort)) this.serialPort = serialPort;
-    if (!isNullOrUndefined(connected)) this.connected = connected;
+    if (!isNullOrUndefined(connectionStatus))
+      this.connectionStatus = connectionStatus;
   };
 }
-
-// TODO: Move this to /common
-export namespace MonitorModel {
-  export interface State {
-    autoscroll: boolean;
-    timestamp: boolean;
-    lineEnding: EOL;
-    interpolate: boolean;
-    darkTheme: boolean;
-    wsPort: number;
-    serialPort: string;
-    connected: boolean;
-  }
-  export namespace State {
-    export interface Change<K extends keyof State> {
-      readonly property: K;
-      readonly value: State[K];
-    }
-  }
-
-  export type EOL = '' | '\n' | '\r' | '\r\n';
-  export namespace EOL {
-    export const DEFAULT: EOL = '\n';
-  }
-}
diff --git a/arduino-ide-extension/src/browser/serial/monitor/monitor-view-contribution.tsx b/arduino-ide-extension/src/browser/serial/monitor/monitor-view-contribution.tsx
@@ -10,6 +10,7 @@ import {
 import { ArduinoToolbar } from '../../toolbar/arduino-toolbar';
 import { ArduinoMenus } from '../../menu/arduino-menus';
 import { nls } from '@theia/core/lib/common';
+import { Event } from '@theia/core/lib/common/event';
 import { MonitorModel } from '../../monitor-model';
 import { MonitorManagerProxyClient } from '../../../common/protocol';
 
@@ -84,13 +85,13 @@ export class MonitorViewContribution
       id: 'monitor-autoscroll',
       render: () => this.renderAutoScrollButton(),
       isVisible: (widget) => widget instanceof MonitorWidget,
-      onDidChange: this.model.onChange as any, // XXX: it's a hack. See: https://github.com/eclipse-theia/theia/pull/6696/
+      onDidChange: this.model.onChange as Event<unknown> as Event<void>,
     });
     registry.registerItem({
       id: 'monitor-timestamp',
       render: () => this.renderTimestampButton(),
       isVisible: (widget) => widget instanceof MonitorWidget,
-      onDidChange: this.model.onChange as any, // XXX: it's a hack. See: https://github.com/eclipse-theia/theia/pull/6696/
+      onDidChange: this.model.onChange as Event<unknown> as Event<void>,
     });
     registry.registerItem({
       id: SerialMonitor.Commands.CLEAR_OUTPUT.id,
@@ -143,8 +144,7 @@ export class MonitorViewContribution
   protected async reset(): Promise<void> {
     const widget = this.tryGetWidget();
     if (widget) {
-      widget.dispose();
-      await this.openView({ activate: true, reveal: true });
+      widget.reset();
     }
   }