Rework listing of discovered ports (#614)

* Removed Protocol type

* Reworked function that groups ports by protocol

* Remove useless protocol check in Port sameAs function

* Reworked port selection menu ordering

Now ports are shown in this order:
1. Serial with recognized boards
2. Serial with unrecognized boards
3. Network with recognized boards
4. Network with unrecognized boards
5. Other protocols with recognized boards
6. Other protocols with unrecognized boards

* Fix ports shown multiple times in menu

* Reworked board selection dropdown ordering

Ordering is now:
1. Serial with recognized boards
2. Serial with guessed boards
3. Serial with incomplete boards
4. Network with recognized boards
5. Other protocols with recognized boards

* Localize some strings

* Fix bug selecting board in boards selector dropdown

* Reworked board selection dialog ordering

* Fix Tools > Port menu not refreshing

* Move Select other board button to bottom of Board selector dropdown and change its style

* Updated arduino-cli to 0.20.0 and generated protocol files

Author: silvanocerza

arduino-ide-extension/package.json

@@ -149,7 +149,7 @@
   ],
   "arduino": {
     "cli": {
-      "version": "0.19.1"
+      "version": "0.20.0"
     },
     "fwuploader": {
       "version": "2.0.0"

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

@@ -10,8 +10,12 @@ import {
   BoardWithPackage,
 } from '../../common/protocol/boards-service';
 import { NotificationCenter } from '../notification-center';
-import { BoardsServiceProvider } from './boards-service-provider';
+import {
+  AvailableBoard,
+  BoardsServiceProvider,
+} from './boards-service-provider';
 import { nls } from '@theia/core/lib/browser/nls';
+import { naturalCompare } from '../../common/utils';
 
 export namespace BoardsConfig {
   export interface Config {
@@ -184,11 +188,50 @@ export class BoardsConfig extends React.Component<
       .filter(notEmpty);
   }
 
+  protected get availableBoards(): AvailableBoard[] {
+    return this.props.boardsServiceProvider.availableBoards;
+  }
+
   protected queryPorts = async (
     availablePorts: MaybePromise<Port[]> = this.availablePorts
   ) => {
-    const ports = await availablePorts;
-    return { knownPorts: ports.sort(Port.compare) };
+    // Available ports must be sorted in this order:
+    // 1. Serial with recognized boards
+    // 2. Serial with guessed boards
+    // 3. Serial with incomplete boards
+    // 4. Network with recognized boards
+    // 5. Other protocols with recognized boards
+    const ports = (await availablePorts).sort((left: Port, right: Port) => {
+      if (left.protocol === 'serial' && right.protocol !== 'serial') {
+        return -1;
+      } else if (left.protocol !== 'serial' && right.protocol === 'serial') {
+        return 1;
+      } else if (left.protocol === 'network' && right.protocol !== 'network') {
+        return -1;
+      } else if (left.protocol !== 'network' && right.protocol === 'network') {
+        return 1;
+      } 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) =>
+          Port.sameAs(board.port, left)
+        );
+        const rightBoard = this.availableBoards.find((board) =>
+          Port.sameAs(board.port, right)
+        );
+        if (leftBoard && !rightBoard) {
+          return -1;
+        } else if (!leftBoard && rightBoard) {
+          return 1;
+        } else if (leftBoard?.state! < rightBoard?.state!) {
+          return -1;
+        } else if (leftBoard?.state! > rightBoard?.state!) {
+          return 1;
+        }
+      }
+      return naturalCompare(left.address, right.address);
+    });
+    return { knownPorts: ports };
   };
 
   protected toggleFilterPorts = () => {
@@ -281,8 +324,24 @@ export class BoardsConfig extends React.Component<
   }
 
   protected renderPorts(): React.ReactNode {
-    const filter = this.state.showAllPorts ? () => true : Port.isBoardPort;
-    const ports = this.state.knownPorts.filter(filter);
+    let ports = [] as Port[];
+    if (this.state.showAllPorts) {
+      ports = this.state.knownPorts;
+    } else {
+      ports = this.state.knownPorts.filter((port) => {
+        if (port.protocol === 'serial') {
+          return true;
+        }
+        // All other ports with different protocol are
+        // only shown if there is a recognized board
+        // connected
+        for (const board of this.availableBoards) {
+          if (board.port?.address === port.address) {
+            return true;
+          }
+        }
+      });
+    }
     return !ports.length ? (
       <div className="loading noselect">No ports discovered</div>
     ) : (

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

@@ -42,6 +42,7 @@ export class BoardsServiceProvider implements FrontendApplicationContribution {
   protected readonly onAvailableBoardsChangedEmitter = new Emitter<
     AvailableBoard[]
   >();
+  protected readonly onAvailablePortsChangedEmitter = new Emitter<Port[]>();
 
   /**
    * Used for the auto-reconnecting. Sometimes, the attached board gets disconnected after uploading something to it.
@@ -67,8 +68,8 @@ export class BoardsServiceProvider implements FrontendApplicationContribution {
    * This event is also emitted when the board package for the currently selected board was uninstalled.
    */
   readonly onBoardsConfigChanged = this.onBoardsConfigChangedEmitter.event;
-  readonly onAvailableBoardsChanged =
-    this.onAvailableBoardsChangedEmitter.event;
+  readonly onAvailableBoardsChanged = this.onAvailableBoardsChangedEmitter.event;
+  readonly onAvailablePortsChanged = this.onAvailablePortsChangedEmitter.event;
 
   onStart(): void {
     this.notificationCenter.onAttachedBoardsChanged(
@@ -88,6 +89,7 @@ export class BoardsServiceProvider implements FrontendApplicationContribution {
     ]).then(([attachedBoards, availablePorts]) => {
       this._attachedBoards = attachedBoards;
       this._availablePorts = availablePorts;
+      this.onAvailablePortsChangedEmitter.fire(this._availablePorts);
       this.reconcileAvailableBoards().then(() => this.tryReconnect());
     });
   }
@@ -102,6 +104,7 @@ export class BoardsServiceProvider implements FrontendApplicationContribution {
     }
     this._attachedBoards = event.newState.boards;
     this._availablePorts = event.newState.ports;
+    this.onAvailablePortsChangedEmitter.fire(this._availablePorts);
     this.reconcileAvailableBoards().then(() => this.tryReconnect());
   }
 
@@ -180,8 +183,8 @@ export class BoardsServiceProvider implements FrontendApplicationContribution {
         const selectedAvailableBoard = AvailableBoard.is(selectedBoard)
           ? selectedBoard
           : this._availableBoards.find((availableBoard) =>
-              Board.sameAs(availableBoard, selectedBoard)
-            );
+            Board.sameAs(availableBoard, selectedBoard)
+          );
         if (
           selectedAvailableBoard &&
           selectedAvailableBoard.selected &&
@@ -358,14 +361,14 @@ export class BoardsServiceProvider implements FrontendApplicationContribution {
     const timeoutTask =
       !!timeout && timeout > 0
         ? new Promise<void>((_, reject) =>
-            setTimeout(
-              () => reject(new Error(`Timeout after ${timeout} ms.`)),
-              timeout
-            )
+          setTimeout(
+            () => reject(new Error(`Timeout after ${timeout} ms.`)),
+            timeout
           )
+        )
         : new Promise<void>(() => {
-            /* never */
-          });
+          /* never */
+        });
     const waitUntilTask = new Promise<void>((resolve) => {
       let candidate = find(what, this.availableBoards);
       if (candidate) {
@@ -384,7 +387,6 @@ export class BoardsServiceProvider implements FrontendApplicationContribution {
   }
 
   protected async reconcileAvailableBoards(): Promise<void> {
-    const attachedBoards = this._attachedBoards;
     const availablePorts = this._availablePorts;
     // Unset the port on the user's config, if it is not available anymore.
     if (
@@ -402,51 +404,64 @@ export class BoardsServiceProvider implements FrontendApplicationContribution {
     const boardsConfig = this.boardsConfig;
     const currentAvailableBoards = this._availableBoards;
     const availableBoards: AvailableBoard[] = [];
-    const availableBoardPorts = availablePorts.filter(Port.isBoardPort);
-    const attachedSerialBoards = attachedBoards.filter(({ port }) => !!port);
+    const attachedBoards = this._attachedBoards.filter(({ port }) => !!port);
+    const availableBoardPorts = availablePorts.filter((port) => {
+      if (port.protocol === "serial") {
+        // We always show all serial ports, even if there
+        // is no recognized board connected to it
+        return true;
+      }
+
+      // All other ports with different protocol are
+      // only shown if there is a recognized board
+      // connected
+      for (const board of attachedBoards) {
+        if (board.port?.address === port.address) {
+          return true;
+        }
+      }
+      return false;
+    });
 
     for (const boardPort of availableBoardPorts) {
-      let state = AvailableBoard.State.incomplete; // Initial pessimism.
-      let board = attachedSerialBoards.find(({ port }) =>
-        Port.sameAs(boardPort, port)
-      );
+      let board = attachedBoards.find(({ port }) => Port.sameAs(boardPort, port));
+      const lastSelectedBoard = await this.getLastSelectedBoardOnPort(boardPort);
+
+      let availableBoard = {} as AvailableBoard;
       if (board) {
-        state = AvailableBoard.State.recognized;
-      } else {
+        availableBoard = {
+          ...board,
+          state: AvailableBoard.State.recognized,
+          selected: BoardsConfig.Config.sameAs(boardsConfig, board),
+          port: boardPort,
+        };
+      } else if (lastSelectedBoard) {
         // If the selected board is not recognized because it is a 3rd party board: https://github.com/arduino/arduino-cli/issues/623
         // We still want to show it without the red X in the boards toolbar: https://github.com/arduino/arduino-pro-ide/issues/198#issuecomment-599355836
-        const lastSelectedBoard = await this.getLastSelectedBoardOnPort(
-          boardPort
-        );
-        if (lastSelectedBoard) {
-          board = {
-            ...lastSelectedBoard,
-            port: boardPort,
-          };
-          state = AvailableBoard.State.guessed;
-        }
-      }
-      if (!board) {
-        availableBoards.push({
-          name: nls.localize('arduino/common/unknown', 'Unknown'),
+        availableBoard = {
+          ...lastSelectedBoard,
+          state: AvailableBoard.State.guessed,
+          selected: BoardsConfig.Config.sameAs(boardsConfig, lastSelectedBoard),
           port: boardPort,
-          state,
-        });
+        };
       } else {
-        const selected = BoardsConfig.Config.sameAs(boardsConfig, board);
-        availableBoards.push({
-          ...board,
-          state,
-          selected,
+        availableBoard = {
+          name: nls.localize('arduino/common/unknown', 'Unknown'),
           port: boardPort,
-        });
+          state: AvailableBoard.State.incomplete,
+        };
       }
+      availableBoards.push(availableBoard);
     }
 
-    if (
-      boardsConfig.selectedBoard &&
-      !availableBoards.some(({ selected }) => selected)
-    ) {
+    if (boardsConfig.selectedBoard && !availableBoards.some(({ selected }) => selected)) {
+      // If the selected board has the same port of an unknown board
+      // that is already in availableBoards we might get a duplicate port.
+      // So we remove the one already in the array and add the selected one.
+      const found = availableBoards.findIndex(board => board.port?.address === boardsConfig.selectedPort?.address);
+      if (found >= 0) {
+        availableBoards.splice(found, 1);
+      }
       availableBoards.push({
         ...boardsConfig.selectedBoard,
         port: boardsConfig.selectedPort,
@@ -455,28 +470,20 @@ export class BoardsServiceProvider implements FrontendApplicationContribution {
       });
     }
 
-    const sortedAvailableBoards = availableBoards.sort(AvailableBoard.compare);
-    let hasChanged =
-      sortedAvailableBoards.length !== currentAvailableBoards.length;
-    for (let i = 0; !hasChanged && i < sortedAvailableBoards.length; i++) {
-      hasChanged =
-        AvailableBoard.compare(
-          sortedAvailableBoards[i],
-          currentAvailableBoards[i]
-        ) !== 0;
+    availableBoards.sort(AvailableBoard.compare);
+
+    let hasChanged = availableBoards.length !== currentAvailableBoards.length;
+    for (let i = 0; !hasChanged && i < availableBoards.length; i++) {
+      const [left, right] = [availableBoards[i], currentAvailableBoards[i]];
+      hasChanged = !!AvailableBoard.compare(left, right) || left.selected !== right.selected;
     }
     if (hasChanged) {
-      this._availableBoards = sortedAvailableBoards;
+      this._availableBoards = availableBoards;
       this.onAvailableBoardsChangedEmitter.fire(this._availableBoards);
     }
   }
 
-  protected async getLastSelectedBoardOnPort(
-    port: Port | string | undefined
-  ): Promise<Board | undefined> {
-    if (!port) {
-      return undefined;
-    }
+  protected async getLastSelectedBoardOnPort(port: Port): Promise<Board | undefined> {
     const key = this.getLastSelectedBoardOnPortKey(port);
     return this.getData<Board>(key);
   }
@@ -497,11 +504,8 @@ 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:${
-      typeof port === 'string' ? port : Port.toString(port)
-    }`;
+  protected getLastSelectedBoardOnPortKey(port: Port): string {
+    return `last-selected-board-on-port:${Port.toString(port)}`;
   }
 
   protected async loadState(): Promise<void> {
@@ -585,35 +589,30 @@ export namespace AvailableBoard {
     return !!board.port;
   }
 
+  // Available boards must be sorted in this order:
+  // 1. Serial with recognized boards
+  // 2. Serial with guessed boards
+  // 3. Serial with incomplete boards
+  // 4. Network with recognized boards
+  // 5. Other protocols with recognized boards
   export const compare = (left: AvailableBoard, right: AvailableBoard) => {
-    if (left.selected && !right.selected) {
+    if (left.port?.protocol === "serial" && right.port?.protocol !== "serial") {
       return -1;
-    }
-    if (right.selected && !left.selected) {
+    } else if (left.port?.protocol !== "serial" && right.port?.protocol === "serial") {
       return 1;
-    }
-    let result = naturalCompare(left.name, right.name);
-    if (result !== 0) {
-      return result;
-    }
-    if (left.fqbn && right.fqbn) {
-      result = naturalCompare(left.fqbn, right.fqbn);
-      if (result !== 0) {
-        return result;
-      }
-    }
-    if (left.port && right.port) {
-      result = Port.compare(left.port, right.port);
-      if (result !== 0) {
-        return result;
-      }
-    }
-    if (!!left.selected && !right.selected) {
+    } else if (left.port?.protocol === "network" && right.port?.protocol !== "network") {
       return -1;
-    }
-    if (!!right.selected && !left.selected) {
+    } else if (left.port?.protocol !== "network" && right.port?.protocol === "network") {
       return 1;
+    } else if (left.port?.protocol === right.port?.protocol) {
+      // We show all ports, including those that have guessed
+      // or unrecognized boards, so we must sort those too.
+      if (left.state < right.state) {
+        return -1;
+      } else if (left.state > right.state) {
+        return 1;
+      }
     }
-    return left.state - right.state;
-  };
+    return naturalCompare(left.port?.address!, right.port?.address!);
+  }
 }