Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit f8539c8

Browse filesBrowse files
author
Akos Kitta
committed
fix: invalid custom board option handling in FQBN
Closes #1588 Signed-off-by: Akos Kitta <a.kitta@arduino.cc>
1 parent 2b5ceea commit f8539c8
Copy full SHA for f8539c8

11 files changed

+586
-139
lines changed

‎arduino-ide-extension/package.json

Copy file name to clipboardExpand all lines: arduino-ide-extension/package.json
+1Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,7 @@
7272
"fast-json-stable-stringify": "^2.1.0",
7373
"fast-safe-stringify": "^2.1.1",
7474
"filename-reserved-regex": "^2.0.0",
75+
"fqbn": "^1.0.5",
7576
"glob": "^7.1.6",
7677
"google-protobuf": "^3.20.1",
7778
"hash.js": "^1.1.7",

‎arduino-ide-extension/src/browser/boards/boards-data-store.ts

Copy file name to clipboardExpand all lines: arduino-ide-extension/src/browser/boards/boards-data-store.ts
+75-32Lines changed: 75 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import { ILogger } from '@theia/core/lib/common/logger';
1212
import { deepClone, deepFreeze } from '@theia/core/lib/common/objects';
1313
import type { Mutable } from '@theia/core/lib/common/types';
1414
import { inject, injectable, named } from '@theia/core/shared/inversify';
15+
import { FQBN } from 'fqbn';
1516
import {
1617
BoardDetails,
1718
BoardsService,
@@ -20,6 +21,7 @@ import {
2021
Programmer,
2122
isBoardIdentifierChangeEvent,
2223
isProgrammer,
24+
sanitizeFqbn,
2325
} from '../../common/protocol';
2426
import { notEmpty } from '../../common/utils';
2527
import type {
@@ -29,6 +31,14 @@ import type {
2931
import { NotificationCenter } from '../notification-center';
3032
import { BoardsServiceProvider } from './boards-service-provider';
3133

34+
export interface SelectConfigOptionParams {
35+
readonly fqbn: string;
36+
readonly optionsToUpdate: readonly Readonly<{
37+
option: string;
38+
selectedValue: string;
39+
}>[];
40+
}
41+
3242
@injectable()
3343
export class BoardsDataStore
3444
implements
@@ -64,7 +74,12 @@ export class BoardsDataStore
6474
this.toDispose.pushAll([
6575
this.boardsServiceProvider.onBoardsConfigDidChange((event) => {
6676
if (isBoardIdentifierChangeEvent(event)) {
67-
this.updateSelectedBoardData(event.selectedBoard?.fqbn);
77+
this.updateSelectedBoardData(
78+
event.selectedBoard?.fqbn,
79+
// If the change event comes from toolbar and the FQBN contains custom board options, change the currently selected options
80+
// https://github.com/arduino/arduino-ide/issues/1588
81+
event.reason === 'toolbar'
82+
);
6883
}
6984
}),
7085
this.notificationCenter.onPlatformDidInstall(async ({ item }) => {
@@ -116,7 +131,7 @@ export class BoardsDataStore
116131
if (!fqbn) {
117132
return undefined;
118133
} else {
119-
const data = await this.getData(fqbn);
134+
const data = await this.getData(sanitizeFqbn(fqbn));
120135
if (data === BoardsDataStore.Data.EMPTY) {
121136
return undefined;
122137
}
@@ -125,9 +140,22 @@ export class BoardsDataStore
125140
}
126141

127142
private async updateSelectedBoardData(
128-
fqbn: string | undefined
143+
fqbn: string | undefined,
144+
updateConfigOptions = false
129145
): Promise<void> {
130146
this._selectedBoardData = await this.getSelectedBoardData(fqbn);
147+
if (fqbn && updateConfigOptions) {
148+
const { options } = new FQBN(fqbn);
149+
if (options) {
150+
const optionsToUpdate = Object.entries(options).map(([key, value]) => ({
151+
option: key,
152+
selectedValue: value,
153+
}));
154+
const params = { fqbn, optionsToUpdate };
155+
await this.selectConfigOption(params);
156+
this._selectedBoardData = await this.getSelectedBoardData(fqbn); // reload the updated data
157+
}
158+
}
131159
}
132160

133161
onStop(): void {
@@ -168,7 +196,7 @@ export class BoardsDataStore
168196
return undefined;
169197
}
170198
const { configOptions } = await this.getData(fqbn);
171-
return ConfigOption.decorate(fqbn, configOptions);
199+
return new FQBN(fqbn).withConfigOptions(...configOptions).toString();
172200
}
173201

174202
async getData(fqbn: string | undefined): Promise<BoardsDataStore.Data> {
@@ -201,48 +229,63 @@ export class BoardsDataStore
201229
fqbn: string;
202230
selectedProgrammer: Programmer;
203231
}): Promise<boolean> {
204-
const storedData = deepClone(await this.getData(fqbn));
232+
const sanitizedFQBN = sanitizeFqbn(fqbn);
233+
const storedData = deepClone(await this.getData(sanitizedFQBN));
205234
const { programmers } = storedData;
206235
if (!programmers.find((p) => Programmer.equals(selectedProgrammer, p))) {
207236
return false;
208237
}
209238

210-
const data = { ...storedData, selectedProgrammer };
211-
await this.setData({ fqbn, data });
212-
this.fireChanged({ fqbn, data });
239+
const change: BoardsDataStoreChange = {
240+
fqbn: sanitizedFQBN,
241+
data: { ...storedData, selectedProgrammer },
242+
};
243+
await this.setData(change);
244+
this.fireChanged(change);
213245
return true;
214246
}
215247

216-
async selectConfigOption({
217-
fqbn,
218-
option,
219-
selectedValue,
220-
}: {
221-
fqbn: string;
222-
option: string;
223-
selectedValue: string;
224-
}): Promise<boolean> {
225-
const data = deepClone(await this.getData(fqbn));
226-
const { configOptions } = data;
227-
const configOption = configOptions.find((c) => c.option === option);
228-
if (!configOption) {
248+
async selectConfigOption(params: SelectConfigOptionParams): Promise<boolean> {
249+
const { fqbn, optionsToUpdate } = params;
250+
if (!optionsToUpdate.length) {
229251
return false;
230252
}
231-
let updated = false;
232-
for (const value of configOption.values) {
233-
const mutable: Mutable<ConfigValue> = value;
234-
if (mutable.value === selectedValue) {
235-
mutable.selected = true;
236-
updated = true;
237-
} else {
238-
mutable.selected = false;
253+
254+
const sanitizedFQBN = sanitizeFqbn(fqbn);
255+
const mutableData = deepClone(await this.getData(sanitizedFQBN));
256+
let didChange = false;
257+
258+
for (const { option, selectedValue } of optionsToUpdate) {
259+
const { configOptions } = mutableData;
260+
const configOption = configOptions.find((c) => c.option === option);
261+
if (configOption) {
262+
const configOptionValueIndex = configOption.values.findIndex(
263+
(configOptionValue) => configOptionValue.value === selectedValue
264+
);
265+
if (configOptionValueIndex >= 0) {
266+
// unselect all
267+
configOption.values
268+
.map((value) => value as Mutable<ConfigValue>)
269+
.forEach((value) => (value.selected = false));
270+
const mutableConfigValue: Mutable<ConfigValue> =
271+
configOption.values[configOptionValueIndex];
272+
// make the new value `selected`
273+
mutableConfigValue.selected = true;
274+
didChange = true;
275+
}
239276
}
240277
}
241-
if (!updated) {
278+
279+
if (!didChange) {
242280
return false;
243281
}
244-
await this.setData({ fqbn, data });
245-
this.fireChanged({ fqbn, data });
282+
283+
const change: BoardsDataStoreChange = {
284+
fqbn: sanitizedFQBN,
285+
data: mutableData,
286+
};
287+
await this.setData(change);
288+
this.fireChanged(change);
246289
return true;
247290
}
248291

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

Copy file name to clipboardExpand all lines: arduino-ide-extension/src/browser/boards/boards-service-provider.ts
+48-12Lines changed: 48 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import { Emitter } from '@theia/core/lib/common/event';
1212
import { ILogger } from '@theia/core/lib/common/logger';
1313
import { MessageService } from '@theia/core/lib/common/message-service';
1414
import { nls } from '@theia/core/lib/common/nls';
15+
import { deepClone } from '@theia/core/lib/common/objects';
1516
import { Deferred } from '@theia/core/lib/common/promise-util';
1617
import type { Mutable } from '@theia/core/lib/common/types';
1718
import { inject, injectable, optional } from '@theia/core/shared/inversify';
@@ -21,31 +22,32 @@ import {
2122
} from '@theia/output/lib/browser/output-channel';
2223
import {
2324
BoardIdentifier,
24-
boardIdentifierEquals,
25+
BoardUserField,
26+
BoardWithPackage,
2527
BoardsConfig,
2628
BoardsConfigChangeEvent,
2729
BoardsPackage,
2830
BoardsService,
29-
BoardUserField,
30-
BoardWithPackage,
3131
DetectedPorts,
32+
Port,
33+
PortIdentifier,
34+
boardIdentifierEquals,
3235
emptyBoardsConfig,
3336
isBoardIdentifier,
3437
isBoardIdentifierChangeEvent,
3538
isPortIdentifier,
3639
isPortIdentifierChangeEvent,
37-
Port,
38-
PortIdentifier,
3940
portIdentifierEquals,
41+
sanitizeFqbn,
4042
serializePlatformIdentifier,
4143
} from '../../common/protocol';
4244
import {
4345
BoardList,
4446
BoardListHistory,
45-
createBoardList,
4647
EditBoardsConfigActionParams,
47-
isBoardListHistory,
4848
SelectBoardsConfigActionParams,
49+
createBoardList,
50+
isBoardListHistory,
4951
} from '../../common/protocol/board-list';
5052
import type { Defined } from '../../common/types';
5153
import type {
@@ -104,6 +106,21 @@ type BoardListHistoryUpdateResult =
104106
type BoardToSelect = BoardIdentifier | undefined | 'ignore-board';
105107
type PortToSelect = PortIdentifier | undefined | 'ignore-port';
106108

109+
function sanitizeBoardToSelectFQBN(board: BoardToSelect): BoardToSelect {
110+
if (isBoardIdentifier(board)) {
111+
return sanitizeBoardIdentifierFQBN(board);
112+
}
113+
return board;
114+
}
115+
function sanitizeBoardIdentifierFQBN(board: BoardIdentifier): BoardIdentifier {
116+
if (board.fqbn) {
117+
const copy: Mutable<BoardIdentifier> = deepClone(board);
118+
copy.fqbn = sanitizeFqbn(board.fqbn);
119+
return copy;
120+
}
121+
return board;
122+
}
123+
107124
interface UpdateBoardListHistoryParams {
108125
readonly portToSelect: PortToSelect;
109126
readonly boardToSelect: BoardToSelect;
@@ -136,6 +153,9 @@ export interface BoardListUIActions {
136153
}
137154
export type BoardListUI = BoardList & BoardListUIActions;
138155

156+
export type BoardsConfigChangeEventUI = BoardsConfigChangeEvent &
157+
Readonly<{ reason?: UpdateBoardsConfigReason }>;
158+
139159
@injectable()
140160
export class BoardListDumper implements Disposable {
141161
@inject(OutputChannelManager)
@@ -190,7 +210,7 @@ export class BoardsServiceProvider
190210
private _ready = new Deferred<void>();
191211

192212
private readonly boardsConfigDidChangeEmitter =
193-
new Emitter<BoardsConfigChangeEvent>();
213+
new Emitter<BoardsConfigChangeEventUI>();
194214
readonly onBoardsConfigDidChange = this.boardsConfigDidChangeEmitter.event;
195215

196216
private readonly boardListDidChangeEmitter = new Emitter<BoardListUI>();
@@ -353,7 +373,8 @@ export class BoardsServiceProvider
353373
portToSelect !== 'ignore-port' &&
354374
!portIdentifierEquals(portToSelect, previousSelectedPort);
355375
const boardDidChangeEvent = boardDidChange
356-
? { selectedBoard: boardToSelect, previousSelectedBoard }
376+
? // The change event must always contain any custom board options. Hence the board to select is not sanitized.
377+
{ selectedBoard: boardToSelect, previousSelectedBoard }
357378
: undefined;
358379
const portDidChangeEvent = portDidChange
359380
? { selectedPort: portToSelect, previousSelectedPort }
@@ -374,16 +395,31 @@ export class BoardsServiceProvider
374395
return false;
375396
}
376397

377-
this.maybeUpdateBoardListHistory({ portToSelect, boardToSelect });
378-
this.maybeUpdateBoardsData({ boardToSelect, reason });
398+
// unlike for the board change event, every persistent state must not contain custom board config options in the FQBN
399+
const sanitizedBoardToSelect = sanitizeBoardToSelectFQBN(boardToSelect);
400+
401+
this.maybeUpdateBoardListHistory({
402+
portToSelect,
403+
boardToSelect: sanitizedBoardToSelect,
404+
});
405+
this.maybeUpdateBoardsData({
406+
boardToSelect: sanitizedBoardToSelect,
407+
reason,
408+
});
379409

380410
if (isBoardIdentifierChangeEvent(event)) {
381-
this._boardsConfig.selectedBoard = event.selectedBoard;
411+
this._boardsConfig.selectedBoard = event.selectedBoard
412+
? sanitizeBoardIdentifierFQBN(event.selectedBoard)
413+
: event.selectedBoard;
382414
}
383415
if (isPortIdentifierChangeEvent(event)) {
384416
this._boardsConfig.selectedPort = event.selectedPort;
385417
}
386418

419+
if (reason) {
420+
event = Object.assign(event, { reason });
421+
}
422+
387423
this.boardsConfigDidChangeEmitter.fire(event);
388424
this.refreshBoardList();
389425
this.saveState();

‎arduino-ide-extension/src/browser/contributions/boards-data-menu-updater.ts

Copy file name to clipboardExpand all lines: arduino-ide-extension/src/browser/contributions/boards-data-menu-updater.ts
+1-2Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -87,8 +87,7 @@ export class BoardsDataMenuUpdater extends Contribution {
8787
execute: () =>
8888
this.boardsDataStore.selectConfigOption({
8989
fqbn,
90-
option,
91-
selectedValue: value.value,
90+
optionsToUpdate: [{ option, selectedValue: value.value }],
9291
}),
9392
isToggled: () => value.selected,
9493
};

‎arduino-ide-extension/src/browser/contributions/ino-language.ts

Copy file name to clipboardExpand all lines: arduino-ide-extension/src/browser/contributions/ino-language.ts
+4-9Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@ import {
99
BoardIdentifier,
1010
BoardsService,
1111
ExecutableService,
12-
assertSanitizedFqbn,
1312
isBoardIdentifierChangeEvent,
1413
sanitizeFqbn,
1514
} from '../../common/protocol';
@@ -159,14 +158,11 @@ export class InoLanguage extends SketchContribution {
159158
this.notificationCenter.onDidReinitialize(() => forceRestart()),
160159
this.boardDataStore.onDidChange((event) => {
161160
if (this.languageServerFqbn) {
162-
const sanitizedFqbn = sanitizeFqbn(this.languageServerFqbn);
163-
if (!sanitizeFqbn) {
164-
throw new Error(
165-
`Failed to sanitize the FQBN of the running language server. FQBN with the board settings was: ${this.languageServerFqbn}`
166-
);
167-
}
161+
const sanitizedFQBN = sanitizeFqbn(this.languageServerFqbn);
162+
// The incoming FQBNs might contain custom boards configs, sanitize them before the comparison.
163+
// https://github.com/arduino/arduino-ide/pull/2113#pullrequestreview-1499998328
168164
const matchingChange = event.changes.find(
169-
(change) => change.fqbn === sanitizedFqbn
165+
(change) => sanitizedFQBN === sanitizeFqbn(change.fqbn)
170166
);
171167
const { boardsConfig } = this.boardsServiceProvider;
172168
if (
@@ -228,7 +224,6 @@ export class InoLanguage extends SketchContribution {
228224
}
229225
return;
230226
}
231-
assertSanitizedFqbn(fqbn);
232227
const fqbnWithConfig = await this.boardDataStore.appendConfigToFqbn(fqbn);
233228
if (!fqbnWithConfig) {
234229
throw new Error(

‎arduino-ide-extension/src/browser/contributions/upload-sketch.ts

Copy file name to clipboardExpand all lines: arduino-ide-extension/src/browser/contributions/upload-sketch.ts
+7-2Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,8 @@
11
import { Emitter } from '@theia/core/lib/common/event';
22
import { nls } from '@theia/core/lib/common/nls';
33
import { inject, injectable } from '@theia/core/shared/inversify';
4-
import { CoreService, sanitizeFqbn } from '../../common/protocol';
4+
import { FQBN } from 'fqbn';
5+
import { CoreService } from '../../common/protocol';
56
import { ArduinoMenus } from '../menu/arduino-menus';
67
import { CurrentSketch } from '../sketches-service-client-impl';
78
import { ArduinoToolbar } from '../toolbar/arduino-toolbar';
@@ -173,7 +174,11 @@ export class UploadSketch extends CoreServiceContribution {
173174
const [fqbn, { selectedProgrammer: programmer }, verify, verbose] =
174175
await Promise.all([
175176
verifyOptions.fqbn, // already decorated FQBN
176-
this.boardsDataStore.getData(sanitizeFqbn(verifyOptions.fqbn)),
177+
this.boardsDataStore.getData(
178+
verifyOptions.fqbn
179+
? new FQBN(verifyOptions.fqbn).toString(true)
180+
: undefined
181+
),
177182
this.preferences.get('arduino.upload.verify'),
178183
this.preferences.get('arduino.upload.verbose'),
179184
]);

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.