From 671d2eabd4eb9b08dd49262f565a369b5cb4e926 Mon Sep 17 00:00:00 2001 From: Alberto Iannaccone Date: Tue, 20 Sep 2022 09:27:09 +0200 Subject: [PATCH] Show user fields dialog again if upload fails (#1415) * Show user fields dialog again if upload fails * move user fields logic into own contribution * apply suggestions --- .../browser/arduino-ide-frontend-module.ts | 2 + .../browser/contributions/upload-sketch.ts | 155 ++++-------------- .../src/browser/contributions/user-fields.ts | 150 +++++++++++++++++ 3 files changed, 180 insertions(+), 127 deletions(-) create mode 100644 arduino-ide-extension/src/browser/contributions/user-fields.ts 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 000e9d0f9..d6e9571f7 100644 --- a/arduino-ide-extension/src/browser/arduino-ide-frontend-module.ts +++ b/arduino-ide-extension/src/browser/arduino-ide-frontend-module.ts @@ -337,6 +337,7 @@ import { CheckForUpdates } from './contributions/check-for-updates'; import { OutputEditorFactory } from './theia/output/output-editor-factory'; import { StartupTaskProvider } from '../electron-common/startup-task'; import { DeleteSketch } from './contributions/delete-sketch'; +import { UserFields } from './contributions/user-fields'; const registerArduinoThemes = () => { const themes: MonacoThemeJson[] = [ @@ -761,6 +762,7 @@ export default new ContainerModule((bind, unbind, isBound, rebind) => { Contribution.configure(bind, OpenBoardsConfig); Contribution.configure(bind, SketchFilesTracker); Contribution.configure(bind, CheckForUpdates); + Contribution.configure(bind, UserFields); Contribution.configure(bind, DeleteSketch); bindContributionProvider(bind, StartupTaskProvider); diff --git a/arduino-ide-extension/src/browser/contributions/upload-sketch.ts b/arduino-ide-extension/src/browser/contributions/upload-sketch.ts index 5d6135cec..f337fb1d7 100644 --- a/arduino-ide-extension/src/browser/contributions/upload-sketch.ts +++ b/arduino-ide-extension/src/browser/contributions/upload-sketch.ts @@ -1,7 +1,7 @@ import { inject, injectable } from '@theia/core/shared/inversify'; import { Emitter } from '@theia/core/lib/common/event'; -import { BoardUserField, CoreService, Port } from '../../common/protocol'; -import { ArduinoMenus, PlaceholderMenuNode } from '../menu/arduino-menus'; +import { CoreService, Port } from '../../common/protocol'; +import { ArduinoMenus } from '../menu/arduino-menus'; import { ArduinoToolbar } from '../toolbar/arduino-toolbar'; import { Command, @@ -11,96 +11,36 @@ import { TabBarToolbarRegistry, CoreServiceContribution, } from './contribution'; -import { UserFieldsDialog } from '../dialogs/user-fields/user-fields-dialog'; -import { deepClone, DisposableCollection, nls } from '@theia/core/lib/common'; +import { deepClone, nls } from '@theia/core/lib/common'; import { CurrentSketch } from '../../common/protocol/sketches-service-client-impl'; import type { VerifySketchParams } from './verify-sketch'; +import { UserFields } from './user-fields'; @injectable() export class UploadSketch extends CoreServiceContribution { - @inject(MenuModelRegistry) - private readonly menuRegistry: MenuModelRegistry; - - @inject(UserFieldsDialog) - private readonly userFieldsDialog: UserFieldsDialog; - - private boardRequiresUserFields = false; - private readonly cachedUserFields: Map = new Map(); - private readonly menuActionsDisposables = new DisposableCollection(); - private readonly onDidChangeEmitter = new Emitter(); private readonly onDidChange = this.onDidChangeEmitter.event; private uploadInProgress = false; - protected override init(): void { - super.init(); - this.boardsServiceProvider.onBoardsConfigChanged(async () => { - const userFields = - await this.boardsServiceProvider.selectedBoardUserFields(); - this.boardRequiresUserFields = userFields.length > 0; - this.registerMenus(this.menuRegistry); - }); - } - - private selectedFqbnAddress(): string { - const { boardsConfig } = this.boardsServiceProvider; - const fqbn = boardsConfig.selectedBoard?.fqbn; - if (!fqbn) { - return ''; - } - const address = - boardsConfig.selectedBoard?.port?.address || - boardsConfig.selectedPort?.address; - if (!address) { - return ''; - } - return fqbn + '|' + address; - } + @inject(UserFields) + private readonly userFields: UserFields; override registerCommands(registry: CommandRegistry): void { registry.registerCommand(UploadSketch.Commands.UPLOAD_SKETCH, { execute: async () => { - const key = this.selectedFqbnAddress(); - if ( - this.boardRequiresUserFields && - key && - !this.cachedUserFields.has(key) - ) { - // Deep clone the array of board fields to avoid editing the cached ones - this.userFieldsDialog.value = ( - await this.boardsServiceProvider.selectedBoardUserFields() - ).map((f) => ({ ...f })); - const result = await this.userFieldsDialog.open(); - if (!result) { - return; - } - this.cachedUserFields.set(key, result); + if (await this.userFields.checkUserFieldsDialog()) { + this.uploadSketch(); } - this.uploadSketch(); }, isEnabled: () => !this.uploadInProgress, }); registry.registerCommand(UploadSketch.Commands.UPLOAD_WITH_CONFIGURATION, { execute: async () => { - const key = this.selectedFqbnAddress(); - if (!key) { - return; - } - - const cached = this.cachedUserFields.get(key); - // Deep clone the array of board fields to avoid editing the cached ones - this.userFieldsDialog.value = ( - cached ?? (await this.boardsServiceProvider.selectedBoardUserFields()) - ).map((f) => ({ ...f })); - - const result = await this.userFieldsDialog.open(); - if (!result) { - return; + if (await this.userFields.checkUserFieldsDialog(true)) { + this.uploadSketch(); } - this.cachedUserFields.set(key, result); - this.uploadSketch(); }, - isEnabled: () => !this.uploadInProgress && this.boardRequiresUserFields, + isEnabled: () => !this.uploadInProgress && this.userFields.isRequired(), }); registry.registerCommand( UploadSketch.Commands.UPLOAD_SKETCH_USING_PROGRAMMER, @@ -120,45 +60,20 @@ export class UploadSketch extends CoreServiceContribution { } override registerMenus(registry: MenuModelRegistry): void { - this.menuActionsDisposables.dispose(); - this.menuActionsDisposables.push( - registry.registerMenuAction(ArduinoMenus.SKETCH__MAIN_GROUP, { - commandId: UploadSketch.Commands.UPLOAD_SKETCH.id, - label: nls.localize('arduino/sketch/upload', 'Upload'), - order: '1', - }) - ); - if (this.boardRequiresUserFields) { - this.menuActionsDisposables.push( - registry.registerMenuAction(ArduinoMenus.SKETCH__MAIN_GROUP, { - commandId: UploadSketch.Commands.UPLOAD_WITH_CONFIGURATION.id, - label: UploadSketch.Commands.UPLOAD_WITH_CONFIGURATION.label, - order: '2', - }) - ); - } else { - this.menuActionsDisposables.push( - registry.registerMenuNode( - ArduinoMenus.SKETCH__MAIN_GROUP, - new PlaceholderMenuNode( - ArduinoMenus.SKETCH__MAIN_GROUP, - // commandId: UploadSketch.Commands.UPLOAD_WITH_CONFIGURATION.id, - UploadSketch.Commands.UPLOAD_WITH_CONFIGURATION.label, - { order: '2' } - ) - ) - ); - } - this.menuActionsDisposables.push( - registry.registerMenuAction(ArduinoMenus.SKETCH__MAIN_GROUP, { - commandId: UploadSketch.Commands.UPLOAD_SKETCH_USING_PROGRAMMER.id, - label: nls.localize( - 'arduino/sketch/uploadUsingProgrammer', - 'Upload Using Programmer' - ), - order: '3', - }) - ); + registry.registerMenuAction(ArduinoMenus.SKETCH__MAIN_GROUP, { + commandId: UploadSketch.Commands.UPLOAD_SKETCH.id, + label: nls.localize('arduino/sketch/upload', 'Upload'), + order: '1', + }); + + registry.registerMenuAction(ArduinoMenus.SKETCH__MAIN_GROUP, { + commandId: UploadSketch.Commands.UPLOAD_SKETCH_USING_PROGRAMMER.id, + label: nls.localize( + 'arduino/sketch/uploadUsingProgrammer', + 'Upload Using Programmer' + ), + order: '3', + }); } override registerKeybindings(registry: KeybindingRegistry): void { @@ -215,18 +130,7 @@ export class UploadSketch extends CoreServiceContribution { return; } - // TODO: This does not belong here. - // IDE2 should not do any preliminary checks but let the CLI fail and then toast a user consumable error message. - if ( - uploadOptions.userFields.length === 0 && - this.boardRequiresUserFields - ) { - this.messageService.error( - nls.localize( - 'arduino/sketch/userFieldsNotFoundError', - "Can't find user fields for connected board" - ) - ); + if (!this.userFields.checkUserFieldsForUpload()) { return; } @@ -242,6 +146,7 @@ export class UploadSketch extends CoreServiceContribution { { timeout: 3000 } ); } catch (e) { + this.userFields.notifyFailedWithError(e); this.handleError(e); } finally { this.uploadInProgress = false; @@ -258,7 +163,7 @@ export class UploadSketch extends CoreServiceContribution { if (!CurrentSketch.isValid(sketch)) { return undefined; } - const userFields = this.userFields(); + const userFields = this.userFields.getUserFields(); const { boardsConfig } = this.boardsServiceProvider; const [fqbn, { selectedProgrammer: programmer }, verify, verbose] = await Promise.all([ @@ -301,10 +206,6 @@ export class UploadSketch extends CoreServiceContribution { return port; } - private userFields(): BoardUserField[] { - return this.cachedUserFields.get(this.selectedFqbnAddress()) ?? []; - } - /** * Converts the `VENDOR:ARCHITECTURE:BOARD_ID[:MENU_ID=OPTION_ID[,MENU2_ID=OPTION_ID ...]]` FQBN to * `VENDOR:ARCHITECTURE:BOARD_ID` format. diff --git a/arduino-ide-extension/src/browser/contributions/user-fields.ts b/arduino-ide-extension/src/browser/contributions/user-fields.ts new file mode 100644 index 000000000..c73ead9e6 --- /dev/null +++ b/arduino-ide-extension/src/browser/contributions/user-fields.ts @@ -0,0 +1,150 @@ +import { inject, injectable } from '@theia/core/shared/inversify'; +import { DisposableCollection, nls } from '@theia/core/lib/common'; +import { BoardUserField, CoreError } from '../../common/protocol'; +import { BoardsServiceProvider } from '../boards/boards-service-provider'; +import { UserFieldsDialog } from '../dialogs/user-fields/user-fields-dialog'; +import { ArduinoMenus, PlaceholderMenuNode } from '../menu/arduino-menus'; +import { MenuModelRegistry, Contribution } from './contribution'; +import { UploadSketch } from './upload-sketch'; + +@injectable() +export class UserFields extends Contribution { + private boardRequiresUserFields = false; + private userFieldsSet = false; + private readonly cachedUserFields: Map = new Map(); + private readonly menuActionsDisposables = new DisposableCollection(); + + @inject(UserFieldsDialog) + private readonly userFieldsDialog: UserFieldsDialog; + + @inject(BoardsServiceProvider) + private readonly boardsServiceProvider: BoardsServiceProvider; + + @inject(MenuModelRegistry) + private readonly menuRegistry: MenuModelRegistry; + + protected override init(): void { + super.init(); + this.boardsServiceProvider.onBoardsConfigChanged(async () => { + const userFields = + await this.boardsServiceProvider.selectedBoardUserFields(); + this.boardRequiresUserFields = userFields.length > 0; + this.registerMenus(this.menuRegistry); + }); + } + + override registerMenus(registry: MenuModelRegistry): void { + this.menuActionsDisposables.dispose(); + if (this.boardRequiresUserFields) { + this.menuActionsDisposables.push( + registry.registerMenuAction(ArduinoMenus.SKETCH__MAIN_GROUP, { + commandId: UploadSketch.Commands.UPLOAD_WITH_CONFIGURATION.id, + label: UploadSketch.Commands.UPLOAD_WITH_CONFIGURATION.label, + order: '2', + }) + ); + } else { + this.menuActionsDisposables.push( + registry.registerMenuNode( + ArduinoMenus.SKETCH__MAIN_GROUP, + new PlaceholderMenuNode( + ArduinoMenus.SKETCH__MAIN_GROUP, + // commandId: UploadSketch.Commands.UPLOAD_WITH_CONFIGURATION.id, + UploadSketch.Commands.UPLOAD_WITH_CONFIGURATION.label, + { order: '2' } + ) + ) + ); + } + } + + private selectedFqbnAddress(): string | undefined { + const { boardsConfig } = this.boardsServiceProvider; + const fqbn = boardsConfig.selectedBoard?.fqbn; + if (!fqbn) { + return undefined; + } + const address = + boardsConfig.selectedBoard?.port?.address || + boardsConfig.selectedPort?.address; + if (!address) { + return undefined; + } + return fqbn + '|' + address; + } + + private async showUserFieldsDialog( + key: string + ): Promise { + const cached = this.cachedUserFields.get(key); + // Deep clone the array of board fields to avoid editing the cached ones + this.userFieldsDialog.value = cached ? cached.slice() : await this.boardsServiceProvider.selectedBoardUserFields(); + const result = await this.userFieldsDialog.open(); + if (!result) { + return; + } + + this.userFieldsSet = true; + this.cachedUserFields.set(key, result); + return result; + } + + async checkUserFieldsDialog(forceOpen = false): Promise { + const key = this.selectedFqbnAddress(); + if (!key) { + return false; + } + /* + If the board requires to be configured with user fields, we want + to show the user fields dialog, but only if they weren't already + filled in or if they were filled in, but the previous upload failed. + */ + if ( + !forceOpen && + (!this.boardRequiresUserFields || + (this.cachedUserFields.has(key) && this.userFieldsSet)) + ) { + return true; + } + const userFieldsFilledIn = Boolean(await this.showUserFieldsDialog(key)); + return userFieldsFilledIn; + } + + checkUserFieldsForUpload(): boolean { + // TODO: This does not belong here. + // IDE2 should not do any preliminary checks but let the CLI fail and then toast a user consumable error message. + if (!this.boardRequiresUserFields || this.getUserFields().length > 0) { + this.userFieldsSet = true; + return true; + } + this.messageService.error( + nls.localize( + 'arduino/sketch/userFieldsNotFoundError', + "Can't find user fields for connected board" + ) + ); + this.userFieldsSet = false; + return false; + } + + getUserFields(): BoardUserField[] { + const fqbnAddress = this.selectedFqbnAddress(); + if (!fqbnAddress) { + return []; + } + return this.cachedUserFields.get(fqbnAddress) ?? []; + } + + isRequired(): boolean { + return this.boardRequiresUserFields; + } + + notifyFailedWithError(e: Error): void { + if ( + this.boardRequiresUserFields && + CoreError.UploadFailed.is(e) + ) { + this.userFieldsSet = false; + } + } +}