Fix upload and serial (#661)

* get serial connection status from BE

* handle serial connect in the BE

* allow breakpoints on vscode (windows)

* Timeout on config change to prevent serial busy

* serial-service tests

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

@@ -48,7 +48,6 @@ export class BurnBootloader extends SketchContribution {
   }
 
   async burnBootloader(): Promise<void> {
-    await this.serialConnection.disconnect();
     try {
       const { boardsConfig } = this.boardsServiceClientImpl;
       const port = boardsConfig.selectedPort;
@@ -87,9 +86,7 @@ export class BurnBootloader extends SketchContribution {
       }
       this.messageService.error(errorMessage);
     } finally {
-      if (this.serialConnection.isSerialOpen()) {
-        await this.serialConnection.connect();
-      }
+      await this.serialConnection.reconnectAfterUpload();
     }
   }
 }

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

@@ -210,7 +210,7 @@ export class UploadSketch extends SketchContribution {
     if (!sketch) {
       return;
     }
-    await this.serialConnection.disconnect();
+
     try {
       const { boardsConfig } = this.boardsServiceClientImpl;
       const [fqbn, { selectedProgrammer }, verify, verbose, sourceOverride] =
@@ -288,27 +288,7 @@ export class UploadSketch extends SketchContribution {
       this.uploadInProgress = false;
       this.onDidChangeEmitter.fire();
 
-      if (
-        this.serialConnection.isSerialOpen() &&
-        this.serialConnection.serialConfig
-      ) {
-        const { board, port } = this.serialConnection.serialConfig;
-        try {
-          await this.boardsServiceClientImpl.waitUntilAvailable(
-            Object.assign(board, { port }),
-            10_000
-          );
-          await this.serialConnection.connect();
-        } catch (waitError) {
-          this.messageService.error(
-            nls.localize(
-              'arduino/sketch/couldNotConnectToSerial',
-              'Could not reconnect to serial port. {0}',
-              waitError.toString()
-            )
-          );
-        }
-      }
+      setTimeout(() => this.serialConnection.reconnectAfterUpload(), 5000);
     }
   }
 }

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

