Skip to content

Commit 138afbf

Browse files
Akos Kittakittaakos
Akos Kitta
authored andcommitted
ATL-469: Fixed various serial-monitor issues.
- Fixed a monitor reconnecting issue after upload. - Serial monitor connection was not disposed when the widget was closed from the toolbar with the magnifier (:mag:) icon. It worked only iff the user closed the view with the `X`. - This commit also fixes a warning that was related to the incorrect focus handling of the widget. - Switched to `board list -w` instead of polling. - Added a singleton for the board discovery to spare the CPU. - Fixed DI scopes on the backend. Each frontend gets its own service. - Switched to the `20201112` nightly CLI. - Fixed the Monitor view's image when the view is on the side-bar. Signed-off-by: Akos Kitta <kittaakos@typefox.io>
1 parent 01e42da commit 138afbf

16 files changed

+1772
-217
lines changed

arduino-ide-extension/package.json

+1-1
Original file line numberDiff line numberDiff line change
@@ -120,7 +120,7 @@
120120
],
121121
"arduino": {
122122
"cli": {
123-
"version": "20201104"
123+
"version": "20201112"
124124
}
125125
}
126126
}

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

+23
Original file line numberDiff line numberDiff line change
@@ -265,6 +265,29 @@ export class BoardsServiceProvider implements FrontendApplicationContribution {
265265
return this._availableBoards;
266266
}
267267

