Skip to content

IDE to run CLI with auto assigned port #673

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 6 commits into from
Dec 9, 2021
Merged
Show file tree
Hide file tree
Changes from 4 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
2 changes: 1 addition & 1 deletion arduino-ide-extension/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@
],
"arduino": {
"cli": {
"version": "0.20.1"
"version": "0.20.2-rc1"
},
"fwuploader": {
"version": "2.0.0"
Expand Down
2 changes: 1 addition & 1 deletion arduino-ide-extension/scripts/download-ls.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
// - https://downloads.arduino.cc/arduino-language-server/clangd/clangd_${VERSION}_${SUFFIX}

(() => {
const DEFAULT_ALS_VERSION = '0.5.0-rc2';
const DEFAULT_ALS_VERSION = '0.5.0-rc6';
const DEFAULT_CLANGD_VERSION = 'snapshot_20210124';

const path = require('path');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -374,7 +374,7 @@ export class ArduinoFrontendContribution
'arduino.languageserver.start',
{
lsPath,
cliDaemonAddr: `localhost:${config.daemon.port}`,
cliDaemonAddr: `localhost:${config.daemon.port}`, // TODO: verify if this port is coming from the BE
clangdPath,
log: currentSketchPath ? currentSketchPath : log,
cliDaemonInstance: '1',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,5 @@ export const ArduinoDaemonPath = '/services/arduino-daemon';
export const ArduinoDaemon = Symbol('ArduinoDaemon');
export interface ArduinoDaemon {
isRunning(): Promise<boolean>;
getPort(): Promise<string>;
}
52 changes: 41 additions & 11 deletions arduino-ide-extension/src/node/arduino-daemon-impl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ export class ArduinoDaemonImpl
protected _running = false;
protected _ready = new Deferred<void>();
protected _execPath: string | undefined;
protected _port: string;

// Backend application lifecycle.

Expand All @@ -55,12 +56,17 @@ export class ArduinoDaemonImpl
return Promise.resolve(this._running);
}

async getPort(): Promise<string> {
return Promise.resolve(this._port);
}

async startDaemon(): Promise<void> {
try {
this.toDispose.dispose(); // This will `kill` the previously started daemon process, if any.
const cliPath = await this.getExecPath();
this.onData(`Starting daemon from ${cliPath}...`);
const daemon = await this.spawnDaemonProcess();
const { daemon, port } = await this.spawnDaemonProcess();
this._port = port;
// Watchdog process for terminating the daemon process when the backend app terminates.
spawn(
process.execPath,
Expand Down Expand Up @@ -148,6 +154,10 @@ export class ArduinoDaemonImpl
const cliConfigPath = join(FileUri.fsPath(configDirUri), CLI_CONFIG);
return [
'daemon',
'--format',
'jsonmini',
'--port',
'0',
'--config-file',
`"${cliConfigPath}"`,
'-v',
Expand All @@ -156,12 +166,15 @@ export class ArduinoDaemonImpl
];
}

protected async spawnDaemonProcess(): Promise<ChildProcess> {
protected async spawnDaemonProcess(): Promise<{
daemon: ChildProcess;
port: string;
}> {
const [cliPath, args] = await Promise.all([
this.getExecPath(),
this.getSpawnArgs(),
]);
const ready = new Deferred<ChildProcess>();
const ready = new Deferred<{ daemon: ChildProcess; port: string }>();
const options = { shell: true };
const daemon = spawn(`"${cliPath}"`, args, options);

Expand All @@ -171,20 +184,37 @@ export class ArduinoDaemonImpl

daemon.stdout.on('data', (data) => {
const message = data.toString();

let port = '';
let address = '';
message
.split('\n')
.filter((line: string) => line.length)
.forEach((line: string) => {
try {
const parsedLine = JSON.parse(line);
if ('Port' in parsedLine) {
port = parsedLine.Port;
}
if ('IP' in parsedLine) {
address = parsedLine.IP;
}
} catch (err) {
// ignore
}
});

this.onData(message);
if (!grpcServerIsReady) {
const error = DaemonError.parse(message);
if (error) {
ready.reject(error);
return;
}
for (const expected of [
'Daemon is listening on TCP port',
'Daemon is now listening on 127.0.0.1',
]) {
if (message.includes(expected)) {
grpcServerIsReady = true;
ready.resolve(daemon);
}

if (port.length && address.length) {
grpcServerIsReady = true;
ready.resolve({ daemon, port });
}
}
});
Expand Down
8 changes: 6 additions & 2 deletions arduino-ide-extension/src/node/config-service-impl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -75,9 +75,11 @@ export class ConfigServiceImpl

async getConfiguration(): Promise<Config> {
await this.ready.promise;
return this.config;
await this.daemon.ready;
return { ...this.config, daemon: { port: await this.daemon.getPort() } };
}

// Used by frontend to update the config.
async setConfiguration(config: Config): Promise<void> {
await this.ready.promise;
if (Config.sameAs(this.config, config)) {
Expand Down Expand Up @@ -108,7 +110,9 @@ export class ConfigServiceImpl
copyDefaultCliConfig.locale = locale || 'en';
const proxy = Network.stringify(network);
copyDefaultCliConfig.network = { proxy };
const { port } = copyDefaultCliConfig.daemon;

// always use the port of the daemon
const port = await this.daemon.getPort();
await this.updateDaemon(port, copyDefaultCliConfig);
await this.writeDaemonState(port);

Expand Down
13 changes: 5 additions & 8 deletions arduino-ide-extension/src/node/core-client-provider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,31 +48,28 @@ export class CoreClientProvider extends GrpcClientProvider<CoreClientProvider.Cl
this._initialized = new Deferred<void>();
}

protected async reconcileClient(
port: string | number | undefined
): Promise<void> {
protected async reconcileClient(): Promise<void> {
const port = await this.daemon.getPort();

if (port && port === this._port) {
// No need to create a new gRPC client, but we have to update the indexes.
if (this._client && !(this._client instanceof Error)) {
await this.updateIndexes(this._client);
this.onClientReadyEmitter.fire();
}
} else {
await super.reconcileClient(port);
await super.reconcileClient();
this.onClientReadyEmitter.fire();
}
}

@postConstruct()
protected async init(): Promise<void> {
this.daemon.ready.then(async () => {
const cliConfig = this.configService.cliConfiguration;
// First create the client and the instance synchronously
// and notify client is ready.
// TODO: Creation failure should probably be handled here
await this.reconcileClient(
cliConfig ? cliConfig.daemon.port : undefined
).then(() => {
await this.reconcileClient().then(() => {
this._created.resolve();
});

Expand Down
9 changes: 4 additions & 5 deletions arduino-ide-extension/src/node/grpc-client-provider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,7 @@ export abstract class GrpcClientProvider<C> {
@postConstruct()
protected init(): void {
const updateClient = () => {
const cliConfig = this.configService.cliConfiguration;
this.reconcileClient(cliConfig ? cliConfig.daemon.port : undefined);
this.reconcileClient();
};
this.configService.onConfigChange(updateClient);
this.daemon.ready.then(updateClient);
Expand All @@ -44,9 +43,9 @@ export abstract class GrpcClientProvider<C> {
}
}

protected async reconcileClient(
port: string | number | undefined
): Promise<void> {
protected async reconcileClient(): Promise<void> {
const port = await this.daemon.getPort();

if (this._port === port) {
return; // Nothing to do.
}
Expand Down
64 changes: 22 additions & 42 deletions arduino-ide-extension/src/test/node/arduino-daemon-impl.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,36 +2,36 @@ import * as fs from 'fs';
// import * as net from 'net';
import * as path from 'path';
import * as temp from 'temp';
import { fail } from 'assert';
import { expect } from 'chai';
import { ChildProcess } from 'child_process';
import { safeLoad, safeDump } from 'js-yaml';
import { DaemonError, ArduinoDaemonImpl } from '../../node/arduino-daemon-impl';
import { ArduinoDaemonImpl } from '../../node/arduino-daemon-impl';
import { spawnCommand } from '../../node/exec-util';
import { CLI_CONFIG } from '../../node/cli-config';

const track = temp.track();

class SilentArduinoDaemonImpl extends ArduinoDaemonImpl {
constructor(
private port: string | number,
private logFormat: 'text' | 'json'
) {
constructor(private logFormat: 'text' | 'json') {
super();
}

onData(data: string): void {
// NOOP
}

async spawnDaemonProcess(): Promise<ChildProcess> {
async spawnDaemonProcess(): Promise<{ daemon: ChildProcess; port: string }> {
return super.spawnDaemonProcess();
}

protected async getSpawnArgs(): Promise<string[]> {
const cliConfigPath = await this.initCliConfig();
return [
'daemon',
'--format',
'jsonmini',
'--port',
'0',
'--config-file',
cliConfigPath,
'-v',
Expand All @@ -53,7 +53,7 @@ class SilentArduinoDaemonImpl extends ArduinoDaemonImpl {
encoding: 'utf8',
});
const cliConfig = safeLoad(content) as any;
cliConfig.daemon.port = String(this.port);
// cliConfig.daemon.port = String(this.port);
const modifiedContent = safeDump(cliConfig);
fs.writeFileSync(path.join(destDir, CLI_CONFIG), modifiedContent, {
encoding: 'utf8',
Expand Down Expand Up @@ -113,43 +113,23 @@ describe('arduino-daemon-impl', () => {
// }
// });

it('should parse an error - unknown address [json]', async () => {
try {
await new SilentArduinoDaemonImpl('foo', 'json').spawnDaemonProcess();
fail('Expected a failure.');
} catch (e) {
expect(e).to.be.instanceOf(DaemonError);
expect(e.code).to.be.equal(DaemonError.UNKNOWN_ADDRESS);
}
});
it('should parse the port address when the log format is json', async () => {
const { daemon, port } = await new SilentArduinoDaemonImpl(
'json'
).spawnDaemonProcess();

it('should parse an error - unknown address [text]', async () => {
try {
await new SilentArduinoDaemonImpl('foo', 'text').spawnDaemonProcess();
fail('Expected a failure.');
} catch (e) {
expect(e).to.be.instanceOf(DaemonError);
expect(e.code).to.be.equal(DaemonError.UNKNOWN_ADDRESS);
}
expect(port).not.to.be.undefined;
expect(port).not.to.be.equal('0');
daemon.kill();
});

it('should parse an error - invalid port [json]', async () => {
try {
await new SilentArduinoDaemonImpl(-1, 'json').spawnDaemonProcess();
fail('Expected a failure.');
} catch (e) {
expect(e).to.be.instanceOf(DaemonError);
expect(e.code).to.be.equal(DaemonError.INVALID_PORT);
}
});
it('should parse the port address when the log format is text', async () => {
const { daemon, port } = await new SilentArduinoDaemonImpl(
'text'
).spawnDaemonProcess();

it('should parse an error - invalid port [text]', async () => {
try {
await new SilentArduinoDaemonImpl(-1, 'text').spawnDaemonProcess();
fail('Expected a failure.');
} catch (e) {
expect(e).to.be.instanceOf(DaemonError);
expect(e.code).to.be.equal(DaemonError.INVALID_PORT);
}
expect(port).not.to.be.undefined;
expect(port).not.to.be.equal('0');
daemon.kill();
});
});