fix clearConsole + refactor monitor connection

Author: Alberto Iannaccone

Diff for: arduino-ide-extension/src/browser/arduino-ide-frontend-module.ts

@@ -81,7 +81,7 @@ import {
 } from '../common/protocol/config-service';
 import { MonitorWidget } from './monitor/monitor-widget';
 import { MonitorViewContribution } from './monitor/monitor-view-contribution';
-import { MonitorConnection } from './monitor/monitor-connection';
+import { SerialConnectionManager } from './monitor/monitor-connection';
 import { MonitorModel } from './monitor/monitor-model';
 import { TabBarDecoratorService as TheiaTabBarDecoratorService } from '@theia/core/lib/browser/shell/tab-bar-decorator';
 import { TabBarDecoratorService } from './theia/core/tab-bar-decorator';
@@ -408,7 +408,7 @@ export default new ContainerModule((bind, unbind, isBound, rebind) => {
       return connection.createProxy(MonitorServicePath, client);
     })
     .inSingletonScope();
-  bind(MonitorConnection).toSelf().inSingletonScope();
+  bind(SerialConnectionManager).toSelf().inSingletonScope();
 
   // Serial monitor service client to receive and delegate notifications from the backend.
   bind(MonitorServiceClient).to(MonitorServiceClientImpl).inSingletonScope();

Diff for: arduino-ide-extension/src/browser/contributions/burn-bootloader.ts

@@ -3,7 +3,7 @@ import { OutputChannelManager } from '@theia/output/lib/common/output-channel';
 import { CoreService } from '../../common/protocol';
 import { ArduinoMenus } from '../menu/arduino-menus';
 import { BoardsDataStore } from '../boards/boards-data-store';
