fix monitor settings client communication

Author: Alberto Iannaccone

arduino-ide-extension/src/browser/arduino-frontend-contribution.tsx

@@ -46,7 +46,10 @@ import {
 } from '@theia/editor/lib/browser';
 import { ProblemContribution } from '@theia/markers/lib/browser/problem/problem-contribution';
 import { MonacoMenus } from '@theia/monaco/lib/browser/monaco-menu';
-import { FileNavigatorCommands, FileNavigatorContribution } from '@theia/navigator/lib/browser/navigator-contribution';
+import {
+  FileNavigatorCommands,
+  FileNavigatorContribution,
+} from '@theia/navigator/lib/browser/navigator-contribution';
 import { OutlineViewContribution } from '@theia/outline-view/lib/browser/outline-view-contribution';
 import { OutputContribution } from '@theia/output/lib/browser/output-contribution';
 import { ScmContribution } from '@theia/scm/lib/browser/scm-contribution';
@@ -84,7 +87,8 @@ export class ArduinoFrontendContribution
     TabBarToolbarContribution,
     CommandContribution,
     MenuContribution,
-    ColorContribution {
+    ColorContribution
+{
   @inject(ILogger)
   protected logger: ILogger;
 
@@ -157,6 +161,7 @@ export class ArduinoFrontendContribution
   @inject(SketchesServiceClientImpl)
   protected readonly sketchServiceClient: SketchesServiceClientImpl;
 
+  @inject(FrontendApplicationStateService)
   protected readonly appStateService: FrontendApplicationStateService;
 
   @inject(LocalStorageService)
@@ -344,7 +349,7 @@ export class ArduinoFrontendContribution
 
     app.shell.leftPanelHandler.removeBottomMenu('settings-menu');
 
-    this.fileSystemFrontendContribution.onDidChangeEditorFile(e => {
+    this.fileSystemFrontendContribution.onDidChangeEditorFile((e) => {
       if (e.type === FileChangeType.DELETED) {
         const editorWidget = e.editor;
         if (SaveableWidget.is(editorWidget)) {
@@ -494,7 +499,7 @@ export class ArduinoFrontendContribution
       EditorCommands.SPLIT_EDITOR_UP,
       EditorCommands.SPLIT_EDITOR_VERTICAL,
       EditorCommands.SPLIT_EDITOR_HORIZONTAL,
-      FileNavigatorCommands.REVEAL_IN_NAVIGATOR
+      FileNavigatorCommands.REVEAL_IN_NAVIGATOR,
     ]) {
       registry.unregisterCommand(command);
     }

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

@@ -24,6 +24,11 @@ export class MonitorManagerProxyClientImpl
   }>();
   readonly onMessagesReceived = this.onMessagesReceivedEmitter.event;
 
+  protected readonly onMonitorSettingsDidChangeEmitter =
+    new Emitter<MonitorSettings>();
+  readonly onMonitorSettingsDidChange =
+    this.onMonitorSettingsDidChangeEmitter.event;
+
   protected readonly onWSConnectionChangedEmitter = new Emitter<boolean>();
   readonly onWSConnectionChanged = this.onWSConnectionChangedEmitter.event;
 
@@ -61,9 +66,11 @@ export class MonitorManagerProxyClientImpl
       return;
     }
 
-    this.webSocket.onmessage = (res) => {
-      const messages = JSON.parse(res.data);
-      this.onMessagesReceivedEmitter.fire({ messages });
+    this.webSocket.onmessage = (message) => {
+      const parsedMessage = JSON.parse(message.data);
+      if (Array.isArray(parsedMessage))
+        this.onMessagesReceivedEmitter.fire({ messages: parsedMessage });
+      else this.onMonitorSettingsDidChangeEmitter.fire(parsedMessage);
     };
     this.wsPort = addressPort;
   }
@@ -93,7 +100,7 @@ export class MonitorManagerProxyClientImpl
     return this.server().startMonitor(board, port, settings);
   }
 
-  getCurrentSettings(board: Board, port: Port): MonitorSettings {
+  getCurrentSettings(board: Board, port: Port): Promise<MonitorSettings> {
     return this.server().getCurrentSettings(board, port);
   }
 

arduino-ide-extension/src/browser/monitor-model.ts

@@ -4,6 +4,9 @@ import {
   LocalStorageService,
 } from '@theia/core/lib/browser';
 import { inject, injectable } from '@theia/core/shared/inversify';
+import { MonitorManagerProxyClient } from '../common/protocol';
+import { isNullOrUndefined } from '../common/utils';
+import { MonitorSettings } from '../node/monitor-settings/monitor-settings-provider';
 
 @injectable()
 export class MonitorModel implements FrontendApplicationContribution {
@@ -12,6 +15,9 @@ export class MonitorModel implements FrontendApplicationContribution {
   @inject(LocalStorageService)
   protected readonly localStorageService: LocalStorageService;
 
+  @inject(MonitorManagerProxyClient)
+  protected readonly monitorManagerProxy: MonitorManagerProxyClient;
+
   protected readonly onChangeEmitter: Emitter<
     MonitorModel.State.Change<keyof MonitorModel.State>
   >;
@@ -35,7 +41,11 @@ export class MonitorModel implements FrontendApplicationContribution {
   onStart(): void {
     this.localStorageService
       .getData<MonitorModel.State>(MonitorModel.STORAGE_ID)
-      .then(this.restoreState);
+      .then(this.restoreState.bind(this));
+
+    this.monitorManagerProxy.onMonitorSettingsDidChange(
+      this.onMonitorSettingsDidChange.bind(this)
+    );
   }
 
   get onChange(): Event<MonitorModel.State.Change<keyof MonitorModel.State>> {
@@ -65,22 +75,34 @@ export class MonitorModel implements FrontendApplicationContribution {
     return this._autoscroll;
   }
 
-  toggleAutoscroll(): void {
-    this._autoscroll = !this._autoscroll;
+  set autoscroll(autoscroll: boolean) {
+    if (autoscroll === this._autoscroll) return;
+    this._autoscroll = autoscroll;
+    this.monitorManagerProxy.changeSettings({
+      monitorUISettings: { autoscroll },
+    });
     this.storeState().then(() => {
       this.onChangeEmitter.fire({
         property: 'autoscroll',
-        value: this._timestamp,
+        value: this._autoscroll,
       });
     });
   }
 
+  toggleAutoscroll(): void {
+    this.autoscroll = !this._autoscroll;
+  }
+
   get timestamp(): boolean {
     return this._timestamp;
   }
 
-  toggleTimestamp(): void {
-    this._timestamp = !this._timestamp;
+  set timestamp(timestamp: boolean) {
+    if (timestamp === this._timestamp) return;
+    this._timestamp = timestamp;
+    this.monitorManagerProxy.changeSettings({
+      monitorUISettings: { timestamp },
+    });
     this.storeState().then(() =>
       this.onChangeEmitter.fire({
         property: 'timestamp',
@@ -89,12 +111,20 @@ export class MonitorModel implements FrontendApplicationContribution {
     );
   }
 
+  toggleTimestamp(): void {
+    this.timestamp = !this._timestamp;
+  }
+
   get lineEnding(): MonitorModel.EOL {
     return this._lineEnding;
   }
 
   set lineEnding(lineEnding: MonitorModel.EOL) {
+    if (lineEnding === this._lineEnding) return;
     this._lineEnding = lineEnding;
+    this.monitorManagerProxy.changeSettings({
+      monitorUISettings: { lineEnding },
+    });
     this.storeState().then(() =>
       this.onChangeEmitter.fire({
         property: 'lineEnding',
@@ -107,15 +137,31 @@ export class MonitorModel implements FrontendApplicationContribution {
     return this._interpolate;
   }
 
-  set interpolate(i: boolean) {
-    this._interpolate = i;
+  set interpolate(interpolate: boolean) {
+    if (interpolate === this._interpolate) return;
+    this._interpolate = interpolate;
+    this.monitorManagerProxy.changeSettings({
+      monitorUISettings: { interpolate },
+    });
     this.storeState().then(() =>
       this.onChangeEmitter.fire({
         property: 'interpolate',
         value: this._interpolate,
       })
     );
   }
+
+  protected onMonitorSettingsDidChange = (settings: MonitorSettings): void => {
+    const { monitorUISettings } = settings;
+    if (!monitorUISettings) return;
+    const { autoscroll, interpolate, lineEnding, timestamp } =
+      monitorUISettings;
+
+    if (!isNullOrUndefined(autoscroll)) this.autoscroll = autoscroll;
+    if (!isNullOrUndefined(interpolate)) this.interpolate = interpolate;
+    if (!isNullOrUndefined(lineEnding)) this.lineEnding = lineEnding;
+    if (!isNullOrUndefined(timestamp)) this.timestamp = timestamp;
+  };
 }
 
 // TODO: Move this to /common

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

@@ -1,5 +1,5 @@
 import * as React from 'react';
-import { postConstruct, injectable, inject } from 'inversify';
+import { injectable, inject } from 'inversify';
 import { OptionsType } from 'react-select/src/types';
 import { Emitter } from '@theia/core/lib/common/event';
 import { Disposable } from '@theia/core/lib/common/disposable';
@@ -26,6 +26,8 @@ export class MonitorWidget extends ReactWidget {
   );
   static readonly ID = 'serial-monitor';
 
+  protected settings: MonitorSettings = {};
+
   protected widgetHeight: number;
 
   /**
@@ -82,10 +84,24 @@ export class MonitorWidget extends ReactWidget {
     );
   }
 
-  @postConstruct()
-  protected init(): void {
+  protected onAfterAttach(msg: Message): void {
     this.update();
     this.toDispose.push(this.monitorModel.onChange(() => this.update()));
+    this.getCurrentSettings().then(this.onMonitorSettingsDidChange.bind(this));
+    this.monitorManagerProxy.onMonitorSettingsDidChange(
+      this.onMonitorSettingsDidChange.bind(this)
+    );
+  }
+
+  onMonitorSettingsDidChange(settings: MonitorSettings): void {
+    this.settings = {
+      ...this.settings,
+      pluggableMonitorSettings: {
+        ...this.settings.pluggableMonitorSettings,
+        ...settings.pluggableMonitorSettings,
+      },
+    };
+    this.update();
   }
 
   clearConsole(): void {
@@ -157,56 +173,33 @@ export class MonitorWidget extends ReactWidget {
     ];
   }
 
-  private getCurrentSettings(): MonitorSettings {
+  private getCurrentSettings(): Promise<MonitorSettings> {
     const board = this.boardsServiceProvider.boardsConfig.selectedBoard;
     const port = this.boardsServiceProvider.boardsConfig.selectedPort;
     if (!board || !port) {
-      return {};
+      return Promise.resolve(this.settings || {});
     }
     return this.monitorManagerProxy.getCurrentSettings(board, port);
   }
 
-  //////////////////////////////////////////////////
-  ////////////////////IMPORTANT/////////////////////
-  //////////////////////////////////////////////////
-  // baudRates and selectedBaudRates as of now are hardcoded
-  // like this to retrieve the baudrate settings from the ones
-  // received by the monitor.
-  // We're doing it like since the frontend as of now doesn't
-  // support a fully customizable list of options that would
-  // be require to support pluggable monitors completely.
-  // As soon as the frontend UI is updated to support
-  // any custom settings this methods MUST be removed and
-  // made generic.
-  //
-  // This breaks if the user tries to open a monitor that
-  // doesn't support the baudrate setting.
-  protected get baudRates(): string[] {
-    const { pluggableMonitorSettings } = this.getCurrentSettings();
-    if (!pluggableMonitorSettings || !pluggableMonitorSettings['baudrate']) {
-      return [];
-    }
-
-    const baudRateSettings = pluggableMonitorSettings['baudrate'];
-
-    return baudRateSettings.values;
-  }
+  protected render(): React.ReactNode {
+    const baudrate = this.settings?.pluggableMonitorSettings
+      ? this.settings.pluggableMonitorSettings.baudrate
+      : undefined;
 
-  protected get selectedBaudRate(): string {
-    const { pluggableMonitorSettings } = this.getCurrentSettings();
-    if (!pluggableMonitorSettings || !pluggableMonitorSettings['baudrate']) {
-      return '';
-    }
-    const baudRateSettings = pluggableMonitorSettings['baudrate'];
-    return baudRateSettings.selectedValue;
-  }
+    const baudrateOptions = baudrate?.values.map((b) => ({
+      label: b + ' baud',
+      value: b,
+    }));
+    const baudrateSelectedOption = baudrateOptions?.find(
+      (b) => b.value === baudrate?.selectedValue
+    );
 
-  protected render(): React.ReactNode {
-    const { baudRates, lineEndings } = this;
     const lineEnding =
-      lineEndings.find((item) => item.value === this.monitorModel.lineEnding) ||
-      lineEndings[1]; // Defaults to `\n`.
-    const baudRate = baudRates.find((item) => item === this.selectedBaudRate);
+      this.lineEndings.find(
+        (item) => item.value === this.monitorModel.lineEnding
+      ) || this.lineEndings[1]; // Defaults to `\n`.
+
     return (
       <div className="serial-monitor">
         <div className="head">
@@ -222,20 +215,22 @@ export class MonitorWidget extends ReactWidget {
             <div className="select">
               <ArduinoSelect
                 maxMenuHeight={this.widgetHeight - 40}
-                options={lineEndings}
+                options={this.lineEndings}
                 value={lineEnding}
                 onChange={this.onChangeLineEnding}
               />
             </div>
-            <div className="select">
-              <ArduinoSelect
-                className="select"
-                maxMenuHeight={this.widgetHeight - 40}
-                options={baudRates}
-                value={baudRate}
-                onChange={this.onChangeBaudRate}
-              />
-            </div>
+            {baudrateOptions && baudrateSelectedOption && (
+              <div className="select">
+                <ArduinoSelect
+                  className="select"
+                  maxMenuHeight={this.widgetHeight - 40}
+                  options={baudrateOptions}
+                  value={baudrateSelectedOption}
+                  onChange={this.onChangeBaudRate}
+                />
+              </div>
+            )}
           </div>
         </div>
         <div className="body">
@@ -257,16 +252,21 @@ export class MonitorWidget extends ReactWidget {
 
   protected readonly onChangeLineEnding = (
     option: SerialMonitorOutput.SelectOption<MonitorModel.EOL>
-  ) => {
+  ): void => {
     this.monitorModel.lineEnding = option.value;
   };
 
-  protected readonly onChangeBaudRate = (value: string) => {
-    const { pluggableMonitorSettings } = this.getCurrentSettings();
-    if (!pluggableMonitorSettings || !pluggableMonitorSettings['baudrate'])
-      return;
-    const baudRateSettings = pluggableMonitorSettings['baudrate'];
-    baudRateSettings.selectedValue = value;
-    this.monitorManagerProxy.changeSettings(pluggableMonitorSettings);
+  protected readonly onChangeBaudRate = ({
+    value,
+  }: {
+    value: string;
+  }): void => {
+    this.getCurrentSettings().then(({ pluggableMonitorSettings }) => {
+      if (!pluggableMonitorSettings || !pluggableMonitorSettings['baudrate'])
+        return;
+      const baudRateSettings = pluggableMonitorSettings['baudrate'];
+      baudRateSettings.selectedValue = value;
+      this.monitorManagerProxy.changeSettings({ pluggableMonitorSettings });
+    });
   };
 }