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 60b3120

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 0987ea8 commit 60b3120
Copy full SHA for 60b3120

File tree

10 files changed

+280
-130
lines changed
Filter options

10 files changed

+280
-130
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.3",
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
+59-27Lines changed: 59 additions & 27 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,
@@ -29,6 +30,14 @@ import type {
2930
import { NotificationCenter } from '../notification-center';
3031
import { BoardsServiceProvider } from './boards-service-provider';
3132

33+
export interface SelectConfigOptionParams {
34+
readonly fqbn: string;
35+
readonly optionsToUpdate: readonly Readonly<{
36+
option: string;
37+
selectedValue: string;
38+
}>[];
39+
}
40+
3241
@injectable()
3342
export class BoardsDataStore
3443
implements
@@ -64,7 +73,12 @@ export class BoardsDataStore
6473
this.toDispose.pushAll([
6574
this.boardsServiceProvider.onBoardsConfigDidChange((event) => {
6675
if (isBoardIdentifierChangeEvent(event)) {
67-
this.updateSelectedBoardData(event.selectedBoard?.fqbn);
76+
this.updateSelectedBoardData(
77+
event.selectedBoard?.fqbn,
78+
// If the change event comes from toolbar and the FQBN contains custom board options, change the currently selected options
79+
// https://github.com/arduino/arduino-ide/issues/1588
80+
event.reason === 'toolbar'
81+
);
6882
}
6983
}),
7084
this.notificationCenter.onPlatformDidInstall(async ({ item }) => {
@@ -125,9 +139,22 @@ export class BoardsDataStore
125139
}
126140

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

133160
onStop(): void {
@@ -168,7 +195,7 @@ export class BoardsDataStore
168195
return undefined;
169196
}
170197
const { configOptions } = await this.getData(fqbn);
171-
return ConfigOption.decorate(fqbn, configOptions);
198+
return new FQBN(fqbn).withConfigOptions(...configOptions).toString();
172199
}
173200

174201
async getData(fqbn: string | undefined): Promise<BoardsDataStore.Data> {
@@ -213,36 +240,41 @@ export class BoardsDataStore
213240
return true;
214241
}
215242

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) {
243+
async selectConfigOption(params: SelectConfigOptionParams): Promise<boolean> {
244+
const { fqbn, optionsToUpdate } = params;
245+
if (!optionsToUpdate.length) {
229246
return false;
230247
}
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;
248+
249+
const mutableData = deepClone(await this.getData(fqbn));
250+
let didChange = false;
251+
252+
for (const { option, selectedValue } of optionsToUpdate) {
253+
const { configOptions } = mutableData;
254+
const configOption = configOptions.find((c) => c.option === option);
255+
if (configOption) {
256+
let updated = false;
257+
for (const value of configOption.values) {
258+
const mutable: Mutable<ConfigValue> = value;
259+
if (mutable.value === selectedValue) {
260+
mutable.selected = true;
261+
updated = true;
262+
} else {
263+
mutable.selected = false;
264+
}
265+
}
266+
if (updated) {
267+
didChange = true;
268+
}
239269
}
240270
}
241-
if (!updated) {
271+
272+
if (!didChange) {
242273
return false;
243274
}
244-
await this.setData({ fqbn, data });
245-
this.fireChanged({ fqbn, data });
275+
276+
await this.setData({ fqbn, data: mutableData });
277+
this.fireChanged({ fqbn, data: mutableData });
246278
return true;
247279
}
248280

‎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
+8-1Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -136,6 +136,9 @@ export interface BoardListUIActions {
136136
}
137137
export type BoardListUI = BoardList & BoardListUIActions;
138138

139+
export type BoardsConfigChangeEventUI = BoardsConfigChangeEvent &
140+
Readonly<{ reason?: UpdateBoardsConfigReason }>;
141+
139142
@injectable()
140143
export class BoardListDumper implements Disposable {
141144
@inject(OutputChannelManager)
@@ -190,7 +193,7 @@ export class BoardsServiceProvider
190193
private _ready = new Deferred<void>();
191194

192195
private readonly boardsConfigDidChangeEmitter =
193-
new Emitter<BoardsConfigChangeEvent>();
196+
new Emitter<BoardsConfigChangeEventUI>();
194197
readonly onBoardsConfigDidChange = this.boardsConfigDidChangeEmitter.event;
195198

196199
private readonly boardListDidChangeEmitter = new Emitter<BoardListUI>();
@@ -384,6 +387,10 @@ export class BoardsServiceProvider
384387
this._boardsConfig.selectedPort = event.selectedPort;
385388
}
386389

390+
if (reason) {
391+
event = Object.assign(event, { reason });
392+
}
393+
387394
this.boardsConfigDidChangeEmitter.fire(event);
388395
this.refreshBoardList();
389396
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
+6-11Lines changed: 6 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -9,10 +9,9 @@ import {
99
BoardIdentifier,
1010
BoardsService,
1111
ExecutableService,
12-
assertSanitizedFqbn,
1312
isBoardIdentifierChangeEvent,
14-
sanitizeFqbn,
1513
} from '../../common/protocol';
14+
import { FQBN } from 'fqbn';
1615
import {
1716
defaultAsyncWorkers,
1817
maxAsyncWorkers,
@@ -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-
}
168-
const matchingChange = event.changes.find(
169-
(change) => change.fqbn === sanitizedFqbn
161+
const sanitizedFqbn = new FQBN(this.languageServerFqbn).sanitize();
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
164+
const matchingChange = event.changes.find((change) =>
165+
new FQBN(change.fqbn).sanitize().equals(sanitizedFqbn)
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
]);

‎arduino-ide-extension/src/common/protocol/boards-service.ts

Copy file name to clipboardExpand all lines: arduino-ide-extension/src/common/protocol/boards-service.ts
+3-60Lines changed: 3 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { nls } from '@theia/core/lib/common/nls';
2+
import { FQBN } from 'fqbn';
23
import type { MaybePromise } from '@theia/core/lib/common/types';
34
import type URI from '@theia/core/lib/common/uri';
45
import {
@@ -367,40 +368,6 @@ export interface ConfigOption {
367368
readonly values: ConfigValue[];
368369
}
369370
export namespace ConfigOption {
370-
/**
371-
* Appends the configuration options to the `fqbn` argument.
372-
* Throws an error if the `fqbn` does not have the `segment(':'segment)*` format.
373-
* The provided output format is always segment(':'segment)*(':'option'='value(','option'='value)*)?
374-
*/
375-
export function decorate(
376-
fqbn: string,
377-
configOptions: ConfigOption[]
378-
): string {
379-
if (!configOptions.length) {
380-
return fqbn;
381-
}
382-
383-
const toValue = (values: ConfigValue[]) => {
384-
const selectedValue = values.find(({ selected }) => selected);
385-
if (!selectedValue) {
386-
console.warn(
387-
`None of the config values was selected. Values were: ${JSON.stringify(
388-
values
389-
)}`
390-
);
391-
return undefined;
392-
}
393-
return selectedValue.value;
394-
};
395-
const options = configOptions
396-
.map(({ option, values }) => [option, toValue(values)])
397-
.filter(([, value]) => !!value)
398-
.map(([option, value]) => `${option}=${value}`)
399-
.join(',');
400-
401-
return `${fqbn}:${options}`;
402-
}
403-
404371
export class ConfigOptionError extends Error {
405372
constructor(message: string) {
406373
super(message);
@@ -574,30 +541,6 @@ export namespace Board {
574541
}
575542
}
576543

577-
/**
578-
* Throws an error if the `fqbn` argument is not sanitized. A sanitized FQBN has the `VENDOR:ARCHITECTURE:BOARD_ID` construct.
579-
*/
580-
export function assertSanitizedFqbn(fqbn: string): void {
581-
if (fqbn.split(':').length !== 3) {
582-
throw new Error(
583-
`Expected a sanitized FQBN with three segments in the following format: 'VENDOR:ARCHITECTURE:BOARD_ID'. Got ${fqbn} instead.`
584-
);
585-
}
586-
}
587-
588-
/**
589-
* Converts the `VENDOR:ARCHITECTURE:BOARD_ID[:MENU_ID=OPTION_ID[,MENU2_ID=OPTION_ID ...]]` FQBN to
590-
* `VENDOR:ARCHITECTURE:BOARD_ID` format.
591-
* See the details of the `{build.fqbn}` entry in the [specs](https://arduino.github.io/arduino-cli/latest/platform-specification/#global-predefined-properties).
592-
*/
593-
export function sanitizeFqbn(fqbn: string | undefined): string | undefined {
594-
if (!fqbn) {
595-
return undefined;
596-
}
597-
const [vendor, arch, id] = fqbn.split(':');
598-
return `${vendor}:${arch}:${id}`;
599-
}
600-
601544
export type PlatformIdentifier = Readonly<{ vendorId: string; arch: string }>;
602545
export function createPlatformIdentifier(
603546
board: BoardWithPackage
@@ -752,8 +695,8 @@ export function boardIdentifierEquals(
752695
return false; // TODO: This a strict now. Maybe compare name in the future.
753696
}
754697
if (left.fqbn && right.fqbn) {
755-
const leftFqbn = options.looseFqbn ? sanitizeFqbn(left.fqbn) : left.fqbn;
756-
const rightFqbn = options.looseFqbn ? sanitizeFqbn(right.fqbn) : right.fqbn;
698+
const leftFqbn = new FQBN(left.fqbn).toString(options.looseFqbn);
699+
const rightFqbn = new FQBN(right.fqbn).toString(options.looseFqbn);
757700
return leftFqbn === rightFqbn;
758701
}
759702
// No more Genuino hack.

‎arduino-ide-extension/src/node/board-discovery.ts

Copy file name to clipboardExpand all lines: arduino-ide-extension/src/node/board-discovery.ts
+4-16Lines changed: 4 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -267,24 +267,12 @@ export class BoardDiscovery
267267
const { port, boards } = detectedPort;
268268
const key = Port.keyOf(port);
269269
if (eventType === EventType.Add) {
270-
const alreadyDetectedPort = newState[key];
271-
if (alreadyDetectedPort) {
272-
console.warn(
273-
`Detected a new port that has been already discovered. The old value will be overridden. Old value: ${JSON.stringify(
274-
alreadyDetectedPort
275-
)}, new value: ${JSON.stringify(detectedPort)}`
276-
);
277-
}
270+
// Note that, the serial discovery might detect port details (such as addressLabel) in chunks.
271+
// For example, first, the Teensy 4.1 port is detected with the `[no_device] Triple Serial` address label,
272+
// Then, when more refinements are available, the same port is detected with `/dev/cu.usbmodem127902301 Triple Serial` address label.
273+
// In such cases, an `add` event is received from the CLI, and the the detected port is overridden in the state.
278274
newState[key] = { port, boards };
279275
} else if (eventType === EventType.Remove) {
280-
const alreadyDetectedPort = newState[key];
281-
if (!alreadyDetectedPort) {
282-
console.warn(
283-
`Detected a port removal but it has not been discovered. This is most likely a bug! Detected port was: ${JSON.stringify(
284-
detectedPort
285-
)}`
286-
);
287-
}
288276
delete newState[key];
289277
}
290278
}

0 commit comments

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