refactor monitor settings interfaces

Author: Alberto Iannaccone

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

@@ -6,7 +6,10 @@ import {
   MonitorManagerProxyClient,
   MonitorManagerProxyFactory,
 } from '../common/protocol/monitor-service';
-import { MonitorSettings } from '../node/monitor-settings/monitor-settings-provider';
+import {
+  PluggableMonitorSettings,
+  MonitorSettings,
+} from '../node/monitor-settings/monitor-settings-provider';
 
 @injectable()
 export class MonitorManagerProxyClientImpl
@@ -85,7 +88,7 @@ export class MonitorManagerProxyClientImpl
   async startMonitor(
     board: Board,
     port: Port,
-    settings?: MonitorSettings
+    settings?: PluggableMonitorSettings
   ): Promise<void> {
     return this.server().startMonitor(board, port, settings);
   }
@@ -116,6 +119,7 @@ export class MonitorManagerProxyClientImpl
       JSON.stringify({
         command: Monitor.Command.CHANGE_SETTINGS,
         // TODO: This might be wrong, verify if it works
+        // SPOILER: It doesn't
         data: settings,
       })
     );

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

@@ -118,6 +118,7 @@ export class MonitorModel implements FrontendApplicationContribution {
   }
 }
 
+// TODO: Move this to /common
 export namespace MonitorModel {
   export interface State {
     autoscroll: boolean;

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

@@ -182,20 +182,22 @@ export class MonitorWidget extends ReactWidget {
   // This breaks if the user tries to open a monitor that
   // doesn't support the baudrate setting.
   protected get baudRates(): string[] {
-    const settings = this.getCurrentSettings();
-    const baudRateSettings = settings['baudrate'];
-    if (!baudRateSettings) {
+    const { pluggableMonitorSettings } = this.getCurrentSettings();
+    if (!pluggableMonitorSettings || !pluggableMonitorSettings['baudrate']) {
       return [];
     }
+
+    const baudRateSettings = pluggableMonitorSettings['baudrate'];
+
     return baudRateSettings.values;
   }
 
   protected get selectedBaudRate(): string {
-    const settings = this.getCurrentSettings();
-    const baudRateSettings = settings['baudrate'];
-    if (!baudRateSettings) {
+    const { pluggableMonitorSettings } = this.getCurrentSettings();
+    if (!pluggableMonitorSettings || !pluggableMonitorSettings['baudrate']) {
       return '';
     }
+    const baudRateSettings = pluggableMonitorSettings['baudrate'];
     return baudRateSettings.selectedValue;
   }
 
@@ -260,8 +262,11 @@ export class MonitorWidget extends ReactWidget {
   };
 
   protected readonly onChangeBaudRate = (value: string) => {
-    const settings = this.getCurrentSettings();
-    settings['baudrate'].selectedValue = value;
-    this.monitorManagerProxy.changeSettings(settings);
+    const { pluggableMonitorSettings } = this.getCurrentSettings();
+    if (!pluggableMonitorSettings || !pluggableMonitorSettings['baudrate'])
+      return;
+    const baudRateSettings = pluggableMonitorSettings['baudrate'];
+    baudRateSettings.selectedValue = value;
+    this.monitorManagerProxy.changeSettings(pluggableMonitorSettings);
   };
 }

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

@@ -88,11 +88,12 @@ export class PlotterFrontendContribution extends Contribution {
     let baudrates: number[] = [];
     let currentBaudrate = -1;
     if (board && port) {
-      const settings = this.monitorManagerProxy.getCurrentSettings(board, port);
-      if ('baudrate' in settings) {
+      const { pluggableMonitorSettings } =
+        this.monitorManagerProxy.getCurrentSettings(board, port);
+      if (pluggableMonitorSettings && 'baudrate' in pluggableMonitorSettings) {
         // Convert from string to numbers
-        baudrates = settings['baudrate'].values.map((b) => +b);
-        currentBaudrate = +settings['baudrate'].selectedValue;
+        baudrates = pluggableMonitorSettings['baudrate'].values.map((b) => +b);
+        currentBaudrate = +pluggableMonitorSettings['baudrate'].selectedValue;
       }
     }
 

arduino-ide-extension/src/common/protocol/monitor-service.ts

@@ -1,5 +1,8 @@
 import { Event, JsonRpcServer } from '@theia/core';
-import { MonitorSettings } from '../../node/monitor-settings/monitor-settings-provider';
+import {
+  PluggableMonitorSettings,
+  MonitorSettings,
+} from '../../node/monitor-settings/monitor-settings-provider';
 import { Board, Port } from './boards-service';
 
 export const MonitorManagerProxyFactory = Symbol('MonitorManagerProxyFactory');
@@ -12,15 +15,15 @@ export interface MonitorManagerProxy
   startMonitor(
     board: Board,
     port: Port,
-    settings?: MonitorSettings
+    settings?: PluggableMonitorSettings
   ): Promise<void>;
   changeMonitorSettings(
     board: Board,
     port: Port,
-    settings: MonitorSettings
+    settings: PluggableMonitorSettings
   ): Promise<void>;
   stopMonitor(board: Board, port: Port): Promise<void>;
-  getCurrentSettings(board: Board, port: Port): MonitorSettings;
+  getCurrentSettings(board: Board, port: Port): PluggableMonitorSettings;
 }
 
 export const MonitorManagerProxyClient = Symbol('MonitorManagerProxyClient');
@@ -34,14 +37,14 @@ export interface MonitorManagerProxyClient {
   startMonitor(
     board: Board,
     port: Port,
-    settings?: MonitorSettings
+    settings?: PluggableMonitorSettings
   ): Promise<void>;
   getCurrentSettings(board: Board, port: Port): MonitorSettings;
   send(message: string): void;
   changeSettings(settings: MonitorSettings): void;
 }
 
-export interface MonitorSetting {
+export interface PluggableMonitorSetting {
   // The setting identifier
   readonly id: string;
   // A human-readable label of the setting (to be displayed on the GUI)

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

@@ -6,7 +6,7 @@ import {
 } from '../common/protocol';
 import { Board, Port } from '../common/protocol';
 import { MonitorManager } from './monitor-manager';
-import { MonitorSettings } from './monitor-settings/monitor-settings-provider';
+import { PluggableMonitorSettings } from './monitor-settings/monitor-settings-provider';
 
 @injectable()
 export class MonitorManagerProxyImpl implements MonitorManagerProxy {
@@ -32,7 +32,7 @@ export class MonitorManagerProxyImpl implements MonitorManagerProxy {
   async startMonitor(
     board: Board,
     port: Port,
-    settings?: MonitorSettings
+    settings?: PluggableMonitorSettings
   ): Promise<void> {
     if (settings) {
       await this.changeMonitorSettings(board, port, settings);
@@ -54,7 +54,7 @@ export class MonitorManagerProxyImpl implements MonitorManagerProxy {
   async changeMonitorSettings(
     board: Board,
     port: Port,
-    settings: MonitorSettings
+    settings: PluggableMonitorSettings
   ): Promise<void> {
     if (!this.manager.isStarted(board, port)) {
       // Monitor is not running, no need to change settings
@@ -79,7 +79,7 @@ export class MonitorManagerProxyImpl implements MonitorManagerProxy {
    * @param port port monitored
    * @returns a map of MonitorSetting
    */
-  getCurrentSettings(board: Board, port: Port): MonitorSettings {
+  getCurrentSettings(board: Board, port: Port): PluggableMonitorSettings {
     return this.manager.currentMonitorSettings(board, port);
   }
 

arduino-ide-extension/src/node/monitor-manager.ts

@@ -3,7 +3,7 @@ import { inject, injectable, named } from '@theia/core/shared/inversify';
 import { Board, Port, Status } from '../common/protocol';
 import { CoreClientAware } from './core-client-provider';
 import { MonitorService } from './monitor-service';
-import { MonitorSettings } from './monitor-settings/monitor-settings-provider';
+import { PluggableMonitorSettings } from './monitor-settings/monitor-settings-provider';
 
 type MonitorID = string;
 
@@ -145,7 +145,11 @@ export class MonitorManager extends CoreClientAware {
    * @param port port to monitor
    * @param settings monitor settings to change
    */
-  changeMonitorSettings(board: Board, port: Port, settings: MonitorSettings) {
+  changeMonitorSettings(
+    board: Board,
+    port: Port,
+    settings: PluggableMonitorSettings
+  ) {
     const monitorID = this.monitorID(board, port);
     let monitor = this.monitorServices.get(monitorID);
     if (!monitor) {
@@ -161,7 +165,7 @@ export class MonitorManager extends CoreClientAware {
    * @param port port monitored
    * @returns map of current monitor settings
    */
-  currentMonitorSettings(board: Board, port: Port): MonitorSettings {
+  currentMonitorSettings(board: Board, port: Port): PluggableMonitorSettings {
     const monitorID = this.monitorID(board, port);
     const monitor = this.monitorServices.get(monitorID);
     if (!monitor) {

arduino-ide-extension/src/node/monitor-service.ts

@@ -15,7 +15,7 @@ import { WebSocketProvider } from './web-socket/web-socket-provider';
 import { Port as gRPCPort } from 'arduino-ide-extension/src/node/cli-protocol/cc/arduino/cli/commands/v1/port_pb';
 import WebSocketProviderImpl from './web-socket/web-socket-provider-impl';
 import {
-  MonitorSettings,
+  PluggableMonitorSettings,
   MonitorSettingsProvider,
 } from './monitor-settings/monitor-settings-provider';
 
@@ -28,7 +28,7 @@ export class MonitorService extends CoreClientAware implements Disposable {
 
   // Settings used by the currently running pluggable monitor.
   // They can be freely modified while running.
-  protected settings: MonitorSettings;
+  protected settings: PluggableMonitorSettings;
 
   // List of messages received from the running pluggable monitor.
   // These are flushed from time to time to the frontend.
@@ -279,7 +279,7 @@ export class MonitorService extends CoreClientAware implements Disposable {
    *
    * @returns map of current monitor settings
    */
-  currentSettings(): MonitorSettings {
+  currentSettings(): PluggableMonitorSettings {
     return this.settings;
   }
 
@@ -294,7 +294,7 @@ export class MonitorService extends CoreClientAware implements Disposable {
   private async portMonitorSettings(
     protocol: string,
     fqbn: string
-  ): Promise<MonitorSettings> {
+  ): Promise<PluggableMonitorSettings> {
     await this.coreClientProvider.initialized;
     const coreClient = await this.coreClient();
     const { client, instance } = coreClient;
@@ -314,7 +314,7 @@ export class MonitorService extends CoreClientAware implements Disposable {
       }
     );
 
-    const settings: MonitorSettings = {};
+    const settings: PluggableMonitorSettings = {};
     for (const iterator of res.getSettingsList()) {
       settings[iterator.getSettingId()] = {
         id: iterator.getSettingId(),
@@ -335,7 +335,7 @@ export class MonitorService extends CoreClientAware implements Disposable {
    * @param settings map of monitor settings to change
    * @returns a status to verify settings have been sent.
    */
-  async changeSettings(settings: MonitorSettings): Promise<Status> {
+  async changeSettings(settings: PluggableMonitorSettings): Promise<Status> {
     const config = new MonitorPortConfiguration();
     for (const id in settings) {
       const s = new MonitorPortSetting();
@@ -384,7 +384,9 @@ export class MonitorService extends CoreClientAware implements Disposable {
               this.send(message.data);
               break;
             case Monitor.Command.CHANGE_SETTINGS:
-              const settings: MonitorSettings = JSON.parse(message.data);
+              const settings: PluggableMonitorSettings = JSON.parse(
+                message.data
+              );
               this.changeSettings(settings);
               break;
           }

arduino-ide-extension/src/node/monitor-settings/monitor-settings-provider-impl.ts

@@ -1,8 +1,9 @@
 import { injectable } from 'inversify';
 import { CoreClientProvider } from '../core-client-provider';
 import {
-  MonitorSettings,
+  PluggableMonitorSettings,
   MonitorSettingsProvider,
+  MonitorSettings,
 } from './monitor-settings-provider';
 
 @injectable()
@@ -18,7 +19,7 @@ export class MonitorSettingsProviderImpl implements MonitorSettingsProvider {
   init(
     id: string,
     coreClientProvider: CoreClientProvider
-  ): Promise<MonitorSettings> {
+  ): Promise<PluggableMonitorSettings> {
     throw new Error('Method not implemented.');
 
     // 1. query the CLI (via coreClientProvider) and return all available settings for the pluggable monitor.
@@ -33,10 +34,10 @@ export class MonitorSettingsProviderImpl implements MonitorSettingsProvider {
     //       and adding those that are missing
     //    ii. save the `monitorSettingsValues` in the file, using the id as the key
   }
-  get(): Promise<MonitorSettings> {
+  get(): Promise<PluggableMonitorSettings> {
     throw new Error('Method not implemented.');
   }
-  set(settings: MonitorSettings): Promise<MonitorSettings> {
+  set(settings: PluggableMonitorSettings): Promise<PluggableMonitorSettings> {
     throw new Error('Method not implemented.');
 
     // 1. parse the settings parameter and remove any setting that is not defined in `monitorSettings`

arduino-ide-extension/src/node/monitor-settings/monitor-settings-provider.ts

@@ -1,14 +1,19 @@
-import { MonitorSetting } from '../../common/protocol';
+import { MonitorModel } from '../../browser/monitor-model';
+import { PluggableMonitorSetting } from '../../common/protocol';
 import { CoreClientProvider } from '../core-client-provider';
 
-export type MonitorSettings = Record<string, MonitorSetting>;
+export type PluggableMonitorSettings = Record<string, PluggableMonitorSetting>;
+export interface MonitorSettings {
+  pluggableMonitorSettings?: PluggableMonitorSettings;
+  monitorUISettings?: Partial<MonitorModel.State>;
+}
 
 export const MonitorSettingsProvider = Symbol('MonitorSettingsProvider');
 export interface MonitorSettingsProvider {
   init(
     id: string,
     coreClientProvider: CoreClientProvider
-  ): Promise<MonitorSettings>;
-  get(): Promise<MonitorSettings>;
-  set(settings: MonitorSettings): Promise<MonitorSettings>;
+  ): Promise<PluggableMonitorSettings>;
+  get(): Promise<PluggableMonitorSettings>;
+  set(settings: PluggableMonitorSettings): Promise<PluggableMonitorSettings>;
 }