From 2831acc5b5b9f0c8744c946f1721925aaf5f8c9c Mon Sep 17 00:00:00 2001 From: Akos Kitta Date: Thu, 5 Nov 2020 09:16:49 +0100 Subject: [PATCH] ATL-530: No checks before upload/verify/burn Made the port/fqbn/programmer optional for upload, verify, and burn bootloader. From now on, the IDE does not warn the user before performing the desired CLI command. Closes arduino/arduino-pro-ide#364 Signed-off-by: Akos Kitta --- .../src/browser/boards/boards-data-store.ts | 21 +++-- .../browser/contributions/burn-bootloader.ts | 22 +---- .../browser/contributions/upload-sketch.ts | 31 ++----- .../browser/contributions/verify-sketch.ts | 8 +- .../src/browser/output-service-impl.ts | 16 +--- .../src/common/protocol/core-service.ts | 16 ++-- .../src/common/protocol/output-service.ts | 2 +- .../src/node/core-service-impl.ts | 87 +++++++++++-------- 8 files changed, 87 insertions(+), 116 deletions(-) diff --git a/arduino-ide-extension/src/browser/boards/boards-data-store.ts b/arduino-ide-extension/src/browser/boards/boards-data-store.ts index 78ff28b4..27bbdc81 100644 --- a/arduino-ide-extension/src/browser/boards/boards-data-store.ts +++ b/arduino-ide-extension/src/browser/boards/boards-data-store.ts @@ -58,17 +58,25 @@ export class BoardsDataStore implements FrontendApplicationContribution { } async appendConfigToFqbn( - fqbn: string, - boardsPackageVersion: MaybePromise = this.getBoardsPackageVersion(fqbn)): Promise { + fqbn: string | undefined, + boardsPackageVersion: MaybePromise = this.getBoardsPackageVersion(fqbn)): Promise { + + if (!fqbn) { + return undefined; + } const { configOptions } = await this.getData(fqbn, boardsPackageVersion); return ConfigOption.decorate(fqbn, configOptions); } async getData( - fqbn: string, + fqbn: string | undefined, boardsPackageVersion: MaybePromise = this.getBoardsPackageVersion(fqbn)): Promise { + if (!fqbn) { + return BoardsDataStore.Data.EMPTY; + } + const version = await boardsPackageVersion; if (!version) { return BoardsDataStore.Data.EMPTY; @@ -76,10 +84,7 @@ export class BoardsDataStore implements FrontendApplicationContribution { const key = this.getStorageKey(fqbn, version); let data = await this.storageService.getData(key, undefined); if (data) { - // If `configOptions` is empty we rather reload the data. See arduino/arduino-cli#954 and arduino/arduino-cli#955. - if (data.configOptions.length && data.programmers !== undefined) { // to be backward compatible. We did not save the `programmers` into the `localStorage`. - return data; - } + return data; } const boardDetails = await this.getBoardDetailsSafe(fqbn); @@ -173,7 +178,7 @@ export class BoardsDataStore implements FrontendApplicationContribution { this.onChangedEmitter.fire(); } - protected async getBoardsPackageVersion(fqbn: string): Promise { + protected async getBoardsPackageVersion(fqbn: string | undefined): Promise { if (!fqbn) { return undefined; } diff --git a/arduino-ide-extension/src/browser/contributions/burn-bootloader.ts b/arduino-ide-extension/src/browser/contributions/burn-bootloader.ts index d4968fb1..434f5a55 100644 --- a/arduino-ide-extension/src/browser/contributions/burn-bootloader.ts +++ b/arduino-ide-extension/src/browser/contributions/burn-bootloader.ts @@ -46,27 +46,11 @@ export class BurnBootloader extends SketchContribution { } try { const { boardsConfig } = this.boardsServiceClientImpl; - if (!boardsConfig || !boardsConfig.selectedBoard) { - throw new Error('No boards selected. Please select a board.'); - } - if (!boardsConfig.selectedBoard.fqbn) { - throw new Error(`No core is installed for the '${boardsConfig.selectedBoard.name}' board. Please install the core.`); - } - const { selectedPort } = boardsConfig; - if (!selectedPort) { - throw new Error('No ports selected. Please select a port.'); - } - - const port = selectedPort.address; + const port = boardsConfig.selectedPort?.address; const [fqbn, { selectedProgrammer: programmer }] = await Promise.all([ - this.boardsDataStore.appendConfigToFqbn(boardsConfig.selectedBoard.fqbn), - this.boardsDataStore.getData(boardsConfig.selectedBoard.fqbn) + this.boardsDataStore.appendConfigToFqbn(boardsConfig.selectedBoard?.fqbn), + this.boardsDataStore.getData(boardsConfig.selectedBoard?.fqbn) ]); - - if (!programmer) { - throw new Error('Programmer is not selected. Please select a programmer from the `Tools` > `Programmer` menu.'); - } - this.outputChannelManager.getChannel('Arduino: bootloader').clear(); await this.coreService.burnBootloader({ fqbn, diff --git a/arduino-ide-extension/src/browser/contributions/upload-sketch.ts b/arduino-ide-extension/src/browser/contributions/upload-sketch.ts index bae20667..a5972502 100644 --- a/arduino-ide-extension/src/browser/contributions/upload-sketch.ts +++ b/arduino-ide-extension/src/browser/contributions/upload-sketch.ts @@ -83,34 +83,19 @@ export class UploadSketch extends SketchContribution { } try { const { boardsConfig } = this.boardsServiceClientImpl; - if (!boardsConfig || !boardsConfig.selectedBoard) { - throw new Error('No boards selected. Please select a board.'); - } - if (!boardsConfig.selectedBoard.fqbn) { - throw new Error(`No core is installed for the '${boardsConfig.selectedBoard.name}' board. Please install the core.`); - } - const [fqbn, { selectedProgrammer }] = await Promise.all([ - this.boardsDataStore.appendConfigToFqbn(boardsConfig.selectedBoard.fqbn), - this.boardsDataStore.getData(boardsConfig.selectedBoard.fqbn) + this.boardsDataStore.appendConfigToFqbn(boardsConfig.selectedBoard?.fqbn), + this.boardsDataStore.getData(boardsConfig.selectedBoard?.fqbn) ]); let options: CoreService.Upload.Options | undefined = undefined; const sketchUri = uri; const optimizeForDebug = this.editorMode.compileForDebug; const { selectedPort } = boardsConfig; + const port = selectedPort?.address; if (usingProgrammer) { const programmer = selectedProgrammer; - if (!programmer) { - throw new Error('Programmer is not selected. Please select a programmer from the `Tools` > `Programmer` menu.'); - } - let port: undefined | string = undefined; - // If the port is set by the user, we pass it to the CLI as it might be required. - // If it is not set but the CLI requires it, we let the CLI to complain. - if (selectedPort) { - port = selectedPort.address; - } options = { sketchUri, fqbn, @@ -119,10 +104,6 @@ export class UploadSketch extends SketchContribution { port }; } else { - if (!selectedPort) { - throw new Error('No ports selected. Please select a port.'); - } - const port = selectedPort.address; options = { sketchUri, fqbn, @@ -131,7 +112,11 @@ export class UploadSketch extends SketchContribution { }; } this.outputChannelManager.getChannel('Arduino: upload').clear(); - await this.coreService.upload(options); + if (usingProgrammer) { + await this.coreService.uploadUsingProgrammer(options); + } else { + await this.coreService.upload(options); + } this.messageService.info('Done uploading.', { timeout: 1000 }); } catch (e) { this.messageService.error(e.toString()); diff --git a/arduino-ide-extension/src/browser/contributions/verify-sketch.ts b/arduino-ide-extension/src/browser/contributions/verify-sketch.ts index a4e99b76..2bc54ae3 100644 --- a/arduino-ide-extension/src/browser/contributions/verify-sketch.ts +++ b/arduino-ide-extension/src/browser/contributions/verify-sketch.ts @@ -63,13 +63,7 @@ export class VerifySketch extends SketchContribution { } try { const { boardsConfig } = this.boardsServiceClientImpl; - if (!boardsConfig || !boardsConfig.selectedBoard) { - throw new Error('No boards selected. Please select a board.'); - } - if (!boardsConfig.selectedBoard.fqbn) { - throw new Error(`No core is installed for the '${boardsConfig.selectedBoard.name}' board. Please install the core.`); - } - const fqbn = await this.boardsDataStore.appendConfigToFqbn(boardsConfig.selectedBoard.fqbn); + const fqbn = await this.boardsDataStore.appendConfigToFqbn(boardsConfig.selectedBoard?.fqbn); this.outputChannelManager.getChannel('Arduino: compile').clear(); await this.coreService.compile({ sketchUri: uri, diff --git a/arduino-ide-extension/src/browser/output-service-impl.ts b/arduino-ide-extension/src/browser/output-service-impl.ts index ef2a2735..c0ebe34d 100644 --- a/arduino-ide-extension/src/browser/output-service-impl.ts +++ b/arduino-ide-extension/src/browser/output-service-impl.ts @@ -1,6 +1,6 @@ import { inject, injectable } from 'inversify'; import { OutputContribution } from '@theia/output/lib/browser/output-contribution'; -import { OutputChannelManager, OutputChannelSeverity } from '@theia/output/lib/common/output-channel'; +import { OutputChannelManager } from '@theia/output/lib/common/output-channel'; import { OutputService, OutputMessage } from '../common/protocol/output-service'; @injectable() @@ -22,19 +22,7 @@ export class OutputServiceImpl implements OutputService { // This will open, reveal but do not activate the Output view. : Promise.resolve(channel.show({ preserveFocus: true })); - show.then(() => channel.append(chunk, this.toOutputSeverity(message))); - } - - protected toOutputSeverity(message: OutputMessage): OutputChannelSeverity { - if (message.severity) { - switch (message.severity) { - case 'error': return OutputChannelSeverity.Error - case 'warning': return OutputChannelSeverity.Warning - case 'info': return OutputChannelSeverity.Info - default: return OutputChannelSeverity.Info - } - } - return OutputChannelSeverity.Info + show.then(() => channel.append(chunk)); } } diff --git a/arduino-ide-extension/src/common/protocol/core-service.ts b/arduino-ide-extension/src/common/protocol/core-service.ts index b8367b60..3cc6a60d 100644 --- a/arduino-ide-extension/src/common/protocol/core-service.ts +++ b/arduino-ide-extension/src/common/protocol/core-service.ts @@ -5,6 +5,7 @@ export const CoreService = Symbol('CoreService'); export interface CoreService { compile(options: CoreService.Compile.Options): Promise; upload(options: CoreService.Upload.Options): Promise; + uploadUsingProgrammer(options: CoreService.Upload.Options): Promise; burnBootloader(options: CoreService.Bootloader.Options): Promise; } @@ -13,22 +14,23 @@ export namespace CoreService { export namespace Compile { export interface Options { readonly sketchUri: string; - readonly fqbn: string; + readonly fqbn?: string | undefined; readonly optimizeForDebug: boolean; } } export namespace Upload { - export type Options = - Compile.Options & Readonly<{ port: string }> | - Compile.Options & Readonly<{ programmer: Programmer, port?: string }>; + export interface Options extends Compile.Options { + readonly port?: string | undefined; + readonly programmer?: Programmer | undefined; + } } export namespace Bootloader { export interface Options { - readonly fqbn: string; - readonly programmer: Programmer; - readonly port: string; + readonly fqbn?: string | undefined; + readonly port?: string | undefined; + readonly programmer?: Programmer | undefined; } } diff --git a/arduino-ide-extension/src/common/protocol/output-service.ts b/arduino-ide-extension/src/common/protocol/output-service.ts index d948fafd..f8e0cba2 100644 --- a/arduino-ide-extension/src/common/protocol/output-service.ts +++ b/arduino-ide-extension/src/common/protocol/output-service.ts @@ -1,7 +1,7 @@ export interface OutputMessage { readonly name: string; readonly chunk: string; - readonly severity?: 'error' | 'warning' | 'info'; + readonly severity?: 'error' | 'warning' | 'info'; // Currently not used! } export const OutputServicePath = '/services/output-service'; diff --git a/arduino-ide-extension/src/node/core-service-impl.ts b/arduino-ide-extension/src/node/core-service-impl.ts index 94272ff1..727d6c8d 100644 --- a/arduino-ide-extension/src/node/core-service-impl.ts +++ b/arduino-ide-extension/src/node/core-service-impl.ts @@ -4,9 +4,12 @@ import { dirname } from 'path'; import { CoreService } from '../common/protocol/core-service'; import { CompileReq, CompileResp } from './cli-protocol/commands/compile_pb'; import { CoreClientProvider } from './core-client-provider'; -import { UploadReq, UploadResp, BurnBootloaderReq, BurnBootloaderResp } from './cli-protocol/commands/upload_pb'; +import { UploadReq, UploadResp, BurnBootloaderReq, BurnBootloaderResp, UploadUsingProgrammerReq, UploadUsingProgrammerResp } from './cli-protocol/commands/upload_pb'; import { OutputService } from '../common/protocol/output-service'; import { NotificationServiceServer } from '../common/protocol'; +import { ClientReadableStream } from '@grpc/grpc-js'; +import { ArduinoCoreClient } from './cli-protocol/commands/commands_grpc_pb'; +import { firstToUpperCase, firstToLowerCase } from '../common/utils'; @injectable() export class CoreServiceImpl implements CoreService { @@ -21,7 +24,7 @@ export class CoreServiceImpl implements CoreService { protected readonly notificationService: NotificationServiceServer; async compile(options: CoreService.Compile.Options): Promise { - this.outputService.append({ name: 'compile', chunk: 'Compiling...\n' + JSON.stringify(options, null, 2) + '\n--------------------------\n' }); + this.outputService.append({ name: 'compile', chunk: 'Compile...\n' + JSON.stringify(options, null, 2) + '\n--------------------------\n' }); const { sketchUri, fqbn } = options; const sketchFilePath = FileUri.fsPath(sketchUri); const sketchpath = dirname(sketchFilePath); @@ -32,14 +35,12 @@ export class CoreServiceImpl implements CoreService { } const { client, instance } = coreClient; - if (!fqbn) { - throw new Error('The selected board has no FQBN.'); - } - const compilerReq = new CompileReq(); compilerReq.setInstance(instance); compilerReq.setSketchpath(sketchpath); - compilerReq.setFqbn(fqbn); + if (fqbn) { + compilerReq.setFqbn(fqbn); + } compilerReq.setOptimizefordebug(options.optimizeForDebug); compilerReq.setPreprocess(false); compilerReq.setVerbose(true); @@ -63,9 +64,23 @@ export class CoreServiceImpl implements CoreService { } async upload(options: CoreService.Upload.Options): Promise { + await this.doUpload(options, () => new UploadReq(), (client, req) => client.upload(req)); + } + + async uploadUsingProgrammer(options: CoreService.Upload.Options): Promise { + await this.doUpload(options, () => new UploadUsingProgrammerReq(), (client, req) => client.uploadUsingProgrammer(req), 'upload using programmer'); + } + + protected async doUpload( + options: CoreService.Upload.Options, + requestProvider: () => UploadReq | UploadUsingProgrammerReq, + responseHandler: (client: ArduinoCoreClient, req: UploadReq | UploadUsingProgrammerReq) => ClientReadableStream, + task: string = 'upload'): Promise { + await this.compile(options); - this.outputService.append({ name: 'upload', chunk: 'Uploading...\n' + JSON.stringify(options, null, 2) + '\n--------------------------\n' }); - const { sketchUri, fqbn } = options; + const chunk = firstToUpperCase(task) + '...\n'; + this.outputService.append({ name: 'upload', chunk: chunk + JSON.stringify(options, null, 2) + '\n--------------------------\n' }); + const { sketchUri, fqbn, port, programmer } = options; const sketchFilePath = FileUri.fsPath(sketchUri); const sketchpath = dirname(sketchFilePath); @@ -75,34 +90,32 @@ export class CoreServiceImpl implements CoreService { } const { client, instance } = coreClient; - if (!fqbn) { - throw new Error('The selected board has no FQBN.'); + const req = requestProvider(); + req.setInstance(instance); + req.setSketchPath(sketchpath); + if (fqbn) { + req.setFqbn(fqbn); } - - const uploadReq = new UploadReq(); - uploadReq.setInstance(instance); - uploadReq.setSketchPath(sketchpath); - uploadReq.setFqbn(fqbn); - if ('programmer' in options) { - uploadReq.setProgrammer(options.programmer.id); + if (port) { + req.setPort(port); } - if (options.port) { - uploadReq.setPort(options.port); + if (programmer) { + req.setProgrammer(programmer.id); } - const result = client.upload(uploadReq); + const result = responseHandler(client, req); try { await new Promise((resolve, reject) => { result.on('data', (resp: UploadResp) => { - this.outputService.append({ name: 'upload', chunk: Buffer.from(resp.getOutStream_asU8()).toString() }); - this.outputService.append({ name: 'upload', chunk: Buffer.from(resp.getErrStream_asU8()).toString() }); + this.outputService.append({ name: task, chunk: Buffer.from(resp.getOutStream_asU8()).toString() }); + this.outputService.append({ name: task, chunk: Buffer.from(resp.getErrStream_asU8()).toString() }); }); result.on('error', error => reject(error)); result.on('end', () => resolve()); }); - this.outputService.append({ name: 'upload', chunk: '\n--------------------------\nUpload complete.\n' }); + this.outputService.append({ name: 'upload', chunk: '\n--------------------------\n' + firstToLowerCase(task) + ' complete.\n' }); } catch (e) { - this.outputService.append({ name: 'upload', chunk: `Upload error: ${e}\n`, severity: 'error' }); + this.outputService.append({ name: 'upload', chunk: `${firstToUpperCase(task)} error: ${e}\n`, severity: 'error' }); throw e; } } @@ -113,19 +126,19 @@ export class CoreServiceImpl implements CoreService { return; } const { fqbn, port, programmer } = options; - if (!fqbn) { - throw new Error('The selected board has no FQBN.'); - } - if (!port) { - throw new Error('Port must be specified.'); - } const { client, instance } = coreClient; - const req = new BurnBootloaderReq(); - req.setFqbn(fqbn); - req.setPort(port); - req.setProgrammer(programmer.id); - req.setInstance(instance); - const result = client.burnBootloader(req); + const burnReq = new BurnBootloaderReq(); + burnReq.setInstance(instance); + if (fqbn) { + burnReq.setFqbn(fqbn); + } + if (port) { + burnReq.setPort(port); + } + if (programmer) { + burnReq.setProgrammer(programmer.id); + } + const result = client.burnBootloader(burnReq); try { await new Promise((resolve, reject) => { result.on('data', (resp: BurnBootloaderResp) => {