Skip to content

Commit 7f2b849

Browse files
authoredJul 6, 2022
#854 fix platform installation only offered if port is selected (#1130)
* ensure desired prompts shown + refactor * pr review changes
1 parent 0ce065e commit 7f2b849

File tree

2 files changed

+259
-97
lines changed

2 files changed

+259
-97
lines changed
 

‎arduino-ide-extension/src/browser/boards/boards-auto-installer.ts

+232-83
Original file line numberDiff line numberDiff line change
@@ -5,19 +5,37 @@ import {
55
BoardsService,
66
BoardsPackage,
77
Board,
8+
Port,
89
} from '../../common/protocol/boards-service';
910
import { BoardsServiceProvider } from './boards-service-provider';
10-
import { BoardsConfig } from './boards-config';
1111
import { Installable, ResponseServiceArduino } from '../../common/protocol';
1212
import { BoardsListWidgetFrontendContribution } from './boards-widget-frontend-contribution';
1313
import { nls } from '@theia/core/lib/common';
14+
import { NotificationCenter } from '../notification-center';
15+
16+
interface AutoInstallPromptAction {
17+
// isAcceptance, whether or not the action indicates acceptance of auto-install proposal
18+
isAcceptance?: boolean;
19+
key: string;
20+
handler: (...args: unknown[]) => unknown;
21+
}
22+
23+
type AutoInstallPromptActions = AutoInstallPromptAction[];
1424

