Skip to content

Commit e47befc

Browse files
kittaakoskittaakos
authored
Merge b0422a7 into d79bc0d 
2 parents d79bc0d + b0422a7 commit e47befc

File tree

3 files changed

+201
-18
lines changed

3 files changed

+201
-18
lines changed

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

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@ import { inject, injectable } from '@theia/core/shared/inversify';
66
import { Mutex } from 'async-mutex';
77
import {
88
ArduinoDaemon,
9-
assertSanitizedFqbn,
109
BoardsService,
1110
ExecutableService,
1211
sanitizeFqbn,
@@ -151,7 +150,6 @@ export class InoLanguage extends SketchContribution {
151150
}
152151
return;
153152
}
154-
assertSanitizedFqbn(fqbn);
155153
const fqbnWithConfig = await this.boardDataStore.appendConfigToFqbn(fqbn);
156154
if (!fqbnWithConfig) {
157155
throw new Error(

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

Lines changed: 92 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -180,7 +180,7 @@ export namespace BoardSearch {
180180
'Partner',
181181
'Arduino@Heart',
182182
] as const;
183-
export type Type = typeof TypeLiterals[number];
183+
export type Type = (typeof TypeLiterals)[number];
184184
export namespace Type {
185185
export function is(arg: unknown): arg is Type {
186186
return typeof arg === 'string' && TypeLiterals.includes(arg as Type);
@@ -347,7 +347,7 @@ export namespace Port {
347347

348348
export namespace Protocols {
349349
export const KnownProtocolLiterals = ['serial', 'network'] as const;
350-
export type KnownProtocol = typeof KnownProtocolLiterals[number];
350+
export type KnownProtocol = (typeof KnownProtocolLiterals)[number];
351351
export namespace KnownProtocol {
352352
export function is(protocol: unknown): protocol is KnownProtocol {
353353
return (
@@ -476,29 +476,105 @@ export namespace ConfigOption {
476476
fqbn: string,
477477
configOptions: ConfigOption[]
478478
): string {
479-
if (!configOptions.length) {
480-
return fqbn;
479+
const failInvalidFQBN = (): never => {
480+
throw new Error(`Invalid FQBN: ${fqbn}`);
481+
};
482+
483+
const [vendor, arch, id, rest] = fqbn.split(':');
484+
if (!vendor || !arch || !id) {
485+
return failInvalidFQBN();
486+
}
487+
488+
const existingOptions: Record<string, string> = {};
489+
if (rest) {
490+
// If rest exists, it must have the key=value(,key=value)* format. Otherwise, fail.
491+
const tuples = rest.split(',');
492+
for (const tuple of tuples) {
493+
const segments = tuple.split('=');
494+
if (segments.length !== 2) {
495+
failInvalidFQBN();
496+
}
497+
const [option, value] = segments;
498+
if (!option || !value) {
499+
failInvalidFQBN();
500+
}
501+
if (existingOptions[option]) {
502+
console.warn(
503+
`Config value already exists for '${option}' on FQBN. Skipping it. Existing value: ${existingOptions[option]}, new value: ${value}, FQBN: ${fqbn}`
504+
);
505+
} else {
506+
existingOptions[option] = value;
507+
}
508+
}
481509
}
482510

483-
const toValue = (values: ConfigValue[]) => {
511+
const newOptions: Record<string, string> = {};
512+
for (const configOption of configOptions) {
513+
const { option, values } = configOption;
514+
if (!option) {
515+
console.warn(
516+
`Detected empty option on config options. Skipping it. ${JSON.stringify(
517+
configOption
518+
)}`
519+
);
520+
continue;
521+
}
484522
const selectedValue = values.find(({ selected }) => selected);
485-
if (!selectedValue) {
523+
if (selectedValue) {
524+
const { value } = selectedValue;
525+
if (!value) {
526+
console.warn(
527+
`Detected empty selected value on config options. Skipping it. ${JSON.stringify(
528+
configOption
529+
)}`
530+
);
531+
continue;
532+
}
533+
if (newOptions[option]) {
534+
console.warn(
535+
`Config value already exists for '${option}' in config options. Skipping it. Existing value: ${
536+
newOptions[option]
537+
}, new value: ${value}, config option: ${JSON.stringify(
538+
configOption
539+
)}`
540+
);
541+
} else {
542+
newOptions[option] = value;
543+
}
544+
} else {
486545
console.warn(
487-
`None of the config values was selected. Values were: ${JSON.stringify(
488-
values
546+
`None of the config values was selected. Config options was: ${JSON.stringify(
547+
configOption
489548
)}`
490549
);
491-
return undefined;
492550
}
493-
return selectedValue.value;
494-
};
495-
const options = configOptions
496-
.map(({ option, values }) => [option, toValue(values)])
497-
.filter(([, value]) => !!value)
551+
}
552+
553+
// Collect all options from the FQBN. Call them existing.
554+
// Collect all incoming options to decorate the FQBN with. Call them new.
555+
// To keep the order, iterate through the existing ones and append to FQBN.
556+
// If a new(er) value exists for the same option, use the new value.
557+
// If there is a new value, "mark" it as visited by deleting it from new. Otherwise, use the existing value.
558+
// Append all new ones to the FQBN.
559+
const mergedOptions: Record<string, string> = {};
560+
for (const existing of Object.entries(existingOptions)) {
561+
const [option, value] = existing;
562+
const newValue = newOptions[option];
563+
if (newValue) {
564+
mergedOptions[option] = newValue;
565+
delete newOptions[option];
566+
} else {
567+
mergedOptions[option] = value;
568+
}
569+
}
570+
Array.from(Object.entries(newOptions)).forEach(
571+
([option, value]) => (mergedOptions[option] = value)
572+
);
573+
574+
const configSuffix = Object.entries(mergedOptions)
498575
.map(([option, value]) => `${option}=${value}`)
499576
.join(',');
500-
501-
return `${fqbn}:${options}`;
577+
return `${vendor}:${arch}:${id}:${configSuffix}`;
502578
}
503579

504580
export class ConfigOptionError extends Error {

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

Lines changed: 109 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import { expect } from 'chai';
44
import {
55
AttachedBoardsChangeEvent,
66
BoardInfo,
7+
ConfigOption,
78
getBoardInfo,
89
noNativeSerialPort,
910
nonSerialPort,
@@ -93,6 +94,114 @@ describe('boards-service', () => {
9394
});
9495
});
9596

97+
describe('ConfigOption#decorate', () => {
98+
['invalid', 'a:b:c:foo=', 'a:b:c:foo=bar=baz', 'a:b:c:foo+bar'].map(
99+
(fqbn) =>
100+
it(`should error when the FQBN is invalid (FQBN: ${fqbn})`, () => {
101+
expect(() => ConfigOption.decorate(fqbn, [])).to.throw(
102+
/^Invalid FQBN:*/
103+
);
104+
})
105+
);
106+
107+
it('should be noop when config options is empty', () => {
108+
const fqbn = 'a:b:c:menu1=value1';
109+
const actual = ConfigOption.decorate(fqbn, []);
110+
expect(actual).to.be.equal(fqbn);
111+
});
112+
113+
it('should not duplicate config options', () => {
114+
const fqbn = 'a:b:c:menu1=value1';
115+
const actual = ConfigOption.decorate(fqbn, [
116+
{
117+
label: 'Menu 1',
118+
option: 'menu1',
119+
values: [
120+
{ label: 'Value 1', value: 'value1', selected: true },
121+
{ label: 'Another Value', value: 'another1', selected: false },
122+
],
123+
},
124+
]);
125+
expect(actual).to.be.equal(fqbn);
126+
});
127+
128+
it('should update config options', () => {
129+
const fqbn = 'a:b:c:menu1=value1,menu2=value2';
130+
const actual = ConfigOption.decorate(fqbn, [
131+
{
132+
label: 'Menu 1',
133+
option: 'menu1',
134+
values: [
135+
{ label: 'Value 1', value: 'value1', selected: false },
136+
{ label: 'Another Value', value: 'another1', selected: true },
137+
],
138+
},
139+
]);
140+
expect(actual).to.be.equal('a:b:c:menu1=another1,menu2=value2');
141+
});
142+
143+
it('should favor first over rest when there are duplicate options (duplicate on FQBN)', () => {
144+
const fqbn = 'a:b:c:menu1=value1,menu1=value2';
145+
const actual = ConfigOption.decorate(fqbn, [
146+
{
147+
label: 'Menu 1',
148+
option: 'menu1',
149+
values: [
150+
{ label: 'Value 1', value: 'value1', selected: false },
151+
{ label: 'Another Value', value: 'another1', selected: true },
152+
],
153+
},
154+
]);
155+
expect(actual).to.be.equal('a:b:c:menu1=another1');
156+
});
157+
158+
it('should favor first over rest when there are duplicate options (duplicate in config)', () => {
159+
const fqbn = 'a:b:c';
160+
const actual = ConfigOption.decorate(fqbn, [
161+
{
162+
label: 'Menu 1',
163+
option: 'menu1',
164+
values: [
165+
{ label: 'Value 1', value: 'value1', selected: false },
166+
{ label: 'Another Value', value: 'another1', selected: true },
167+
],
168+
},
169+
{
170+
label: 'Menu 1',
171+
option: 'menu1',
172+
values: [
173+
{ label: 'Value 1', value: 'value1', selected: true },
174+
{ label: 'Another Value', value: 'another1', selected: false },
175+
],
176+
},
177+
]);
178+
expect(actual).to.be.equal('a:b:c:menu1=another1');
179+
});
180+
181+
it('should not change the existing config order', () => {
182+
const fqbn = 'a:b:c:menu1=value1';
183+
const actual = ConfigOption.decorate(fqbn, [
184+
{
185+
label: 'Menu 0',
186+
option: 'menu0',
187+
values: [
188+
{ label: 'Value 0', value: 'value0', selected: true },
189+
{ label: 'Another Value', value: 'another0', selected: false },
190+
],
191+
},
192+
{
193+
label: 'Menu 1',
194+
option: 'menu1',
195+
values: [
196+
{ label: 'Value 1', value: 'value1', selected: false },
197+
{ label: 'Another Value', value: 'another1', selected: true },
198+
],
199+
},
200+
]);
201+
expect(actual).to.be.equal('a:b:c:menu1=another1,menu0=value0');
202+
});
203+
});
204+
96205
describe('getBoardInfo', () => {
97206
const vid = '0x0';
98207
const pid = '0x1';

0 commit comments

Comments
 (0)