Solve ports conflicts with same address and different protocol

arduino-ide-extension/src/browser/arduino-frontend-contribution.tsx

@@ -3,7 +3,6 @@ import * as React from 'react';
 import { remote } from 'electron';
 import {
   BoardsService,
-  Port,
   SketchesService,
   ExecutableService,
   Sketch,
@@ -216,7 +215,7 @@ export class ArduinoFrontendContribution
             ? nls.localize(
                 'arduino/common/selectedOn',
                 'on {0}',
-                Port.toString(selectedPort)
+                selectedPort.address
               )
             : nls.localize('arduino/common/notConnected', '[not connected]'),
           className: 'arduino-selected-port',

arduino-ide-extension/src/browser/boards/boards-config.tsx

@@ -167,7 +167,7 @@ export class BoardsConfig extends React.Component<
     this.queryPorts(Promise.resolve(ports)).then(({ knownPorts }) => {
       let { selectedPort } = this.state;
       // If the currently selected port is not available anymore, unset the selected port.
-      if (removedPorts.some((port) => Port.equals(port, selectedPort))) {
+      if (removedPorts.some((port) => Port.sameAs(port, selectedPort))) {
         selectedPort = undefined;
       }
       this.setState({ knownPorts, selectedPort }, () =>
@@ -213,11 +213,11 @@ export class BoardsConfig extends React.Component<
       } else if (left.protocol === right.protocol) {
         // We show ports, including those that have guessed
         // or unrecognized boards, so we must sort those too.
-        const leftBoard = this.availableBoards.find((board) =>
+        const leftBoard = this.availableBoards.find(
-          Port.sameAs(board.port, left)
+          (board) => board.port === left
         );
-        const rightBoard = this.availableBoards.find((board) =>
+        const rightBoard = this.availableBoards.find(
-          Port.sameAs(board.port, right)
+          (board) => board.port === right
         );
         if (leftBoard && !rightBoard) {
           return -1;
@@ -348,10 +348,10 @@ export class BoardsConfig extends React.Component<
       <div className="ports list">
         {ports.map((port) => (
           <Item<Port>
-            key={Port.toString(port)}
+            key={`${port.id}`}
             item={port}
             label={Port.toString(port)}
-            selected={Port.equals(this.state.selectedPort, port)}
+            selected={Port.sameAs(this.state.selectedPort, port)}
             onClick={this.selectPort}
           />
         ))}
@@ -410,7 +410,7 @@ export namespace BoardsConfig {
         return options.default;
       }
       const { name } = selectedBoard;
-      return `${name}${port ? ' at ' + Port.toString(port) : ''}`;
+      return `${name}${port ? ` at ${port.address}` : ''}`;
     }
 
     export function setConfig(

arduino-ide-extension/src/browser/boards/boards-service-provider.ts

@@ -244,7 +244,7 @@ export class BoardsServiceProvider implements FrontendApplicationContribution {
   }
 
   set boardsConfig(config: BoardsConfig.Config) {
-    this.doSetBoardsConfig(config);
+    this.setBoardsConfig(config);
     this.saveState().finally(() =>
       this.reconcileAvailableBoards().finally(() =>
         this.onBoardsConfigChangedEmitter.fire(this._boardsConfig)
@@ -256,7 +256,7 @@ export class BoardsServiceProvider implements FrontendApplicationContribution {
     return this._boardsConfig;
   }
 
-  protected doSetBoardsConfig(config: BoardsConfig.Config): void {
+  protected setBoardsConfig(config: BoardsConfig.Config): void {
     this.logger.info('Board config changed: ', JSON.stringify(config));
     this._boardsConfig = config;
     this.latestBoardsConfig = this._boardsConfig;
@@ -370,7 +370,7 @@ export class BoardsServiceProvider implements FrontendApplicationContribution {
     const find = (needle: Board & { port: Port }, haystack: AvailableBoard[]) =>
       haystack.find(
         (board) =>
-          Board.equals(needle, board) && Port.equals(needle.port, board.port)
+          Board.equals(needle, board) && Port.sameAs(needle.port, board.port)
       );
     const timeoutTask =
       !!timeout && timeout > 0
@@ -409,7 +409,7 @@ export class BoardsServiceProvider implements FrontendApplicationContribution {
         Port.sameAs(port, this.boardsConfig.selectedPort)
       )
     ) {
-      this.doSetBoardsConfig({
+      this.setBoardsConfig({
         selectedBoard: this.boardsConfig.selectedBoard,
         selectedPort: undefined,
       });
@@ -533,8 +533,7 @@ export class BoardsServiceProvider implements FrontendApplicationContribution {
 
   protected getLastSelectedBoardOnPortKey(port: Port | string): string {
     // TODO: we lose the port's `protocol` info (`serial`, `network`, etc.) here if the `port` is a `string`.
-    return `last-selected-board-on-port:${
+    return `last-selected-board-on-port:${typeof port === 'string' ? port : port.address
-      typeof port === 'string' ? port : Port.toString(port)
       }`;
   }
 

arduino-ide-extension/src/browser/contributions/board-selection.ts

@@ -204,8 +204,7 @@ PID: ${PID}`;
 
       const packageLabel =
         packageName +
-        `${
+        `${manuallyInstalled
-          manuallyInstalled
           ? nls.localize('arduino/board/inSketchbook', ' (in Sketchbook)')
           : ''
         }`;
@@ -255,8 +254,8 @@ PID: ${PID}`;
       protocolOrder: number,
       ports: AvailablePorts
     ) => {
-      const addresses = Object.keys(ports);
+      const portIDs = Object.keys(ports);
-      if (!addresses.length) {
+      if (!portIDs.length) {
         return;
       }
 
@@ -279,27 +278,27 @@ PID: ${PID}`;
 
       // First we show addresses with recognized boards connected,
       // then all the rest.
-      const sortedAddresses = Object.keys(ports);
+      const sortedIDs = Object.keys(ports);
-      sortedAddresses.sort((left: string, right: string): number => {
+      sortedIDs.sort((left: string, right: string): number => {
         const [, leftBoards] = ports[left];
         const [, rightBoards] = ports[right];
         return rightBoards.length - leftBoards.length;
       });
 
-      for (let i = 0; i < sortedAddresses.length; i++) {
+      for (let i = 0; i < sortedIDs.length; i++) {
-        const address = sortedAddresses[i];
+        const portID = sortedIDs[i];
-        const [port, boards] = ports[address];
+        const [port, boards] = ports[portID];
-        let label = `${address}`;
+        let label = `${port.address}`;
         if (boards.length) {
           const boardsList = boards.map((board) => board.name).join(', ');
           label = `${label} (${boardsList})`;
         }
-        const id = `arduino-select-port--${address}`;
+        const id = `arduino-select-port--${portID}`;
         const command = { id };
         const handler = {
           execute: () => {
             if (
-              !Port.equals(
+              !Port.sameAs(
                 port,
                 this.boardsServiceProvider.boardsConfig.selectedPort
               )
@@ -312,7 +311,7 @@ PID: ${PID}`;
             }
           },
           isToggled: () =>
-            Port.equals(
+            Port.sameAs(
               port,
               this.boardsServiceProvider.boardsConfig.selectedPort
             ),

arduino-ide-extension/src/browser/serial/monitor/serial-monitor-send-input.tsx

@@ -1,6 +1,6 @@
 import * as React from 'react';
 import { Key, KeyCode } from '@theia/core/lib/browser/keys';
-import { Board, Port } from '../../../common/protocol/boards-service';
+import { Board } from '../../../common/protocol/boards-service';
 import { isOSX } from '@theia/core/lib/common/os';
 import { DisposableCollection, nls } from '@theia/core/lib/common';
 import { SerialConnectionManager } from '../serial-connection-manager';
@@ -87,7 +87,7 @@ export class SerialMonitorSendInput extends React.Component<
             useFqbn: false,
           })
         : 'unknown',
-      port ? Port.toString(port) : 'unknown'
+      port ? port.address : 'unknown'
     );
   }
 

arduino-ide-extension/src/browser/serial/serial-connection-manager.ts

@@ -10,7 +10,6 @@ import {
 } from '../../common/protocol/serial-service';
 import { BoardsServiceProvider } from '../boards/boards-service-provider';
 import {
-  Port,
   Board,
   BoardsService,
 } from '../../common/protocol/boards-service';
@@ -217,7 +216,7 @@ export class SerialConnectionManager {
           nls.localize(
             'arduino/serial/connectionBusy',
             'Connection failed. Serial port is busy: {0}',
-            Port.toString(port)
+            port.address
           ),
           options
         );
@@ -232,7 +231,7 @@ export class SerialConnectionManager {
             Board.toString(board, {
               useFqbn: false,
             }),
-            Port.toString(port)
+            port.address
           ),
           options
         );
@@ -244,7 +243,7 @@ export class SerialConnectionManager {
             'arduino/serial/unexpectedError',
             'Unexpected error. Reconnecting {0} on port {1}.',
             Board.toString(board),
-            Port.toString(port)
+            port.address
           ),
           options
         );
@@ -262,7 +261,7 @@ export class SerialConnectionManager {
             Board.toString(board, {
               useFqbn: false,
             }),
-            Port.toString(port)
+            port.address
           )
         );
         this.serialErrors.length = 0;
@@ -280,7 +279,7 @@ export class SerialConnectionManager {
             Board.toString(board, {
               useFqbn: false,
             }),
-            Port.toString(port),
+            port.address,
             attempts.toString()
           )
         );
@@ -351,7 +350,7 @@ export namespace Serial {
     export function toString(config: Partial<SerialConfig>): string {
       if (!isSerialConfig(config)) return '';
       const { board, port } = config;
-      return `${Board.toString(board)} ${Port.toString(port)}`;
+      return `${Board.toString(board)} ${port.address}`;
     }
   }
 }

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

@@ -7,13 +7,13 @@ export type AvailablePorts = Record<string, [Port, Array<Board>]>;
 export namespace AvailablePorts {
   export function byProtocol(availablePorts: AvailablePorts): Map<string, AvailablePorts> {
     const grouped = new Map<string, AvailablePorts>();
-    for (const address of Object.keys(availablePorts)) {
+    for (const portID of Object.keys(availablePorts)) {
-      const [port, boards] = availablePorts[address];
+      const [port, boards] = availablePorts[portID];
       let ports = grouped.get(port.protocol);
       if (!ports) {
         ports = {} as AvailablePorts;
       }
-      ports[address] = [port, boards];
+      ports[portID] = [port, boards];
       grouped.set(port.protocol, ports);
     }
     return grouped;
@@ -43,7 +43,7 @@ export namespace AttachedBoardsChangeEvent {
       const visitedDetachedPorts: Port[] = [];
       for (const board of attached.boards) {
         const port = board.port
-          ? ` on ${Port.toString(board.port, { useLabel: true })}`
+          ? ` on ${Port.toString(board.port)}`
           : '';
         rows.push(` - Attached board: ${Board.toString(board)}${port}`);
         if (board.port) {
@@ -52,7 +52,7 @@ export namespace AttachedBoardsChangeEvent {
       }
       for (const board of detached.boards) {
         const port = board.port
-          ? ` from ${Port.toString(board.port, { useLabel: true })}`
+          ? ` from ${Port.toString(board.port)}`
           : '';
         rows.push(` - Detached board: ${Board.toString(board)}${port}`);
         if (board.port) {
@@ -62,18 +62,14 @@ export namespace AttachedBoardsChangeEvent {
       for (const port of attached.ports) {
         if (!visitedAttachedPorts.find((p) => Port.sameAs(port, p))) {
           rows.push(
-            ` - New port is available on ${Port.toString(port, {
+            ` - New port is available on ${Port.toString(port)}`
-              useLabel: true,
-            })}`
           );
         }
       }
       for (const port of detached.ports) {
         if (!visitedDetachedPorts.find((p) => Port.sameAs(port, p))) {
           rows.push(
-            ` - Port is no longer available on ${Port.toString(port, {
+            ` - Port is no longer available on ${Port.toString(port)}`
-              useLabel: true,
-            })}`
           );
         }
       }
@@ -147,12 +143,14 @@ export interface BoardsService
 }
 
 export interface Port {
+  // id is the combination of address and protocol
+  // formatted like "<address>|<protocol>" used
+  // to univocally recognize a port
+  readonly id: string;
   readonly address: string;
+  readonly addressLabel: string;
   readonly protocol: string;
-  /**
+  readonly protocolLabel: string;
-   * Optional label for the protocol. For example: `Serial Port (USB)`.
-   */
-  readonly label?: string;
 }
 export namespace Port {
   export function is(arg: any): arg is Port {
@@ -165,14 +163,8 @@ export namespace Port {
     );
   }
 
-  export function toString(
+  export function toString(port: Port): string {
-    port: Port,
+    return `${port.addressLabel} ${port.protocolLabel}`;
-    options: { useLabel: boolean } = { useLabel: false }
-  ): string {
-    if (options.useLabel && port.label) {
-      return `${port.address} ${port.label}`;
-    }
-    return port.address;
   }
 
   export function compare(left: Port, right: Port): number {
@@ -192,29 +184,12 @@ export namespace Port {
     return naturalCompare(left.address!, right.address!);
   }
 
-  export function equals(
+  export function sameAs(
     left: Port | undefined,
     right: Port | undefined
   ): boolean {
     if (left && right) {
-      return (
+      return left.address === right.address && left.protocol === right.protocol;
-        left.address === right.address &&
-        left.protocol === right.protocol &&
-        (left.label || '') === (right.label || '')
-      );
-    }
-    return left === right;
-  }
-
-  export function sameAs(
-    left: Port | undefined,
-    right: Port | string | undefined
-  ) {
-    if (left && right) {
-      if (typeof right === 'string') {
-        return left.address === right;
-      }
-      return left.address === right.address;
     }
     return false;
   }

arduino-ide-extension/src/node/board-discovery.ts

@@ -124,8 +124,22 @@ export class BoardDiscovery extends CoreClientAware {
 
         const address = (detectedPort as any).getPort().getAddress();
         const protocol = (detectedPort as any).getPort().getProtocol();
+        // Different discoveries can detect the same port with different
+        // protocols, so we consider the combination of address and protocol
+        // to be the id of a certain port to distinguish it from others.
+        // If we'd use only the address of a port to store it in a map
+        // we can have conflicts the same port is found with multiple
+        // protocols.
+        const portID = `${address}|${protocol}`;
         const label = (detectedPort as any).getPort().getLabel();
-        const port = { address, protocol, label };
+        const protocolLabel = (detectedPort as any).getPort().getProtocolLabel();
+        const port = {
+          id: portID,
+          address,
+          addressLabel: label,
+          protocol,
+          protocolLabel,
+        };
         const boards: Board[] = [];
         for (const item of detectedPort.getMatchingBoardsList()) {
           boards.push({
@@ -136,23 +150,21 @@ export class BoardDiscovery extends CoreClientAware {
         }
 
         if (eventType === 'add') {
-          if (newState[port.address]) {
+          if (newState[portID]) {
-            const [, knownBoards] = newState[port.address];
+            const [, knownBoards] = newState[portID];
             console.warn(
-              `Port '${
+              `Port '${Port.toString(port)}' was already available. Known boards before override: ${JSON.stringify(
-                port.address
-              }' was already available. Known boards before override: ${JSON.stringify(
                 knownBoards
               )}`
             );
           }
-          newState[port.address] = [port, boards];
+          newState[portID] = [port, boards];
         } else if (eventType === 'remove') {
-          if (!newState[port.address]) {
+          if (!newState[portID]) {
-            console.warn(`Port '${port.address}' was not available. Skipping`);
+            console.warn(`Port '${Port.toString(port)}' was not available. Skipping`);
             return;
           }
-          delete newState[port.address];
+          delete newState[portID];
         }
 
         const oldAvailablePorts = this.getAvailablePorts(oldState);
@@ -179,8 +191,8 @@ export class BoardDiscovery extends CoreClientAware {
 
   getAttachedBoards(state: AvailablePorts = this.state): Board[] {
     const attachedBoards: Board[] = [];
-    for (const address of Object.keys(state)) {
+    for (const portID of Object.keys(state)) {
-      const [, boards] = state[address];
+      const [, boards] = state[portID];
       attachedBoards.push(...boards);
     }
     return attachedBoards;
@@ -188,8 +200,8 @@ export class BoardDiscovery extends CoreClientAware {
 
   getAvailablePorts(state: AvailablePorts = this.state): Port[] {
     const availablePorts: Port[] = [];
-    for (const address of Object.keys(state)) {
+    for (const portID of Object.keys(state)) {
-      const [port] = state[address];
+      const [port] = state[portID];
       availablePorts.push(port);
     }
     return availablePorts;

arduino-ide-extension/src/node/core-service-impl.ts

@@ -159,8 +159,9 @@ export class CoreServiceImpl extends CoreClientAware implements CoreService {
     const p = new Port();
     if (port) {
       p.setAddress(port.address);
-      p.setLabel(port.label || '');
+      p.setLabel(port.addressLabel);
       p.setProtocol(port.protocol);
+      p.setProtocolLabel(port.protocolLabel);
     }
     req.setPort(p);
     if (programmer) {
@@ -229,8 +230,9 @@ export class CoreServiceImpl extends CoreClientAware implements CoreService {
     const p = new Port();
     if (port) {
       p.setAddress(port.address);
-      p.setLabel(port.label || '');
+      p.setLabel(port.addressLabel);
       p.setProtocol(port.protocol);
+      p.setProtocolLabel(port.protocolLabel);
     }
     burnReq.setPort(p);
     if (programmer) {

arduino-ide-extension/src/node/serial/serial-service-impl.ts

@@ -16,7 +16,7 @@ import {
   MonitorConfig as GrpcMonitorConfig,
 } from '../cli-protocol/cc/arduino/cli/monitor/v1/monitor_pb';
 import { MonitorClientProvider } from './monitor-client-provider';
-import { Board, Port } from '../../common/protocol/boards-service';
+import { Board } from '../../common/protocol/boards-service';
 import { WebSocketService } from '../web-socket/web-socket-service';
 import { SerialPlotter } from '../../browser/serial/plotter/protocol';
 import { Disposable } from '@theia/core/shared/vscode-languageserver-protocol';
@@ -88,7 +88,7 @@ export class SerialServiceImpl implements SerialService {
 
     @inject(WebSocketService)
     protected readonly webSocketService: WebSocketService
-  ) {}
+  ) { }
 
   async isSerialPortOpen(): Promise<boolean> {
     return !!this.serialConnection;
@@ -153,7 +153,7 @@ export class SerialServiceImpl implements SerialService {
     this.logger.info(
       `>>> Creating serial connection for ${Board.toString(
         this.serialConfig.board
-      )} on port ${Port.toString(this.serialConfig.port)}...`
+      )} on port ${this.serialConfig.port.address}...`
     );
 
     if (this.serialConnection) {
@@ -225,7 +225,7 @@ export class SerialServiceImpl implements SerialService {
             default:
               break;
           }
-        } catch (error) {}
+        } catch (error) { }
       }
     );
 
@@ -277,7 +277,7 @@ export class SerialServiceImpl implements SerialService {
             : 'unknown board';
 
           const portName = this.serialConfig?.port
-            ? Port.toString(this.serialConfig.port)
+            ? this.serialConfig.port.address
             : 'unknown port';
           this.logger.info(
             `<<< Serial connection created for ${boardName} on port ${portName}.`
@@ -330,7 +330,7 @@ export class SerialServiceImpl implements SerialService {
         this.logger.info(
           `<<< Disposed serial connection for ${Board.toString(config.board, {
             useFqbn: false,
-          })} on port ${Port.toString(config.port)}.`
+          })} on port ${config.port.address}.`
         );
 
         duplex.cancel();

arduino-ide-extension/src/test/browser/fixtures/boards.ts

@@ -4,11 +4,20 @@ import { Board, BoardsPackage, Port } from '../../../common/protocol';
 export const aBoard: Board = {
   fqbn: 'some:board:fqbn',
   name: 'Some Arduino Board',
-  port: { address: '/lol/port1234', protocol: 'serial' },
+  port: {
+    id: '/lol/port1234|serial',
+    address: '/lol/port1234',
+    addressLabel: '/lol/port1234',
+    protocol: 'serial',
+    protocolLabel: 'Serial Port (USB)',
+  },
 };
 export const aPort: Port = {
+  id: aBoard.port!.id,
   address: aBoard.port!.address,
+  addressLabel: aBoard.port!.addressLabel,
   protocol: aBoard.port!.protocol,
+  protocolLabel: aBoard.port!.protocolLabel,
 };
 export const aBoardConfig: BoardsConfig.Config = {
   selectedBoard: aBoard,
@@ -17,11 +26,20 @@ export const aBoardConfig: BoardsConfig.Config = {
 export const anotherBoard: Board = {
   fqbn: 'another:board:fqbn',
   name: 'Another Arduino Board',
-  port: { address: '/kek/port5678', protocol: 'serial' },
+  port: {
+    id: '/kek/port5678|serial',
+    address: '/kek/port5678',
+    addressLabel: '/kek/port5678',
+    protocol: 'serial',
+    protocolLabel: 'Serial Port (USB)',
+  },
 };
 export const anotherPort: Port = {
+  id: anotherBoard.port!.id,
   address: anotherBoard.port!.address,
+  addressLabel: anotherBoard.port!.addressLabel,
   protocol: anotherBoard.port!.protocol,
+  protocolLabel: anotherBoard.port!.protocolLabel,
 };
 export const anotherBoardConfig: BoardsConfig.Config = {
   selectedBoard: anotherBoard,

arduino-ide-extension/src/test/node/serial-service-impl.test.ts

@@ -86,7 +86,7 @@ describe('SerialServiceImpl', () => {
 
   context('when a disconnection is requested', () => {
     const sandbox = createSandbox();
-    beforeEach(() => {});
+    beforeEach(() => { });
 
     afterEach(function () {
       sandbox.restore();
@@ -132,11 +132,11 @@ describe('SerialServiceImpl', () => {
           return {
             streamingOpen: () => {
               return {
-                on: (str: string, cb: any) => {},
+                on: (str: string, cb: any) => { },
                 write: (chunk: any, cb: any) => {
                   cb();
                 },
-                cancel: () => {},
+                cancel: () => { },
               };
             },
           } as MonitorServiceClient;
@@ -146,7 +146,7 @@ describe('SerialServiceImpl', () => {
 
       await subject.setSerialConfig({
         board: { name: 'test' },
-        port: { address: 'test', protocol: 'test' },
+        port: { id: 'test|test', address: 'test', addressLabel: 'test', protocol: 'test', protocolLabel: 'test' },
       });
     });