From c43d360b46bf6c78009d707402a436ff7c266603 Mon Sep 17 00:00:00 2001 From: Akos Kitta Date: Fri, 17 Mar 2023 15:41:34 +0100 Subject: [PATCH] fix: propagate monitor errors to the frontend - Handle when the board's platform is not installed (Closes #1974) - UX: Smoother monitor widget reset (Closes #1985) - Fixed monitor readOnly state (Closes #1984) - Set monitor widget header color (Ref #682) Closes #1508 Signed-off-by: Akos Kitta --- .../browser/arduino-ide-frontend-module.ts | 10 +- .../monitor-manager-proxy-client-impl.ts | 92 +++-- .../src/browser/monitor-model.ts | 84 ++-- .../monitor/monitor-view-contribution.tsx | 8 +- .../browser/serial/monitor/monitor-widget.tsx | 94 +++-- .../monitor/serial-monitor-send-input.tsx | 86 +++- .../src/browser/style/index.css | 32 +- .../src/browser/style/monitor.css | 33 +- .../src/common/protocol/core-service.ts | 10 +- .../src/common/protocol/monitor-service.ts | 186 ++++++++- .../src/node/core-service-impl.ts | 4 +- .../src/node/monitor-manager-proxy-impl.ts | 14 +- .../src/node/monitor-manager.ts | 33 +- .../src/node/monitor-service.ts | 372 ++++++++++-------- .../monitor-settings-provider.ts | 5 +- i18n/en.json | 8 + 16 files changed, 718 insertions(+), 353 deletions(-) diff --git a/arduino-ide-extension/src/browser/arduino-ide-frontend-module.ts b/arduino-ide-extension/src/browser/arduino-ide-frontend-module.ts index d2161ebf2..09475690b 100644 --- a/arduino-ide-extension/src/browser/arduino-ide-frontend-module.ts +++ b/arduino-ide-extension/src/browser/arduino-ide-frontend-module.ts @@ -496,15 +496,7 @@ export default new ContainerModule((bind, unbind, isBound, rebind) => { bind(TabBarToolbarContribution).toService(MonitorViewContribution); bind(WidgetFactory).toDynamicValue((context) => ({ id: MonitorWidget.ID, - createWidget: () => { - return new MonitorWidget( - context.container.get(MonitorModel), - context.container.get( - MonitorManagerProxyClient - ), - context.container.get(BoardsServiceProvider) - ); - }, + createWidget: () => context.container.get(MonitorWidget), })); bind(MonitorManagerProxyFactory).toFactory( 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 94ab4d0f5..ebbafa385 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 @@ -1,11 +1,14 @@ import { - CommandRegistry, + ApplicationError, Disposable, Emitter, MessageService, nls, } from '@theia/core'; +import { Deferred } from '@theia/core/lib/common/promise-util'; import { inject, injectable } from '@theia/core/shared/inversify'; +import { NotificationManager } from '@theia/messages/lib/browser/notifications-manager'; +import { MessageType } from '@theia/core/lib/common/message-service-protocol'; import { Board, Port } from '../common/protocol'; import { Monitor, @@ -23,21 +26,31 @@ import { BoardsServiceProvider } from './boards/boards-service-provider'; export class MonitorManagerProxyClientImpl implements MonitorManagerProxyClient { + @inject(MessageService) + private readonly messageService: MessageService; + // This is necessary to call the backend methods from the frontend + @inject(MonitorManagerProxyFactory) + private readonly server: MonitorManagerProxyFactory; + @inject(BoardsServiceProvider) + private readonly boardsServiceProvider: BoardsServiceProvider; + @inject(NotificationManager) + private readonly notificationManager: NotificationManager; + // When pluggable monitor messages are received from the backend // this event is triggered. // Ideally a frontend component is connected to this event // to update the UI. - protected readonly onMessagesReceivedEmitter = new Emitter<{ + private readonly onMessagesReceivedEmitter = new Emitter<{ messages: string[]; }>(); readonly onMessagesReceived = this.onMessagesReceivedEmitter.event; - protected readonly onMonitorSettingsDidChangeEmitter = + private readonly onMonitorSettingsDidChangeEmitter = new Emitter(); readonly onMonitorSettingsDidChange = this.onMonitorSettingsDidChangeEmitter.event; - protected readonly onMonitorShouldResetEmitter = new Emitter(); + private readonly onMonitorShouldResetEmitter = new Emitter(); readonly onMonitorShouldReset = this.onMonitorShouldResetEmitter.event; // WebSocket used to handle pluggable monitor communication between @@ -51,29 +64,16 @@ export class MonitorManagerProxyClientImpl return this.wsPort; } - constructor( - @inject(MessageService) - protected messageService: MessageService, - - // This is necessary to call the backend methods from the frontend - @inject(MonitorManagerProxyFactory) - protected server: MonitorManagerProxyFactory, - - @inject(CommandRegistry) - protected readonly commandRegistry: CommandRegistry, - - @inject(BoardsServiceProvider) - protected readonly boardsServiceProvider: BoardsServiceProvider - ) {} - /** * Connects a localhost WebSocket using the specified port. * @param addressPort port of the WebSocket */ async connect(addressPort: number): Promise { - if (!!this.webSocket) { - if (this.wsPort === addressPort) return; - else this.disconnect(); + if (this.webSocket) { + if (this.wsPort === addressPort) { + return; + } + this.disconnect(); } try { this.webSocket = new WebSocket(`ws://localhost:${addressPort}`); @@ -87,6 +87,9 @@ export class MonitorManagerProxyClientImpl return; } + const opened = new Deferred(); + this.webSocket.onopen = () => opened.resolve(); + this.webSocket.onerror = () => opened.reject(); this.webSocket.onmessage = (message) => { const parsedMessage = JSON.parse(message.data); if (Array.isArray(parsedMessage)) @@ -99,19 +102,26 @@ export class MonitorManagerProxyClientImpl } }; this.wsPort = addressPort; + return opened.promise; } /** * Disconnects the WebSocket if connected. */ disconnect(): void { - if (!this.webSocket) return; + if (!this.webSocket) { + return; + } this.onBoardsConfigChanged?.dispose(); this.onBoardsConfigChanged = undefined; try { - this.webSocket?.close(); + this.webSocket.close(); this.webSocket = undefined; - } catch { + } catch (err) { + console.error( + 'Could not close the websocket connection for the monitor.', + err + ); this.messageService.error( nls.localize( 'arduino/monitor/unableToCloseWebSocket', @@ -126,6 +136,7 @@ export class MonitorManagerProxyClientImpl } async startMonitor(settings?: PluggableMonitorSettings): Promise { + await this.boardsServiceProvider.reconciled; this.lastConnectedBoard = { selectedBoard: this.boardsServiceProvider.boardsConfig.selectedBoard, selectedPort: this.boardsServiceProvider.boardsConfig.selectedPort, @@ -150,11 +161,11 @@ export class MonitorManagerProxyClientImpl ? Port.keyOf(this.lastConnectedBoard.selectedPort) : undefined) ) { - this.onMonitorShouldResetEmitter.fire(null); this.lastConnectedBoard = { selectedBoard: selectedBoard, selectedPort: selectedPort, }; + this.onMonitorShouldResetEmitter.fire(); } else { // a board is plugged and it's the same as prev, rerun "this.startMonitor" to // recreate the listener callback @@ -167,7 +178,14 @@ export class MonitorManagerProxyClientImpl const { selectedBoard, selectedPort } = this.boardsServiceProvider.boardsConfig; if (!selectedBoard || !selectedBoard.fqbn || !selectedPort) return; - await this.server().startMonitor(selectedBoard, selectedPort, settings); + try { + this.clearVisibleNotification(); + await this.server().startMonitor(selectedBoard, selectedPort, settings); + } catch (err) { + const message = ApplicationError.is(err) ? err.message : String(err); + this.previousNotificationId = this.notificationId(message); + this.messageService.error(message); + } } getCurrentSettings(board: Board, port: Port): Promise { @@ -199,4 +217,24 @@ export class MonitorManagerProxyClientImpl }) ); } + + /** + * This is the internal (Theia) ID of the notification that is currently visible. + * It's stored here as a field to be able to close it before starting a new monitor connection. It's a hack. + */ + private previousNotificationId: string | undefined; + private clearVisibleNotification(): void { + if (this.previousNotificationId) { + this.notificationManager.clear(this.previousNotificationId); + this.previousNotificationId = undefined; + } + } + + private notificationId(message: string, ...actions: string[]): string { + return this.notificationManager['getMessageId']({ + text: message, + actions, + type: MessageType.Error, + }); + } } diff --git a/arduino-ide-extension/src/browser/monitor-model.ts b/arduino-ide-extension/src/browser/monitor-model.ts index 3ebea1819..c3c8a26f4 100644 --- a/arduino-ide-extension/src/browser/monitor-model.ts +++ b/arduino-ide-extension/src/browser/monitor-model.ts @@ -4,7 +4,14 @@ import { LocalStorageService, } from '@theia/core/lib/browser'; import { inject, injectable } from '@theia/core/shared/inversify'; -import { MonitorManagerProxyClient } from '../common/protocol'; +import { + isMonitorConnected, + MonitorConnectionStatus, + monitorConnectionStatusEquals, + MonitorEOL, + MonitorManagerProxyClient, + MonitorState, +} from '../common/protocol'; import { isNullOrUndefined } from '../common/utils'; import { MonitorSettings } from '../node/monitor-settings/monitor-settings-provider'; @@ -19,36 +26,36 @@ export class MonitorModel implements FrontendApplicationContribution { protected readonly monitorManagerProxy: MonitorManagerProxyClient; protected readonly onChangeEmitter: Emitter< - MonitorModel.State.Change + MonitorState.Change >; protected _autoscroll: boolean; protected _timestamp: boolean; - protected _lineEnding: MonitorModel.EOL; + protected _lineEnding: MonitorEOL; protected _interpolate: boolean; protected _darkTheme: boolean; protected _wsPort: number; protected _serialPort: string; - protected _connected: boolean; + protected _connectionStatus: MonitorConnectionStatus; constructor() { this._autoscroll = true; this._timestamp = false; this._interpolate = false; - this._lineEnding = MonitorModel.EOL.DEFAULT; + this._lineEnding = MonitorEOL.DEFAULT; this._darkTheme = false; this._wsPort = 0; this._serialPort = ''; - this._connected = true; + this._connectionStatus = 'not-connected'; this.onChangeEmitter = new Emitter< - MonitorModel.State.Change + MonitorState.Change >(); } onStart(): void { this.localStorageService - .getData(MonitorModel.STORAGE_ID) + .getData(MonitorModel.STORAGE_ID) .then(this.restoreState.bind(this)); this.monitorManagerProxy.onMonitorSettingsDidChange( @@ -56,11 +63,11 @@ export class MonitorModel implements FrontendApplicationContribution { ); } - get onChange(): Event> { + get onChange(): Event> { return this.onChangeEmitter.event; } - protected restoreState(state: MonitorModel.State): void { + protected restoreState(state: MonitorState): void { if (!state) { return; } @@ -125,11 +132,11 @@ export class MonitorModel implements FrontendApplicationContribution { this.timestamp = !this._timestamp; } - get lineEnding(): MonitorModel.EOL { + get lineEnding(): MonitorEOL { return this._lineEnding; } - set lineEnding(lineEnding: MonitorModel.EOL) { + set lineEnding(lineEnding: MonitorEOL) { if (lineEnding === this._lineEnding) return; this._lineEnding = lineEnding; this.monitorManagerProxy.changeSettings({ @@ -211,19 +218,26 @@ export class MonitorModel implements FrontendApplicationContribution { ); } - get connected(): boolean { - return this._connected; + get connectionStatus(): MonitorConnectionStatus { + return this._connectionStatus; } - set connected(connected: boolean) { - if (connected === this._connected) return; - this._connected = connected; + set connectionStatus(connectionStatus: MonitorConnectionStatus) { + if ( + monitorConnectionStatusEquals(connectionStatus, this.connectionStatus) + ) { + return; + } + this._connectionStatus = connectionStatus; this.monitorManagerProxy.changeSettings({ - monitorUISettings: { connected }, + monitorUISettings: { + connectionStatus, + connected: isMonitorConnected(connectionStatus), + }, }); this.onChangeEmitter.fire({ - property: 'connected', - value: this._connected, + property: 'connectionStatus', + value: this._connectionStatus, }); } @@ -238,7 +252,7 @@ export class MonitorModel implements FrontendApplicationContribution { darkTheme, wsPort, serialPort, - connected, + connectionStatus, } = monitorUISettings; if (!isNullOrUndefined(autoscroll)) this.autoscroll = autoscroll; @@ -248,31 +262,7 @@ export class MonitorModel implements FrontendApplicationContribution { if (!isNullOrUndefined(darkTheme)) this.darkTheme = darkTheme; if (!isNullOrUndefined(wsPort)) this.wsPort = wsPort; if (!isNullOrUndefined(serialPort)) this.serialPort = serialPort; - if (!isNullOrUndefined(connected)) this.connected = connected; + if (!isNullOrUndefined(connectionStatus)) + this.connectionStatus = connectionStatus; }; } - -// TODO: Move this to /common -export namespace MonitorModel { - export interface State { - autoscroll: boolean; - timestamp: boolean; - lineEnding: EOL; - interpolate: boolean; - darkTheme: boolean; - wsPort: number; - serialPort: string; - connected: boolean; - } - export namespace State { - export interface Change { - readonly property: K; - readonly value: State[K]; - } - } - - export type EOL = '' | '\n' | '\r' | '\r\n'; - export namespace EOL { - export const DEFAULT: EOL = '\n'; - } -} diff --git a/arduino-ide-extension/src/browser/serial/monitor/monitor-view-contribution.tsx b/arduino-ide-extension/src/browser/serial/monitor/monitor-view-contribution.tsx index 36f13c3b2..925d221ec 100644 --- a/arduino-ide-extension/src/browser/serial/monitor/monitor-view-contribution.tsx +++ b/arduino-ide-extension/src/browser/serial/monitor/monitor-view-contribution.tsx @@ -10,6 +10,7 @@ import { import { ArduinoToolbar } from '../../toolbar/arduino-toolbar'; import { ArduinoMenus } from '../../menu/arduino-menus'; import { nls } from '@theia/core/lib/common'; +import { Event } from '@theia/core/lib/common/event'; import { MonitorModel } from '../../monitor-model'; import { MonitorManagerProxyClient } from '../../../common/protocol'; @@ -84,13 +85,13 @@ export class MonitorViewContribution id: 'monitor-autoscroll', render: () => this.renderAutoScrollButton(), isVisible: (widget) => widget instanceof MonitorWidget, - onDidChange: this.model.onChange as any, // XXX: it's a hack. See: https://github.com/eclipse-theia/theia/pull/6696/ + onDidChange: this.model.onChange as Event as Event, }); registry.registerItem({ id: 'monitor-timestamp', render: () => this.renderTimestampButton(), isVisible: (widget) => widget instanceof MonitorWidget, - onDidChange: this.model.onChange as any, // XXX: it's a hack. See: https://github.com/eclipse-theia/theia/pull/6696/ + onDidChange: this.model.onChange as Event as Event, }); registry.registerItem({ id: SerialMonitor.Commands.CLEAR_OUTPUT.id, @@ -143,8 +144,7 @@ export class MonitorViewContribution protected async reset(): Promise { const widget = this.tryGetWidget(); if (widget) { - widget.dispose(); - await this.openView({ activate: true, reveal: true }); + widget.reset(); } } 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 ec83f5a0a..c2311a11c 100644 --- a/arduino-ide-extension/src/browser/serial/monitor/monitor-widget.tsx +++ b/arduino-ide-extension/src/browser/serial/monitor/monitor-widget.tsx @@ -1,7 +1,14 @@ import * as React from '@theia/core/shared/react'; -import { injectable, inject } from '@theia/core/shared/inversify'; +import { + injectable, + inject, + postConstruct, +} from '@theia/core/shared/inversify'; import { Emitter } from '@theia/core/lib/common/event'; -import { Disposable } from '@theia/core/lib/common/disposable'; +import { + Disposable, + DisposableCollection, +} from '@theia/core/lib/common/disposable'; import { ReactWidget, Message, @@ -13,9 +20,13 @@ import { SerialMonitorSendInput } from './serial-monitor-send-input'; import { SerialMonitorOutput } from './serial-monitor-send-output'; import { BoardsServiceProvider } from '../../boards/boards-service-provider'; import { nls } from '@theia/core/lib/common'; -import { MonitorManagerProxyClient } from '../../../common/protocol'; +import { + MonitorEOL, + MonitorManagerProxyClient, +} from '../../../common/protocol'; import { MonitorModel } from '../../monitor-model'; import { MonitorSettings } from '../../../node/monitor-settings/monitor-settings-provider'; +import { FrontendApplicationStateService } from '@theia/core/lib/browser/frontend-application-state'; @injectable() export class MonitorWidget extends ReactWidget { @@ -40,40 +51,46 @@ export class MonitorWidget extends ReactWidget { protected closing = false; protected readonly clearOutputEmitter = new Emitter(); - constructor( - @inject(MonitorModel) - protected readonly monitorModel: MonitorModel, + @inject(MonitorModel) + private readonly monitorModel: MonitorModel; + @inject(MonitorManagerProxyClient) + private readonly monitorManagerProxy: MonitorManagerProxyClient; + @inject(BoardsServiceProvider) + private readonly boardsServiceProvider: BoardsServiceProvider; + @inject(FrontendApplicationStateService) + private readonly appStateService: FrontendApplicationStateService; - @inject(MonitorManagerProxyClient) - protected readonly monitorManagerProxy: MonitorManagerProxyClient, + private readonly toDisposeOnReset: DisposableCollection; - @inject(BoardsServiceProvider) - protected readonly boardsServiceProvider: BoardsServiceProvider - ) { + constructor() { super(); this.id = MonitorWidget.ID; this.title.label = MonitorWidget.LABEL; this.title.iconClass = 'monitor-tab-icon'; this.title.closable = true; this.scrollOptions = undefined; + this.toDisposeOnReset = new DisposableCollection(); this.toDispose.push(this.clearOutputEmitter); - this.toDispose.push( - Disposable.create(() => this.monitorManagerProxy.disconnect()) - ); } - protected override onBeforeAttach(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) - ); + @postConstruct() + protected init(): void { + this.toDisposeOnReset.dispose(); + this.toDisposeOnReset.pushAll([ + Disposable.create(() => this.monitorManagerProxy.disconnect()), + this.monitorModel.onChange(() => this.update()), + this.monitorManagerProxy.onMonitorSettingsDidChange((event) => + this.updateSettings(event) + ), + ]); + this.startMonitor(); + } - this.monitorManagerProxy.startMonitor(); + reset(): void { + this.init(); } - onMonitorSettingsDidChange(settings: MonitorSettings): void { + private updateSettings(settings: MonitorSettings): void { this.settings = { ...this.settings, pluggableMonitorSettings: { @@ -90,6 +107,7 @@ export class MonitorWidget extends ReactWidget { } override dispose(): void { + this.toDisposeOnReset.dispose(); super.dispose(); } @@ -122,7 +140,7 @@ export class MonitorWidget extends ReactWidget { this.update(); } - protected onFocusResolved = (element: HTMLElement | undefined) => { + protected onFocusResolved = (element: HTMLElement | undefined): void => { if (this.closing || !this.isAttached) { return; } @@ -132,7 +150,7 @@ export class MonitorWidget extends ReactWidget { ); }; - protected get lineEndings(): SerialMonitorOutput.SelectOption[] { + protected get lineEndings(): SerialMonitorOutput.SelectOption[] { return [ { label: nls.localize('arduino/serial/noLineEndings', 'No Line Ending'), @@ -156,11 +174,23 @@ export class MonitorWidget extends ReactWidget { ]; } - private getCurrentSettings(): Promise { + private async startMonitor(): Promise { + await this.appStateService.reachedState('ready'); + await this.boardsServiceProvider.reconciled; + await this.syncSettings(); + await this.monitorManagerProxy.startMonitor(); + } + + private async syncSettings(): Promise { + const settings = await this.getCurrentSettings(); + this.updateSettings(settings); + } + + private async getCurrentSettings(): Promise { const board = this.boardsServiceProvider.boardsConfig.selectedBoard; const port = this.boardsServiceProvider.boardsConfig.selectedPort; if (!board || !port) { - return Promise.resolve(this.settings || {}); + return this.settings || {}; } return this.monitorManagerProxy.getCurrentSettings(board, port); } @@ -171,7 +201,7 @@ export class MonitorWidget extends ReactWidget { : undefined; const baudrateOptions = baudrate?.values.map((b) => ({ - label: b + ' baud', + label: nls.localize('arduino/monitor/baudRate', '{0} baud', b), value: b, })); const baudrateSelectedOption = baudrateOptions?.find( @@ -181,7 +211,7 @@ export class MonitorWidget extends ReactWidget { const lineEnding = this.lineEndings.find( (item) => item.value === this.monitorModel.lineEnding - ) || this.lineEndings[1]; // Defaults to `\n`. + ) || MonitorEOL.DEFAULT; return (
@@ -228,13 +258,13 @@ export class MonitorWidget extends ReactWidget { ); } - protected readonly onSend = (value: string) => this.doSend(value); - protected async doSend(value: string): Promise { + protected readonly onSend = (value: string): void => this.doSend(value); + protected doSend(value: string): void { this.monitorManagerProxy.send(value); } protected readonly onChangeLineEnding = ( - option: SerialMonitorOutput.SelectOption + option: SerialMonitorOutput.SelectOption ): void => { this.monitorModel.lineEnding = option.value; }; diff --git a/arduino-ide-extension/src/browser/serial/monitor/serial-monitor-send-input.tsx b/arduino-ide-extension/src/browser/serial/monitor/serial-monitor-send-input.tsx index 163fdbafd..21e54e2fd 100644 --- a/arduino-ide-extension/src/browser/serial/monitor/serial-monitor-send-input.tsx +++ b/arduino-ide-extension/src/browser/serial/monitor/serial-monitor-send-input.tsx @@ -5,6 +5,10 @@ import { DisposableCollection, nls } from '@theia/core/lib/common'; import { BoardsServiceProvider } from '../../boards/boards-service-provider'; import { MonitorModel } from '../../monitor-model'; import { Unknown } from '../../../common/nls'; +import { + isMonitorConnectionError, + MonitorConnectionStatus, +} from '../../../common/protocol'; class HistoryList { private readonly items: string[] = []; @@ -62,7 +66,7 @@ export namespace SerialMonitorSendInput { } export interface State { text: string; - connected: boolean; + connectionStatus: MonitorConnectionStatus; history: HistoryList; } } @@ -75,18 +79,27 @@ export class SerialMonitorSendInput extends React.Component< constructor(props: Readonly) { super(props); - this.state = { text: '', connected: true, history: new HistoryList() }; + this.state = { + text: '', + connectionStatus: 'not-connected', + history: new HistoryList(), + }; this.onChange = this.onChange.bind(this); this.onSend = this.onSend.bind(this); this.onKeyDown = this.onKeyDown.bind(this); } override componentDidMount(): void { - this.setState({ connected: this.props.monitorModel.connected }); + this.setState({ + connectionStatus: this.props.monitorModel.connectionStatus, + }); this.toDisposeBeforeUnmount.push( this.props.monitorModel.onChange(({ property }) => { - if (property === 'connected') - this.setState({ connected: this.props.monitorModel.connected }); + if (property === 'connected' || property === 'connectionStatus') { + this.setState({ + connectionStatus: this.props.monitorModel.connectionStatus, + }); + } }) ); } @@ -97,44 +110,83 @@ export class SerialMonitorSendInput extends React.Component< } override render(): React.ReactNode { + const status = this.state.connectionStatus; + const input = this.renderInput(status); + if (status !== 'connecting') { + return input; + } + return ; + } + + private renderInput(status: MonitorConnectionStatus): React.ReactNode { + const inputClassName = this.inputClassName(status); + const placeholder = this.placeholder; + const readOnly = Boolean(inputClassName); return ( onChange={this.onChange} onKeyDown={this.onKeyDown} /> ); } + private inputClassName( + status: MonitorConnectionStatus + ): 'error' | 'warning' | '' { + if (isMonitorConnectionError(status)) { + return 'error'; + } + if (status === 'connected') { + return ''; + } + return 'warning'; + } + protected shouldShowWarning(): boolean { const board = this.props.boardsServiceProvider.boardsConfig.selectedBoard; const port = this.props.boardsServiceProvider.boardsConfig.selectedPort; - return !this.state.connected || !board || !port; + return !this.state.connectionStatus || !board || !port; } protected get placeholder(): string { - if (this.shouldShowWarning()) { + const status = this.state.connectionStatus; + if (isMonitorConnectionError(status)) { + return status.errorMessage; + } + if (status === 'not-connected') { return nls.localize( 'arduino/serial/notConnected', 'Not connected. Select a board and a port to connect automatically.' ); } - const board = this.props.boardsServiceProvider.boardsConfig.selectedBoard; const port = this.props.boardsServiceProvider.boardsConfig.selectedPort; + const boardLabel = board + ? Board.toString(board, { + useFqbn: false, + }) + : Unknown; + const portLabel = port ? port.address : Unknown; + if (status === 'connecting') { + return nls.localize( + 'arduino/serial/connecting', + "Connecting to '{0}' on '{1}'...", + boardLabel, + portLabel + ); + } return nls.localize( 'arduino/serial/message', "Message (Enter to send message to '{0}' on '{1}')", - board - ? Board.toString(board, { - useFqbn: false, - }) - : Unknown, - port ? port.address : Unknown + boardLabel, + portLabel ); } diff --git a/arduino-ide-extension/src/browser/style/index.css b/arduino-ide-extension/src/browser/style/index.css index d0ac1e45e..4698d4a33 100644 --- a/arduino-ide-extension/src/browser/style/index.css +++ b/arduino-ide-extension/src/browser/style/index.css @@ -29,9 +29,11 @@ /* https://github.com/arduino/arduino-ide/pull/1662#issuecomment-1324997134 */ body { --theia-icon-loading: url(../icons/loading-light.svg); + --theia-icon-loading-warning: url(../icons/loading-dark.svg); } body.theia-dark { --theia-icon-loading: url(../icons/loading-dark.svg); + --theia-icon-loading-warning: url(../icons/loading-light.svg); } .theia-input.warning:focus { @@ -48,22 +50,32 @@ body.theia-dark { } .theia-input.warning::placeholder { - /* Chrome, Firefox, Opera, Safari 10.1+ */ color: var(--theia-warningForeground); background-color: var(--theia-warningBackground); - opacity: 1; /* Firefox */ } -.theia-input.warning:-ms-input-placeholder { - /* Internet Explorer 10-11 */ - color: var(--theia-warningForeground); - background-color: var(--theia-warningBackground); +.hc-black.hc-theia.theia-hc .theia-input.warning, +.hc-black.hc-theia.theia-hc .theia-input.warning::placeholder { + color: var(--theia-warningBackground); + background-color: var(--theia-warningForeground); } -.theia-input.warning::-ms-input-placeholder { - /* Microsoft Edge */ - color: var(--theia-warningForeground); - background-color: var(--theia-warningBackground); +.theia-input.error:focus { + outline-width: 1px; + outline-style: solid; + outline-offset: -1px; + opacity: 1 !important; + color: var(--theia-errorForeground); + background-color: var(--theia-errorBackground); +} + +.theia-input.error { + background-color: var(--theia-errorBackground); +} + +.theia-input.error::placeholder { + color: var(--theia-errorForeground); + background-color: var(--theia-errorBackground); } /* Makes the sidepanel a bit wider when opening the widget */ diff --git a/arduino-ide-extension/src/browser/style/monitor.css b/arduino-ide-extension/src/browser/style/monitor.css index fdcdfc21c..6787ea4cb 100644 --- a/arduino-ide-extension/src/browser/style/monitor.css +++ b/arduino-ide-extension/src/browser/style/monitor.css @@ -20,22 +20,47 @@ .serial-monitor .head { display: flex; - padding: 5px; + padding: 0px 5px 5px 0px; height: 27px; + background-color: var(--theia-activityBar-background); } .serial-monitor .head .send { display: flex; flex: 1; - margin-right: 2px; } -.serial-monitor .head .send > input { +.serial-monitor .head .send > label:before { + content: ""; + position: absolute; + top: -1px; + background: var(--theia-icon-loading-warning) center center no-repeat; + animation: theia-spin 1.25s linear infinite; + width: 30px; + height: 30px; +} + +.serial-monitor .head .send > label { + position: relative; + width: 100%; + display: flex; + align-self: baseline; +} + +.serial-monitor .head .send > input, +.serial-monitor .head .send > label > input { line-height: var(--theia-content-line-height); + height: 27px; width: 100%; } -.serial-monitor .head .send > input:focus { +.serial-monitor .head .send > label > input { + padding-left: 30px; + box-sizing: border-box; +} + +.serial-monitor .head .send > input:focus, +.serial-monitor .head .send > label > input:focus { border-color: var(--theia-focusBorder); } diff --git a/arduino-ide-extension/src/common/protocol/core-service.ts b/arduino-ide-extension/src/common/protocol/core-service.ts index 809f3a7c6..7dfcfb896 100644 --- a/arduino-ide-extension/src/common/protocol/core-service.ts +++ b/arduino-ide-extension/src/common/protocol/core-service.ts @@ -73,12 +73,12 @@ export namespace CoreError { UploadUsingProgrammer: 4003, BurnBootloader: 4004, }; - export const VerifyFailed = create(Codes.Verify); - export const UploadFailed = create(Codes.Upload); - export const UploadUsingProgrammerFailed = create( + export const VerifyFailed = declareCoreError(Codes.Verify); + export const UploadFailed = declareCoreError(Codes.Upload); + export const UploadUsingProgrammerFailed = declareCoreError( Codes.UploadUsingProgrammer ); - export const BurnBootloaderFailed = create(Codes.BurnBootloader); + export const BurnBootloaderFailed = declareCoreError(Codes.BurnBootloader); export function is( error: unknown ): error is ApplicationError { @@ -88,7 +88,7 @@ export namespace CoreError { Object.values(Codes).includes(error.code) ); } - function create( + function declareCoreError( code: number ): ApplicationError.Constructor { return ApplicationError.declare( diff --git a/arduino-ide-extension/src/common/protocol/monitor-service.ts b/arduino-ide-extension/src/common/protocol/monitor-service.ts index 7374951db..6c9bbcac9 100644 --- a/arduino-ide-extension/src/common/protocol/monitor-service.ts +++ b/arduino-ide-extension/src/common/protocol/monitor-service.ts @@ -1,4 +1,4 @@ -import { Event, JsonRpcServer } from '@theia/core'; +import { ApplicationError, Event, JsonRpcServer, nls } from '@theia/core'; import { PluggableMonitorSettings, MonitorSettings, @@ -31,7 +31,7 @@ export interface MonitorManagerProxyClient { onMessagesReceived: Event<{ messages: string[] }>; onMonitorSettingsDidChange: Event; onMonitorShouldReset: Event; - connect(addressPort: number): void; + connect(addressPort: number): Promise; disconnect(): void; getWebSocketPort(): number | undefined; isWSConnected(): Promise; @@ -46,7 +46,7 @@ export interface PluggableMonitorSetting { readonly id: string; // A human-readable label of the setting (to be displayed on the GUI) readonly label: string; - // The setting type (at the moment only "enum" is avaiable) + // The setting type (at the moment only "enum" is available) readonly type: string; // The values allowed on "enum" types readonly values: string[]; @@ -72,24 +72,168 @@ export namespace Monitor { }; } -export interface Status {} -export type OK = Status; -export interface ErrorStatus extends Status { - readonly message: string; +export const MonitorErrorCodes = { + ConnectionFailed: 6001, + NotConnected: 6002, + AlreadyConnected: 6003, + MissingConfiguration: 6004, +} as const; + +export const ConnectionFailedError = declareMonitorError( + MonitorErrorCodes.ConnectionFailed +); +export const NotConnectedError = declareMonitorError( + MonitorErrorCodes.NotConnected +); +export const AlreadyConnectedError = declareMonitorError( + MonitorErrorCodes.AlreadyConnected +); +export const MissingConfigurationError = declareMonitorError( + MonitorErrorCodes.MissingConfiguration +); + +export function createConnectionFailedError( + port: Port, + details?: string +): ApplicationError { + const { protocol, address } = port; + let message; + if (details) { + const detailsWithPeriod = details.endsWith('.') ? details : `${details}.`; + message = nls.localize( + 'arduino/monitor/connectionFailedErrorWithDetails', + '{0} Could not connect to {1} {2} port.', + detailsWithPeriod, + address, + protocol + ); + } else { + message = nls.localize( + 'arduino/monitor/connectionFailedError', + 'Could not connect to {0} {1} port.', + address, + protocol + ); + } + return ConnectionFailedError(message, { protocol, address }); +} +export function createNotConnectedError( + port: Port +): ApplicationError { + const { protocol, address } = port; + return NotConnectedError( + nls.localize( + 'arduino/monitor/notConnectedError', + 'Not connected to {0} {1} port.', + address, + protocol + ), + { protocol, address } + ); +} +export function createAlreadyConnectedError( + port: Port +): ApplicationError { + const { protocol, address } = port; + return AlreadyConnectedError( + nls.localize( + 'arduino/monitor/alreadyConnectedError', + 'Could not connect to {0} {1} port. Already connected.', + address, + protocol + ), + { protocol, address } + ); +} +export function createMissingConfigurationError( + port: Port +): ApplicationError { + const { protocol, address } = port; + return MissingConfigurationError( + nls.localize( + 'arduino/monitor/missingConfigurationError', + 'Could not connect to {0} {1} port. The monitor configuration is missing.', + address, + protocol + ), + { protocol, address } + ); +} + +/** + * Bare minimum representation of a port. Supports neither UI labels nor properties. + */ +interface PortDescriptor { + readonly protocol: string; + readonly address: string; +} +function declareMonitorError( + code: number +): ApplicationError.Constructor { + return ApplicationError.declare( + code, + (message: string, data: PortDescriptor) => ({ data, message }) + ); +} + +export interface MonitorConnectionError { + readonly errorMessage: string; +} + +export type MonitorConnectionStatus = + | 'connecting' + | 'connected' + | 'not-connected' + | MonitorConnectionError; + +export function monitorConnectionStatusEquals( + left: MonitorConnectionStatus, + right: MonitorConnectionStatus +): boolean { + if (typeof left === 'object' && typeof right === 'object') { + return left.errorMessage === right.errorMessage; + } + return left === right; +} + +/** + * @deprecated see `MonitorState#connected` + */ +export function isMonitorConnected( + status: MonitorConnectionStatus +): status is 'connected' { + return status === 'connected'; } -export namespace Status { - export function isOK(status: Status & { message?: string }): status is OK { - return !!status && typeof status.message !== 'string'; + +export function isMonitorConnectionError( + status: MonitorConnectionStatus +): status is MonitorConnectionError { + return typeof status === 'object'; +} + +export interface MonitorState { + autoscroll: boolean; + timestamp: boolean; + lineEnding: MonitorEOL; + interpolate: boolean; + darkTheme: boolean; + wsPort: number; + serialPort: string; + connectionStatus: MonitorConnectionStatus; + /** + * @deprecated This property is never get by IDE2 only set. This value is present to be backward compatible with the plotter app. + * IDE2 uses `MonitorState#connectionStatus`. + */ + connected: boolean; +} +export namespace MonitorState { + export interface Change { + readonly property: K; + readonly value: MonitorState[K]; } - export const OK: OK = {}; - export const NOT_CONNECTED: ErrorStatus = { message: 'Not connected.' }; - export const ALREADY_CONNECTED: ErrorStatus = { - message: 'Already connected.', - }; - export const CONFIG_MISSING: ErrorStatus = { - message: 'Serial Config missing.', - }; - export const UPLOAD_IN_PROGRESS: ErrorStatus = { - message: 'Upload in progress.', - }; +} + +export type MonitorEOL = '' | '\n' | '\r' | '\r\n'; +export namespace MonitorEOL { + export const DEFAULT: MonitorEOL = '\n'; } diff --git a/arduino-ide-extension/src/node/core-service-impl.ts b/arduino-ide-extension/src/node/core-service-impl.ts index f438ca2e6..08cfe4260 100644 --- a/arduino-ide-extension/src/node/core-service-impl.ts +++ b/arduino-ide-extension/src/node/core-service-impl.ts @@ -23,7 +23,7 @@ import { UploadUsingProgrammerResponse, } from './cli-protocol/cc/arduino/cli/commands/v1/upload_pb'; import { ResponseService } from '../common/protocol/response-service'; -import { OutputMessage, Port, Status } from '../common/protocol'; +import { OutputMessage, Port } from '../common/protocol'; import { ArduinoCoreServiceClient } from './cli-protocol/cc/arduino/cli/commands/v1/commands_grpc_pb'; import { Port as RpcPort } from './cli-protocol/cc/arduino/cli/commands/v1/port_pb'; import { ApplicationError, CommandService, Disposable, nls } from '@theia/core'; @@ -392,7 +392,7 @@ export class CoreServiceImpl extends CoreClientAware implements CoreService { }: { fqbn?: string | undefined; port?: Port | undefined; - }): Promise { + }): Promise { this.boardDiscovery.setUploadInProgress(false); return this.monitorManager.notifyUploadFinished(fqbn, port); } 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 7f284ac3f..079b85d48 100644 --- a/arduino-ide-extension/src/node/monitor-manager-proxy-impl.ts +++ b/arduino-ide-extension/src/node/monitor-manager-proxy-impl.ts @@ -2,7 +2,6 @@ import { inject, injectable } from '@theia/core/shared/inversify'; import { MonitorManagerProxy, MonitorManagerProxyClient, - Status, } from '../common/protocol'; import { Board, Port } from '../common/protocol'; import { MonitorManager } from './monitor-manager'; @@ -41,11 +40,16 @@ export class MonitorManagerProxyImpl implements MonitorManagerProxy { await this.changeMonitorSettings(board, port, settings); } - const connectToClient = (status: Status) => { - if (status === Status.ALREADY_CONNECTED || status === Status.OK) { - // Monitor started correctly, connect it with the frontend - this.client.connect(this.manager.getWebsocketAddressPort(board, port)); + const connectToClient = async () => { + const address = this.manager.getWebsocketAddressPort(board, port); + if (!this.client) { + throw new Error( + `No client was connected to this monitor manager. Board: ${ + board.fqbn ?? board.name + }, port: ${port.address}, address: ${address}` + ); } + await this.client.connect(address); }; return this.manager.startMonitor(board, port, connectToClient); } diff --git a/arduino-ide-extension/src/node/monitor-manager.ts b/arduino-ide-extension/src/node/monitor-manager.ts index 38360a39f..ea63aa8cb 100644 --- a/arduino-ide-extension/src/node/monitor-manager.ts +++ b/arduino-ide-extension/src/node/monitor-manager.ts @@ -1,6 +1,11 @@ import { ILogger } from '@theia/core'; import { inject, injectable, named } from '@theia/core/shared/inversify'; -import { Board, BoardsService, Port, Status } from '../common/protocol'; +import { + AlreadyConnectedError, + Board, + BoardsService, + Port, +} from '../common/protocol'; import { CoreClientAware } from './core-client-provider'; import { MonitorService } from './monitor-service'; import { MonitorServiceFactory } from './monitor-service-factory'; @@ -36,7 +41,7 @@ export class MonitorManager extends CoreClientAware { private monitorServiceStartQueue: { monitorID: string; serviceStartParams: [Board, Port]; - connectToClient: (status: Status) => void; + connectToClient: () => Promise; }[] = []; @inject(MonitorServiceFactory) @@ -104,7 +109,7 @@ export class MonitorManager extends CoreClientAware { async startMonitor( board: Board, port: Port, - connectToClient: (status: Status) => void + connectToClient: () => Promise ): Promise { const monitorID = this.monitorID(board.fqbn, port); @@ -127,8 +132,14 @@ export class MonitorManager extends CoreClientAware { return; } - const result = await monitor.start(); - connectToClient(result); + try { + await connectToClient(); + await monitor.start(); + } catch (err) { + if (!AlreadyConnectedError.is(err)) { + throw err; + } + } } /** @@ -202,8 +213,7 @@ export class MonitorManager extends CoreClientAware { async notifyUploadFinished( fqbn?: string | undefined, port?: Port - ): Promise { - let status: Status = Status.NOT_CONNECTED; + ): Promise { let portDidChangeOnUpload = false; // We have no way of knowing which monitor @@ -214,7 +224,7 @@ export class MonitorManager extends CoreClientAware { const monitor = this.monitorServices.get(monitorID); if (monitor) { - status = await monitor.start(); + await monitor.start(); } // this monitorID will only be present in "disposedForUpload" @@ -232,7 +242,6 @@ export class MonitorManager extends CoreClientAware { } await this.startQueuedServices(portDidChangeOnUpload); - return status; } async startQueuedServices(portDidChangeOnUpload: boolean): Promise { @@ -246,7 +255,7 @@ export class MonitorManager extends CoreClientAware { for (const { monitorID, - serviceStartParams: [_, port], + serviceStartParams: [, port], connectToClient, } of queued) { const boardsState = await this.boardsService.getState(); @@ -261,8 +270,8 @@ export class MonitorManager extends CoreClientAware { const monitorService = this.monitorServices.get(monitorID); if (monitorService) { - const result = await monitorService.start(); - connectToClient(result); + await connectToClient(); + await monitorService.start(); } } } diff --git a/arduino-ide-extension/src/node/monitor-service.ts b/arduino-ide-extension/src/node/monitor-service.ts index db5f100d7..ff7a9019f 100644 --- a/arduino-ide-extension/src/node/monitor-service.ts +++ b/arduino-ide-extension/src/node/monitor-service.ts @@ -1,8 +1,23 @@ -import { ClientDuplexStream } from '@grpc/grpc-js'; -import { Disposable, Emitter, ILogger } from '@theia/core'; +import { ClientDuplexStream, status } from '@grpc/grpc-js'; +import { + ApplicationError, + Disposable, + Emitter, + ILogger, + nls, +} from '@theia/core'; import { inject, named, postConstruct } from '@theia/core/shared/inversify'; import { diff, Operation } from 'just-diff'; -import { Board, Port, Status, Monitor } from '../common/protocol'; +import { + Board, + Port, + Monitor, + createAlreadyConnectedError, + createMissingConfigurationError, + createNotConnectedError, + createConnectionFailedError, + isMonitorConnected, +} from '../common/protocol'; import { EnumerateMonitorPortSettingsRequest, EnumerateMonitorPortSettingsResponse, @@ -19,8 +34,13 @@ import { PluggableMonitorSettings, MonitorSettingsProvider, } from './monitor-settings/monitor-settings-provider'; -import { Deferred } from '@theia/core/lib/common/promise-util'; +import { + Deferred, + retry, + timeoutReject, +} from '@theia/core/lib/common/promise-util'; import { MonitorServiceFactoryOptions } from './monitor-service-factory'; +import { ServiceError } from './service-error'; export const MonitorServiceName = 'monitor-service'; type DuplexHandlerKeys = @@ -76,7 +96,7 @@ export class MonitorService extends CoreClientAware implements Disposable { readonly onDispose = this.onDisposeEmitter.event; private _initialized = new Deferred(); - private creating: Deferred; + private creating: Deferred; private readonly board: Board; private readonly port: Port; private readonly monitorID: string; @@ -114,7 +134,7 @@ export class MonitorService extends CoreClientAware implements Disposable { this.updateClientsSettings(this.settings); }); - this.portMonitorSettings(this.port.protocol, this.board.fqbn!).then( + this.portMonitorSettings(this.port.protocol, this.board.fqbn!, true).then( async (settings) => { this.settings = { ...this.settings, @@ -154,74 +174,85 @@ export class MonitorService extends CoreClientAware implements Disposable { /** * Start and connects a monitor using currently set board and port. - * If a monitor is already started or board fqbn, port address and/or protocol - * are missing nothing happens. - * @returns a status to verify connection has been established. + * If a monitor is already started, the promise will reject with an `AlreadyConnectedError`. + * If the board fqbn, port address and/or protocol are missing, the promise rejects with a `MissingConfigurationError`. */ - async start(): Promise { + async start(): Promise { if (this.creating?.state === 'unresolved') return this.creating.promise; this.creating = new Deferred(); if (this.duplex) { this.updateClientsSettings({ - monitorUISettings: { connected: true, serialPort: this.port.address }, + monitorUISettings: { + connectionStatus: 'connected', + connected: true, // TODO: should be removed when plotter app understand the `connectionStatus` message + serialPort: this.port.address, + }, }); - this.creating.resolve(Status.ALREADY_CONNECTED); + this.creating.reject(createAlreadyConnectedError(this.port)); return this.creating.promise; } if (!this.board?.fqbn || !this.port?.address || !this.port?.protocol) { - this.updateClientsSettings({ monitorUISettings: { connected: false } }); + this.updateClientsSettings({ + monitorUISettings: { + connectionStatus: 'not-connected', + connected: false, // TODO: should be removed when plotter app understand the `connectionStatus` message + }, + }); - this.creating.resolve(Status.CONFIG_MISSING); + this.creating.reject(createMissingConfigurationError(this.port)); return this.creating.promise; } this.logger.info('starting monitor'); - // get default monitor settings from the CLI - const defaultSettings = await this.portMonitorSettings( - this.port.protocol, - this.board.fqbn - ); - // get actual settings from the settings provider - this.settings = { - ...this.settings, - pluggableMonitorSettings: { - ...this.settings.pluggableMonitorSettings, - ...(await this.monitorSettingsProvider.getSettings( - this.monitorID, - defaultSettings - )), - }, - }; + try { + // get default monitor settings from the CLI + const defaultSettings = await this.portMonitorSettings( + this.port.protocol, + this.board.fqbn + ); - const coreClient = await this.coreClient; + this.updateClientsSettings({ + monitorUISettings: { connectionStatus: 'connecting' }, + }); - const { instance } = coreClient; - const monitorRequest = new MonitorRequest(); - monitorRequest.setInstance(instance); - if (this.board?.fqbn) { - monitorRequest.setFqbn(this.board.fqbn); - } - if (this.port?.address && this.port?.protocol) { - const rpcPort = new RpcPort(); - rpcPort.setAddress(this.port.address); - rpcPort.setProtocol(this.port.protocol); - monitorRequest.setPort(rpcPort); - } - const config = new MonitorPortConfiguration(); - for (const id in this.settings.pluggableMonitorSettings) { - const s = new MonitorPortSetting(); - s.setSettingId(id); - s.setValue(this.settings.pluggableMonitorSettings[id].selectedValue); - config.addSettings(s); - } - monitorRequest.setPortConfiguration(config); + // get actual settings from the settings provider + this.settings = { + ...this.settings, + pluggableMonitorSettings: { + ...this.settings.pluggableMonitorSettings, + ...(await this.monitorSettingsProvider.getSettings( + this.monitorID, + defaultSettings + )), + }, + }; - const wroteToStreamSuccessfully = await this.pollWriteToStream( - monitorRequest - ); - if (wroteToStreamSuccessfully) { + const coreClient = await this.coreClient; + + const { instance } = coreClient; + const monitorRequest = new MonitorRequest(); + monitorRequest.setInstance(instance); + if (this.board?.fqbn) { + monitorRequest.setFqbn(this.board.fqbn); + } + if (this.port?.address && this.port?.protocol) { + const rpcPort = new RpcPort(); + rpcPort.setAddress(this.port.address); + rpcPort.setProtocol(this.port.protocol); + monitorRequest.setPort(rpcPort); + } + const config = new MonitorPortConfiguration(); + for (const id in this.settings.pluggableMonitorSettings) { + const s = new MonitorPortSetting(); + s.setSettingId(id); + s.setValue(this.settings.pluggableMonitorSettings[id].selectedValue); + config.addSettings(s); + } + monitorRequest.setPortConfiguration(config); + + await this.pollWriteToStream(monitorRequest); // Only store the config, if the monitor has successfully started. this.currentPortConfigSnapshot = MonitorPortConfiguration.toObject( false, @@ -237,15 +268,34 @@ export class MonitorService extends CoreClientAware implements Disposable { `started monitor to ${this.port?.address} using ${this.port?.protocol}` ); this.updateClientsSettings({ - monitorUISettings: { connected: true, serialPort: this.port.address }, + monitorUISettings: { + connectionStatus: 'connected', + connected: true, // TODO: should be removed when plotter app understand the `connectionStatus` message + serialPort: this.port.address, + }, }); - this.creating.resolve(Status.OK); + this.creating.resolve(); return this.creating.promise; - } else { + } catch (err) { this.logger.warn( `failed starting monitor to ${this.port?.address} using ${this.port?.protocol}` ); - this.creating.resolve(Status.NOT_CONNECTED); + const appError = ApplicationError.is(err) + ? err + : createConnectionFailedError( + this.port, + ServiceError.is(err) + ? err.details + : err instanceof Error + ? err.message + : String(err) + ); + this.creating.reject(appError); + this.updateClientsSettings({ + monitorUISettings: { + connectionStatus: { errorMessage: appError.message }, + }, + }); return this.creating.promise; } } @@ -264,19 +314,29 @@ export class MonitorService extends CoreClientAware implements Disposable { // default handlers duplex .on('close', () => { - this.duplex = null; - this.updateClientsSettings({ - monitorUISettings: { connected: false }, - }); + if (duplex === this.duplex) { + this.duplex = null; + this.updateClientsSettings({ + monitorUISettings: { + connected: false, // TODO: should be removed when plotter app understand the `connectionStatus` message + connectionStatus: 'not-connected', + }, + }); + } this.logger.info( `monitor to ${this.port?.address} using ${this.port?.protocol} closed by client` ); }) .on('end', () => { - this.duplex = null; - this.updateClientsSettings({ - monitorUISettings: { connected: false }, - }); + if (duplex === this.duplex) { + this.duplex = null; + this.updateClientsSettings({ + monitorUISettings: { + connected: false, // TODO: should be removed when plotter app understand the `connectionStatus` message + connectionStatus: 'not-connected', + }, + }); + } this.logger.info( `monitor to ${this.port?.address} using ${this.port?.protocol} closed by server` ); @@ -287,21 +347,17 @@ export class MonitorService extends CoreClientAware implements Disposable { } } - pollWriteToStream(request: MonitorRequest): Promise { - let attemptsRemaining = MAX_WRITE_TO_STREAM_TRIES; - const writeTimeoutMs = WRITE_TO_STREAM_TIMEOUT_MS; - + pollWriteToStream(request: MonitorRequest): Promise { const createWriteToStreamExecutor = (duplex: ClientDuplexStream) => - (resolve: (value: boolean) => void, reject: () => void) => { + (resolve: () => void, reject: (reason?: unknown) => void) => { const resolvingDuplexHandlers: DuplexHandler[] = [ { key: 'error', callback: async (err: Error) => { this.logger.error(err); - resolve(false); - // TODO - // this.theiaFEClient?.notifyError() + const details = ServiceError.is(err) ? err.details : err.message; + reject(createConnectionFailedError(this.port, details)); }, }, { @@ -313,79 +369,47 @@ export class MonitorService extends CoreClientAware implements Disposable { return; } if (monitorResponse.getSuccess()) { - resolve(true); + resolve(); return; } const data = monitorResponse.getRxData(); const message = typeof data === 'string' ? data - : this.streamingTextDecoder.decode(data, {stream:true}); + : this.streamingTextDecoder.decode(data, { stream: true }); this.messages.push(...splitLines(message)); }, }, ]; this.setDuplexHandlers(duplex, resolvingDuplexHandlers); - - setTimeout(() => { - reject(); - }, writeTimeoutMs); duplex.write(request); }; - const pollWriteToStream = new Promise((resolve) => { - const startPolling = async () => { - // here we create a new duplex but we don't yet - // set "this.duplex", nor do we use "this.duplex" in our poll - // as duplex 'end' / 'close' events (which we do not "await") - // will set "this.duplex" to null - const createdDuplex = await this.createDuplex(); - - let pollingIsSuccessful; - // attempt a "writeToStream" and "await" CLI response: success (true) or error (false) - // if we get neither within WRITE_TO_STREAM_TIMEOUT_MS or an error we get undefined - try { - const writeToStream = createWriteToStreamExecutor(createdDuplex); - pollingIsSuccessful = await new Promise(writeToStream); - } catch (error) { - this.logger.error(error); - } - - // CLI confirmed port opened successfully - if (pollingIsSuccessful) { - this.duplex = createdDuplex; - resolve(true); - return; - } - - // if "pollingIsSuccessful" is false - // the CLI gave us an error, lets try again - // after waiting 2 seconds if we've not already - // reached MAX_WRITE_TO_STREAM_TRIES - if (pollingIsSuccessful === false) { - attemptsRemaining -= 1; - if (attemptsRemaining > 0) { - setTimeout(startPolling, 2000); - return; - } else { - resolve(false); - return; + return Promise.race([ + retry( + async () => { + let createdDuplex = undefined; + try { + createdDuplex = await this.createDuplex(); + await new Promise(createWriteToStreamExecutor(createdDuplex)); + this.duplex = createdDuplex; + } catch (err) { + createdDuplex?.end(); + throw err; } - } - - // "pollingIsSuccessful" remains undefined: - // we got no response from the CLI within 30 seconds - // resolve to false and end the duplex connection - resolve(false); - createdDuplex.end(); - return; - }; - - startPolling(); - }); - - return pollWriteToStream; + }, + 2_000, + MAX_WRITE_TO_STREAM_TRIES + ), + timeoutReject( + WRITE_TO_STREAM_TIMEOUT_MS, + nls.localize( + 'arduino/monitor/connectionTimeout', + "Timeout. The IDE has not received the 'success' message from the monitor after successfully connecting to it" + ) + ), + ]) as Promise as Promise; } /** @@ -429,9 +453,9 @@ export class MonitorService extends CoreClientAware implements Disposable { * @param message string sent to running monitor * @returns a status to verify message has been sent. */ - async send(message: string): Promise { + async send(message: string): Promise { if (!this.duplex) { - return Status.NOT_CONNECTED; + throw createNotConnectedError(this.port); } const coreClient = await this.coreClient; const { instance } = coreClient; @@ -439,14 +463,12 @@ export class MonitorService extends CoreClientAware implements Disposable { const req = new MonitorRequest(); req.setInstance(instance); req.setTxData(new TextEncoder().encode(message)); - return new Promise((resolve) => { + return new Promise((resolve, reject) => { if (this.duplex) { - this.duplex?.write(req, () => { - resolve(Status.OK); - }); + this.duplex?.write(req, resolve); return; } - this.stop().then(() => resolve(Status.NOT_CONNECTED)); + this.stop().then(() => reject(createNotConnectedError(this.port))); }); } @@ -469,7 +491,8 @@ export class MonitorService extends CoreClientAware implements Disposable { */ private async portMonitorSettings( protocol: string, - fqbn: string + fqbn: string, + swallowsPlatformNotFoundError = false ): Promise { const coreClient = await this.coreClient; const { client, instance } = coreClient; @@ -478,19 +501,33 @@ export class MonitorService extends CoreClientAware implements Disposable { req.setPortProtocol(protocol); req.setFqbn(fqbn); - const res = await new Promise( - (resolve, reject) => { - client.enumerateMonitorPortSettings(req, (err, resp) => { - if (!!err) { - reject(err); + const resp = await new Promise< + EnumerateMonitorPortSettingsResponse | undefined + >((resolve, reject) => { + client.enumerateMonitorPortSettings(req, async (err, resp) => { + if (err) { + // Check whether the platform is installed: https://github.com/arduino/arduino-ide/issues/1974. + // No error codes. Look for `Unknown FQBN: platform arduino:mbed_nano is not installed` message similarities: https://github.com/arduino/arduino-cli/issues/1762. + if ( + swallowsPlatformNotFoundError && + ServiceError.is(err) && + err.code === status.NOT_FOUND && + err.details.includes('FQBN') && + err.details.includes(fqbn.split(':', 2).join(':')) // create a platform ID from the FQBN + ) { + resolve(undefined); } - resolve(resp); - }); - } - ); + reject(err); + } + resolve(resp); + }); + }); const settings: PluggableMonitorSettings = {}; - for (const iterator of res.getSettingsList()) { + if (!resp) { + return settings; + } + for (const iterator of resp.getSettingsList()) { settings[iterator.getSettingId()] = { id: iterator.getSettingId(), label: iterator.getLabel(), @@ -510,7 +547,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: MonitorSettings): Promise { const config = new MonitorPortConfiguration(); const { pluggableMonitorSettings } = settings; const reconciledSettings = await this.monitorSettingsProvider.setSettings( @@ -527,17 +564,23 @@ export class MonitorService extends CoreClientAware implements Disposable { } } + const connectionStatus = Boolean(this.duplex) + ? 'connected' + : 'not-connected'; this.updateClientsSettings({ monitorUISettings: { ...settings.monitorUISettings, - connected: !!this.duplex, + connectionStatus, serialPort: this.port.address, + connected: isMonitorConnected(connectionStatus), // TODO: should be removed when plotter app understand the `connectionStatus` message }, pluggableMonitorSettings: reconciledSettings, }); if (!this.duplex) { - return Status.NOT_CONNECTED; + // instead of throwing an error, return silently like the original logic + // https://github.com/arduino/arduino-ide/blob/9b49712669b06c97bda68a1e5f04eee4664c13f8/arduino-ide-extension/src/node/monitor-service.ts#L540 + return; } const diffConfig = this.maybeUpdatePortConfigSnapshot(config); @@ -545,7 +588,7 @@ export class MonitorService extends CoreClientAware implements Disposable { this.logger.info( `No port configuration changes have been detected. No need to send configure commands to the running monitor ${this.port.protocol}:${this.port.address}.` ); - return Status.OK; + return; } const coreClient = await this.coreClient; @@ -560,7 +603,6 @@ export class MonitorService extends CoreClientAware implements Disposable { req.setInstance(instance); req.setPortConfiguration(diffConfig); this.duplex.write(req); - return Status.OK; } /** @@ -688,6 +730,26 @@ export class MonitorService extends CoreClientAware implements Disposable { updateClientsSettings(settings: MonitorSettings): void { this.settings = { ...this.settings, ...settings }; + if ( + settings.monitorUISettings?.connectionStatus && + !('connected' in settings.monitorUISettings) + ) { + // Make sure the deprecated `connected` prop is set. + settings.monitorUISettings.connected = isMonitorConnected( + settings.monitorUISettings.connectionStatus + ); + } + if ( + typeof settings.monitorUISettings?.connected === 'boolean' && + !('connectionStatus' in settings.monitorUISettings) + ) { + // Set the connectionStatus if the message was sent by the plotter which does not handle the new protocol. Assuming that the plotter can send anything. + // https://github.com/arduino/arduino-serial-plotter-webapp#monitor-settings + settings.monitorUISettings.connectionStatus = settings.monitorUISettings + .connected + ? 'connected' + : 'not-connected'; + } const command: Monitor.Message = { command: Monitor.MiddlewareCommand.ON_SETTINGS_DID_CHANGE, data: settings, 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 e8949a60b..293751781 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,10 +1,9 @@ -import { MonitorModel } from '../../browser/monitor-model'; -import { PluggableMonitorSetting } from '../../common/protocol'; +import { MonitorState, PluggableMonitorSetting } from '../../common/protocol'; export type PluggableMonitorSettings = Record; export interface MonitorSettings { pluggableMonitorSettings?: PluggableMonitorSettings; - monitorUISettings?: Partial; + monitorUISettings?: Partial; } export const MonitorSettingsProvider = Symbol('MonitorSettingsProvider'); diff --git a/i18n/en.json b/i18n/en.json index 22419c5bf..320be23f1 100644 --- a/i18n/en.json +++ b/i18n/en.json @@ -328,6 +328,13 @@ "tools": "Tools" }, "monitor": { + "alreadyConnectedError": "Could not connect to {0} {1} port. Already connected.", + "baudRate": "{0} baud", + "connectionFailedError": "Could not connect to {0} {1} port.", + "connectionFailedErrorWithDetails": "{0} Could not connect to {1} {2} port.", + "connectionTimeout": "Timeout. The IDE has not received the 'success' message from the monitor after successfully connecting to it", + "missingConfigurationError": "Could not connect to {0} {1} port. The monitor configuration is missing.", + "notConnectedError": "Not connected to {0} {1} port.", "unableToCloseWebSocket": "Unable to close websocket", "unableToConnectToWebSocket": "Unable to connect to websocket" }, @@ -408,6 +415,7 @@ "serial": { "autoscroll": "Autoscroll", "carriageReturn": "Carriage Return", + "connecting": "Connecting to '{0}' on '{1}'...", "message": "Message (Enter to send message to '{0}' on '{1}')", "newLine": "New Line", "newLineCarriageReturn": "Both NL & CR",