From a4e5e65286e66891040f5c82b329b9b54dbe5895 Mon Sep 17 00:00:00 2001 From: Akos Kitta Date: Tue, 3 Dec 2019 17:19:32 +0100 Subject: [PATCH] simplified monitor connection API. we have one connenction per editor anyways. Signed-off-by: Akos Kitta --- .vscode/launch.json | 101 +++++++---- .../src/browser/monitor/monitor-connection.ts | 90 +++++----- .../monitor/monitor-service-client-impl.ts | 4 +- .../src/browser/monitor/monitor-widget.tsx | 7 +- .../src/common/protocol/monitor-service.ts | 31 ++-- .../src/node/monitor/monitor-service-impl.ts | 170 +++++++----------- 6 files changed, 195 insertions(+), 208 deletions(-) diff --git a/.vscode/launch.json b/.vscode/launch.json index 522b90c3..6e810669 100644 --- a/.vscode/launch.json +++ b/.vscode/launch.json @@ -1,39 +1,64 @@ { - // Use IntelliSense to learn about possible attributes. - // Hover to view descriptions of existing attributes. - // For more information, visit: https://go.microsoft.com/fwlink/?linkid=830387 - "version": "0.2.0", - "configurations": [ - { - "type": "node", - "request": "launch", - "name": "Launch Electron Packager", - "program": "${workspaceRoot}/electron/packager/index.js", - "cwd": "${workspaceFolder}/electron/packager" - }, - { - "type": "node", - "request": "launch", - "name": "Launch Backend", - "program": "${workspaceRoot}/browser-app/src-gen/backend/main.js", - "args": [ - "--hostname=0.0.0.0", - "--port=3000", - "--no-cluster", - "--no-app-auto-install" - ], - "env": { - "NODE_ENV": "development" - }, - "sourceMaps": true, - "outFiles": [ - "${workspaceRoot}/browser-app/src-gen/backend/*.js", - "${workspaceRoot}/browser-app/lib/**/*.js", - "${workspaceRoot}/arduino-ide-extension/*/lib/**/*.js" - ], - "smartStep": true, - "internalConsoleOptions": "openOnSessionStart", - "outputCapture": "std" - } - ] -} \ No newline at end of file + // Use IntelliSense to learn about possible attributes. + // Hover to view descriptions of existing attributes. + // For more information, visit: https://go.microsoft.com/fwlink/?linkid=830387 + "version": "0.2.0", + "configurations": [ + { + "type": "node", + "request": "launch", + "name": "Launch Electron Packager", + "program": "${workspaceRoot}/electron/packager/index.js", + "cwd": "${workspaceFolder}/electron/packager" + }, + { + "type": "node", + "request": "launch", + "name": "Launch Backend", + "program": "${workspaceRoot}/browser-app/src-gen/backend/main.js", + "args": [ + "--hostname=0.0.0.0", + "--port=3000", + "--no-cluster", + "--no-app-auto-install" + ], + "env": { + "NODE_ENV": "development" + }, + "sourceMaps": true, + "outFiles": [ + "${workspaceRoot}/browser-app/src-gen/backend/*.js", + "${workspaceRoot}/browser-app/lib/**/*.js", + "${workspaceRoot}/arduino-ide-extension/*/lib/**/*.js" + ], + "smartStep": true, + "internalConsoleOptions": "openOnSessionStart", + "outputCapture": "std" + }, + { + "type": "node", + "request": "launch", + "name": "Launch Backend (Debug CLI daemon)", + "program": "${workspaceRoot}/browser-app/src-gen/backend/main.js", + "args": [ + "--hostname=0.0.0.0", + "--port=3000", + "--no-cluster", + "--no-app-auto-install", + "--debug-cli=true" + ], + "env": { + "NODE_ENV": "development" + }, + "sourceMaps": true, + "outFiles": [ + "${workspaceRoot}/browser-app/src-gen/backend/*.js", + "${workspaceRoot}/browser-app/lib/**/*.js", + "${workspaceRoot}/arduino-ide-extension/*/lib/**/*.js" + ], + "smartStep": true, + "internalConsoleOptions": "openOnSessionStart", + "outputCapture": "std" + } + ] +} diff --git a/arduino-ide-extension/src/browser/monitor/monitor-connection.ts b/arduino-ide-extension/src/browser/monitor/monitor-connection.ts index f8efd723..37b51fc8 100644 --- a/arduino-ide-extension/src/browser/monitor/monitor-connection.ts +++ b/arduino-ide-extension/src/browser/monitor/monitor-connection.ts @@ -2,7 +2,7 @@ import { injectable, inject, postConstruct } from 'inversify'; import { Emitter, Event } from '@theia/core/lib/common/event'; // import { ConnectionStatusService } from '@theia/core/lib/browser/connection-status-service'; import { MessageService } from '@theia/core/lib/common/message-service'; -import { MonitorService, MonitorConfig, MonitorError } from '../../common/protocol/monitor-service'; +import { MonitorService, MonitorConfig, MonitorError, Status } from '../../common/protocol/monitor-service'; import { BoardsServiceClientImpl } from '../boards/boards-service-client-impl'; import { Port, Board } from '../../common/protocol/boards-service'; import { MonitorServiceClientImpl } from './monitor-service-client-impl'; @@ -26,82 +26,81 @@ export class MonitorConnection { // protected readonly connectionStatusService: ConnectionStatusService; protected state: MonitorConnection.State | undefined; - protected readonly onConnectionChangedEmitter = new Emitter(); + protected readonly onConnectionChangedEmitter = new Emitter(); - readonly onConnectionChanged: Event = this.onConnectionChangedEmitter.event; + readonly onConnectionChanged: Event = this.onConnectionChangedEmitter.event; @postConstruct() protected init(): void { this.monitorServiceClient.onError(async error => { let shouldReconnect = false; if (this.state) { - const { code, connectionId, config } = error; - if (this.state.connectionId === connectionId) { - switch (code) { - case MonitorError.ErrorCodes.CLIENT_CANCEL: { - console.debug(`Connection was canceled by client: ${MonitorConnection.State.toString(this.state)}.`); - break; - } - case MonitorError.ErrorCodes.DEVICE_BUSY: { - const { port } = config; - this.messageService.warn(`Connection failed. Serial port is busy: ${Port.toString(port)}.`); - break; - } - case MonitorError.ErrorCodes.DEVICE_NOT_CONFIGURED: { - const { port } = config; - this.messageService.info(`Disconnected from ${Port.toString(port)}.`); - break; - } - case MonitorError.ErrorCodes.interrupted_system_call: { - const { board, port } = config; - this.messageService.warn(`Unexpectedly interrupted by backend. Reconnecting ${Board.toString(board)} on port ${Port.toString(port)}.`); - shouldReconnect = true; - } + const { code, config } = error; + switch (code) { + case MonitorError.ErrorCodes.CLIENT_CANCEL: { + console.debug(`Connection was canceled by client: ${MonitorConnection.State.toString(this.state)}.`); + break; } - const oldState = this.state; - this.state = undefined; - if (shouldReconnect) { - await this.connect(oldState.config); + case MonitorError.ErrorCodes.DEVICE_BUSY: { + const { port } = config; + this.messageService.warn(`Connection failed. Serial port is busy: ${Port.toString(port)}.`); + break; } - } else { - console.warn(`Received an error from unexpected connection: ${MonitorConnection.State.toString({ connectionId, config })}.`); + case MonitorError.ErrorCodes.DEVICE_NOT_CONFIGURED: { + const { port } = config; + this.messageService.info(`Disconnected from ${Port.toString(port)}.`); + break; + } + case undefined: { + const { board, port } = config; + this.messageService.error(`Unexpected error. Reconnecting ${Board.toString(board)} on port ${Port.toString(port)}.`); + console.error(JSON.stringify(error)); + shouldReconnect = true; + } + } + const oldState = this.state; + this.state = undefined; + if (shouldReconnect) { + await this.connect(oldState.config); } } }); } - get connectionId(): string | undefined { - return this.state ? this.state.connectionId : undefined; + get connected(): boolean { + return !!this.state; } get connectionConfig(): MonitorConfig | undefined { return this.state ? this.state.config : undefined; } - async connect(config: MonitorConfig): Promise { + async connect(config: MonitorConfig): Promise { if (this.state) { throw new Error(`Already connected to ${MonitorConnection.State.toString(this.state)}.`); } - const { connectionId } = await this.monitorService.connect(config); - this.state = { connectionId, config }; - this.onConnectionChangedEmitter.fire(connectionId); - return connectionId; + const status = await this.monitorService.connect(config); + if (Status.isOK(status)) { + this.state = { config }; + this.onConnectionChangedEmitter.fire(true); + } + return Status.isOK(status); } - async disconnect(): Promise { + async disconnect(): Promise { if (!this.state) { throw new Error('Not connected. Nothing to disconnect.'); } console.log('>>> Disposing existing monitor connection before establishing a new one...'); - const result = await this.monitorService.disconnect(this.state.connectionId); - if (result) { + const status = await this.monitorService.disconnect(); + if (Status.isOK(status)) { console.log(`<<< Disposed connection. Was: ${MonitorConnection.State.toString(this.state)}`); } else { console.warn(`<<< Could not dispose connection. Activate connection: ${MonitorConnection.State.toString(this.state)}`); } this.state = undefined; - this.onConnectionChangedEmitter.fire(undefined); - return result; + this.onConnectionChangedEmitter.fire(false); + return status; } } @@ -109,15 +108,14 @@ export class MonitorConnection { export namespace MonitorConnection { export interface State { - readonly connectionId: string; readonly config: MonitorConfig; } export namespace State { export function toString(state: State): string { - const { connectionId, config } = state; + const { config } = state; const { board, port } = config; - return `${Board.toString(board)} ${Port.toString(port)} [ID: ${connectionId}]`; + return `${Board.toString(board)} ${Port.toString(port)}`; } } diff --git a/arduino-ide-extension/src/browser/monitor/monitor-service-client-impl.ts b/arduino-ide-extension/src/browser/monitor/monitor-service-client-impl.ts index 3e6367b7..88c6cb8d 100644 --- a/arduino-ide-extension/src/browser/monitor/monitor-service-client-impl.ts +++ b/arduino-ide-extension/src/browser/monitor/monitor-service-client-impl.ts @@ -12,8 +12,8 @@ export class MonitorServiceClientImpl implements MonitorServiceClient { notifyRead(event: MonitorReadEvent): void { this.onReadEmitter.fire(event); - const { connectionId, data } = event; - console.debug(`Received data from ${connectionId}: ${data}`); + const { data } = event; + console.debug(`Received data: ${data}`); } notifyError(error: MonitorError): void { diff --git a/arduino-ide-extension/src/browser/monitor/monitor-widget.tsx b/arduino-ide-extension/src/browser/monitor/monitor-widget.tsx index a76e1ea5..64e5816c 100644 --- a/arduino-ide-extension/src/browser/monitor/monitor-widget.tsx +++ b/arduino-ide-extension/src/browser/monitor/monitor-widget.tsx @@ -202,7 +202,7 @@ export class MonitorWidget extends ReactWidget { protected onBeforeDetach(msg: Message): void { super.onBeforeDetach(msg); - if (this.connection.connectionId) { + if (this.connection.connected) { this.connection.disconnect(); } } @@ -294,9 +294,8 @@ export class MonitorWidget extends ReactWidget { protected readonly onSend = (value: string) => this.doSend(value); protected async doSend(value: string) { - const { connectionId } = this.connection; - if (connectionId) { - this.monitorService.send(connectionId, value + this.model.lineEnding); + if (this.connection.connected) { + this.monitorService.send(value + this.model.lineEnding); } } diff --git a/arduino-ide-extension/src/common/protocol/monitor-service.ts b/arduino-ide-extension/src/common/protocol/monitor-service.ts index 1eaf5199..f7b11f1d 100644 --- a/arduino-ide-extension/src/common/protocol/monitor-service.ts +++ b/arduino-ide-extension/src/common/protocol/monitor-service.ts @@ -1,12 +1,26 @@ import { JsonRpcServer } from '@theia/core/lib/common/messaging/proxy-factory'; import { Board, Port } from './boards-service'; +export interface Status { } +export interface OK extends Status { } +export interface ErrorStatus extends Status { + readonly message: string; +} +export namespace Status { + export function isOK(status: Status & { message?: string }): status is OK { + return typeof status.message !== 'string'; + } + export const OK: OK = {}; + export const NOT_CONNECTED: ErrorStatus = { message: 'Not connected.' }; + export const ALREADY_CONNECTED: ErrorStatus = { message: 'Already connected.' }; +} + export const MonitorServicePath = '/services/serial-monitor'; export const MonitorService = Symbol('MonitorService'); export interface MonitorService extends JsonRpcServer { - connect(config: MonitorConfig): Promise<{ connectionId: string }>; - disconnect(connectionId: string): Promise; - send(connectionId: string, data: string | Uint8Array): Promise; + connect(config: MonitorConfig): Promise; + disconnect(): Promise; + send(data: string | Uint8Array): Promise; } export interface MonitorConfig { @@ -42,14 +56,15 @@ export interface MonitorServiceClient { } export interface MonitorReadEvent { - readonly connectionId: string; readonly data: string; } export interface MonitorError { - readonly connectionId: string; readonly message: string; - readonly code: number; + /** + * If no `code` is available, clients must reestablish the serial-monitor connection. + */ + readonly code: number | undefined; readonly config: MonitorConfig; } export namespace MonitorError { @@ -66,9 +81,5 @@ export namespace MonitorError { * Another serial monitor was opened on this port. For another electron-instance, Java IDE. */ export const DEVICE_BUSY = 3; - /** - * Another serial monitor was opened on this port. For another electron-instance, Java IDE. - */ - export const interrupted_system_call = 4; } } diff --git a/arduino-ide-extension/src/node/monitor/monitor-service-impl.ts b/arduino-ide-extension/src/node/monitor/monitor-service-impl.ts index f943b262..95f8fe51 100644 --- a/arduino-ide-extension/src/node/monitor/monitor-service-impl.ts +++ b/arduino-ide-extension/src/node/monitor/monitor-service-impl.ts @@ -1,48 +1,36 @@ -import { v4 } from 'uuid'; -import { Chance } from 'chance'; import { ClientDuplexStream } from '@grpc/grpc-js'; import { TextDecoder, TextEncoder } from 'util'; import { injectable, inject, named } from 'inversify'; import { Struct } from 'google-protobuf/google/protobuf/struct_pb'; -import { ILogger, Disposable, DisposableCollection } from '@theia/core'; -import { MonitorService, MonitorServiceClient, MonitorConfig, MonitorError } from '../../common/protocol/monitor-service'; +import { ILogger } from '@theia/core/lib/common/logger'; +import { MonitorService, MonitorServiceClient, MonitorConfig, MonitorError, Status } from '../../common/protocol/monitor-service'; import { StreamingOpenReq, StreamingOpenResp, MonitorConfig as GrpcMonitorConfig } from '../cli-protocol/monitor/monitor_pb'; import { MonitorClientProvider } from './monitor-client-provider'; import { Board, Port } from '../../common/protocol/boards-service'; -export interface MonitorDuplex { - readonly toDispose: Disposable; - readonly duplex: ClientDuplexStream; -} - interface ErrorWithCode extends Error { readonly code: number; } namespace ErrorWithCode { - export function is(error: Error & { code?: number }): error is ErrorWithCode { - return typeof error.code === 'number'; - } - export function toMonitorError(error: Error, connectionId: string, config: MonitorConfig): MonitorError | undefined { + export function toMonitorError(error: Error, config: MonitorConfig): MonitorError { + const { message } = error; + let code = undefined; if (is(error)) { // TODO: const `mapping`. Use regex for the `message`. const mapping = new Map(); mapping.set('1 CANCELLED: Cancelled on client', MonitorError.ErrorCodes.CLIENT_CANCEL); mapping.set('2 UNKNOWN: device not configured', MonitorError.ErrorCodes.DEVICE_NOT_CONFIGURED); mapping.set('2 UNKNOWN: error opening serial monitor: Serial port busy', MonitorError.ErrorCodes.DEVICE_BUSY); - mapping.set('2 UNKNOWN: interrupted system call', MonitorError.ErrorCodes.interrupted_system_call); - const { message } = error; - const code = mapping.get(message); - if (typeof code === 'number') { - return { - connectionId, - message, - code, - config - } - } - console.warn(`Unhandled error with code:`, error); + code = mapping.get(message); } - return undefined; + return { + message, + code, + config + }; + } + function is(error: Error & { code?: number }): error is ErrorWithCode { + return typeof error.code === 'number'; } } @@ -57,7 +45,7 @@ export class MonitorServiceImpl implements MonitorService { protected readonly monitorClientProvider: MonitorClientProvider; protected client?: MonitorServiceClient; - protected readonly connections = new Map(); + protected connection?: ClientDuplexStream; setClient(client: MonitorServiceClient | undefined): void { this.client = client; @@ -65,51 +53,37 @@ export class MonitorServiceImpl implements MonitorService { dispose(): void { this.logger.info('>>> Disposing monitor service...'); - for (const [connectionId, duplex] of this.connections.entries()) { - this.doDisconnect(connectionId, duplex); + if (this.connection) { + this.disconnect(); } this.logger.info('<<< Disposing monitor service...'); this.client = undefined; } - async connect(config: MonitorConfig): Promise<{ connectionId: string }> { + async connect(config: MonitorConfig): Promise { this.logger.info(`>>> Creating serial monitor connection for ${Board.toString(config.board)} on port ${Port.toString(config.port)}...`); + if (this.connection) { + return Status.ALREADY_CONNECTED; + } const client = await this.monitorClientProvider.client; - const duplex = client.streamingOpen(); - const connectionId = `${new Chance(v4()).animal().replace(/\s+/g, '-').toLowerCase()}-monitor-connection`; - const toDispose = new DisposableCollection( - Disposable.create(() => this.disconnect(connectionId)) - ); - - duplex.on('error', ((error: Error) => { - if (ErrorWithCode.is(error)) { - const monitorError = ErrorWithCode.toMonitorError(error, connectionId, config); - if (monitorError) { - if (this.client) { - this.client.notifyError(monitorError); - } - // Do not log the error, it was expected. The client will take care of the rest. - if (monitorError.code === MonitorError.ErrorCodes.interrupted_system_call) { - console.log('jajjajaja'); - if (!toDispose.disposed) { - toDispose.dispose(); - } - } - return; + this.connection = client.streamingOpen(); + this.connection.on('error', ((error: Error) => { + const monitorError = ErrorWithCode.toMonitorError(error, config); + if (monitorError.code === undefined) { + this.logger.error(error); + } + ((monitorError.code === undefined ? this.disconnect() : Promise.resolve()) as Promise).then(() => { + if (this.client) { + this.client.notifyError(monitorError); } - } - if (!toDispose.disposed) { - toDispose.dispose(); - } - this.logger.error(`Error occurred for connection ${connectionId}.`, error); + }) }).bind(this)); - duplex.on('data', ((resp: StreamingOpenResp) => { + this.connection.on('data', ((resp: StreamingOpenResp) => { if (this.client) { const raw = resp.getData(); const data = typeof raw === 'string' ? raw : new TextDecoder('utf8').decode(raw); - this.logger.info('NOTIFY READ', data); - this.client.notifyRead({ connectionId, data }); + this.client.notifyRead({ data }); } }).bind(this)); @@ -123,54 +97,42 @@ export class MonitorServiceImpl implements MonitorService { } req.setMonitorconfig(monitorConfig); - return new Promise<{ connectionId: string }>(resolve => { - duplex.write(req, () => { - this.connections.set(connectionId, { toDispose, duplex }); - this.logger.info(`<<< Serial monitor connection created for ${Board.toString(config.board)} on port ${Port.toString(config.port)}. ID: [${connectionId}]`); - resolve({ connectionId }); - }); + return new Promise(resolve => { + if (this.connection) { + this.connection.write(req, () => { + this.logger.info(`<<< Serial monitor connection created for ${Board.toString(config.board)} on port ${Port.toString(config.port)}.`); + resolve(Status.OK); + }); + return; + } + resolve(Status.NOT_CONNECTED); }); } - async disconnect(connectionId: string): Promise { - this.logger.info(`>>> Received disconnect request for connection: ${connectionId}`); - const disposable = this.connections.get(connectionId); - if (!disposable) { - this.logger.warn(`<<< No connection was found for ID: ${connectionId}`); - return false; + async disconnect(): Promise { + if (!this.connection) { + return Status.NOT_CONNECTED; } - const result = await this.doDisconnect(connectionId, disposable); - if (result) { - this.logger.info(`<<< Successfully disconnected from ${connectionId}.`); - } else { - this.logger.info(`<<< Could not disconnected from ${connectionId}.`); - } - return result; + this.connection.cancel(); + this.connection = undefined; + return Status.OK; } - protected async doDisconnect(connectionId: string, monitorDuplex: MonitorDuplex): Promise { - const { duplex } = monitorDuplex; - this.logger.info(`>>> Disposing monitor connection: ${connectionId}...`); - try { - duplex.cancel(); - this.connections.delete(connectionId); - this.logger.info(`<<< Connection disposed: ${connectionId}.`); - return true; - } catch (e) { - this.logger.error(`<<< Error occurred when disposing monitor connection: ${connectionId}. ${e}`); - return false; - } - } - - async send(connectionId: string, data: string): Promise { - const duplex = this.duplex(connectionId); - if (duplex) { - const req = new StreamingOpenReq(); - req.setData(new TextEncoder().encode(data)); - return new Promise(resolve => duplex.duplex.write(req, resolve)); - } else { - throw new Error(`No connection with ID: ${connectionId}.`); + async send(data: string): Promise { + if (!this.connection) { + return Status.NOT_CONNECTED; } + const req = new StreamingOpenReq(); + req.setData(new TextEncoder().encode(data)); + return new Promise(resolve => { + if (this.connection) { + this.connection.write(req, () => { + resolve(Status.OK); + }); + return; + } + resolve(Status.NOT_CONNECTED); + }); } protected mapType(type?: MonitorConfig.ConnectionType): GrpcMonitorConfig.TargetType { @@ -180,12 +142,4 @@ export class MonitorServiceImpl implements MonitorService { } } - protected duplex(connectionId: string): MonitorDuplex | undefined { - const monitorClient = this.connections.get(connectionId); - if (!monitorClient) { - this.logger.warn(`Could not find monitor client for connection ID: ${connectionId}`); - } - return monitorClient; - } - }