slightly better reconnecting experience.

Akos Kitta · Akos Kitta · commit c7242ca34f06 · 2019-12-10T12:01:54.000+01:00
Signed-off-by: Akos Kitta &lt;kittaakos@typefox.io&gt;
diff --git a/arduino-ide-extension/src/browser/boards/boards-service-client-impl.ts b/arduino-ide-extension/src/browser/boards/boards-service-client-impl.ts
@@ -107,6 +107,20 @@ export class BoardsServiceClientImpl implements BoardsServiceClient {
         return this._boardsConfig;
     }
 
+    /**
+     * `true` if the `config.selectedBoard` is defined; hence can compile against the board. Otherwise, `false`.
+     */
+    canVerify(config: BoardsConfig.Config | undefined = this.boardsConfig): config is BoardsConfig.Config & { selectedBoard: Board } {
+        return !!config && !!config.selectedBoard;
+    }
+
+    /**
+     * `true` if the `canVerify` and the `config.selectedPort` is also set with FQBN, hence can upload to board. Otherwise, `false`.
+     */
+    canUploadTo(config: BoardsConfig.Config | undefined = this.boardsConfig): config is RecursiveRequired<BoardsConfig.Config> {
+        return this.canVerify(config) && !!config.selectedPort && !!config.selectedBoard.fqbn;
+    }
+
     protected saveState(): Promise<void> {
         return this.storageService.setData('latest-valid-boards-config', this.latestValidBoardsConfig);
     }
@@ -118,12 +132,4 @@ export class BoardsServiceClientImpl implements BoardsServiceClient {
         }
     }
 
-    protected canVerify(config: BoardsConfig.Config | undefined): config is BoardsConfig.Config & { selectedBoard: Board } {
-        return !!config && !!config.selectedBoard;
-    }
-
-    protected canUploadTo(config: BoardsConfig.Config | undefined): config is RecursiveRequired<BoardsConfig.Config> {
-        return this.canVerify(config) && !!config.selectedPort && !!config.selectedBoard.fqbn;
-    }
-
 }
diff --git a/arduino-ide-extension/src/browser/monitor/monitor-connection.ts b/arduino-ide-extension/src/browser/monitor/monitor-connection.ts
@@ -47,8 +47,8 @@ export class MonitorConnection {
                         break;
                     }
                     case MonitorError.ErrorCodes.DEVICE_NOT_CONFIGURED: {
-                        const { port } = config;
-                        this.messageService.info(`Disconnected from ${Port.toString(port)}.`);
+                        const { port, board } = config;
+                        this.messageService.info(`Disconnected ${Board.toString(board, { useFqbn: false })} from ${Port.toString(port)}.`);
                         break;
                     }
                     case undefined: {
@@ -77,19 +77,22 @@ export class MonitorConnection {
 
     async connect(config: MonitorConfig): Promise<Status> {
         if (this.state) {
-            throw new Error(`Already connected to ${MonitorConnection.State.toString(this.state)}.`);
+            const disconnectStatus = await this.disconnect();
+            if (!Status.isOK(disconnectStatus)) {
+                return disconnectStatus;
+            }
         }
-        const status = await this.monitorService.connect(config);
-        if (Status.isOK(status)) {
+        const connectStatus = await this.monitorService.connect(config);
+        if (Status.isOK(connectStatus)) {
             this.state = { config };
             this.onConnectionChangedEmitter.fire(true);
         }
-        return Status.isOK(status);
+        return Status.isOK(connectStatus);
     }
 
     async disconnect(): Promise<Status> {
         if (!this.state) {
-            throw new Error('Not connected. Nothing to disconnect.');
+            return Status.OK;
         }
         console.log('>>> Disposing existing monitor connection before establishing a new one...');
         const status = await this.monitorService.disconnect();
diff --git a/arduino-ide-extension/src/browser/monitor/monitor-widget.tsx b/arduino-ide-extension/src/browser/monitor/monitor-widget.tsx
@@ -9,7 +9,7 @@ import { MessageService } from '@theia/core/lib/common/message-service';
 import { ReactWidget, Message, Widget } from '@theia/core/lib/browser';
 import { MonitorServiceClientImpl } from './monitor-service-client-impl';
 import { MonitorConfig, MonitorService } from '../../common/protocol/monitor-service';
-import { AttachedSerialBoard, BoardsService } from '../../common/protocol/boards-service';
+import { AttachedSerialBoard, BoardsService, AttachedBoardsChangeEvent } from '../../common/protocol/boards-service';
 import { BoardsConfig } from '../boards/boards-config';
 import { BoardsServiceClientImpl } from '../boards/boards-service-client-impl';
 import { MonitorModel } from './monitor-model';
@@ -171,16 +171,24 @@ export class MonitorWidget extends ReactWidget {
                 }
             }),
             this.boardsServiceClient.onBoardsConfigChanged(config => {
-                const { selectedBoard, selectedPort } = config;
-                if (selectedBoard && selectedPort) {
+                if (this.boardsServiceClient.canUploadTo(config)) {
                     this.boardsService.getAttachedBoards().then(({ boards }) => {
                         if (boards.filter(AttachedSerialBoard.is).some(board => BoardsConfig.Config.sameAs(config, board))) {
-                            this.clearConsole();
                             this.connect();
                         }
                     });
                 }
-            })]);
+            }),
+            this.boardsServiceClient.onBoardsChanged(event => {
+                const { attached } = AttachedBoardsChangeEvent.diff(event);
+                if (!this.connection.connected && this.boardsServiceClient.canUploadTo()) {
+                    const { boardsConfig } = this.boardsServiceClient;
+                    if (attached.boards.filter(AttachedSerialBoard.is).some(board => BoardsConfig.Config.sameAs(boardsConfig, board))) {
+                        this.connect();
+                    }
+                }
+            })
+        ]);
         this.update();
     }
 
