Skip to content

fix: programmer is optional for debug -I check #47

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Feb 19, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{
"private": true,
"name": "vscode-arduino-tools",
"version": "0.1.2",
"version": "0.1.3",
"description": "Arduino Tools extension for VS Code",
"license": "Apache-2.0",
"author": "Arduino SA",
Expand All @@ -25,7 +25,7 @@
"compile-tests": "tsc -p . --outDir lib",
"format": "prettier --write . && prettier-package-json --write ./package.json",
"generate": "node ./scripts/generate.js 1.5.1",
"postinstall": "node ./scripts/cli 0.35.2",
"postinstall": "node ./scripts/cli 0.35.3",
"lint": "eslint src --ext ts",
"prepackage": "yarn clean && yarn compile && yarn lint && yarn webpack",
"package": "mkdirp build-artifacts && vsce package --out ./build-artifacts",
Expand Down
34 changes: 20 additions & 14 deletions src/debug.ts
Original file line number Diff line number Diff line change
Expand Up @@ -142,9 +142,6 @@ async function createLaunchConfig(
params: StartDebugParams
): Promise<ArduinoDebugLaunchConfig> {
const { programmer, board } = params;
if (!programmer) {
throw new Error('Missing programmer');
}
const { file, args } = buildDebugInfoArgs(params);
const [stdout, customConfigs] = await Promise.all([
withProgress(
Expand Down Expand Up @@ -206,8 +203,14 @@ async function parseRawDebugInfo(
return config;
}

function createConfigId(board: BoardIdentifier, programmer: string): string {
function createConfigId(
board: BoardIdentifier,
programmer: string | undefined
): string {
const fqbn = new FQBN(board.fqbn);
if (!programmer) {
return fqbn.toString();
}
if (hasConfigOptions(fqbn)) {
// if already has config options, append the programmer as an ordinary custom board config option
return `${fqbn.toString()},programmer=${programmer}`;
Expand Down Expand Up @@ -294,7 +297,7 @@ function resolveCliConfigPath(

async function mergeLaunchConfig(
board: BoardIdentifier,
programmer: string,
programmer: string | undefined,
debugInfo: DebugInfo & Executable,
customConfigs: CustomDebugConfigs
): Promise<ArduinoDebugLaunchConfig> {
Expand Down Expand Up @@ -326,7 +329,10 @@ async function mergeLaunchConfig(
return launchConfig;
}

function createName(board: BoardIdentifier, programmer: string): string {
function createName(
board: BoardIdentifier,
programmer: string | undefined
): string {
if (!board.name) {
const configId = createConfigId(board, programmer);
return `Arduino (${configId})`;
Expand All @@ -336,9 +342,9 @@ function createName(board: BoardIdentifier, programmer: string): string {
const options = Object.entries(fqbn.options)
.map(([key, value]) => `${key}=${value}`)
.join(',');
return `${board.name} (${options},${programmer})`;
return `${board.name} (${options}${programmer ? `,${programmer}` : ''})`;
}
return `${board.name} (${programmer})`;
return `${board.name}${programmer ? ` (${programmer})` : ''}`;
}

function replaceValue(
Expand Down Expand Up @@ -494,12 +500,12 @@ export class CliError extends Error {
}
}

// https://github.com/arduino/arduino-cli/blob/b41f4044cac6ab7f7d853e368bc31e5d626d63d4/internal/cli/feedback/errorcodes.go#L57-L58
const missingProgrammerCode = 11 as const;
function isMissingProgrammerError(
// https://github.com/arduino/arduino-cli/blob/b41f4044cac6ab7f7d853e368bc31e5d626d63d4/internal/cli/feedback/errorcodes.go#L43-L44
const badArgumentCode = 7 as const;
function isBadArgumentError(
arg: unknown
): arg is CliError & { exitCode: typeof missingProgrammerCode } {
return arg instanceof CliError && arg.exitCode === missingProgrammerCode;
): arg is CliError & { exitCode: typeof badArgumentCode } {
return arg instanceof CliError && arg.exitCode === badArgumentCode;
}

// Remove index signature
Expand Down Expand Up @@ -536,6 +542,6 @@ export const __tests__ = {
mergeLaunchConfig,
updateLaunchConfigs,
isCommandError,
isMissingProgrammerError,
isBadArgumentError,
cliExec,
} as const;
15 changes: 6 additions & 9 deletions src/test/suite/debug.slow-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,8 @@ import { __tests__ } from '../../debug';
import { exec } from '../../exec';
import { TestEnv } from '../testEnv';

const {
CliError,
buildDebugInfoArgs,
createLaunchConfig,
isMissingProgrammerError,
} = __tests__;
const { CliError, buildDebugInfoArgs, createLaunchConfig, isBadArgumentError } =
__tests__;

describe('debug (slow)', function () {
this.slow(2_000);
Expand Down Expand Up @@ -168,14 +164,15 @@ describe('debug (slow)', function () {
});

['en', 'it'].map((locale) =>
it(`should fail when the programmer is missing (locale: ${locale})`, async function () {
// Note: this test used to test the missing programmer error, but the programmer is optional after https://github.com/arduino/arduino-cli/pull/2544 so it's testing the bad request.
it(`should fail with bad argument (error code 7) if the debugging is not supported by the board (locale: ${locale})`, async function () {
if (!testEnv.cliConfigPaths[locale]) {
this.skip();
}
await assert.rejects(
// Can be any arbitrary board that does not have a default programmer defined in the platform. Otherwise, the error does not occur.
// Can be any arbitrary board that does not support debugging.
cliExec(['debug', '-I', '-b', 'arduino:avr:uno', sketchPath], locale),
(reason) => isMissingProgrammerError(reason)
(reason) => isBadArgumentError(reason)
);
})
);
Expand Down
29 changes: 29 additions & 0 deletions src/test/suite/debug.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -363,10 +363,21 @@ describe('debug', () => {
const actual = createConfigId({ fqbn: 'a:b:c' }, 'p');
assert.strictEqual(actual, 'a:b:c:programmer=p');
});

it('should create the configuration ID when the FQBN has no custom board options (no programmer)', () => {
const actual = createConfigId({ fqbn: 'a:b:c' }, undefined);
assert.strictEqual(actual, 'a:b:c');
});

it('should create the configuration ID when the FQBN has custom board options', () => {
const actual = createConfigId({ fqbn: 'a:b:c:o1=v1' }, 'p');
assert.strictEqual(actual, 'a:b:c:o1=v1,programmer=p');
});

it('should create the configuration ID when the FQBN has custom board options (no programmer)', () => {
const actual = createConfigId({ fqbn: 'a:b:c:o1=v1' }, undefined);
assert.strictEqual(actual, 'a:b:c:o1=v1');
});
});
describe('createName', () => {
it('should use the generated config ID if the board name is absent', () => {
Expand All @@ -386,19 +397,37 @@ describe('debug', () => {
);
});

it('should use the generated config ID with the custom board options if the board name is absent (no programmer)', () => {
const board = { fqbn: 'a:b:c:UsbMode=default' };
const actual = createName(board, undefined);
assert.strictEqual(actual, 'Arduino (a:b:c:UsbMode=default)');
});

it('should use the board name', () => {
const board = { fqbn: 'a:b:c', name: 'board name' };
const programmer = 'p1';
const actual = createName(board, programmer);
assert.strictEqual(actual, 'board name (p1)');
});

it('should use the board name (no programmer)', () => {
const board = { fqbn: 'a:b:c', name: 'board name' };
const actual = createName(board, undefined);
assert.strictEqual(actual, 'board name');
});

it('should use the board name and all custom board options', () => {
const board = { fqbn: 'a:b:c:UsbMode=default', name: 'board name' };
const programmer = 'p1';
const actual = createName(board, programmer);
assert.strictEqual(actual, 'board name (UsbMode=default,p1)');
});

it('should use the board name and all custom board options (no programmer)', () => {
const board = { fqbn: 'a:b:c:UsbMode=default', name: 'board name' };
const actual = createName(board, undefined);
assert.strictEqual(actual, 'board name (UsbMode=default)');
});
});

describe('isCustomDebugConfig', () => {
Expand Down
2 changes: 1 addition & 1 deletion src/test/testEnv.ts
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ async function prepareWithCli(params: PrepareTestEnvParams): Promise<void> {
log(`Installing ${toInstall}...`);
const args = ['core', 'install', toInstall, '--skip-post-install'];
await cliExec(args, cliConfigPath);
log(`Done. Installed ${platform}`);
log(`Done. Installed ${toInstall}`);
}
log('Done');
}
Expand Down