fix 180: prevent erroneous "auto-reconnect"(s) in board selector (#1328)

* ensure auto-select doesn't select wrong port

prevent auto-select of unattached boards Co-authored-by: Francesco Spissu <francescospissu@users.noreply.github.com>

* clean up

* add "uploadInProgress" prop to boards change event

* remove unused methods and deps

* [WIP]: leverage upload event to derived new port

* remove timeout

* refine port selection logic

* clean up naming

* refine port selection logic & add delayed clean up

* renaming & refactoring

* method syntax & remove unnecessary methods
This commit is contained in:
Dave Simpson 2022-08-24 12:31:51 +02:00 committed by GitHub
parent 125bd64c91
commit cc5cf3b165
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 122 additions and 21 deletions

View File

@ -65,6 +65,10 @@ export class BoardsServiceProvider implements FrontendApplicationContribution {
protected _availablePorts: Port[] = [];
protected _availableBoards: AvailableBoard[] = [];
private lastBoardsConfigOnUpload: BoardsConfig.Config | undefined;
private lastAvailablePortsOnUpload: Port[] | undefined;
private boardConfigToAutoSelect: BoardsConfig.Config | undefined;
/**
* Unlike `onAttachedBoardsChanged` this event fires when the user modifies the selected board in the IDE.\
* This event also fires, when the boards package was not available for the currently selected board,
@ -112,6 +116,84 @@ export class BoardsServiceProvider implements FrontendApplicationContribution {
return this._reconciled.promise;
}
snapshotBoardDiscoveryOnUpload(): void {
this.lastBoardsConfigOnUpload = this._boardsConfig;
this.lastAvailablePortsOnUpload = this._availablePorts;
}
clearBoardDiscoverySnapshot(): void {
this.lastBoardsConfigOnUpload = undefined;
this.lastAvailablePortsOnUpload = undefined;
}
private portToAutoSelectCanBeDerived(): boolean {
return Boolean(
this.lastBoardsConfigOnUpload && this.lastAvailablePortsOnUpload
);
}
attemptPostUploadAutoSelect(): void {
setTimeout(() => {
if (this.portToAutoSelectCanBeDerived()) {
this.attemptAutoSelect({
ports: this._availablePorts,
boards: this._availableBoards,
});
}
}, 2000); // 2 second delay same as IDE 1.8
}
private attemptAutoSelect(
newState: AttachedBoardsChangeEvent['newState']
): void {
this.deriveBoardConfigToAutoSelect(newState);
this.tryReconnect();
}
private deriveBoardConfigToAutoSelect(
newState: AttachedBoardsChangeEvent['newState']
): void {
if (!this.portToAutoSelectCanBeDerived()) {
this.boardConfigToAutoSelect = undefined;
return;
}
const oldPorts = this.lastAvailablePortsOnUpload!;
const { ports: newPorts, boards: newBoards } = newState;
const appearedPorts =
oldPorts.length > 0
? newPorts.filter((newPort: Port) =>
oldPorts.every((oldPort: Port) => !Port.sameAs(newPort, oldPort))
)
: newPorts;
for (const port of appearedPorts) {
const boardOnAppearedPort = newBoards.find((board: Board) =>
Port.sameAs(board.port, port)
);
const lastBoardsConfigOnUpload = this.lastBoardsConfigOnUpload!;
if (
boardOnAppearedPort &&
lastBoardsConfigOnUpload.selectedBoard &&
Board.sameAs(
boardOnAppearedPort,
lastBoardsConfigOnUpload.selectedBoard
)
) {
this.clearBoardDiscoverySnapshot();
this.boardConfigToAutoSelect = {
selectedBoard: boardOnAppearedPort,
selectedPort: port,
};
return;
}
}
}
protected notifyAttachedBoardsChanged(
event: AttachedBoardsChangeEvent
): void {
@ -120,10 +202,18 @@ export class BoardsServiceProvider implements FrontendApplicationContribution {
this.logger.info(AttachedBoardsChangeEvent.toString(event));
this.logger.info('------------------------------------------');
}
this._attachedBoards = event.newState.boards;
this._availablePorts = event.newState.ports;
this.onAvailablePortsChangedEmitter.fire(this._availablePorts);
this.reconcileAvailableBoards().then(() => this.tryReconnect());
this.reconcileAvailableBoards().then(() => {
const { uploadInProgress } = event;
// avoid attempting "auto-selection" while an
// upload is in progress
if (!uploadInProgress) {
this.attemptAutoSelect(event.newState);
}
});
}
protected notifyPlatformInstalled(event: { item: BoardsPackage }): void {
@ -239,24 +329,12 @@ export class BoardsServiceProvider implements FrontendApplicationContribution {
return true;
}
}
// If we could not find an exact match, we compare the board FQBN-name pairs and ignore the port, as it might have changed.
// See documentation on `latestValidBoardsConfig`.
for (const board of this.availableBoards.filter(
({ state }) => state !== AvailableBoard.State.incomplete
)) {
if (
this.latestValidBoardsConfig.selectedBoard.fqbn === board.fqbn &&
this.latestValidBoardsConfig.selectedBoard.name === board.name &&
this.latestValidBoardsConfig.selectedPort.protocol ===
board.port?.protocol
) {
this.boardsConfig = {
...this.latestValidBoardsConfig,
selectedPort: board.port,
};
return true;
}
}
if (!this.boardConfigToAutoSelect) return false;
this.boardsConfig = this.boardConfigToAutoSelect;
this.boardConfigToAutoSelect = undefined;
return true;
}
return false;
}
@ -450,6 +528,11 @@ export class BoardsServiceProvider implements FrontendApplicationContribution {
const board = attachedBoards.find(({ port }) =>
Port.sameAs(boardPort, port)
);
// "board" will always be falsey for
// port that was originally mapped
// to unknown board and then selected
// manually by user
const lastSelectedBoard = await this.getLastSelectedBoardOnPort(
boardPort
);
@ -468,7 +551,9 @@ export class BoardsServiceProvider implements FrontendApplicationContribution {
availableBoard = {
...lastSelectedBoard,
state: AvailableBoard.State.guessed,
selected: BoardsConfig.Config.sameAs(boardsConfig, lastSelectedBoard),
selected:
BoardsConfig.Config.sameAs(boardsConfig, lastSelectedBoard) &&
Port.sameAs(boardPort, boardsConfig.selectedPort), // to avoid double selection
port: boardPort,
};
} else {
@ -483,7 +568,7 @@ export class BoardsServiceProvider implements FrontendApplicationContribution {
if (
boardsConfig.selectedBoard &&
!availableBoards.some(({ selected }) => selected)
availableBoards.every(({ 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.

View File

@ -190,6 +190,7 @@ export class UploadSketch extends CoreServiceContribution {
// toggle the toolbar button and menu item state.
// uploadInProgress will be set to false whether the upload fails or not
this.uploadInProgress = true;
this.boardsServiceProvider.snapshotBoardDiscoveryOnUpload();
this.onDidChangeEmitter.fire();
this.clearVisibleNotification();
@ -243,6 +244,7 @@ export class UploadSketch extends CoreServiceContribution {
this.handleError(e);
} finally {
this.uploadInProgress = false;
this.boardsServiceProvider.attemptPostUploadAutoSelect();
this.onDidChangeEmitter.fire();
}
}

View File

@ -25,6 +25,7 @@ export namespace AvailablePorts {
export interface AttachedBoardsChangeEvent {
readonly oldState: Readonly<{ boards: Board[]; ports: Port[] }>;
readonly newState: Readonly<{ boards: Board[]; ports: Port[] }>;
readonly uploadInProgress: boolean;
}
export namespace AttachedBoardsChangeEvent {
export function isEmpty(event: AttachedBoardsChangeEvent): boolean {

View File

@ -54,6 +54,8 @@ export class BoardDiscovery
private readonly onStreamDidCancelEmitter = new Emitter<void>(); // when the watcher is canceled by the IDE2
private readonly toDisposeOnStopWatch = new DisposableCollection();
private uploadInProgress = false;
/**
* Keys are the `address` of the ports.
*
@ -123,6 +125,10 @@ export class BoardDiscovery
});
}
setUploadInProgress(uploadAttemptInProgress: boolean): void {
this.uploadInProgress = uploadAttemptInProgress;
}
private createTimeout(
after: number,
onTimeout: (error: Error) => void
@ -318,6 +324,7 @@ export class BoardDiscovery
ports: newAvailablePorts,
boards: newAttachedBoards,
},
uploadInProgress: this.uploadInProgress,
};
this._availablePorts = newState;

View File

@ -34,6 +34,7 @@ import { Instance } from './cli-protocol/cc/arduino/cli/commands/v1/common_pb';
import { firstToUpperCase, notEmpty } from '../common/utils';
import { ServiceError } from './service-error';
import { ExecuteWithProgress, ProgressResponse } from './grpc-progressible';
import { BoardDiscovery } from './board-discovery';
namespace Uploadable {
export type Request = UploadRequest | UploadUsingProgrammerRequest;
@ -51,6 +52,9 @@ export class CoreServiceImpl extends CoreClientAware implements CoreService {
@inject(CommandService)
private readonly commandService: CommandService;
@inject(BoardDiscovery)
private readonly boardDiscovery: BoardDiscovery;
async compile(options: CoreService.Options.Compile): Promise<void> {
const coreClient = await this.coreClient;
const { client, instance } = coreClient;
@ -378,6 +382,7 @@ export class CoreServiceImpl extends CoreClientAware implements CoreService {
fqbn?: string | undefined;
port?: Port | undefined;
}): Promise<void> {
this.boardDiscovery.setUploadInProgress(true);
return this.monitorManager.notifyUploadStarted(fqbn, port);
}
@ -388,6 +393,7 @@ export class CoreServiceImpl extends CoreClientAware implements CoreService {
fqbn?: string | undefined;
port?: Port | undefined;
}): Promise<Status> {
this.boardDiscovery.setUploadInProgress(false);
return this.monitorManager.notifyUploadFinished(fqbn, port);
}