Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 12 commits into from
Aug 24, 2022
104 changes: 101 additions & 3 deletions arduino-ide-extension/src/browser/boards/boards-service-provider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,11 @@ export class BoardsServiceProvider implements FrontendApplicationContribution {
protected _availablePorts: Port[] = [];
protected _availableBoards: AvailableBoard[] = [];

private lastItemRemovedForUpload: { board: Board; port: Port } | undefined;
// "lastPersistingUploadPort", is a port created during an upload, that persisted after
// the upload finished, it's "substituting" the port selected when the user invoked the upload
private lastPersistingUploadPort: Port | undefined;

/**
* Unlike `onAttachedBoardsChanged` this even fires when the user modifies the selected board in the IDE.\
* This even also fires, when the boards package was not available for the currently selected board,
Expand Down Expand Up @@ -111,6 +116,64 @@ export class BoardsServiceProvider implements FrontendApplicationContribution {
return this._reconciled.promise;
}

private checkForItemRemoved(event: AttachedBoardsChangeEvent): void {
if (!this.lastItemRemovedForUpload) {
const {
oldState: { ports: oldPorts, boards: oldBoards },
newState: { ports: newPorts },
} = event;

const disappearedPorts = oldPorts.filter((oldPort: Port) =>
newPorts.every((newPort: Port) => !Port.sameAs(oldPort, newPort))
);

if (disappearedPorts.length > 0) {
this.lastItemRemovedForUpload = {
board: oldBoards.find((board: Board) =>
Port.sameAs(board.port, disappearedPorts[0])
) as Board,
port: disappearedPorts[0],
};
}

return;
}
}

private checkForPersistingPort(event: AttachedBoardsChangeEvent): void {
if (this.lastItemRemovedForUpload) {
const {
oldState: { ports: oldPorts },
newState: { ports: newPorts, boards: newBoards },
} = event;

const disappearedItem = this.lastItemRemovedForUpload;
this.lastItemRemovedForUpload = undefined;

const appearedPorts = newPorts.filter((newPort: Port) =>
oldPorts.every((oldPort: Port) => !Port.sameAs(newPort, oldPort))
);

if (appearedPorts.length > 0) {
const boardOnAppearedPort = newBoards.find((board: Board) =>
Port.sameAs(board.port, appearedPorts[0])
);

if (
boardOnAppearedPort &&
Board.sameAs(boardOnAppearedPort, disappearedItem.board)
) {
this.lastPersistingUploadPort = appearedPorts[0];
return;
}
}

return;
}

this.lastPersistingUploadPort = undefined;
}

protected notifyAttachedBoardsChanged(
event: AttachedBoardsChangeEvent
): void {
Expand All @@ -119,10 +182,23 @@ export class BoardsServiceProvider implements FrontendApplicationContribution {
this.logger.info(AttachedBoardsChangeEvent.toString(event));
this.logger.info('------------------------------------------');
}

const { uploadInProgress } = event;

if (uploadInProgress) {
this.checkForItemRemoved(event);
} else {
this.checkForPersistingPort(event);
}

this._attachedBoards = event.newState.boards;
this._availablePorts = event.newState.ports;
this.onAvailablePortsChangedEmitter.fire(this._availablePorts);
this.reconcileAvailableBoards().then(() => this.tryReconnect());
this.reconcileAvailableBoards().then(() => {
if (!uploadInProgress) {
this.tryReconnect();
}
});
}

protected notifyPlatformInstalled(event: { item: BoardsPackage }): void {
Expand Down Expand Up @@ -240,6 +316,21 @@ export class BoardsServiceProvider implements FrontendApplicationContribution {
}
// 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`.

if (!this.lastPersistingUploadPort) return false;

const lastPersistingUploadPort = this.lastPersistingUploadPort;
this.lastPersistingUploadPort = undefined;

if (
!Port.sameAs(
lastPersistingUploadPort,
this.latestValidBoardsConfig.selectedPort
)
) {
return false;
}

for (const board of this.availableBoards.filter(
({ state }) => state !== AvailableBoard.State.incomplete
)) {
Expand Down Expand Up @@ -458,6 +549,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
);
Expand All @@ -476,7 +572,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 {
Expand All @@ -491,7 +589,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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
7 changes: 7 additions & 0 deletions arduino-ide-extension/src/node/board-discovery.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*
Expand Down Expand Up @@ -123,6 +125,10 @@ export class BoardDiscovery
});
}

public setUploadInProgress(uploadAttemptInProgress: boolean): void {
this.uploadInProgress = uploadAttemptInProgress;
}

private createTimeout(
after: number,
onTimeout: (error: Error) => void
Expand Down Expand Up @@ -318,6 +324,7 @@ export class BoardDiscovery
ports: newAvailablePorts,
boards: newAttachedBoards,
},
uploadInProgress: this.uploadInProgress,
};

this._availablePorts = newState;
Expand Down
6 changes: 6 additions & 0 deletions arduino-ide-extension/src/node/core-service-impl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -51,6 +52,9 @@ export class CoreServiceImpl extends CoreClientAware implements CoreService {
@inject(CommandService)
private readonly commandService: CommandService;

@inject(BoardDiscovery)
protected readonly boardDiscovery: BoardDiscovery;

async compile(options: CoreService.Options.Compile): Promise<void> {
const coreClient = await this.coreClient;
const { client, instance } = coreClient;
Expand Down Expand Up @@ -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);
}

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

Expand Down