From d564e246f3ffd08843d2de856464afd9c14d3169 Mon Sep 17 00:00:00 2001 From: Francesco Stasi Date: Tue, 7 Dec 2021 16:22:16 +0100 Subject: [PATCH 1/6] get daemon port from CLI stdout --- arduino-ide-extension/package.json | 2 +- .../browser/arduino-frontend-contribution.tsx | 2 +- .../src/common/protocol/arduino-daemon.ts | 1 + .../src/node/arduino-daemon-impl.ts | 52 +++++++++++---- .../src/node/core-client-provider.ts | 13 ++-- .../src/node/grpc-client-provider.ts | 9 ++- .../src/test/node/arduino-daemon-impl.test.ts | 64 ++++++------------- 7 files changed, 73 insertions(+), 70 deletions(-) diff --git a/arduino-ide-extension/package.json b/arduino-ide-extension/package.json index 47b28fbf4..fad4c7bf6 100644 --- a/arduino-ide-extension/package.json +++ b/arduino-ide-extension/package.json @@ -151,7 +151,7 @@ ], "arduino": { "cli": { - "version": "0.20.1" + "version": "0.20.2-rc1" }, "fwuploader": { "version": "2.0.0" diff --git a/arduino-ide-extension/src/browser/arduino-frontend-contribution.tsx b/arduino-ide-extension/src/browser/arduino-frontend-contribution.tsx index 369828217..b72ba4e29 100644 --- a/arduino-ide-extension/src/browser/arduino-frontend-contribution.tsx +++ b/arduino-ide-extension/src/browser/arduino-frontend-contribution.tsx @@ -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', diff --git a/arduino-ide-extension/src/common/protocol/arduino-daemon.ts b/arduino-ide-extension/src/common/protocol/arduino-daemon.ts index 641cc32ef..783590048 100644 --- a/arduino-ide-extension/src/common/protocol/arduino-daemon.ts +++ b/arduino-ide-extension/src/common/protocol/arduino-daemon.ts @@ -2,4 +2,5 @@ export const ArduinoDaemonPath = '/services/arduino-daemon'; export const ArduinoDaemon = Symbol('ArduinoDaemon'); export interface ArduinoDaemon { isRunning(): Promise; + getPort(): Promise; } diff --git a/arduino-ide-extension/src/node/arduino-daemon-impl.ts b/arduino-ide-extension/src/node/arduino-daemon-impl.ts index efa8a33c2..e8ff9f27b 100644 --- a/arduino-ide-extension/src/node/arduino-daemon-impl.ts +++ b/arduino-ide-extension/src/node/arduino-daemon-impl.ts @@ -42,6 +42,7 @@ export class ArduinoDaemonImpl protected _running = false; protected _ready = new Deferred(); protected _execPath: string | undefined; + protected _port: string; // Backend application lifecycle. @@ -55,12 +56,17 @@ export class ArduinoDaemonImpl return Promise.resolve(this._running); } + async getPort(): Promise { + return Promise.resolve(this._port); + } + async startDaemon(): Promise { 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, @@ -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', @@ -156,12 +166,15 @@ export class ArduinoDaemonImpl ]; } - protected async spawnDaemonProcess(): Promise { + protected async spawnDaemonProcess(): Promise<{ + daemon: ChildProcess; + port: string; + }> { const [cliPath, args] = await Promise.all([ this.getExecPath(), this.getSpawnArgs(), ]); - const ready = new Deferred(); + const ready = new Deferred<{ daemon: ChildProcess; port: string }>(); const options = { shell: true }; const daemon = spawn(`"${cliPath}"`, args, options); @@ -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 }); } } }); diff --git a/arduino-ide-extension/src/node/core-client-provider.ts b/arduino-ide-extension/src/node/core-client-provider.ts index aac884879..fc792a8bd 100644 --- a/arduino-ide-extension/src/node/core-client-provider.ts +++ b/arduino-ide-extension/src/node/core-client-provider.ts @@ -48,9 +48,9 @@ export class CoreClientProvider extends GrpcClientProvider(); } - protected async reconcileClient( - port: string | number | undefined - ): Promise { + protected async reconcileClient(): Promise { + 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)) { @@ -58,7 +58,7 @@ export class CoreClientProvider extends GrpcClientProvider { 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(); }); diff --git a/arduino-ide-extension/src/node/grpc-client-provider.ts b/arduino-ide-extension/src/node/grpc-client-provider.ts index d87ac026d..c4e5655c7 100644 --- a/arduino-ide-extension/src/node/grpc-client-provider.ts +++ b/arduino-ide-extension/src/node/grpc-client-provider.ts @@ -21,8 +21,7 @@ export abstract class GrpcClientProvider { @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); @@ -44,9 +43,9 @@ export abstract class GrpcClientProvider { } } - protected async reconcileClient( - port: string | number | undefined - ): Promise { + protected async reconcileClient(): Promise { + const port = await this.daemon.getPort(); + if (this._port === port) { return; // Nothing to do. } diff --git a/arduino-ide-extension/src/test/node/arduino-daemon-impl.test.ts b/arduino-ide-extension/src/test/node/arduino-daemon-impl.test.ts index 9d3aed9af..b1fa30046 100644 --- a/arduino-ide-extension/src/test/node/arduino-daemon-impl.test.ts +++ b/arduino-ide-extension/src/test/node/arduino-daemon-impl.test.ts @@ -2,21 +2,17 @@ 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(); } @@ -24,7 +20,7 @@ class SilentArduinoDaemonImpl extends ArduinoDaemonImpl { // NOOP } - async spawnDaemonProcess(): Promise { + async spawnDaemonProcess(): Promise<{ daemon: ChildProcess; port: string }> { return super.spawnDaemonProcess(); } @@ -32,6 +28,10 @@ class SilentArduinoDaemonImpl extends ArduinoDaemonImpl { const cliConfigPath = await this.initCliConfig(); return [ 'daemon', + '--format', + 'jsonmini', + '--port', + '0', '--config-file', cliConfigPath, '-v', @@ -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', @@ -113,43 +113,19 @@ 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 instance = await new SilentArduinoDaemonImpl( + 'json' + ).spawnDaemonProcess(); + expect(instance.port).not.to.be.undefined; + expect(instance.port).not.to.be.equal('0'); }); - 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); - } - }); - - 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 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); - } + it('should parse the port address when the log format is text', async () => { + const instance = await new SilentArduinoDaemonImpl( + 'text' + ).spawnDaemonProcess(); + expect(instance.port).not.to.be.undefined; + expect(instance.port).not.to.be.equal('0'); }); }); From 7cdd93757ac019d025e2abfd65c0b4a02499d21a Mon Sep 17 00:00:00 2001 From: Francesco Stasi Date: Thu, 9 Dec 2021 11:51:14 +0100 Subject: [PATCH 2/6] config-service to use CLI daemon port --- arduino-ide-extension/src/node/config-service-impl.ts | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/arduino-ide-extension/src/node/config-service-impl.ts b/arduino-ide-extension/src/node/config-service-impl.ts index fd215f9e0..66bf58375 100644 --- a/arduino-ide-extension/src/node/config-service-impl.ts +++ b/arduino-ide-extension/src/node/config-service-impl.ts @@ -75,9 +75,11 @@ export class ConfigServiceImpl async getConfiguration(): Promise { 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 { await this.ready.promise; if (Config.sameAs(this.config, config)) { @@ -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); From 4711c935e3ee53f5476d028c79bd34ba0f37d865 Mon Sep 17 00:00:00 2001 From: Francesco Stasi Date: Thu, 9 Dec 2021 12:01:31 +0100 Subject: [PATCH 3/6] updating LS --- arduino-ide-extension/scripts/download-ls.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arduino-ide-extension/scripts/download-ls.js b/arduino-ide-extension/scripts/download-ls.js index 6e53f9dcc..b7fb82145 100755 --- a/arduino-ide-extension/scripts/download-ls.js +++ b/arduino-ide-extension/scripts/download-ls.js @@ -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'); From 4998e0fde963e28fff56274fadb88225dab3c98a Mon Sep 17 00:00:00 2001 From: Francesco Stasi Date: Thu, 9 Dec 2021 12:59:20 +0100 Subject: [PATCH 4/6] fixed tests --- .../src/test/node/arduino-daemon-impl.test.ts | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/arduino-ide-extension/src/test/node/arduino-daemon-impl.test.ts b/arduino-ide-extension/src/test/node/arduino-daemon-impl.test.ts index b1fa30046..0fd6cf4d9 100644 --- a/arduino-ide-extension/src/test/node/arduino-daemon-impl.test.ts +++ b/arduino-ide-extension/src/test/node/arduino-daemon-impl.test.ts @@ -114,18 +114,22 @@ describe('arduino-daemon-impl', () => { // }); it('should parse the port address when the log format is json', async () => { - const instance = await new SilentArduinoDaemonImpl( + const { daemon, port } = await new SilentArduinoDaemonImpl( 'json' ).spawnDaemonProcess(); - expect(instance.port).not.to.be.undefined; - expect(instance.port).not.to.be.equal('0'); + + expect(port).not.to.be.undefined; + expect(port).not.to.be.equal('0'); + daemon.kill(); }); it('should parse the port address when the log format is text', async () => { - const instance = await new SilentArduinoDaemonImpl( + const { daemon, port } = await new SilentArduinoDaemonImpl( 'text' ).spawnDaemonProcess(); - expect(instance.port).not.to.be.undefined; - expect(instance.port).not.to.be.equal('0'); + + expect(port).not.to.be.undefined; + expect(port).not.to.be.equal('0'); + daemon.kill(); }); }); From 5cab51dca4ef7eb6a0a6d16c2abbcfdcb498c0d3 Mon Sep 17 00:00:00 2001 From: Alberto Iannaccone Date: Thu, 9 Dec 2021 14:45:08 +0100 Subject: [PATCH 5/6] fix upload blocked when selectedBoard.port is undefined --- .../src/browser/contributions/upload-sketch.ts | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/arduino-ide-extension/src/browser/contributions/upload-sketch.ts b/arduino-ide-extension/src/browser/contributions/upload-sketch.ts index fd294ed28..df196cb7f 100644 --- a/arduino-ide-extension/src/browser/contributions/upload-sketch.ts +++ b/arduino-ide-extension/src/browser/contributions/upload-sketch.ts @@ -63,7 +63,9 @@ export class UploadSketch extends SketchContribution { if (!fqbn) { return ''; } - const address = boardsConfig.selectedBoard?.port?.address; + const address = + boardsConfig.selectedBoard?.port?.address || + boardsConfig.selectedPort?.address; if (!address) { return ''; } @@ -277,8 +279,8 @@ export class UploadSketch extends SketchContribution { { timeout: 3000 } ); } catch (e) { - let errorMessage = ""; - if (typeof e === "string") { + let errorMessage = ''; + if (typeof e === 'string') { errorMessage = e; } else { errorMessage = e.toString(); From 1130487e79821afc3c0d13b96be377a234795f40 Mon Sep 17 00:00:00 2001 From: Alberto Iannaccone Date: Thu, 9 Dec 2021 14:53:21 +0100 Subject: [PATCH 6/6] bump arduino-cli to 0.20.2 --- arduino-ide-extension/package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arduino-ide-extension/package.json b/arduino-ide-extension/package.json index fad4c7bf6..d22f96f18 100644 --- a/arduino-ide-extension/package.json +++ b/arduino-ide-extension/package.json @@ -151,7 +151,7 @@ ], "arduino": { "cli": { - "version": "0.20.2-rc1" + "version": "0.20.2" }, "fwuploader": { "version": "2.0.0"