IDE to run CLI with auto assigned port (#673)

* get daemon port from CLI stdout

* config-service to use CLI daemon port

* updating LS

* fixed tests

* fix upload blocked when selectedBoard.port is undefined

* bump arduino-cli to 0.20.2

Co-authored-by: Alberto Iannaccone <[email protected]>

Author: fstasi

Diff for: arduino-ide-extension/package.json

@@ -151,7 +151,7 @@
   ],
   "arduino": {
     "cli": {
-      "version": "0.20.1"
+      "version": "0.20.2"
     },
     "fwuploader": {
       "version": "2.0.0"

Diff for: 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');

Diff for: 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 for: 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();

Diff for: 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<boolean>;
+  getPort(): Promise<string>;
 }

Diff for: arduino-ide-extension/src/node/arduino-daemon-impl.ts

@@ -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.
 
@@ -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,
@@ -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<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);
 
@@ -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 for: arduino-ide-extension/src/node/config-service-impl.ts

@@ -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)) {
@@ -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);
 

Diff for: arduino-ide-extension/src/node/core-client-provider.ts

@@ -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();
       });
 

Diff for: arduino-ide-extension/src/node/grpc-client-provider.ts

@@ -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);
@@ -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.
     }

Diff for: arduino-ide-extension/src/test/node/arduino-daemon-impl.test.ts

@@ -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',
@@ -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,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();
   });
 });