268+
async waitUntilAvailable(what: Board & { port: Port }, timeout?: number): Promise<void> {
269+
const find = (needle: Board & { port: Port }, haystack: AvailableBoard[]) =>
270+
haystack.find(board => Board.equals(needle, board) && Port.equals(needle.port, board.port));
271+
const timeoutTask = !!timeout && timeout > 0
272+
? new Promise<void>((_, reject) => setTimeout(() => reject(new Error(`Timeout after ${timeout} ms.`)), timeout))
273+
: new Promise<void>(() => { /* never */ });
274+
const waitUntilTask = new Promise<void>(resolve => {
275+
let candidate = find(what, this.availableBoards);
276+
if (candidate) {
277+
resolve();
278+
return;
279+
}
280+
const disposable = this.onAvailableBoardsChanged(availableBoards => {
281+
candidate = find(what, availableBoards);
282+
if (candidate) {
283+
disposable.dispose();
284+
resolve();
285+
}
286+
});
287+
});
288+
return await Promise.race([waitUntilTask, timeoutTask]);
289+
}
290+
268291
protected async reconcileAvailableBoards(): Promise<void> {
269292
const attachedBoards = this._attachedBoards;
270293
const availablePorts = this._availablePorts;

arduino-ide-extension/src/browser/contributions/upload-sketch.ts

+17-1
Original file line numberDiff line numberDiff line change
@@ -77,9 +77,14 @@ export class UploadSketch extends SketchContribution {
7777
if (!uri) {
7878
return;
7979
}
80+
let shouldAutoConnect = false;
8081
const monitorConfig = this.monitorConnection.monitorConfig;
8182
if (monitorConfig) {
8283
await this.monitorConnection.disconnect();
84+
if (this.monitorConnection.autoConnect) {
85+
shouldAutoConnect = true;
86+
}
87+
this.monitorConnection.autoConnect = false;
8388
}
8489
try {
8590
const { boardsConfig } = this.boardsServiceClientImpl;
@@ -122,7 +127,18 @@ export class UploadSketch extends SketchContribution {
122127
this.messageService.error(e.toString());
123128
} finally {
124129
if (monitorConfig) {
125-
await this.monitorConnection.connect(monitorConfig);
130+
const { board, port } = monitorConfig;
131+
try {
132+
await this.boardsServiceClientImpl.waitUntilAvailable(Object.assign(board, { port }), 10_000);
133+
if (shouldAutoConnect) {
134+
// Enabling auto-connect will trigger a connect.
135+
this.monitorConnection.autoConnect = true;
136+
} else {
137+
await this.monitorConnection.connect(monitorConfig);
138+
}
139+
} catch (waitError) {
140+
this.messageService.error(`Could not reconnect to serial monitor. ${waitError.toString()}`);
141+
}
126142
}
127143
}
128144
}
Loading

arduino-ide-extension/src/browser/monitor/monitor-widget.tsx

+11-6
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import { OptionsType } from 'react-select/src/types';
55
import { isOSX } from '@theia/core/lib/common/os';
66
import { Event, Emitter } from '@theia/core/lib/common/event';
77
import { Key, KeyCode } from '@theia/core/lib/browser/keys';
8-
import { DisposableCollection } from '@theia/core/lib/common/disposable'
8+
import { DisposableCollection, Disposable } from '@theia/core/lib/common/disposable'
99
import { ReactWidget, Message, Widget, MessageLoop } from '@theia/core/lib/browser/widgets';
1010
import { Board, Port } from '../../common/protocol/boards-service';
1111
import { MonitorConfig } from '../../common/protocol/monitor-service';
@@ -45,10 +45,16 @@ export class MonitorWidget extends ReactWidget {
4545
super();
4646
this.id = MonitorWidget.ID;
4747
this.title.label = 'Serial Monitor';
48-
this.title.iconClass = 'arduino-serial-monitor-tab-icon';
48+
this.title.iconClass = 'monitor-tab-icon';
4949
this.title.closable = true;
5050
this.scrollOptions = undefined;
5151
this.toDispose.push(this.clearOutputEmitter);
52+
this.toDispose.push(Disposable.create(() => {
53+
this.monitorConnection.autoConnect = false;
54+
if (this.monitorConnection.connected) {
55+
this.monitorConnection.disconnect();
56+
}
57+
}));
5258
}
5359

5460
@postConstruct()
@@ -73,10 +79,6 @@ export class MonitorWidget extends ReactWidget {
7379

7480
onCloseRequest(msg: Message): void {
7581
this.closing = true;
76-
this.monitorConnection.autoConnect = false;
77-
if (this.monitorConnection.connected) {
78-
this.monitorConnection.disconnect();
79-
}
8082
super.onCloseRequest(msg);
8183
}
8284

@@ -100,6 +102,9 @@ export class MonitorWidget extends ReactWidget {
100102
}
101103

102104
protected onFocusResolved = (element: HTMLElement | undefined) => {
105+
if (this.closing || !this.isAttached) {
106+
return;
107+
}
103108
this.focusNode = element;
104109
requestAnimationFrame(() => MessageLoop.sendMessage(this, Widget.Msg.ActivateRequest));
105110
}

arduino-ide-extension/src/browser/style/monitor.css

+3-5
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,6 @@
1-
.p-TabBar.theia-app-centers .p-TabBar-tabIcon.arduino-serial-monitor-tab-icon {
2-
background: url(../icons/buttons.svg);
3-
background-size: 800%;
4-
background-position-y: 41px;
5-
background-position-x: 19px;
1+
.monitor-tab-icon {
2+
-webkit-mask: url('../icons/monitor-tab-icon.svg');
3+
mask: url('../icons/monitor-tab-icon.svg');
64
}
75

86
.serial-monitor {

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

+11
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@ import { Searchable } from './searchable';
44
import { Installable } from './installable';
55
import { ArduinoComponent } from './arduino-component';
66

7+
export type AvailablePorts = Record<string, [Port, Array<Board>]>;
8+
79
export interface AttachedBoardsChangeEvent {
810
readonly oldState: Readonly<{ boards: Board[], ports: Port[] }>;
911
readonly newState: Readonly<{ boards: Board[], ports: Port[] }>;
@@ -86,8 +88,17 @@ export namespace AttachedBoardsChangeEvent {
8688
export const BoardsServicePath = '/services/boards-service';
8789
export const BoardsService = Symbol('BoardsService');
8890
export interface BoardsService extends Installable<BoardsPackage>, Searchable<BoardsPackage> {
91+
/**
92+
* Deprecated. `getState` should be used to correctly map a board with a port.
93+
* @deprecated
94+
*/
8995
getAttachedBoards(): Promise<Board[]>;
96+
/**
97+
* Deprecated. `getState` should be used to correctly map a board with a port.
98+
* @deprecated
99+
*/
90100
getAvailablePorts(): Promise<Port[]>;
101+
getState(): Promise<AvailablePorts>;
91102
getBoardDetails(options: { fqbn: string }): Promise<BoardDetails>;
92103
getBoardPackage(options: { id: string }): Promise<BoardsPackage | undefined>;
93104
getContainerBoardPackage(options: { fqbn: string }): Promise<BoardsPackage | undefined>;

arduino-ide-extension/src/node/arduino-ide-backend-module.ts

+12-25
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ import { CoreServiceImpl } from './core-service-impl';
1313
import { CoreService, CoreServicePath } from '../common/protocol/core-service';
1414
import { ConnectionContainerModule } from '@theia/core/lib/node/messaging/connection-container-module';
1515
import { CoreClientProvider } from './core-client-provider';
16-
import { ConnectionHandler, JsonRpcConnectionHandler, JsonRpcProxy } from '@theia/core';
16+
import { ConnectionHandler, JsonRpcConnectionHandler } from '@theia/core';
1717
import { DefaultWorkspaceServer } from './theia/workspace/default-workspace-server';
1818
import { WorkspaceServer as TheiaWorkspaceServer } from '@theia/workspace/lib/common';
1919
import { SketchesServiceImpl } from './sketches-service-impl';
@@ -39,6 +39,7 @@ import { OutputServicePath, OutputService } from '../common/protocol/output-serv
3939
import { NotificationServiceServerImpl } from './notification-service-server';
4040
import { NotificationServiceServer, NotificationServiceClient, NotificationServicePath } from '../common/protocol';
4141
import { BackendApplication } from './theia/core/backend-application';
42+
import { BoardDiscovery } from './board-discovery';
4243

4344
export default new ContainerModule((bind, unbind, isBound, rebind) => {
4445
bind(BackendApplication).toSelf().inSingletonScope();
@@ -60,9 +61,6 @@ export default new ContainerModule((bind, unbind, isBound, rebind) => {
6061

6162
// Examples service. One per backend, each connected FE gets a proxy.
6263
bind(ConnectionContainerModule).toConstantValue(ConnectionContainerModule.create(({ bind, bindBackendService }) => {
63-
// const ExamplesServiceProxy = Symbol('ExamplesServiceProxy');
64-
// bind(ExamplesServiceProxy).toDynamicValue(ctx => new Proxy(ctx.container.get(ExamplesService), {}));
65-
// bindBackendService(ExamplesServicePath, ExamplesServiceProxy);
6664
bind(ExamplesServiceImpl).toSelf().inSingletonScope();
6765
bind(ExamplesService).toService(ExamplesServiceImpl);
6866
bindBackendService(ExamplesServicePath, ExamplesService);
@@ -75,9 +73,6 @@ export default new ContainerModule((bind, unbind, isBound, rebind) => {
7573

7674
// Library service. Singleton per backend, each connected FE gets its proxy.
7775
bind(ConnectionContainerModule).toConstantValue(ConnectionContainerModule.create(({ bind, bindBackendService }) => {
78-
// const LibraryServiceProxy = Symbol('LibraryServiceProxy');
79-
// bind(LibraryServiceProxy).toDynamicValue(ctx => new Proxy(ctx.container.get(LibraryService), {}));
80-
// bindBackendService(LibraryServicePath, LibraryServiceProxy);
8176
bind(LibraryServiceImpl).toSelf().inSingletonScope();
8277
bind(LibraryService).toService(LibraryServiceImpl);
8378
bindBackendService(LibraryServicePath, LibraryService);
@@ -88,27 +83,21 @@ export default new ContainerModule((bind, unbind, isBound, rebind) => {
8883
bind(SketchesService).toService(SketchesServiceImpl);
8984
bind(ConnectionHandler).toDynamicValue(context => new JsonRpcConnectionHandler(SketchesServicePath, () => context.container.get(SketchesService))).inSingletonScope();
9085

91-
// Boards service. One singleton per backend that does the board and port polling. Each connected FE gets its proxy.
86+
// Boards service. One instance per connected frontend.
9287
bind(ConnectionContainerModule).toConstantValue(ConnectionContainerModule.create(({ bind, bindBackendService }) => {
93-
// const BoardsServiceProxy = Symbol('BoardsServiceProxy');
94-
// bind(BoardsServiceProxy).toDynamicValue(ctx => new Proxy(ctx.container.get(BoardsService), {}));
95-
// bindBackendService(BoardsServicePath, BoardsServiceProxy);
9688
bind(BoardsServiceImpl).toSelf().inSingletonScope();
9789
bind(BoardsService).toService(BoardsServiceImpl);
98-
bindBackendService<BoardsServiceImpl, JsonRpcProxy<object>>(BoardsServicePath, BoardsService, (service, client) => {
99-
client.onDidCloseConnection(() => service.dispose());
100-
return service;
101-
});
90+
bindBackendService(BoardsServicePath, BoardsService);
10291
}));
10392

10493
// Shared Arduino core client provider service for the backend.
10594
bind(CoreClientProvider).toSelf().inSingletonScope();
10695

96+
// Shared port/board discovery for the server
97+
bind(BoardDiscovery).toSelf().inSingletonScope();
98+
10799
// Core service -> `verify` and `upload`. Singleton per BE, each FE connection gets its proxy.
108100
bind(ConnectionContainerModule).toConstantValue(ConnectionContainerModule.create(({ bind, bindBackendService }) => {
109-
// const CoreServiceProxy = Symbol('CoreServiceProxy');
110-
// bind(CoreServiceProxy).toDynamicValue(ctx => new Proxy(ctx.container.get(CoreService), {}));
111-
// bindBackendService(CoreServicePath, CoreServiceProxy);
112101
bind(CoreServiceImpl).toSelf().inSingletonScope();
113102
bind(CoreService).toService(CoreServiceImpl);
114103
bindBackendService(CoreServicePath, CoreService);
@@ -127,14 +116,12 @@ export default new ContainerModule((bind, unbind, isBound, rebind) => {
127116

128117
// #endregion Theia customizations
129118

130-
// Shared monitor client provider service for the backend.
131-
bind(MonitorClientProvider).toSelf().inSingletonScope();
132-
bind(MonitorServiceImpl).toSelf().inSingletonScope();
133-
bind(MonitorService).toService(MonitorServiceImpl);
119+
// Monitor client provider per connected frontend.
134120
bind(ConnectionContainerModule).toConstantValue(ConnectionContainerModule.create(({ bind, bindBackendService }) => {
135-
const MonitorServiceProxy = Symbol('MonitorServiceProxy');
136-
bind(MonitorServiceProxy).toDynamicValue(ctx => new Proxy(ctx.container.get(MonitorService), {}));
137-
bindBackendService<MonitorService, MonitorServiceClient>(MonitorServicePath, MonitorServiceProxy, (service, client) => {
121+
bind(MonitorClientProvider).toSelf().inSingletonScope();
122+
bind(MonitorServiceImpl).toSelf().inSingletonScope();
123+
bind(MonitorService).toService(MonitorServiceImpl);
124+
bindBackendService<MonitorService, MonitorServiceClient>(MonitorServicePath, MonitorService, (service, client) => {
138125
service.setClient(client);
139126
client.onDidCloseConnection(() => service.dispose());
140127
return service;

0 commit comments

Comments
 (0)