Skip to content

Commit 48b9c4d

Browse files
prevent monitor side effects on upload (WIP)
1 parent 9585991 commit 48b9c4d

File tree

3 files changed

+62
-24
lines changed

3 files changed

+62
-24
lines changed

arduino-ide-extension/src/node/core-service-impl.ts

+4-2
Original file line numberDiff line numberDiff line change
@@ -140,7 +140,6 @@ export class CoreServiceImpl extends CoreClientAware implements CoreService {
140140

141141
this.uploading = true;
142142
const { sketchUri, board, port, programmer } = options;
143-
await this.monitorManager.notifyUploadStarted(board, port);
144143

145144
const sketchPath = FileUri.fsPath(sketchUri);
146145

@@ -175,6 +174,8 @@ export class CoreServiceImpl extends CoreClientAware implements CoreService {
175174
const result = responseHandler(client, req);
176175

177176
try {
177+
await this.monitorManager.notifyUploadStarted(board, port);
178+
178179
await new Promise<void>((resolve, reject) => {
179180
result.on('data', (resp: UploadResponse) => {
180181
this.responseService.appendToOutput({
@@ -207,7 +208,8 @@ export class CoreServiceImpl extends CoreClientAware implements CoreService {
207208
throw new Error(errorMessage);
208209
} finally {
209210
this.uploading = false;
210-
this.monitorManager.notifyUploadFinished(board, port);
211+
await this.monitorManager.notifyUploadFinished(board, port);
212+
await this.monitorManager.startQueuedServices();
211213
}
212214
}
213215

arduino-ide-extension/src/node/monitor-manager-proxy-impl.ts

+8-5
Original file line numberDiff line numberDiff line change
@@ -40,11 +40,14 @@ export class MonitorManagerProxyImpl implements MonitorManagerProxy {
4040
if (settings) {
4141
await this.changeMonitorSettings(board, port, settings);
4242
}
43-
const status = await this.manager.startMonitor(board, port);
44-
if (status === Status.ALREADY_CONNECTED || status === Status.OK) {
45-
// Monitor started correctly, connect it with the frontend
46-
this.client.connect(this.manager.getWebsocketAddressPort(board, port));
47-
}
43+
44+
const onFinish = (status: Status) => {
45+
if (status === Status.ALREADY_CONNECTED || status === Status.OK) {
46+
// Monitor started correctly, connect it with the frontend
47+
this.client.connect(this.manager.getWebsocketAddressPort(board, port));
48+
}
49+
};
50+
return this.manager.startMonitor(board, port, onFinish);
4851
}
4952

5053
/**

arduino-ide-extension/src/node/monitor-manager.ts

+50-17
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { ILogger } from '@theia/core';
22
import { inject, injectable, named } from '@theia/core/shared/inversify';
3-
import { Board, Port, Status } from '../common/protocol';
3+
import { Board, BoardsService, Port, Status } from '../common/protocol';
44
import { CoreClientAware } from './core-client-provider';
55
import { MonitorService } from './monitor-service';
66
import { MonitorServiceFactory } from './monitor-service-factory';
@@ -15,14 +15,20 @@ export const MonitorManagerName = 'monitor-manager';
1515

1616
@injectable()
1717
export class MonitorManager extends CoreClientAware {
18+
@inject(BoardsService)
19+
protected boardsService: BoardsService;
20+
1821
// Map of monitor services that manage the running pluggable monitors.
1922
// Each service handles the lifetime of one, and only one, monitor.
2023
// If either the board or port managed changes, a new service must
2124
// be started.
2225
private monitorServices = new Map<MonitorID, MonitorService>();
2326
private isUploadInProgress: boolean;
2427

25-
private startMonitorPendingRequests: Array<MonitorID> = [];
28+
private startMonitorPendingRequests: [
29+
[Board, Port],
30+
(status: Status) => void
31+
][] = [];
2632

2733
@inject(MonitorServiceFactory)
2834
private monitorServiceFactory: MonitorServiceFactory;
@@ -59,17 +65,25 @@ export class MonitorManager extends CoreClientAware {
5965
* @returns a Status object to know if the process has been
6066
* started or if there have been errors.
6167
*/
62-
async startMonitor(board: Board, port: Port): Promise<Status> {
68+
async startMonitor(
69+
board: Board,
70+
port: Port,
71+
postStartCallback: (status: Status) => void
72+
): Promise<void> {
6373
const monitorID = this.monitorID(board, port);
74+
6475
let monitor = this.monitorServices.get(monitorID);
6576
if (!monitor) {
6677
monitor = this.createMonitor(board, port);
6778
}
79+
6880
if (this.isUploadInProgress) {
69-
this.startMonitorPendingRequests.push(monitorID);
70-
return Status.UPLOAD_IN_PROGRESS;
81+
this.startMonitorPendingRequests.push([[board, port], postStartCallback]);
82+
return;
7183
}
72-
return monitor.start();
84+
85+
const result = await monitor.start();
86+
postStartCallback(result);
7387
}
7488

7589
/**
@@ -113,6 +127,8 @@ export class MonitorManager extends CoreClientAware {
113127
* @param port port to monitor
114128
*/
115129
async notifyUploadStarted(board?: Board, port?: Port): Promise<void> {
130+
this.isUploadInProgress = true;
131+
116132
if (!board || !port) {
117133
// We have no way of knowing which monitor
118134
// to retrieve if we don't have this information.
@@ -124,8 +140,7 @@ export class MonitorManager extends CoreClientAware {
124140
// There's no monitor running there, bail
125141
return;
126142
}
127-
this.isUploadInProgress = true;
128-
return await monitor.pause();
143+
return monitor.pause();
129144
}
130145

131146
/**
@@ -137,14 +152,8 @@ export class MonitorManager extends CoreClientAware {
137152
* started or if there have been errors.
138153
*/
139154
async notifyUploadFinished(board?: Board, port?: Port): Promise<Status> {
140-
try {
141-
for (const id of this.startMonitorPendingRequests) {
142-
const m = this.monitorServices.get(id);
143-
if (m) m.start();
144-
}
145-
} finally {
146-
this.startMonitorPendingRequests = [];
147-
}
155+
this.isUploadInProgress = false;
156+
148157
if (!board || !port) {
149158
// We have no way of knowing which monitor
150159
// to retrieve if we don't have this information.
@@ -156,11 +165,35 @@ export class MonitorManager extends CoreClientAware {
156165
// There's no monitor running there, bail
157166
return Status.NOT_CONNECTED;
158167
}
159-
this.isUploadInProgress = false;
160168

161169
return monitor.start();
162170
}
163171

172+
async startQueuedServices(): Promise<void> {
173+
const queued = this.startMonitorPendingRequests;
174+
this.startMonitorPendingRequests = [];
175+
176+
for (const [[board, port], onFinish] of queued) {
177+
const boardsState = await this.boardsService.getState();
178+
const boardIsStillOnPort = Object.keys(boardsState)
179+
.map((connection: string) => {
180+
const portAddress = connection.split('|')[0];
181+
return portAddress;
182+
})
183+
.some((portAddress: string) => port.address === portAddress);
184+
185+
if (boardIsStillOnPort) {
186+
const monitorID = this.monitorID(board, port);
187+
const monitorService = this.monitorServices.get(monitorID);
188+
189+
if (monitorService) {
190+
const result = await monitorService.start();
191+
onFinish(result);
192+
}
193+
}
194+
}
195+
}
196+
164197
/**
165198
* Changes the settings of a pluggable monitor even if it's running.
166199
* If monitor is not running they're going to be used as soon as it's started.

0 commit comments

Comments
 (0)