-import { MonitorConnection } from '../monitor/monitor-connection';
+import { SerialConnectionManager } from '../monitor/monitor-connection';
 import { BoardsServiceProvider } from '../boards/boards-service-provider';
 import {
   SketchContribution,
@@ -17,8 +17,8 @@ export class BurnBootloader extends SketchContribution {
   @inject(CoreService)
   protected readonly coreService: CoreService;
 
-  @inject(MonitorConnection)
-  protected readonly monitorConnection: MonitorConnection;
+  @inject(SerialConnectionManager)
+  protected readonly monitorConnection: SerialConnectionManager;
 
   @inject(BoardsDataStore)
   protected readonly boardsDataStore: BoardsDataStore;

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

@@ -4,7 +4,7 @@ import { CoreService } from '../../common/protocol';
 import { ArduinoMenus } from '../menu/arduino-menus';
 import { ArduinoToolbar } from '../toolbar/arduino-toolbar';
 import { BoardsDataStore } from '../boards/boards-data-store';
-import { MonitorConnection } from '../monitor/monitor-connection';
+import { SerialConnectionManager } from '../monitor/monitor-connection';
 import { BoardsServiceProvider } from '../boards/boards-service-provider';
 import {
   SketchContribution,
@@ -20,8 +20,8 @@ export class UploadSketch extends SketchContribution {
   @inject(CoreService)
   protected readonly coreService: CoreService;
 
-  @inject(MonitorConnection)
-  protected readonly monitorConnection: MonitorConnection;
+  @inject(SerialConnectionManager)
+  protected readonly monitorConnection: SerialConnectionManager;
 
   @inject(BoardsDataStore)
   protected readonly boardsDataStore: BoardsDataStore;

Diff for: arduino-ide-extension/src/browser/monitor/monitor-connection.ts

@@ -2,7 +2,6 @@ import { injectable, inject, postConstruct } from 'inversify';
 import { deepClone } from '@theia/core/lib/common/objects';
 import { Emitter, Event } from '@theia/core/lib/common/event';
 import { MessageService } from '@theia/core/lib/common/message-service';
-import { FrontendApplicationStateService } from '@theia/core/lib/browser/frontend-application-state';
 import {
   MonitorService,
   MonitorConfig,
@@ -22,13 +21,8 @@ import { MonitorModel } from './monitor-model';
 import { NotificationCenter } from '../notification-center';
 import { ThemeService } from '@theia/core/lib/browser/theming';
 
-export enum SerialType {
-  Monitor = 'Monitor',
-  Plotter = 'Plotter',
-}
-
 @injectable()
-export class MonitorConnection {
+export class SerialConnectionManager {
   @inject(MonitorModel)
   protected readonly monitorModel: MonitorModel;
 
@@ -50,26 +44,19 @@ export class MonitorConnection {
   @inject(MessageService)
   protected messageService: MessageService;
 
-  @inject(FrontendApplicationStateService)
-  protected readonly applicationState: FrontendApplicationStateService;
-
-  @inject(MonitorModel)
-  protected readonly model: MonitorModel;
-
   @inject(ThemeService)
   protected readonly themeService: ThemeService;
-
-  protected _state: MonitorConnection.State = [];
+  protected _state: Serial.State = [];
   protected _connected: boolean;
 
   /**
    * Note: The idea is to toggle this property from the UI (`Monitor` view)
    * and the boards config and the boards attachment/detachment logic can be at on place, here.
    */
-  protected readonly onConnectionChangedEmitter =
-    new Emitter<MonitorConnection.State>();
+  protected readonly onConnectionChangedEmitter = new Emitter<boolean>();
+
   /**
-   * This emitter forwards all read events **iff** the connection is established.
+   * This emitter forwards all read events **if** the connection is established.
    */
   protected readonly onReadEmitter = new Emitter<{ messages: string[] }>();
 
@@ -81,8 +68,13 @@ export class MonitorConnection {
   protected monitorErrors: MonitorError[] = [];
   protected reconnectTimeout?: number;
 
+  /**
+   * When the websocket server is up on the backend, we save the port here, so that the client knows how to connect to it
+   * */
   protected wsPort?: number;
+
   protected webSocket?: WebSocket;
+
   protected config: Partial<MonitorConfig> = {
     board: undefined,
     port: undefined,
@@ -116,15 +108,21 @@ export class MonitorConnection {
     });
   }
 
+  /**
+   * Set the config passing only the properties that has changed. If some has changed and the serial is open,
+   * we try to reconnect
+   *
+   * @param newConfig the porperties of the config that has changed
+   */
   protected setConfig(newConfig: Partial<MonitorConfig>): void {
-    let shouldReconnect = false;
+    let configHasChanged = false;
     Object.keys(this.config).forEach((key: keyof MonitorConfig) => {
       if (newConfig[key] && newConfig[key] !== this.config[key]) {
-        shouldReconnect = true;
+        configHasChanged = true;
         this.config = { ...this.config, [key]: newConfig[key] };
       }
     });
-    if (shouldReconnect && this.isSerialOpen()) {
+    if (configHasChanged && this.isSerialOpen()) {
       this.disconnect().then(() => this.connect());
     }
   }
@@ -133,10 +131,13 @@ export class MonitorConnection {
     return this.wsPort;
   }
 
-  handleWebSocketChanged(wsPort: number): void {
+  protected handleWebSocketChanged(wsPort: number): void {
     this.wsPort = wsPort;
   }
 
+  /**
+   * When the serial monitor is open and the frontend is connected to the serial, we create the websocket here
+   */
   protected createWsConnection(): boolean {
     if (this.wsPort) {
       try {
@@ -153,10 +154,17 @@ export class MonitorConnection {
     return false;
   }
 
-  protected async setState(s: MonitorConnection.State): Promise<Status> {
+  /**
+   * Sets the types of connections needed by the client.
+   *
+   * @param s The new types of connections (can be 'Monitor', 'Plotter', none or both).
+   *          If the previuos state was empty and 's' is not, it tries to reconnect to the serial service
+   *          If the provios state was NOT empty and now it is, it disconnects to the serial service
+   * @returns The status of the operation
+   */
+  protected async setState(s: Serial.State): Promise<Status> {
     const oldState = deepClone(this._state);
     this._state = s;
-    this.onConnectionChangedEmitter.fire(this._state);
     let status = Status.OK;
 
     if (this.isSerialOpen(oldState) && !this.isSerialOpen()) {
@@ -165,34 +173,14 @@ export class MonitorConnection {
       status = await this.connect();
     }
 
-    // if (this.connected) {
-    //   switch (this.state.connected) {
-    //     case SerialType.All:
-    //       return Status.OK;
-    //     case SerialType.Plotter:
-    //       if (type === SerialType.Monitor) {
-    //         if (this.createWsConnection()) {
-    //           this.state = { ...this.state, connected: SerialType.All };
-    //           return Status.OK;
-    //         }
-    //         return Status.NOT_CONNECTED;
-    //       }
-    //       return Status.OK;
-    //     case SerialType.Monitor:
-    //       if (type === SerialType.Plotter)
-    //         this.state = { ...this.state, connected: SerialType.All };
-    //       return SerialType.All;
-    //   }
-    // }
-
     return status;
   }
 
-  protected get state(): MonitorConnection.State {
+  protected get state(): Serial.State {
     return this._state;
   }
 
-  isSerialOpen(state?: MonitorConnection.State): boolean {
+  isSerialOpen(state?: Serial.State): boolean {
     return (state ? state : this._state).length > 0;
   }
 
@@ -208,19 +196,32 @@ export class MonitorConnection {
 
   set connected(c: boolean) {
     this._connected = c;
+    this.onConnectionChangedEmitter.fire(this._connected);
   }
-
-  async openSerial(type: SerialType): Promise<Status> {
+  /**
+   * Called when a client opens the serial from the GUI
+   *
+   * @param type could be either 'Monitor' or 'Plotter'. If it's 'Monitor' we also connect to the websocket and
+   *             listen to the message events
+   * @returns the status of the operation
+   */
+  async openSerial(type: Serial.Type): Promise<Status> {
     if (this.state.includes(type)) return Status.OK;
     const newState = deepClone(this.state);
     newState.push(type);
     const status = await this.setState(newState);
-    if (Status.isOK(status) && type === SerialType.Monitor)
+    if (Status.isOK(status) && type === Serial.Type.Monitor)
       this.createWsConnection();
     return status;
   }
 
-  async closeSerial(type: SerialType): Promise<Status> {
+  /**
+   * Called when a client closes the serial from the GUI
+   *
+   * @param type could be either 'Monitor' or 'Plotter'. If it's 'Monitor' we close the websocket connection
+   * @returns the status of the operation
+   */
+  async closeSerial(type: Serial.Type): Promise<Status> {
     const index = this.state.indexOf(type);
     let status = Status.OK;
     if (index >= 0) {
@@ -229,7 +230,7 @@ export class MonitorConnection {
       status = await this.setState(newState);
       if (
         Status.isOK(status) &&
-        type === SerialType.Monitor &&
+        type === Serial.Type.Monitor &&
         this.webSocket
       ) {
         this.webSocket.close();
@@ -239,6 +240,9 @@ export class MonitorConnection {
     return status;
   }
 
+  /**
+   * Handles error on the MonitorServiceClient and try to reconnect, eventually
+   */
   handleError(error: MonitorError): void {
     if (!this.connected) return;
     const { code, config } = error;
@@ -247,7 +251,7 @@ export class MonitorConnection {
     switch (code) {
       case MonitorError.ErrorCodes.CLIENT_CANCEL: {
         console.debug(
-          `Serial connection was canceled by client: ${MonitorConnection.Config.toString(
+          `Serial connection was canceled by client: ${Serial.Config.toString(
             this.config
           )}.`
         );
@@ -375,14 +379,14 @@ export class MonitorConnection {
     if (Status.isOK(status)) {
       this.connected = false;
       console.log(
-        `<<< Disposed serial connection. Was: ${MonitorConnection.Config.toString(
+        `<<< Disposed serial connection. Was: ${Serial.Config.toString(
           this.config
         )}`
       );
       this.wsPort = undefined;
     } else {
       console.warn(
-        `<<< Could not dispose serial connection. Activate connection: ${MonitorConnection.Config.toString(
+        `<<< Could not dispose serial connection. Activate connection: ${Serial.Config.toString(
           this.config
         )}`
       );
@@ -407,7 +411,7 @@ export class MonitorConnection {
     });
   }
 
-  get onConnectionChanged(): Event<MonitorConnection.State> {
+  get onConnectionChanged(): Event<boolean> {
     return this.onConnectionChangedEmitter.event;
   }
 
@@ -440,8 +444,18 @@ export class MonitorConnection {
   }
 }
 
-export namespace MonitorConnection {
-  export type State = SerialType[];
+export namespace Serial {
+  export enum Type {
+    Monitor = 'Monitor',
+    Plotter = 'Plotter',
+  }
+
+  /**
+   * The state represents which types of connections are needed by the client, and it should match whether the Serial Monitor
+   * or the Serial Plotter are open or not in the GUI. It's an array cause it's possible to have both, none or only one of
+   * them open
+   */
+  export type State = Serial.Type[];
 
   export namespace Config {
     export function toString(config: Partial<MonitorConfig>): string {

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

@@ -12,7 +12,7 @@ import {
 import { MonitorConfig } from '../../common/protocol/monitor-service';
 import { ArduinoSelect } from '../widgets/arduino-select';
 import { MonitorModel } from './monitor-model';
-import { MonitorConnection, SerialType } from './monitor-connection';
+import { Serial, SerialConnectionManager } from './monitor-connection';
 import { SerialMonitorSendInput } from './serial-monitor-send-input';
 import { SerialMonitorOutput } from './serial-monitor-send-output';
 import { BoardsServiceProvider } from '../boards/boards-service-provider';
@@ -24,8 +24,8 @@ export class MonitorWidget extends ReactWidget {
   @inject(MonitorModel)
   protected readonly monitorModel: MonitorModel;
 
-  @inject(MonitorConnection)
-  protected readonly monitorConnection: MonitorConnection;
+  @inject(SerialConnectionManager)
+  protected readonly monitorConnection: SerialConnectionManager;
 
   @inject(BoardsServiceProvider)
   protected readonly boardsServiceProvider: BoardsServiceProvider;
@@ -53,7 +53,7 @@ export class MonitorWidget extends ReactWidget {
     this.toDispose.push(this.clearOutputEmitter);
     this.toDispose.push(
       Disposable.create(() =>
-        this.monitorConnection.closeSerial(SerialType.Monitor)
+        this.monitorConnection.closeSerial(Serial.Type.Monitor)
       )
     );
   }
@@ -77,9 +77,7 @@ export class MonitorWidget extends ReactWidget {
 
   protected onAfterAttach(msg: Message): void {
     super.onAfterAttach(msg);
-    // const { boardsConfig } = this.boardsServiceProvider;
-
-    this.monitorConnection.openSerial(SerialType.Monitor);
+    this.monitorConnection.openSerial(Serial.Type.Monitor);
   }
 
   onCloseRequest(msg: Message): void {

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

@@ -3,7 +3,7 @@ import { Event } from '@theia/core/lib/common/event';
 import { DisposableCollection } from '@theia/core/lib/common/disposable';
 import { areEqual, FixedSizeList as List } from 'react-window';
 import { MonitorModel } from './monitor-model';
-import { MonitorConnection } from './monitor-connection';
+import { SerialConnectionManager } from './monitor-connection';
 import dateFormat = require('dateformat');
 import { messagesToLines, truncateLines } from './monitor-utils';
 
@@ -124,7 +124,7 @@ const Row = React.memo(_Row, areEqual);
 export namespace SerialMonitorOutput {
   export interface Props {
     readonly monitorModel: MonitorModel;
-    readonly monitorConnection: MonitorConnection;
+    readonly monitorConnection: SerialConnectionManager;
     readonly clearConsoleEvent: Event<void>;
     readonly height: number;
   }