@@ -214,6 +222,7 @@ export class MonitorWidget extends ReactWidget {
     }
 
     protected async connect(): Promise<void> {
+        this.clearConsole();
         const config = await this.getConnectionConfig();
         if (config) {
             this.connection.connect(config);
diff --git a/arduino-ide-extension/src/browser/tool-output/client-service-impl.ts b/arduino-ide-extension/src/browser/tool-output/client-service-impl.ts
@@ -13,11 +13,11 @@ export class ToolOutputServiceClientImpl implements ToolOutputServiceClient {
     protected readonly outputContribution: OutputContribution;
 
     onNewOutput(tool: string, chunk: string): void {
-        this.outputContribution.openView({ reveal: true }).then(() => {
+        this.outputContribution.openView({ activate: true }).then(() => {
             const channel = this.outputChannelManager.getChannel(`Arduino: ${tool}`);
             channel.setVisibility(true);
             channel.append(chunk);
-        })
+        });
     }
 
-}
+}
diff --git a/arduino-ide-extension/src/common/protocol/boards-service.ts b/arduino-ide-extension/src/common/protocol/boards-service.ts
@@ -214,8 +214,8 @@ export namespace Board {
         return !!board.fqbn;
     }
 
-    export function toString(board: Board): string {
-        const fqbn = board.fqbn ? ` [${board.fqbn}]` : '';
+    export function toString(board: Board, options: { useFqbn: boolean } = { useFqbn: true }): string {
+        const fqbn = options && options.useFqbn && board.fqbn ? ` [${board.fqbn}]` : '';
         return `${board.name}${fqbn}`;
     }
 
diff --git a/arduino-ide-extension/src/node/monitor/monitor-service-impl.ts b/arduino-ide-extension/src/node/monitor/monitor-service-impl.ts
@@ -45,7 +45,7 @@ export class MonitorServiceImpl implements MonitorService {
     protected readonly monitorClientProvider: MonitorClientProvider;
 
     protected client?: MonitorServiceClient;
-    protected connection?: ClientDuplexStream<StreamingOpenReq, StreamingOpenResp>;
+    protected connection?: { duplex: ClientDuplexStream<StreamingOpenReq, StreamingOpenResp>, config: MonitorConfig };
 
     setClient(client: MonitorServiceClient | undefined): void {
         this.client = client;
@@ -66,20 +66,23 @@ export class MonitorServiceImpl implements MonitorService {
             return Status.ALREADY_CONNECTED;
         }
         const client = await this.monitorClientProvider.client;
-        this.connection = client.streamingOpen();
-        this.connection.on('error', ((error: Error) => {
+        const duplex = client.streamingOpen();
+        this.connection = { duplex, config };
+
+        duplex.on('error', ((error: Error) => {
             const monitorError = ErrorWithCode.toMonitorError(error, config);
-            if (monitorError.code === undefined) {
-                this.logger.error(error);
-            }
-            ((monitorError.code === undefined ? this.disconnect() : Promise.resolve()) as Promise<any>).then(() => {
+            this.disconnect().then(() => {
                 if (this.client) {
                     this.client.notifyError(monitorError);
                 }
-            })
+                if (monitorError.code === undefined) {
+                    // Log the original, unexpected error.
+                    this.logger.error(error);
+                }
+            });
         }).bind(this));
 
-        this.connection.on('data', ((resp: StreamingOpenResp) => {
+        duplex.on('data', ((resp: StreamingOpenResp) => {
             if (this.client) {
                 const raw = resp.getData();
                 const data = typeof raw === 'string' ? raw : new TextDecoder('utf8').decode(raw);
@@ -99,21 +102,25 @@ export class MonitorServiceImpl implements MonitorService {
 
         return new Promise<Status>(resolve => {
             if (this.connection) {
-                this.connection.write(req, () => {
-                    this.logger.info(`<<< Serial monitor connection created for ${Board.toString(config.board)} on port ${Port.toString(config.port)}.`);
+                this.connection.duplex.write(req, () => {
+                    this.logger.info(`<<< Serial monitor connection created for ${Board.toString(config.board, { useFqbn: false })} on port ${Port.toString(config.port)}.`);
                     resolve(Status.OK);
                 });
                 return;
             }
-            resolve(Status.NOT_CONNECTED);
+            this.disconnect().then(() => resolve(Status.NOT_CONNECTED));
         });
     }
 
     async disconnect(): Promise<Status> {
+        this.logger.info(`>>> Disposing monitor connection...`);
         if (!this.connection) {
+            this.logger.warn(`<<< Not connected. Nothing to dispose.`);
             return Status.NOT_CONNECTED;
         }
-        this.connection.cancel();
+        const { duplex, config } = this.connection;
+        duplex.cancel();
+        this.logger.info(`<<< Disposed monitor connection for ${Board.toString(config.board, { useFqbn: false })} on port ${Port.toString(config.port)}.`);
         this.connection = undefined;
         return Status.OK;
     }
@@ -126,12 +133,12 @@ export class MonitorServiceImpl implements MonitorService {
         req.setData(new TextEncoder().encode(data));
         return new Promise<Status>(resolve => {
             if (this.connection) {
-                this.connection.write(req, () => {
+                this.connection.duplex.write(req, () => {
                     resolve(Status.OK);
                 });
                 return;
             }
-            resolve(Status.NOT_CONNECTED);
+            this.disconnect().then(() => resolve(Status.NOT_CONNECTED));
         });
     }
 

Original file line number	Diff line number	Diff line change
`@@ -214,8 +214,8 @@ export namespace Board {`
`214`	`214`	`return !!board.fqbn;`
`215`	`215`	`}`
`216`	`216`
`217`		`- export function toString(board: Board): string {`
`218`		- const fqbn = board.fqbn ? ` [${board.fqbn}]` : '';
	`217`	`+ export function toString(board: Board, options: { useFqbn: boolean } = { useFqbn: true }): string {`
	`218`	+ const fqbn = options && options.useFqbn && board.fqbn ? ` [${board.fqbn}]` : '';
`219`	`219`	return `${board.name}${fqbn}`;
`220`	`220`	`}`
`221`	`221`