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]>

Author: Akos Kitta

Diff for: 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 for: arduino-ide-extension/src/browser/monitor-manager-proxy-client-impl.ts

@@ -1,11 +1,14 @@
 import {
-  CommandRegistry,
+  ApplicationError,
   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 { NotificationManager } from '@theia/messages/lib/browser/notifications-manager';
+import { MessageType } from '@theia/core/lib/common/message-service-protocol';
 import { Board, Port } from '../common/protocol';
 import {
   Monitor,
@@ -23,21 +26,31 @@ import { BoardsServiceProvider } from './boards/boards-service-provider';
 export class MonitorManagerProxyClientImpl
   implements MonitorManagerProxyClient
 {
+  @inject(MessageService)
+  private readonly messageService: MessageService;
+  // This is necessary to call the backend methods from the frontend
+  @inject(MonitorManagerProxyFactory)
+  private readonly server: MonitorManagerProxyFactory;
+  @inject(BoardsServiceProvider)
+  private readonly boardsServiceProvider: BoardsServiceProvider;
+  @inject(NotificationManager)
+  private readonly notificationManager: NotificationManager;
+
   // When pluggable monitor messages are received from the backend
   // this event is triggered.
   // Ideally a frontend component is connected to this event
   // to update the UI.
-  protected readonly onMessagesReceivedEmitter = new Emitter<{
+  private readonly onMessagesReceivedEmitter = new Emitter<{
     messages: string[];
   }>();
   readonly onMessagesReceived = this.onMessagesReceivedEmitter.event;
 
-  protected readonly onMonitorSettingsDidChangeEmitter =
+  private readonly onMonitorSettingsDidChangeEmitter =
     new Emitter<MonitorSettings>();
   readonly onMonitorSettingsDidChange =
     this.onMonitorSettingsDidChangeEmitter.event;
 
-  protected readonly onMonitorShouldResetEmitter = new Emitter();
+  private readonly onMonitorShouldResetEmitter = new Emitter<void>();
   readonly onMonitorShouldReset = this.onMonitorShouldResetEmitter.event;
 
   // WebSocket used to handle pluggable monitor communication between
@@ -51,29 +64,16 @@ export class MonitorManagerProxyClientImpl
     return this.wsPort;
   }
 
-  constructor(
-    @inject(MessageService)
-    protected messageService: MessageService,
-
-    // This is necessary to call the backend methods from the frontend
-    @inject(MonitorManagerProxyFactory)
-    protected server: MonitorManagerProxyFactory,
-
-    @inject(CommandRegistry)
-    protected readonly commandRegistry: CommandRegistry,
-
-    @inject(BoardsServiceProvider)
-    protected readonly boardsServiceProvider: BoardsServiceProvider
-  ) {}
-
   /**
    * Connects a localhost WebSocket using the specified port.
    * @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 +87,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 +102,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 +136,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,11 +161,11 @@ export class MonitorManagerProxyClientImpl
                   ? Port.keyOf(this.lastConnectedBoard.selectedPort)
                   : undefined)
             ) {
-              this.onMonitorShouldResetEmitter.fire(null);
               this.lastConnectedBoard = {
                 selectedBoard: selectedBoard,
                 selectedPort: selectedPort,
               };
+              this.onMonitorShouldResetEmitter.fire();
             } else {
               // a board is plugged and it's the same as prev, rerun "this.startMonitor" to
               // recreate the listener callback
@@ -167,7 +178,14 @@ export class MonitorManagerProxyClientImpl
     const { selectedBoard, selectedPort } =
       this.boardsServiceProvider.boardsConfig;
     if (!selectedBoard || !selectedBoard.fqbn || !selectedPort) return;
-    await this.server().startMonitor(selectedBoard, selectedPort, settings);
+    try {
+      this.clearVisibleNotification();
+      await this.server().startMonitor(selectedBoard, selectedPort, settings);
+    } catch (err) {
+      const message = ApplicationError.is(err) ? err.message : String(err);
+      this.previousNotificationId = this.notificationId(message);
+      this.messageService.error(message);
+    }
   }
 
   getCurrentSettings(board: Board, port: Port): Promise<MonitorSettings> {
@@ -199,4 +217,24 @@ export class MonitorManagerProxyClientImpl
       })
     );
   }
+
+  /**
+   * This is the internal (Theia) ID of the notification that is currently visible.
+   * It's stored here as a field to be able to close it before starting a new monitor connection. It's a hack.
+   */
+  private previousNotificationId: string | undefined;
+  private clearVisibleNotification(): void {
+    if (this.previousNotificationId) {
+      this.notificationManager.clear(this.previousNotificationId);
+      this.previousNotificationId = undefined;
+    }
+  }
+
+  private notificationId(message: string, ...actions: string[]): string {
+    return this.notificationManager['getMessageId']({
+      text: message,
+      actions,
+      type: MessageType.Error,
+    });
+  }
 }

Diff for: 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';
-  }
-}