From 7063f9219028908b981615d02b2803ead9b1c857 Mon Sep 17 00:00:00 2001 From: Alberto Iannaccone Date: Thu, 30 Sep 2021 14:42:50 +0200 Subject: [PATCH 1/2] wip tests --- .../src/browser/monitor/monitor-connection.ts | 237 +++++++++--------- 1 file changed, 117 insertions(+), 120 deletions(-) diff --git a/arduino-ide-extension/src/browser/monitor/monitor-connection.ts b/arduino-ide-extension/src/browser/monitor/monitor-connection.ts index e37d15f3d..f04e33b2d 100644 --- a/arduino-ide-extension/src/browser/monitor/monitor-connection.ts +++ b/arduino-ide-extension/src/browser/monitor/monitor-connection.ts @@ -71,129 +71,15 @@ export class MonitorConnection { @postConstruct() protected init(): void { - this.monitorServiceClient.onMessage(async (port) => { - const w = new WebSocket(`ws://localhost:${port}`); - w.onmessage = (res) => { - const messages = JSON.parse(res.data); - this.onReadEmitter.fire({ messages }); - }; - }); - - // let i = 0; - // this.monitorServiceClient.onMessage(async (msg) => { - // // msg received - // i++; - // if (i % 1000 === 0) { - // // console.log(msg); - // } - // }); - - this.monitorServiceClient.onError(async (error) => { - let shouldReconnect = false; - if (this.state) { - const { code, config } = error; - const { board, port } = config; - const options = { timeout: 3000 }; - 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: { - this.messageService.warn( - `Connection failed. Serial port is busy: ${Port.toString(port)}.`, - options - ); - shouldReconnect = this.autoConnect; - this.monitorErrors.push(error); - break; - } - case MonitorError.ErrorCodes.DEVICE_NOT_CONFIGURED: { - this.messageService.info( - `Disconnected ${Board.toString(board, { - useFqbn: false, - })} from ${Port.toString(port)}.`, - options - ); - break; - } - case undefined: { - this.messageService.error( - `Unexpected error. Reconnecting ${Board.toString( - board - )} on port ${Port.toString(port)}.`, - options - ); - console.error(JSON.stringify(error)); - shouldReconnect = this.connected && this.autoConnect; - break; - } - } - const oldState = this.state; - this.state = undefined; - this.onConnectionChangedEmitter.fire(this.state); - if (shouldReconnect) { - if (this.monitorErrors.length >= 10) { - this.messageService.warn( - `Failed to reconnect ${Board.toString(board, { - useFqbn: false, - })} to the the serial-monitor after 10 consecutive attempts. The ${Port.toString( - port - )} serial port is busy. after 10 consecutive attempts.` - ); - this.monitorErrors.length = 0; - } else { - const attempts = this.monitorErrors.length || 1; - if (this.reconnectTimeout !== undefined) { - // Clear the previous timer. - window.clearTimeout(this.reconnectTimeout); - } - const timeout = attempts * 1000; - this.messageService.warn( - `Reconnecting ${Board.toString(board, { - useFqbn: false, - })} to ${Port.toString(port)} in ${attempts} seconds...`, - { timeout } - ); - this.reconnectTimeout = window.setTimeout( - () => this.connect(oldState.config), - timeout - ); - } - } - } - }); + this.monitorServiceClient.onMessage(this.handleMessage.bind(this)); + this.monitorServiceClient.onError(this.handleError.bind(this)); this.boardsServiceProvider.onBoardsConfigChanged( this.handleBoardConfigChange.bind(this) ); - this.notificationCenter.onAttachedBoardsChanged((event) => { - if (this.autoConnect && this.connected) { - const { boardsConfig } = this.boardsServiceProvider; - if ( - this.boardsServiceProvider.canUploadTo(boardsConfig, { - silent: false, - }) - ) { - const { attached } = AttachedBoardsChangeEvent.diff(event); - if ( - attached.boards.some( - (board) => - !!board.port && BoardsConfig.Config.sameAs(boardsConfig, board) - ) - ) { - const { selectedBoard: board, selectedPort: port } = boardsConfig; - const { baudRate } = this.monitorModel; - this.disconnect().then(() => - this.connect({ board, port, baudRate }) - ); - } - } - } - }); + this.notificationCenter.onAttachedBoardsChanged( + this.handleAttachedBoardsChanged.bind(this) + ); + // Handles the `baudRate` changes by reconnecting if required. this.monitorModel.onChange(({ property }) => { if (property === 'baudRate' && this.autoConnect && this.connected) { @@ -203,6 +89,14 @@ export class MonitorConnection { }); } + async handleMessage(port: string): Promise { + const w = new WebSocket(`ws://localhost:${port}`); + w.onmessage = (res) => { + const messages = JSON.parse(res.data); + this.onReadEmitter.fire({ messages }); + }; + } + get connected(): boolean { return !!this.state; } @@ -234,6 +128,109 @@ export class MonitorConnection { } } + handleError(error: MonitorError): void { + let shouldReconnect = false; + if (this.state) { + const { code, config } = error; + const { board, port } = config; + const options = { timeout: 3000 }; + 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: { + this.messageService.warn( + `Connection failed. Serial port is busy: ${Port.toString(port)}.`, + options + ); + shouldReconnect = this.autoConnect; + this.monitorErrors.push(error); + break; + } + case MonitorError.ErrorCodes.DEVICE_NOT_CONFIGURED: { + this.messageService.info( + `Disconnected ${Board.toString(board, { + useFqbn: false, + })} from ${Port.toString(port)}.`, + options + ); + break; + } + case undefined: { + this.messageService.error( + `Unexpected error. Reconnecting ${Board.toString( + board + )} on port ${Port.toString(port)}.`, + options + ); + console.error(JSON.stringify(error)); + shouldReconnect = this.connected && this.autoConnect; + break; + } + } + const oldState = this.state; + this.state = undefined; + this.onConnectionChangedEmitter.fire(this.state); + if (shouldReconnect) { + if (this.monitorErrors.length >= 10) { + this.messageService.warn( + `Failed to reconnect ${Board.toString(board, { + useFqbn: false, + })} to the the serial-monitor after 10 consecutive attempts. The ${Port.toString( + port + )} serial port is busy. after 10 consecutive attempts.` + ); + this.monitorErrors.length = 0; + } else { + const attempts = this.monitorErrors.length || 1; + if (this.reconnectTimeout !== undefined) { + // Clear the previous timer. + window.clearTimeout(this.reconnectTimeout); + } + const timeout = attempts * 1000; + this.messageService.warn( + `Reconnecting ${Board.toString(board, { + useFqbn: false, + })} to ${Port.toString(port)} in ${attempts} seconds...`, + { timeout } + ); + this.reconnectTimeout = window.setTimeout( + () => this.connect(oldState.config), + timeout + ); + } + } + } + } + + handleAttachedBoardsChanged(event: AttachedBoardsChangeEvent): void { + if (this.autoConnect && this.connected) { + const { boardsConfig } = this.boardsServiceProvider; + if ( + this.boardsServiceProvider.canUploadTo(boardsConfig, { + silent: false, + }) + ) { + const { attached } = AttachedBoardsChangeEvent.diff(event); + if ( + attached.boards.some( + (board) => + !!board.port && BoardsConfig.Config.sameAs(boardsConfig, board) + ) + ) { + const { selectedBoard: board, selectedPort: port } = boardsConfig; + const { baudRate } = this.monitorModel; + this.disconnect().then(() => this.connect({ board, port, baudRate })); + } + } + } + } + async connect(config: MonitorConfig): Promise { if (this.connected) { const disconnectStatus = await this.disconnect(); From 51423052f1d9eed4ae5b822461132924255dd574 Mon Sep 17 00:00:00 2001 From: Alberto Iannaccone Date: Mon, 4 Oct 2021 18:12:28 +0200 Subject: [PATCH 2/2] test monitor utils --- arduino-ide-extension/package.json | 1 + .../src/browser/monitor/monitor-utils.ts | 14 +- .../monitor/serial-monitor-send-output.tsx | 12 +- .../src/test/browser/monitor-utils.test.ts | 171 ++++++++++++++++++ yarn.lock | 5 + 5 files changed, 190 insertions(+), 13 deletions(-) create mode 100644 arduino-ide-extension/src/test/browser/monitor-utils.test.ts diff --git a/arduino-ide-extension/package.json b/arduino-ide-extension/package.json index 70847582b..40495b9d2 100644 --- a/arduino-ide-extension/package.json +++ b/arduino-ide-extension/package.json @@ -102,6 +102,7 @@ "download": "^7.1.0", "grpc_tools_node_protoc_ts": "^4.1.0", "mocha": "^7.0.0", + "mockdate": "^3.0.5", "moment": "^2.24.0", "protoc": "^1.0.4", "shelljs": "^0.8.3", diff --git a/arduino-ide-extension/src/browser/monitor/monitor-utils.ts b/arduino-ide-extension/src/browser/monitor/monitor-utils.ts index 30607d724..586eea146 100644 --- a/arduino-ide-extension/src/browser/monitor/monitor-utils.ts +++ b/arduino-ide-extension/src/browser/monitor/monitor-utils.ts @@ -1,14 +1,14 @@ import { Line, SerialMonitorOutput } from './serial-monitor-send-output'; -export function messageToLines( +export function messagesToLines( messages: string[], - prevLines: Line[], + prevLines: Line[] = [], + charCount = 0, separator = '\n' ): [Line[], number] { const linesToAdd: Line[] = prevLines.length ? [prevLines[prevLines.length - 1]] : [{ message: '', lineLen: 0 }]; - let charCount = 0; for (const message of messages) { const messageLen = message.length; @@ -38,11 +38,12 @@ export function messageToLines( export function truncateLines( lines: Line[], - charCount: number + charCount: number, + maxCharacters: number = SerialMonitorOutput.MAX_CHARACTERS ): [Line[], number] { - let charsToDelete = charCount - SerialMonitorOutput.MAX_CHARACTERS; + let charsToDelete = charCount - maxCharacters; let lineIndex = 0; - while (charsToDelete > 0) { + while (charsToDelete > 0 || lineIndex > 0) { const firstLineLength = lines[lineIndex]?.lineLen; if (charsToDelete >= firstLineLength) { @@ -55,6 +56,7 @@ export function truncateLines( // delete all previous lines lines.splice(0, lineIndex); + lineIndex = 0; const newFirstLine = lines[0]?.message?.substring(charsToDelete); const deletedCharsCount = firstLineLength - newFirstLine.length; diff --git a/arduino-ide-extension/src/browser/monitor/serial-monitor-send-output.tsx b/arduino-ide-extension/src/browser/monitor/serial-monitor-send-output.tsx index 78b40fff1..8086fb28c 100644 --- a/arduino-ide-extension/src/browser/monitor/serial-monitor-send-output.tsx +++ b/arduino-ide-extension/src/browser/monitor/serial-monitor-send-output.tsx @@ -6,7 +6,7 @@ import AutoSizer from 'react-virtualized-auto-sizer'; import { MonitorModel } from './monitor-model'; import { MonitorConnection } from './monitor-connection'; import dateFormat = require('dateformat'); -import { messageToLines, truncateLines } from './monitor-utils'; +import { messagesToLines, truncateLines } from './monitor-utils'; export type Line = { message: string; timestamp?: Date; lineLen: number }; @@ -66,14 +66,12 @@ export class SerialMonitorOutput extends React.Component< this.scrollToBottom(); this.toDisposeBeforeUnmount.pushAll([ this.props.monitorConnection.onRead(({ messages }) => { - const [newLines, charsToAddCount] = messageToLines( + const [newLines, totalCharCount] = messagesToLines( messages, - this.state.lines - ); - const [lines, charCount] = truncateLines( - newLines, - this.state.charCount + charsToAddCount + this.state.lines, + this.state.charCount ); + const [lines, charCount] = truncateLines(newLines, totalCharCount); this.setState({ lines, diff --git a/arduino-ide-extension/src/test/browser/monitor-utils.test.ts b/arduino-ide-extension/src/test/browser/monitor-utils.test.ts new file mode 100644 index 000000000..3e1e90e6d --- /dev/null +++ b/arduino-ide-extension/src/test/browser/monitor-utils.test.ts @@ -0,0 +1,171 @@ +import { expect } from 'chai'; +import { + messagesToLines, + truncateLines, +} from '../../browser/monitor/monitor-utils'; +import { Line } from '../../browser/monitor/serial-monitor-send-output'; +import { set, reset } from 'mockdate'; + +type TestLine = { + messages: string[]; + prevLines?: { lines: Line[]; charCount: number }; + expected: { lines: Line[]; charCount: number }; + expectedTruncated?: { + lines: Line[]; + charCount: number; + maxCharacters?: number; + }; +}; + +const date = new Date(); +const testLines: TestLine[] = [ + { + messages: ['Hello'], + expected: { lines: [{ message: 'Hello', lineLen: 5 }], charCount: 5 }, + }, + { + messages: ['Hello', 'Dog!'], + expected: { lines: [{ message: 'HelloDog!', lineLen: 9 }], charCount: 9 }, + }, + { + messages: ['Hello\n', 'Dog!'], + expected: { + lines: [ + { message: 'Hello\n', lineLen: 6 }, + { message: 'Dog!', lineLen: 4 }, + ], + charCount: 10, + }, + }, + { + messages: ['Dog!'], + prevLines: { lines: [{ message: 'Hello\n', lineLen: 6 }], charCount: 6 }, + expected: { + lines: [ + { message: 'Hello\n', lineLen: 6 }, + { message: 'Dog!', lineLen: 4 }, + ], + charCount: 10, + }, + }, + { + messages: [' Dog!\n', " Who's a good ", 'boy?\n', "You're a good boy!"], + prevLines: { lines: [{ message: 'Hello', lineLen: 5 }], charCount: 5 }, + expected: { + lines: [ + { message: 'Hello Dog!\n', lineLen: 11 }, + { message: " Who's a good boy?\n", lineLen: 19 }, + { message: "You're a good boy!", lineLen: 8 }, + ], + charCount: 48, + }, + expectedTruncated: { + maxCharacters: 20, + charCount: 20, + lines: [ + { message: '?\n', lineLen: 2 }, + { message: "You're a good boy!", lineLen: 8 }, + ], + }, + }, + { + messages: ['boy?\n', "You're a good boy!"], + prevLines: { + lines: [ + { message: 'Hello Dog!\n', lineLen: 11 }, + { message: " Who's a good ", lineLen: 14 }, + ], + charCount: 25, + }, + expected: { + lines: [ + { message: 'Hello Dog!\n', lineLen: 11 }, + { message: " Who's a good boy?\n", lineLen: 19 }, + { message: "You're a good boy!", lineLen: 8 }, + ], + charCount: 48, + }, + expectedTruncated: { + maxCharacters: 20, + charCount: 20, + lines: [ + { message: '?\n', lineLen: 2 }, + { message: "You're a good boy!", lineLen: 8 }, + ], + }, + }, + { + messages: ["Who's a good boy?\n", 'Yo'], + prevLines: { + lines: [{ message: 'Hello Dog!\n', lineLen: 11 }], + charCount: 11, + }, + expected: { + lines: [ + { message: 'Hello Dog!\n', lineLen: 11 }, + { message: "Who's a good boy?\n", lineLen: 18 }, + { message: 'Yo', lineLen: 2 }, + ], + charCount: 31, + }, + expectedTruncated: { + maxCharacters: 20, + charCount: 20, + lines: [ + { message: "Who's a good boy?\n", lineLen: 18 }, + { message: 'Yo', lineLen: 2 }, + ], + }, + }, +]; + +testLines.forEach((t) => + [...t.expected.lines, ...(t.prevLines?.lines || [])].forEach( + (l) => (l.timestamp = date) + ) +); + +describe.only('Monitor Utils', () => { + beforeEach(() => { + set(date); + }); + + afterEach(() => { + reset(); + }); + + testLines.forEach((testLine) => { + context('when converting messages', () => { + it('should give the right result', () => { + const [newLines, addedCharCount] = messagesToLines( + testLine.messages, + testLine.prevLines?.lines, + testLine.prevLines?.charCount + ); + newLines.forEach((line, index) => { + expect(line.message).to.equal(testLine.expected.lines[index].message); + expect(line.timestamp).to.deep.equal( + testLine.expected.lines[index].timestamp + ); + }); + expect(addedCharCount).to.equal(testLine.expected.charCount); + + const [truncatedLines, totalCharCount] = truncateLines( + newLines, + addedCharCount, + testLine.expectedTruncated?.maxCharacters + ); + let charCount = 0; + if (testLine.expectedTruncated) { + truncatedLines.forEach((line, index) => { + expect(line.message).to.equal( + testLine.expectedTruncated?.lines[index].message + ); + charCount += line.message.length; + }); + expect(totalCharCount).to.equal(charCount); + } + }); + }); + }); +}); diff --git a/yarn.lock b/yarn.lock index ec04154f3..9ca2ebebd 100644 --- a/yarn.lock +++ b/yarn.lock @@ -11743,6 +11743,11 @@ mocha@^7.0.0: yargs-parser "13.1.2" yargs-unparser "1.6.0" +mockdate@^3.0.5: + version "3.0.5" + resolved "https://registry.yarnpkg.com/mockdate/-/mockdate-3.0.5.tgz#789be686deb3149e7df2b663d2bc4392bc3284fb" + integrity sha512-iniQP4rj1FhBdBYS/+eQv7j1tadJ9lJtdzgOpvsOHng/GbcDh2Fhdeq+ZRldrPYdXvCyfFUmFeEwEGXZB5I/AQ== + modify-values@^1.0.0: version "1.0.1" resolved "https://registry.yarnpkg.com/modify-values/-/modify-values-1.0.1.tgz#b3939fa605546474e3e3e3c63d64bd43b4ee6022"