From 0427759fdb98856ec588b0d3bb92c35a7e7e2ff3 Mon Sep 17 00:00:00 2001 From: Alberto Iannaccone Date: Thu, 19 May 2022 11:06:23 +0200 Subject: [PATCH] refactor monitor settings interfaces --- .../monitor-manager-proxy-client-impl.ts | 8 +++++-- .../src/browser/monitor-model.ts | 1 + .../browser/serial/monitor/monitor-widget.tsx | 23 +++++++++++-------- .../plotter/plotter-frontend-contribution.ts | 9 ++++---- .../src/common/protocol/monitor-service.ts | 15 +++++++----- .../src/node/monitor-manager-proxy-impl.ts | 8 +++---- .../src/node/monitor-manager.ts | 10 +++++--- .../src/node/monitor-service.ts | 16 +++++++------ .../monitor-settings-provider-impl.ts | 9 ++++---- .../monitor-settings-provider.ts | 15 ++++++++---- 10 files changed, 70 insertions(+), 44 deletions(-) diff --git a/arduino-ide-extension/src/browser/monitor-manager-proxy-client-impl.ts b/arduino-ide-extension/src/browser/monitor-manager-proxy-client-impl.ts index acde1998..5a9293d4 100644 --- a/arduino-ide-extension/src/browser/monitor-manager-proxy-client-impl.ts +++ b/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 { 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, }) ); diff --git a/arduino-ide-extension/src/browser/monitor-model.ts b/arduino-ide-extension/src/browser/monitor-model.ts index ccbd073c..41baca1a 100644 --- a/arduino-ide-extension/src/browser/monitor-model.ts +++ b/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; diff --git a/arduino-ide-extension/src/browser/serial/monitor/monitor-widget.tsx b/arduino-ide-extension/src/browser/serial/monitor/monitor-widget.tsx index 9d9a0830..ce098be6 100644 --- a/arduino-ide-extension/src/browser/serial/monitor/monitor-widget.tsx +++ b/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); }; } diff --git a/arduino-ide-extension/src/browser/serial/plotter/plotter-frontend-contribution.ts b/arduino-ide-extension/src/browser/serial/plotter/plotter-frontend-contribution.ts index 1cc5e0f6..083477e0 100644 --- a/arduino-ide-extension/src/browser/serial/plotter/plotter-frontend-contribution.ts +++ b/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; } } diff --git a/arduino-ide-extension/src/common/protocol/monitor-service.ts b/arduino-ide-extension/src/common/protocol/monitor-service.ts index 4cfb76a3..18f67367 100644 --- a/arduino-ide-extension/src/common/protocol/monitor-service.ts +++ b/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; changeMonitorSettings( board: Board, port: Port, - settings: MonitorSettings + settings: PluggableMonitorSettings ): Promise; stopMonitor(board: Board, port: Port): Promise; - 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; 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) diff --git a/arduino-ide-extension/src/node/monitor-manager-proxy-impl.ts b/arduino-ide-extension/src/node/monitor-manager-proxy-impl.ts index 9e78274b..75ade745 100644 --- a/arduino-ide-extension/src/node/monitor-manager-proxy-impl.ts +++ b/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 { 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 { 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); } diff --git a/arduino-ide-extension/src/node/monitor-manager.ts b/arduino-ide-extension/src/node/monitor-manager.ts index d4a56557..265f1f18 100644 --- a/arduino-ide-extension/src/node/monitor-manager.ts +++ b/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) { diff --git a/arduino-ide-extension/src/node/monitor-service.ts b/arduino-ide-extension/src/node/monitor-service.ts index 405fceaa..1696756f 100644 --- a/arduino-ide-extension/src/node/monitor-service.ts +++ b/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 { + ): Promise { 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 { + async changeSettings(settings: PluggableMonitorSettings): Promise { 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; } diff --git a/arduino-ide-extension/src/node/monitor-settings/monitor-settings-provider-impl.ts b/arduino-ide-extension/src/node/monitor-settings/monitor-settings-provider-impl.ts index 49d150ed..40dca6d7 100644 --- a/arduino-ide-extension/src/node/monitor-settings/monitor-settings-provider-impl.ts +++ b/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 { + ): Promise { 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 { + get(): Promise { throw new Error('Method not implemented.'); } - set(settings: MonitorSettings): Promise { + set(settings: PluggableMonitorSettings): Promise { throw new Error('Method not implemented.'); // 1. parse the settings parameter and remove any setting that is not defined in `monitorSettings` diff --git a/arduino-ide-extension/src/node/monitor-settings/monitor-settings-provider.ts b/arduino-ide-extension/src/node/monitor-settings/monitor-settings-provider.ts index 959ff1ad..5ca5ad4a 100644 --- a/arduino-ide-extension/src/node/monitor-settings/monitor-settings-provider.ts +++ b/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; +export type PluggableMonitorSettings = Record; +export interface MonitorSettings { + pluggableMonitorSettings?: PluggableMonitorSettings; + monitorUISettings?: Partial; +} export const MonitorSettingsProvider = Symbol('MonitorSettingsProvider'); export interface MonitorSettingsProvider { init( id: string, coreClientProvider: CoreClientProvider - ): Promise; - get(): Promise; - set(settings: MonitorSettings): Promise; + ): Promise; + get(): Promise; + set(settings: PluggableMonitorSettings): Promise; }