Skip to content

Commit

Permalink
Merge 0caf0ca into d79bc0d
Browse files Browse the repository at this point in the history
  • Loading branch information
@kittaakos
kittaakos committed Jun 27, 2023
2 parents d79bc0d + 0caf0ca commit f4e766a
Show file tree
Hide file tree
Showing 3 changed files with 210 additions and 18 deletions.
6 changes: 3 additions & 3 deletions arduino-ide-extension/src/browser/contributions/ino-language.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import { inject, injectable } from '@theia/core/shared/inversify';
import { Mutex } from 'async-mutex';
import {
ArduinoDaemon,
assertSanitizedFqbn,
BoardsService,
ExecutableService,
sanitizeFqbn,
Expand Down Expand Up @@ -93,8 +92,10 @@ export class InoLanguage extends SketchContribution {
`Failed to sanitize the FQBN of the running language server. FQBN with the board settings was: ${this.languageServerFqbn}`
);
}
// The incoming FQBNs might contain custom boards configs, sanitize them before the comparison.
// https://github.com/arduino/arduino-ide/pull/2113#pullrequestreview-1499998328
const matchingFqbn = dataChangePerFqbn.find(
(fqbn) => sanitizedFqbn === fqbn
(fqbn) => sanitizedFqbn === sanitizeFqbn(fqbn)
);
const { boardsConfig } = this.boardsServiceProvider;
if (
Expand Down Expand Up @@ -151,7 +152,6 @@ export class InoLanguage extends SketchContribution {
}
return;
}
assertSanitizedFqbn(fqbn);
const fqbnWithConfig = await this.boardDataStore.appendConfigToFqbn(fqbn);
if (!fqbnWithConfig) {
throw new Error(
Expand Down
107 changes: 92 additions & 15 deletions arduino-ide-extension/src/common/protocol/boards-service.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@ export namespace BoardSearch {
'Partner',
'Arduino@Heart',
] as const;
export type Type = typeof TypeLiterals[number];
export type Type = (typeof TypeLiterals)[number];
export namespace Type {
export function is(arg: unknown): arg is Type {
return typeof arg === 'string' && TypeLiterals.includes(arg as Type);
Expand Down Expand Up @@ -347,7 +347,7 @@ export namespace Port {

export namespace Protocols {
export const KnownProtocolLiterals = ['serial', 'network'] as const;
export type KnownProtocol = typeof KnownProtocolLiterals[number];
export type KnownProtocol = (typeof KnownProtocolLiterals)[number];
export namespace KnownProtocol {
export function is(protocol: unknown): protocol is KnownProtocol {
return (
Expand Down Expand Up @@ -476,29 +476,106 @@ export namespace ConfigOption {
fqbn: string,
configOptions: ConfigOption[]
): string {
if (!configOptions.length) {
return fqbn;
const failInvalidFQBN = (): never => {
throw new Error(`Invalid FQBN: ${fqbn}`);
};

const [vendor, arch, id, rest] = fqbn.split(':');
if (!vendor || !arch || !id) {
return failInvalidFQBN();
}

const toValue = (values: ConfigValue[]) => {
const existingOptions: Record<string, string> = {};
if (rest) {
// If rest exists, it must have the key=value(,key=value)* format. Otherwise, fail.
const tuples = rest.split(',');
for (const tuple of tuples) {
const segments = tuple.split('=');
if (segments.length !== 2) {
failInvalidFQBN();
}
const [option, value] = segments;
if (!option || !value) {
failInvalidFQBN();
}
if (existingOptions[option]) {
console.warn(
`Config value already exists for '${option}' on FQBN. Skipping it. Existing value: ${existingOptions[option]}, new value: ${value}, FQBN: ${fqbn}`
);
} else {
existingOptions[option] = value;
}
}
}

const newOptions: Record<string, string> = {};
for (const configOption of configOptions) {
const { option, values } = configOption;
if (!option) {
console.warn(
`Detected empty option on config options. Skipping it. ${JSON.stringify(
configOption
)}`
);
continue;
}
const selectedValue = values.find(({ selected }) => selected);
if (!selectedValue) {
if (selectedValue) {
const { value } = selectedValue;
if (!value) {
console.warn(
`Detected empty selected value on config options. Skipping it. ${JSON.stringify(
configOption
)}`
);
continue;
}
if (newOptions[option]) {
console.warn(
`Config value already exists for '${option}' in config options. Skipping it. Existing value: ${
newOptions[option]
}, new value: ${value}, config option: ${JSON.stringify(
configOption
)}`
);
} else {
newOptions[option] = value;
}
} else {
console.warn(
`None of the config values was selected. Values were: ${JSON.stringify(
values
`None of the config values was selected. Config options was: ${JSON.stringify(
configOption
)}`
);
return undefined;
}
return selectedValue.value;
};
const options = configOptions
.map(({ option, values }) => [option, toValue(values)])
.filter(([, value]) => !!value)
}

// Collect all options from the FQBN. Call them existing.
// Collect all incoming options to decorate the FQBN with. Call them new.
// To keep the order, iterate through the existing ones and append to FQBN.
// If a new(er) value exists for the same option, use the new value.
// If there is a new value, "mark" it as visited by deleting it from new. Otherwise, use the existing value.
// Append all new ones to the FQBN.
const mergedOptions: Record<string, string> = {};
for (const existing of Object.entries(existingOptions)) {
const [option, value] = existing;
const newValue = newOptions[option];
if (newValue) {
mergedOptions[option] = newValue;
delete newOptions[option];
} else {
mergedOptions[option] = value;
}
}
Array.from(Object.entries(newOptions)).forEach(
([option, value]) => (mergedOptions[option] = value)
);

const configSuffix = Object.entries(mergedOptions)
.map(([option, value]) => `${option}=${value}`)
.join(',');

return `${fqbn}:${options}`;
return `${vendor}:${arch}:${id}${configSuffix ? `:${configSuffix}` : ''}`;
}

export class ConfigOptionError extends Error {
Expand Down
115 changes: 115 additions & 0 deletions arduino-ide-extension/src/test/common/boards-service.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { expect } from 'chai';
import {
AttachedBoardsChangeEvent,
BoardInfo,
ConfigOption,
getBoardInfo,
noNativeSerialPort,
nonSerialPort,
Expand Down Expand Up @@ -93,6 +94,120 @@ describe('boards-service', () => {
});
});

describe('ConfigOption#decorate', () => {
['invalid', 'a:b:c:foo=', 'a:b:c:foo=bar=baz', 'a:b:c:foo+bar'].map(
(fqbn) =>
it(`should error when the FQBN is invalid (FQBN: ${fqbn})`, () => {
expect(() => ConfigOption.decorate(fqbn, [])).to.throw(
/^Invalid FQBN:*/
);
})
);

it('should not append the trailing boards config part to FQBN if configs is empty', () => {
const fqbn = 'a:b:c';
const actual = ConfigOption.decorate(fqbn, []);
expect(actual).to.be.equal(fqbn);
});

it('should be noop when config options is empty', () => {
const fqbn = 'a:b:c:menu1=value1';
const actual = ConfigOption.decorate(fqbn, []);
expect(actual).to.be.equal(fqbn);
});

it('should not duplicate config options', () => {
const fqbn = 'a:b:c:menu1=value1';
const actual = ConfigOption.decorate(fqbn, [
{
label: 'Menu 1',
option: 'menu1',
values: [
{ label: 'Value 1', value: 'value1', selected: true },
{ label: 'Another Value', value: 'another1', selected: false },
],
},
]);
expect(actual).to.be.equal(fqbn);
});

it('should update config options', () => {
const fqbn = 'a:b:c:menu1=value1,menu2=value2';
const actual = ConfigOption.decorate(fqbn, [
{
label: 'Menu 1',
option: 'menu1',
values: [
{ label: 'Value 1', value: 'value1', selected: false },
{ label: 'Another Value', value: 'another1', selected: true },
],
},
]);
expect(actual).to.be.equal('a:b:c:menu1=another1,menu2=value2');
});

it('should favor first over rest when there are duplicate options (duplicate on FQBN)', () => {
const fqbn = 'a:b:c:menu1=value1,menu1=value2';
const actual = ConfigOption.decorate(fqbn, [
{
label: 'Menu 1',
option: 'menu1',
values: [
{ label: 'Value 1', value: 'value1', selected: false },
{ label: 'Another Value', value: 'another1', selected: true },
],
},
]);
expect(actual).to.be.equal('a:b:c:menu1=another1');
});

it('should favor first over rest when there are duplicate options (duplicate in config)', () => {
const fqbn = 'a:b:c';
const actual = ConfigOption.decorate(fqbn, [
{
label: 'Menu 1',
option: 'menu1',
values: [
{ label: 'Value 1', value: 'value1', selected: false },
{ label: 'Another Value', value: 'another1', selected: true },
],
},
{
label: 'Menu 1',
option: 'menu1',
values: [
{ label: 'Value 1', value: 'value1', selected: true },
{ label: 'Another Value', value: 'another1', selected: false },
],
},
]);
expect(actual).to.be.equal('a:b:c:menu1=another1');
});

it('should not change the existing config order', () => {
const fqbn = 'a:b:c:menu1=value1';
const actual = ConfigOption.decorate(fqbn, [
{
label: 'Menu 0',
option: 'menu0',
values: [
{ label: 'Value 0', value: 'value0', selected: true },
{ label: 'Another Value', value: 'another0', selected: false },
],
},
{
label: 'Menu 1',
option: 'menu1',
values: [
{ label: 'Value 1', value: 'value1', selected: false },
{ label: 'Another Value', value: 'another1', selected: true },
],
},
]);
expect(actual).to.be.equal('a:b:c:menu1=another1,menu0=value0');
});
});

describe('getBoardInfo', () => {
const vid = '0x0';
const pid = '0x1';
Expand Down

0 comments on commit f4e766a

Please sign in to comment.