Skip to content

Commit

Permalink
avoid starting monitor when upload is in progress
Browse files Browse the repository at this point in the history
  • Loading branch information
Alberto Iannaccone committed Jun 14, 2022
1 parent fff6075 commit cc12bc0
Show file tree
Hide file tree
Showing 5 changed files with 20 additions and 41 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -219,12 +219,6 @@ export class UploadSketch extends SketchContribution {
}

try {
// here we inform the "monitorManagerProxyClient" an upload is in progress,
// setting a boolean flag, this is to prevent triggering of the
// "usual side effects" if a serial port change occurs during upload
// (expected on windows for some boards)
this.monitorManagerProxyClient.setUploadInProgress(true);

const { boardsConfig } = this.boardsServiceClientImpl;
const [fqbn, { selectedProgrammer }, verify, verbose, sourceOverride] =
await Promise.all([
Expand Down Expand Up @@ -305,8 +299,6 @@ export class UploadSketch extends SketchContribution {
} finally {
this.uploadInProgress = false;

this.monitorManagerProxyClient.setUploadInProgress(false);

this.onDidChangeEmitter.fire();
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,6 @@ export class MonitorManagerProxyClientImpl
private lastConnectedBoard: BoardsConfig.Config;
private onBoardsConfigChanged: Disposable | undefined;

private uploadInProgress = false;

getWebSocketPort(): number | undefined {
return this.wsPort;
}
Expand Down Expand Up @@ -137,14 +135,6 @@ export class MonitorManagerProxyClientImpl
this.onBoardsConfigChanged =
this.boardsServiceProvider.onBoardsConfigChanged(
async ({ selectedBoard, selectedPort }) => {
const changeTriggeredDuringUpload = this.uploadInProgress;
if (
changeTriggeredDuringUpload ||
typeof selectedBoard === 'undefined' ||
typeof selectedPort === 'undefined'
)
return;

// a board is plugged and it's different from the old connected board
if (
selectedBoard?.fqbn !==
Expand Down Expand Up @@ -200,8 +190,4 @@ export class MonitorManagerProxyClientImpl
})
);
}

public setUploadInProgress(value: boolean): void {
this.uploadInProgress = value;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@ export interface MonitorManagerProxyClient {
getCurrentSettings(board: Board, port: Port): Promise<MonitorSettings>;
send(message: string): void;
changeSettings(settings: MonitorSettings): void;
setUploadInProgress(value: boolean): void;
}

export interface PluggableMonitorSetting {
Expand Down
24 changes: 20 additions & 4 deletions arduino-ide-extension/src/node/monitor-manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,9 @@ export class MonitorManager extends CoreClientAware {
// If either the board or port managed changes, a new service must
// be started.
private monitorServices = new Map<MonitorID, MonitorService>();
private isUploadInProgress: boolean;

private startMonitorPendingRequests: Array<MonitorID> = [];

@inject(MonitorServiceFactory)
private monitorServiceFactory: MonitorServiceFactory;
Expand Down Expand Up @@ -62,7 +65,11 @@ export class MonitorManager extends CoreClientAware {
if (!monitor) {
monitor = this.createMonitor(board, port);
}
return await monitor.start();
if (this.isUploadInProgress) {
this.startMonitorPendingRequests.push(monitorID);
return Status.UPLOAD_IN_PROGRESS;
}
return monitor.start();
}

/**
Expand Down Expand Up @@ -117,7 +124,7 @@ export class MonitorManager extends CoreClientAware {
// There's no monitor running there, bail
return;
}
monitor.setUploadInProgress(true);
this.isUploadInProgress = true;
return await monitor.pause();
}

Expand All @@ -130,6 +137,14 @@ export class MonitorManager extends CoreClientAware {
* started or if there have been errors.
*/
async notifyUploadFinished(board?: Board, port?: Port): Promise<Status> {
try {
for (const id of this.startMonitorPendingRequests) {
const m = this.monitorServices.get(id);
if (m) m.start();
}
} finally {
this.startMonitorPendingRequests = [];
}
if (!board || !port) {
// We have no way of knowing which monitor
// to retrieve if we don't have this information.
Expand All @@ -141,8 +156,9 @@ export class MonitorManager extends CoreClientAware {
// There's no monitor running there, bail
return Status.NOT_CONNECTED;
}
monitor.setUploadInProgress(false);
return await monitor.start();
this.isUploadInProgress = false;

return monitor.start();
}

/**
Expand Down
14 changes: 0 additions & 14 deletions arduino-ide-extension/src/node/monitor-service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,6 @@ export class MonitorService extends CoreClientAware implements Disposable {
protected readonly onDisposeEmitter = new Emitter<void>();
readonly onDispose = this.onDisposeEmitter.event;

protected uploadInProgress = false;
protected _initialized = new Deferred<void>();
protected creating: Deferred<Status>;

Expand Down Expand Up @@ -114,10 +113,6 @@ export class MonitorService extends CoreClientAware implements Disposable {
return this._initialized.promise;
}

setUploadInProgress(status: boolean): void {
this.uploadInProgress = status;
}

getWebsocketAddressPort(): number {
return this.webSocketProvider.getAddress().port;
}
Expand Down Expand Up @@ -161,15 +156,6 @@ export class MonitorService extends CoreClientAware implements Disposable {
return this.creating.promise;
}

if (this.uploadInProgress) {
this.updateClientsSettings({
monitorUISettings: { connected: false, serialPort: this.port.address },
});

this.creating.resolve(Status.UPLOAD_IN_PROGRESS);
return this.creating.promise;
}

this.logger.info('starting monitor');

// get default monitor settings from the CLI
Expand Down

0 comments on commit cc12bc0

Please sign in to comment.