1525
/**
1626
* Listens on `BoardsConfig.Config` changes, if a board is selected which does not
1727
* have the corresponding core installed, it proposes the user to install the core.
1828
*/
29+
30+
// * Cases in which we do not show the auto-install prompt:
31+
// 1. When a related platform is already installed
32+
// 2. When a prompt is already showing in the UI
33+
// 3. When a board is unplugged
1934
@injectable()
2035
export class BoardsAutoInstaller implements FrontendApplicationContribution {
36+
@inject(NotificationCenter)
37+
private readonly notificationCenter: NotificationCenter;
38+
2139
@inject(MessageService)
2240
protected readonly messageService: MessageService;
2341

@@ -36,97 +54,228 @@ export class BoardsAutoInstaller implements FrontendApplicationContribution {
3654
// Workaround for https://github.com/eclipse-theia/theia/issues/9349
3755
protected notifications: Board[] = [];
3856

57+
// * "refusal" meaning a "prompt action" not accepting the auto-install offer ("X" or "install manually")
58+
// we can use "portSelectedOnLastRefusal" to deduce when a board is unplugged after a user has "refused"
59+
// an auto-install prompt. Important to know as we do not want "an unplug" to trigger a "refused" prompt
60+
// showing again
61+
private portSelectedOnLastRefusal: Port | undefined;
62+
private lastRefusedPackageId: string | undefined;
63+
3964
onStart(): void {
40-
this.boardsServiceClient.onBoardsConfigChanged(
41-
this.ensureCoreExists.bind(this)
42-
);
43-
this.ensureCoreExists(this.boardsServiceClient.boardsConfig);
44-
}
65+
const setEventListeners = () => {
66+
this.boardsServiceClient.onBoardsConfigChanged((config) => {
67+
const { selectedBoard, selectedPort } = config;
68+
69+
const boardWasUnplugged =
70+
!selectedPort && this.portSelectedOnLastRefusal;
71+
72+
this.clearLastRefusedPromptInfo();
4573

46-
protected ensureCoreExists(config: BoardsConfig.Config): void {
47-
const { selectedBoard, selectedPort } = config;
48-
if (
49-
selectedBoard &&
50-
selectedPort &&
51-
!this.notifications.find((board) => Board.sameAs(board, selectedBoard))
52-
) {
53-
this.notifications.push(selectedBoard);
54-
this.boardsService.search({}).then((packages) => {
55-
// filter packagesForBoard selecting matches from the cli (installed packages)
56-
// and matches based on the board name
57-
// NOTE: this ensures the Deprecated & new packages are all in the array
58-
// so that we can check if any of the valid packages is already installed
59-
const packagesForBoard = packages.filter(
60-
(pkg) =>
61-
BoardsPackage.contains(selectedBoard, pkg) ||
62-
pkg.boards.some((board) => board.name === selectedBoard.name)
63-
);
64-
65-
// check if one of the packages for the board is already installed. if so, no hint
6674
if (
67-
packagesForBoard.some(({ installedVersion }) => !!installedVersion)
75+
boardWasUnplugged ||
76+
!selectedBoard ||
77+
this.promptAlreadyShowingForBoard(selectedBoard)
6878
) {
6979
return;
7080
}
7181

72-
// filter the installable (not installed) packages,
73-
// CLI returns the packages already sorted with the deprecated ones at the end of the list
74-
// in order to ensure the new ones are preferred
75-
const candidates = packagesForBoard.filter(
76-
({ installable, installedVersion }) =>
77-
installable && !installedVersion
78-
);
79-
80-
const candidate = candidates[0];
81-
if (candidate) {
82-
const version = candidate.availableVersions[0]
83-
? `[v ${candidate.availableVersions[0]}]`
84-
: '';
85-
const yes = nls.localize('vscode/extensionsUtils/yes', 'Yes');
86-
const manualInstall = nls.localize(
87-
'arduino/board/installManually',
88-
'Install Manually'
89-
);
90-
// tslint:disable-next-line:max-line-length
91-
this.messageService
92-
.info(
93-
nls.localize(
94-
'arduino/board/installNow',
95-
'The "{0} {1}" core has to be installed for the currently selected "{2}" board. Do you want to install it now?',
96-
candidate.name,
97-
version,
98-
selectedBoard.name
99-
),
100-
manualInstall,
101-
yes
102-
)
103-
.then(async (answer) => {
104-
const index = this.notifications.findIndex((board) =>
105-
Board.sameAs(board, selectedBoard)
106-
);
107-
if (index !== -1) {
108-
this.notifications.splice(index, 1);
109-
}
110-
if (answer === yes) {
111-
await Installable.installWithProgress({
112-
installable: this.boardsService,
113-
item: candidate,
114-
messageService: this.messageService,
115-
responseService: this.responseService,
116-
version: candidate.availableVersions[0],
117-
});
118-
return;
119-
}
120-
if (answer === manualInstall) {
121-
this.boardsManagerFrontendContribution
122-
.openView({ reveal: true })
123-
.then((widget) =>
124-
widget.refresh(candidate.name.toLocaleLowerCase())
125-
);
126-
}
127-
});
82+
this.ensureCoreExists(selectedBoard, selectedPort);
83+
});
84+
85+
// we "clearRefusedPackageInfo" if a "refused" package is eventually
86+
// installed, though this is not strictly necessary. It's more of a
87+
// cleanup, to ensure the related variables are representative of
88+
// current state.
89+
this.notificationCenter.onPlatformInstalled((installed) => {
90+
if (this.lastRefusedPackageId === installed.item.id) {
91+
this.clearLastRefusedPromptInfo();
12892
}
12993
});
94+
};
95+
96+
// we should invoke this.ensureCoreExists only once we're sure
97+
// everything has been reconciled
98+
this.boardsServiceClient.reconciled.then(() => {
99+
const { selectedBoard, selectedPort } =
100+
this.boardsServiceClient.boardsConfig;
101+
102+
if (selectedBoard) {
103+
this.ensureCoreExists(selectedBoard, selectedPort);
104+
}
105+
106+
setEventListeners();
107+
});
108+
}
109+
110+
private removeNotificationByBoard(selectedBoard: Board): void {
111+
const index = this.notifications.findIndex((notification) =>
112+
Board.sameAs(notification, selectedBoard)
113+
);
114+
if (index !== -1) {
115+
this.notifications.splice(index, 1);
116+
}
117+
}
118+
119+
private clearLastRefusedPromptInfo(): void {
120+
this.lastRefusedPackageId = undefined;
121+
this.portSelectedOnLastRefusal = undefined;
122+
}
123+
124+
private setLastRefusedPromptInfo(
125+
packageId: string,
126+
selectedPort?: Port
127+
): void {
128+
this.lastRefusedPackageId = packageId;
129+
this.portSelectedOnLastRefusal = selectedPort;
130+
}
131+
132+
private promptAlreadyShowingForBoard(board: Board): boolean {
133+
return Boolean(
134+
this.notifications.find((notification) =>
135+
Board.sameAs(notification, board)
136+
)
137+
);
138+
}
139+
140+
protected ensureCoreExists(selectedBoard: Board, selectedPort?: Port): void {
141+
this.notifications.push(selectedBoard);
142+
this.boardsService.search({}).then((packages) => {
143+
const candidate = this.getInstallCandidate(packages, selectedBoard);
144+
145+
if (candidate) {
146+
this.showAutoInstallPrompt(candidate, selectedBoard, selectedPort);
147+
} else {
148+
this.removeNotificationByBoard(selectedBoard);
149+
}
150+
});
151+
}
152+
153+
private getInstallCandidate(
154+
packages: BoardsPackage[],
155+
selectedBoard: Board
156+
): BoardsPackage | undefined {
157+
// filter packagesForBoard selecting matches from the cli (installed packages)
158+
// and matches based on the board name
159+
// NOTE: this ensures the Deprecated & new packages are all in the array
160+
// so that we can check if any of the valid packages is already installed
161+
const packagesForBoard = packages.filter(
162+
(pkg) =>
163+
BoardsPackage.contains(selectedBoard, pkg) ||
164+
pkg.boards.some((board) => board.name === selectedBoard.name)
165+
);
166+
167+
// check if one of the packages for the board is already installed. if so, no hint
168+
if (packagesForBoard.some(({ installedVersion }) => !!installedVersion)) {
169+
return;
130170
}
171+
172+
// filter the installable (not installed) packages,
173+
// CLI returns the packages already sorted with the deprecated ones at the end of the list
174+
// in order to ensure the new ones are preferred
175+
const candidates = packagesForBoard.filter(
176+
({ installable, installedVersion }) => installable && !installedVersion
177+
);
178+
179+
return candidates[0];
180+
}
181+
182+
private showAutoInstallPrompt(
183+
candidate: BoardsPackage,
184+
selectedBoard: Board,
185+
selectedPort?: Port
186+
): void {
187+
const candidateName = candidate.name;
188+
const version = candidate.availableVersions[0]
189+
? `[v ${candidate.availableVersions[0]}]`
190+
: '';
191+
192+
const info = this.generatePromptInfoText(
193+
candidateName,
194+
version,
195+
selectedBoard.name
196+
);
197+
198+
const actions = this.createPromptActions(candidate);
199+
200+
const onRefuse = () => {
201+
this.setLastRefusedPromptInfo(candidate.id, selectedPort);
202+
};
203+
const handleAction = this.createOnAnswerHandler(actions, onRefuse);
204+
205+
const onAnswer = (answer: string) => {
206+
this.removeNotificationByBoard(selectedBoard);
207+
208+
handleAction(answer);
209+
};
210+
211+
this.messageService
212+
.info(info, ...actions.map((action) => action.key))
213+
.then(onAnswer);
214+
}
215+
216+
private generatePromptInfoText(
217+
candidateName: string,
218+
version: string,
219+
boardName: string
220+
): string {
221+
return nls.localize(
222+
'arduino/board/installNow',
223+
'The "{0} {1}" core has to be installed for the currently selected "{2}" board. Do you want to install it now?',
224+
candidateName,
225+
version,
226+
boardName
227+
);
228+
}
229+
230+
private createPromptActions(
231+
candidate: BoardsPackage
232+
): AutoInstallPromptActions {
233+
const yes = nls.localize('vscode/extensionsUtils/yes', 'Yes');
234+
const manualInstall = nls.localize(
235+
'arduino/board/installManually',
236+
'Install Manually'
237+
);
238+
239+
const actions: AutoInstallPromptActions = [
240+
{
241+
isAcceptance: true,
242+
key: yes,
243+
handler: () => {
244+
return Installable.installWithProgress({
245+
installable: this.boardsService,
246+
item: candidate,
247+
messageService: this.messageService,
248+
responseService: this.responseService,
249+
version: candidate.availableVersions[0],
250+
});
251+
},
252+
},
253+
{
254+
key: manualInstall,
255+
handler: () => {
256+
this.boardsManagerFrontendContribution
257+
.openView({ reveal: true })
258+
.then((widget) =>
259+
widget.refresh(candidate.name.toLocaleLowerCase())
260+
);
261+
},
262+
},
263+
];
264+
265+
return actions;
266+
}
267+
268+
private createOnAnswerHandler(
269+
actions: AutoInstallPromptActions,
270+
onRefuse?: () => void
271+
): (answer: string) => void {
272+
return (answer) => {
273+
const actionToHandle = actions.find((action) => action.key === answer);
274+
actionToHandle?.handler();
275+
276+
if (!actionToHandle?.isAcceptance && onRefuse) {
277+
onRefuse();
278+
}
279+
};
131280
}
132281
}

‎arduino-ide-extension/src/browser/boards/boards-service-provider.ts

+27-14
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import { NotificationCenter } from '../notification-center';
2020
import { ArduinoCommands } from '../arduino-commands';
2121
import { StorageWrapper } from '../storage-wrapper';
2222
import { nls } from '@theia/core/lib/common';
23+
import { Deferred } from '@theia/core/lib/common/promise-util';
2324

2425
@injectable()
2526
export class BoardsServiceProvider implements FrontendApplicationContribution {
@@ -73,6 +74,8 @@ export class BoardsServiceProvider implements FrontendApplicationContribution {
7374
this.onAvailableBoardsChangedEmitter.event;
7475
readonly onAvailablePortsChanged = this.onAvailablePortsChangedEmitter.event;
7576

77+
private readonly _reconciled = new Deferred<void>();
78+
7679
onStart(): void {
7780
this.notificationCenter.onAttachedBoardsChanged(
7881
this.notifyAttachedBoardsChanged.bind(this)
@@ -88,14 +91,22 @@ export class BoardsServiceProvider implements FrontendApplicationContribution {
8891
this.boardsService.getAttachedBoards(),
8992
this.boardsService.getAvailablePorts(),
9093
this.loadState(),
91-
]).then(([attachedBoards, availablePorts]) => {
94+
]).then(async ([attachedBoards, availablePorts]) => {
9295
this._attachedBoards = attachedBoards;
9396
this._availablePorts = availablePorts;
9497
this.onAvailablePortsChangedEmitter.fire(this._availablePorts);
95-
this.reconcileAvailableBoards().then(() => this.tryReconnect());
98+
99+
await this.reconcileAvailableBoards();
100+
101+
this.tryReconnect();
102+
this._reconciled.resolve();
96103
});
97104
}
98105

106+
get reconciled(): Promise<void> {
107+
return this._reconciled.promise;
108+
}
109+
99110
protected notifyAttachedBoardsChanged(
100111
event: AttachedBoardsChangeEvent
101112
): void {
@@ -185,8 +196,8 @@ export class BoardsServiceProvider implements FrontendApplicationContribution {
185196
const selectedAvailableBoard = AvailableBoard.is(selectedBoard)
186197
? selectedBoard
187198
: this._availableBoards.find((availableBoard) =>
188-
Board.sameAs(availableBoard, selectedBoard)
189-
);
199+
Board.sameAs(availableBoard, selectedBoard)
200+
);
190201
if (
191202
selectedAvailableBoard &&
192203
selectedAvailableBoard.selected &&
@@ -209,7 +220,7 @@ export class BoardsServiceProvider implements FrontendApplicationContribution {
209220
}
210221
}
211222

212-
protected async tryReconnect(): Promise<boolean> {
223+
protected tryReconnect(): boolean {
213224
if (this.latestValidBoardsConfig && !this.canUploadTo(this.boardsConfig)) {
214225
for (const board of this.availableBoards.filter(
215226
({ state }) => state !== AvailableBoard.State.incomplete
@@ -231,7 +242,8 @@ export class BoardsServiceProvider implements FrontendApplicationContribution {
231242
if (
232243
this.latestValidBoardsConfig.selectedBoard.fqbn === board.fqbn &&
233244
this.latestValidBoardsConfig.selectedBoard.name === board.name &&
234-
this.latestValidBoardsConfig.selectedPort.protocol === board.port?.protocol
245+
this.latestValidBoardsConfig.selectedPort.protocol ===
246+
board.port?.protocol
235247
) {
236248
this.boardsConfig = {
237249
...this.latestValidBoardsConfig,
@@ -376,14 +388,14 @@ export class BoardsServiceProvider implements FrontendApplicationContribution {
376388
const timeoutTask =
377389
!!timeout && timeout > 0
378390
? new Promise<void>((_, reject) =>
379-
setTimeout(
380-
() => reject(new Error(`Timeout after ${timeout} ms.`)),
381-
timeout
391+
setTimeout(
392+
() => reject(new Error(`Timeout after ${timeout} ms.`)),
393+
timeout
394+
)
382395
)
383-
)
384396
: new Promise<void>(() => {
385-
/* never */
386-
});
397+
/* never */
398+
});
387399
const waitUntilTask = new Promise<void>((resolve) => {
388400
let candidate = find(what, this.availableBoards);
389401
if (candidate) {
@@ -534,8 +546,9 @@ export class BoardsServiceProvider implements FrontendApplicationContribution {
534546

535547
protected getLastSelectedBoardOnPortKey(port: Port | string): string {
536548
// TODO: we lose the port's `protocol` info (`serial`, `network`, etc.) here if the `port` is a `string`.
537-
return `last-selected-board-on-port:${typeof port === 'string' ? port : port.address
538-
}`;
549+
return `last-selected-board-on-port:${
550+
typeof port === 'string' ? port : port.address
551+
}`;
539552
}
540553

541554
protected async loadState(): Promise<void> {

0 commit comments

Comments
 (0)
Please sign in to comment.