@@ -12,7 +12,7 @@ import {
 import { SerialConfig } from '../../../common/protocol/serial-service';
 import { ArduinoSelect } from '../../widgets/arduino-select';
 import { SerialModel } from '../serial-model';
-import { Serial, SerialConnectionManager } from '../serial-connection-manager';
+import { SerialConnectionManager } from '../serial-connection-manager';
 import { SerialMonitorSendInput } from './serial-monitor-send-input';
 import { SerialMonitorOutput } from './serial-monitor-send-output';
 import { BoardsServiceProvider } from '../../boards/boards-service-provider';
@@ -57,9 +57,7 @@ export class MonitorWidget extends ReactWidget {
     this.scrollOptions = undefined;
     this.toDispose.push(this.clearOutputEmitter);
     this.toDispose.push(
-      Disposable.create(() =>
-        this.serialConnection.closeSerial(Serial.Type.Monitor)
-      )
+      Disposable.create(() => this.serialConnection.closeWStoBE())
     );
   }
 
@@ -83,7 +81,7 @@ export class MonitorWidget extends ReactWidget {
 
   protected onAfterAttach(msg: Message): void {
     super.onAfterAttach(msg);
-    this.serialConnection.openSerial(Serial.Type.Monitor);
+    this.serialConnection.openWSToBE();
   }
 
   onCloseRequest(msg: Message): void {
@@ -171,7 +169,7 @@ export class MonitorWidget extends ReactWidget {
         <div className="head">
           <div className="send">
             <SerialMonitorSendInput
-              serialConfig={this.serialConnection.serialConfig}
+              serialConnection={this.serialConnection}
               resolveFocus={this.onFocusResolved}
               onSend={this.onSend}
             />

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

@@ -1,18 +1,20 @@
 import * as React from 'react';
 import { Key, KeyCode } from '@theia/core/lib/browser/keys';
 import { Board, Port } from '../../../common/protocol/boards-service';
-import { SerialConfig } from '../../../common/protocol/serial-service';
 import { isOSX } from '@theia/core/lib/common/os';
-import { nls } from '@theia/core/lib/common';
+import { DisposableCollection, nls } from '@theia/core/lib/common';
+import { SerialConnectionManager } from '../serial-connection-manager';
+import { SerialPlotter } from '../plotter/protocol';
 
 export namespace SerialMonitorSendInput {
   export interface Props {
-    readonly serialConfig?: SerialConfig;
+    readonly serialConnection: SerialConnectionManager;
     readonly onSend: (text: string) => void;
     readonly resolveFocus: (element: HTMLElement | undefined) => void;
   }
   export interface State {
     text: string;
+    connected: boolean;
   }
 }
 
@@ -20,20 +22,45 @@ export class SerialMonitorSendInput extends React.Component<
   SerialMonitorSendInput.Props,
   SerialMonitorSendInput.State
 > {
+  protected toDisposeBeforeUnmount = new DisposableCollection();
+
   constructor(props: Readonly<SerialMonitorSendInput.Props>) {
     super(props);
-    this.state = { text: '' };
+    this.state = { text: '', connected: false };
     this.onChange = this.onChange.bind(this);
     this.onSend = this.onSend.bind(this);
     this.onKeyDown = this.onKeyDown.bind(this);
   }
 
+  componentDidMount(): void {
+    this.props.serialConnection.isBESerialConnected().then((connected) => {
+      this.setState({ connected });
+    });
+
+    this.toDisposeBeforeUnmount.pushAll([
+      this.props.serialConnection.onRead(({ messages }) => {
+        if (
+          messages.command ===
+            SerialPlotter.Protocol.Command.MIDDLEWARE_CONFIG_CHANGED &&
+          'connected' in messages.data
+        ) {
+          this.setState({ connected: messages.data.connected });
+        }
+      }),
+    ]);
+  }
+
+  componentWillUnmount(): void {
+    // TODO: "Your preferred browser's local storage is almost full." Discard `content` before saving layout?
+    this.toDisposeBeforeUnmount.dispose();
+  }
+
   render(): React.ReactNode {
     return (
       <input
         ref={this.setRef}
         type="text"
-        className={`theia-input ${this.props.serialConfig ? '' : 'warning'}`}
+        className={`theia-input ${this.state.connected ? '' : 'warning'}`}
         placeholder={this.placeholder}
         value={this.state.text}
         onChange={this.onChange}
@@ -43,8 +70,8 @@ export class SerialMonitorSendInput extends React.Component<
   }
 
   protected get placeholder(): string {
-    const { serialConfig } = this.props;
-    if (!serialConfig) {
+    const serialConfig = this.props.serialConnection.getConfig();
+    if (!this.state.connected || !serialConfig) {
       return nls.localize(
         'arduino/serial/notConnected',
         'Not connected. Select a board and a port to connect automatically.'
@@ -55,10 +82,12 @@ export class SerialMonitorSendInput extends React.Component<
       'arduino/serial/message',
       "Message ({0} + Enter to send message to '{1}' on '{2}'",
       isOSX ? '⌘' : nls.localize('vscode/keybindingLabels/ctrlKey', 'Ctrl'),
-      Board.toString(board, {
-        useFqbn: false,
-      }),
-      Port.toString(port)
+      board
+        ? Board.toString(board, {
+            useFqbn: false,
+          })
+        : 'unknown',
+      port ? Port.toString(port) : 'unknown'
     );
   }
 

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

@@ -11,8 +11,8 @@ import { ArduinoMenus } from '../../menu/arduino-menus';
 import { Contribution } from '../../contributions/contribution';
 import { Endpoint, FrontendApplication } from '@theia/core/lib/browser';
 import { ipcRenderer } from '@theia/core/shared/electron';
-import { SerialConfig, Status } from '../../../common/protocol';
-import { Serial, SerialConnectionManager } from '../serial-connection-manager';
+import { SerialConfig } from '../../../common/protocol';
+import { SerialConnectionManager } from '../serial-connection-manager';
 import { SerialPlotter } from './protocol';
 import { BoardsServiceProvider } from '../../boards/boards-service-provider';
 const queryString = require('query-string');
@@ -51,10 +51,8 @@ export class PlotterFrontendContribution extends Contribution {
     ipcRenderer.on('CLOSE_CHILD_WINDOW', async () => {
       if (!!this.window) {
         this.window = null;
-        await this.serialConnection.closeSerial(Serial.Type.Plotter);
       }
     });
-
     return super.onStart(app);
   }
 
@@ -77,17 +75,15 @@ export class PlotterFrontendContribution extends Contribution {
       this.window.focus();
       return;
     }
-    const status = await this.serialConnection.openSerial(Serial.Type.Plotter);
     const wsPort = this.serialConnection.getWsPort();
-    if (Status.isOK(status) && wsPort) {
+    if (wsPort) {
       this.open(wsPort);
     } else {
-      this.serialConnection.closeSerial(Serial.Type.Plotter);
       this.messageService.error(`Couldn't open serial plotter`);
     }
   }
 
-  protected open(wsPort: number): void {
+  protected async open(wsPort: number): Promise<void> {
     const initConfig: Partial<SerialPlotter.Config> = {
       baudrates: SerialConfig.BaudRates.map((b) => b),
       currentBaudrate: this.model.baudRate,
@@ -95,7 +91,7 @@ export class PlotterFrontendContribution extends Contribution {
       darkTheme: this.themeService.getCurrentTheme().type === 'dark',
       wsPort,
       interpolate: this.model.interpolate,
-      connected: this.serialConnection.connected,
+      connected: await this.serialConnection.isBESerialConnected(),
       serialPort: this.boardsServiceProvider.boardsConfig.selectedPort?.address,
     };
     const urlWithParams = queryString.stringifyUrl(

arduino-ide-extension/src/browser/serial/serial-connection-manager.ts

@@ -1,5 +1,4 @@
 import { injectable, inject } 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 {
@@ -23,8 +22,6 @@ import { nls } from '@theia/core/lib/common/nls';
 
 @injectable()
 export class SerialConnectionManager {
-  protected _state: Serial.State = [];
-  protected _connected = false;
   protected config: Partial<SerialConfig> = {
     board: undefined,
     port: undefined,
@@ -62,7 +59,9 @@ export class SerialConnectionManager {
     protected readonly boardsServiceProvider: BoardsServiceProvider,
     @inject(MessageService) protected messageService: MessageService,
     @inject(ThemeService) protected readonly themeService: ThemeService,
-    @inject(CoreService) protected readonly core: CoreService
+    @inject(CoreService) protected readonly core: CoreService,
+    @inject(BoardsServiceProvider)
+    protected readonly boardsServiceClientImpl: BoardsServiceProvider
   ) {
     this.serialServiceClient.onWebSocketChanged(
       this.handleWebSocketChanged.bind(this)
@@ -89,8 +88,11 @@ export class SerialConnectionManager {
     );
 
     // Handles the `baudRate` changes by reconnecting if required.
-    this.serialModel.onChange(({ property }) => {
-      if (property === 'baudRate' && this.connected) {
+    this.serialModel.onChange(async ({ property }) => {
+      if (
+        property === 'baudRate' &&
+        (await this.serialService.isSerialPortOpen())
+      ) {
         const { boardsConfig } = this.boardsServiceProvider;
         this.handleBoardConfigChange(boardsConfig);
       }
@@ -114,8 +116,8 @@ export class SerialConnectionManager {
   }
 
   /**
-   * Set the config passing only the properties that has changed. If some has changed and the serial is open,
-   * we try to reconnect
+   * Updated the config in the BE passing only the properties that has changed.
+   * BE will create a new connection if needed.
    *
    * @param newConfig the porperties of the config that has changed
    */
@@ -127,17 +129,16 @@ export class SerialConnectionManager {
         this.config = { ...this.config, [key]: newConfig[key] };
       }
     });
-    if (
-      configHasChanged &&
-      this.isSerialOpen() &&
-      !(await this.core.isUploading())
-    ) {
+
+    if (configHasChanged) {
       this.serialService.updateWsConfigParam({
         currentBaudrate: this.config.baudRate,
         serialPort: this.config.port?.address,
       });
-      await this.disconnect();
-      await this.connect();
+
+      if (isSerialConfig(this.config)) {
+        this.serialService.setSerialConfig(this.config);
+      }
     }
   }
 
@@ -149,134 +150,56 @@ export class SerialConnectionManager {
     return this.wsPort;
   }
 
-  isWebSocketConnected(): boolean {
-    return !!this.webSocket?.url;
-  }
-
   protected handleWebSocketChanged(wsPort: number): void {
     this.wsPort = wsPort;
   }
 
-  /**
-   * When the serial is open and the frontend is connected to the serial, we create the websocket here
-   */
-  protected createWsConnection(): boolean {
-    if (this.wsPort) {
-      try {
-        this.webSocket = new WebSocket(`ws://localhost:${this.wsPort}`);
-        this.webSocket.onmessage = (res) => {
-          const messages = JSON.parse(res.data);
-          this.onReadEmitter.fire({ messages });
-        };
-        return true;
-      } catch {
-        return false;
-      }
-    }
-    return false;
-  }
-
-  /**
-   * Sets the types of connections needed by the client.
-   *
-   * @param newState The array containing the list of desired connections.
-   *          If the previuos state was empty and 'newState' 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(newState: Serial.State): Promise<Status> {
-    const oldState = deepClone(this._state);
-    let status = Status.OK;
-
-    if (this.isSerialOpen(oldState) && !this.isSerialOpen(newState)) {
-      status = await this.disconnect();
-    } else if (!this.isSerialOpen(oldState) && this.isSerialOpen(newState)) {
-      if (await this.core.isUploading()) {
-        this.messageService.error(`Cannot open serial port when uploading`);
-        return Status.NOT_CONNECTED;
-      }
-      status = await this.connect();
-    }
-    this._state = newState;
-    return status;
-  }
-
-  protected get state(): Serial.State {
-    return this._state;
-  }
-
-  isSerialOpen(state?: Serial.State): boolean {
-    return (state ? state : this._state).length > 0;
-  }
-
   get serialConfig(): SerialConfig | undefined {
     return isSerialConfig(this.config)
       ? (this.config as SerialConfig)
       : undefined;
   }
 
-  get connected(): boolean {
-    return this._connected;
+  async isBESerialConnected(): Promise<boolean> {
+    return await this.serialService.isSerialPortOpen();
   }
 
-  set connected(c: boolean) {
-    this._connected = c;
-    this.serialService.updateWsConfigParam({ connected: c });
-    this.onConnectionChangedEmitter.fire(this._connected);
-  }
-  /**
-   * 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> {
+  openWSToBE(): void {
     if (!isSerialConfig(this.config)) {
       this.messageService.error(
         `Please select a board and a port to open the serial connection.`
       );
-      return Status.NOT_CONNECTED;
     }
-    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 === Serial.Type.Monitor)
-      this.createWsConnection();
-    return 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) {
-      const newState = deepClone(this.state);
-      newState.splice(index, 1);
-      status = await this.setState(newState);
-      if (
-        Status.isOK(status) &&
-        type === Serial.Type.Monitor &&
-        this.webSocket
-      ) {
-        this.webSocket.close();
-        this.webSocket = undefined;
+    if (!this.webSocket && this.wsPort) {
+      try {
+        this.webSocket = new WebSocket(`ws://localhost:${this.wsPort}`);
+        this.webSocket.onmessage = (res) => {
+          const messages = JSON.parse(res.data);
+          this.onReadEmitter.fire({ messages });
+        };
+      } catch {
+        this.messageService.error(`Unable to connect to websocket`);
+      }
+    }
+  }
+
+  closeWStoBE(): void {
+    if (this.webSocket) {
+      try {
+        this.webSocket.close();
+        this.webSocket = undefined;
+      } catch {
+        this.messageService.error(`Unable to close websocket`);
       }
     }
-    return status;
   }
 
   /**
    * Handles error on the SerialServiceClient and try to reconnect, eventually
    */
-  handleError(error: SerialError): void {
-    if (!this.connected) return;
+  async handleError(error: SerialError): Promise<void> {
+    if (!(await this.serialService.isSerialPortOpen())) return;
     const { code, config } = error;
     const { board, port } = config;
     const options = { timeout: 3000 };
@@ -329,9 +252,8 @@ export class SerialConnectionManager {
         break;
       }
     }
-    this.connected = false;
 
-    if (this.isSerialOpen()) {
+    if ((await this.serialService.clientsAttached()) > 0) {
       if (this.serialErrors.length >= 10) {
         this.messageService.warn(
           nls.localize(
@@ -363,59 +285,31 @@ export class SerialConnectionManager {
           )
         );
         this.reconnectTimeout = window.setTimeout(
-          () => this.connect(),
+          () => this.reconnectAfterUpload(),
           timeout
         );
       }
     }
   }
 
-  async connect(): Promise<Status> {
-    if (this.connected) return Status.ALREADY_CONNECTED;
-    if (!isSerialConfig(this.config)) return Status.NOT_CONNECTED;
-
-    console.info(
-      `>>> Creating serial connection for ${Board.toString(
-        this.config.board
-      )} on port ${Port.toString(this.config.port)}...`
-    );
-    const connectStatus = await this.serialService.connect(this.config);
-    if (Status.isOK(connectStatus)) {
-      this.connected = true;
-      console.info(
-        `<<< Serial connection created for ${Board.toString(this.config.board, {
-          useFqbn: false,
-        })} on port ${Port.toString(this.config.port)}.`
+  async reconnectAfterUpload(): Promise<void> {
+    try {
+      if (isSerialConfig(this.config)) {
+        await this.boardsServiceClientImpl.waitUntilAvailable(
+          Object.assign(this.config.board, { port: this.config.port }),
+          10_000
+        );
+        this.serialService.connectSerialIfRequired();
+      }
+    } catch (waitError) {
+      this.messageService.error(
+        nls.localize(
+          'arduino/sketch/couldNotConnectToSerial',
+          'Could not reconnect to serial port. {0}',
+          waitError.toString()
+        )
       );
     }
-
-    return Status.isOK(connectStatus);
-  }
-
-  async disconnect(): Promise<Status> {
-    if (!this.connected) {
-      return Status.OK;
-    }
-
-    console.log('>>> Disposing existing serial connection...');
-    const status = await this.serialService.disconnect();
-    if (Status.isOK(status)) {
-      this.connected = false;
-      console.log(
-        `<<< Disposed serial connection. Was: ${Serial.Config.toString(
-          this.config
-        )}`
-      );
-      this.wsPort = undefined;
-    } else {
-      console.warn(
-        `<<< Could not dispose serial connection. Activate connection: ${Serial.Config.toString(
-          this.config
-        )}`
-      );
-    }
-
-    return status;
   }
 
   /**
@@ -424,7 +318,7 @@ export class SerialConnectionManager {
    * It is a NOOP if connected.
    */
   async send(data: string): Promise<Status> {
-    if (!this.connected) {
+    if (!(await this.serialService.isSerialPortOpen())) {
       return Status.NOT_CONNECTED;
     }
     return new Promise<Status>((resolve) => {
@@ -438,7 +332,7 @@ export class SerialConnectionManager {
     return this.onConnectionChangedEmitter.event;
   }
 
-  get onRead(): Event<{ messages: string[] }> {
+  get onRead(): Event<{ messages: any }> {
     return this.onReadEmitter.event;
   }
 
@@ -453,18 +347,6 @@ export class SerialConnectionManager {
 }
 
 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<SerialConfig>): string {
       if (!isSerialConfig(config)) return '';