Skip to content

Commit bd623db

Browse files
committed
fix(plugin): decouple state update from the LS
To enhance the reliability of Arduino IDE extensions, the update process for `ArduinoState` has been modified to ensure independence from the language server's availability. This change addresses issues caused by `compileSummary` being `undefined` due to potential startup failures of the Arduino Language Server, as noted in dankeboy36/esp-exception-decoder#28 (comment). The `compile` command now resolves with a `CompileSummary` rather than `void`, facilitating a more reliable way for extensions to access necessary data. Furthermore, the command has been adjusted to allow resolution with `undefined` when the compiled data is partial. By transitioning to direct usage of the resolved compile value for state updates, the reliance on executed commands for extensions is eliminated. This update also moves the VSIX command execution to the frontend without altering existing IDE behavior. Closes arduino#2642 Signed-off-by: dankeboy36 <[email protected]>
1 parent 6eef09e commit bd623db

File tree

7 files changed

+111
-114
lines changed

7 files changed

+111
-114
lines changed

arduino-ide-extension/src/browser/arduino-ide-frontend-module.ts

+6-1
Original file line numberDiff line numberDiff line change
@@ -131,7 +131,10 @@ import { OpenSketch } from './contributions/open-sketch';
131131
import { Close } from './contributions/close';
132132
import { SaveAsSketch } from './contributions/save-as-sketch';
133133
import { SaveSketch } from './contributions/save-sketch';
134-
import { VerifySketch } from './contributions/verify-sketch';
134+
import {
135+
CompileSummaryProvider,
136+
VerifySketch,
137+
} from './contributions/verify-sketch';
135138
import { UploadSketch } from './contributions/upload-sketch';
136139
import { CommonFrontendContribution } from './theia/core/common-frontend-contribution';
137140
import { EditContributions } from './contributions/edit-contributions';
@@ -788,6 +791,8 @@ export default new ContainerModule((bind, unbind, isBound, rebind) => {
788791
Contribution.configure(bind, BoardsDataMenuUpdater);
789792
Contribution.configure(bind, AutoSelectProgrammer);
790793

794+
bind(CompileSummaryProvider).toService(VerifySketch);
795+
791796
bindContributionProvider(bind, StartupTaskProvider);
792797
bind(StartupTaskProvider).toService(BoardsServiceProvider); // to reuse the boards config in another window
793798

arduino-ide-extension/src/browser/contributions/update-arduino-state.ts

+16-11
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,11 @@
11
import { DisposableCollection } from '@theia/core/lib/common/disposable';
22
import URI from '@theia/core/lib/common/uri';
33
import { inject, injectable } from '@theia/core/shared/inversify';
4-
import { HostedPluginSupport } from '../hosted/hosted-plugin-support';
54
import type { ArduinoState } from 'vscode-arduino-api';
65
import {
6+
BoardsConfig,
77
BoardsService,
88
CompileSummary,
9-
isCompileSummary,
10-
BoardsConfig,
119
PortIdentifier,
1210
resolveDetectedPort,
1311
} from '../../common/protocol';
@@ -18,8 +16,10 @@ import {
1816
} from '../../common/protocol/arduino-context-mapper';
1917
import { BoardsDataStore } from '../boards/boards-data-store';
2018
import { BoardsServiceProvider } from '../boards/boards-service-provider';
19+
import { HostedPluginSupport } from '../hosted/hosted-plugin-support';
2120
import { CurrentSketch } from '../sketches-service-client-impl';
2221
import { SketchContribution } from './contribution';
22+
import { CompileSummaryProvider } from './verify-sketch';
2323

2424
/**
2525
* (non-API) exported for tests
@@ -43,6 +43,8 @@ export class UpdateArduinoState extends SketchContribution {
4343
private readonly boardsDataStore: BoardsDataStore;
4444
@inject(HostedPluginSupport)
4545
private readonly hostedPluginSupport: HostedPluginSupport;
46+
@inject(CompileSummaryProvider)
47+
private readonly compileSummaryProvider: CompileSummaryProvider;
4648

4749
private readonly toDispose = new DisposableCollection();
4850

@@ -60,14 +62,9 @@ export class UpdateArduinoState extends SketchContribution {
6062
this.configService.onDidChangeSketchDirUri((userDirUri) =>
6163
this.updateUserDirPath(userDirUri)
6264
),
63-
this.commandService.onDidExecuteCommand(({ commandId, args }) => {
64-
if (
65-
commandId === 'arduino.languageserver.notifyBuildDidComplete' &&
66-
isCompileSummary(args[0])
67-
) {
68-
this.updateCompileSummary(args[0]);
69-
}
70-
}),
65+
this.compileSummaryProvider.onDidChangeCompileSummary(() =>
66+
this.maybeUpdateCompileSummary()
67+
),
7168
this.boardsDataStore.onDidChange((event) => {
7269
const selectedFqbn =
7370
this.boardsServiceProvider.boardsConfig.selectedBoard?.fqbn;
@@ -88,12 +85,20 @@ export class UpdateArduinoState extends SketchContribution {
8885
this.updateSketchPath(this.sketchServiceClient.tryGetCurrentSketch());
8986
this.updateUserDirPath(this.configService.tryGetSketchDirUri());
9087
this.updateDataDirPath(this.configService.tryGetDataDirUri());
88+
this.maybeUpdateCompileSummary();
9189
}
9290

9391
onStop(): void {
9492
this.toDispose.dispose();
9593
}
9694

95+
private maybeUpdateCompileSummary() {
96+
const { compileSummary } = this.compileSummaryProvider;
97+
if (compileSummary) {
98+
this.updateCompileSummary(compileSummary);
99+
}
100+
}
101+
97102
private async updateSketchPath(
98103
sketch: CurrentSketch | undefined
99104
): Promise<void> {

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

+52-4
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
1-
import { Emitter } from '@theia/core/lib/common/event';
1+
import { Emitter, Event } from '@theia/core/lib/common/event';
22
import { nls } from '@theia/core/lib/common/nls';
33
import { inject, injectable } from '@theia/core/shared/inversify';
4-
import type { CoreService } from '../../common/protocol';
4+
import type { CompileSummary, CoreService } from '../../common/protocol';
55
import { ArduinoMenus } from '../menu/arduino-menus';
66
import { CurrentSketch } from '../sketches-service-client-impl';
77
import { ArduinoToolbar } from '../toolbar/arduino-toolbar';
@@ -15,6 +15,12 @@ import {
1515
} from './contribution';
1616
import { CoreErrorHandler } from './core-error-handler';
1717

18+
export const CompileSummaryProvider = Symbol('CompileSummaryProvider');
19+
export interface CompileSummaryProvider {
20+
readonly compileSummary: CompileSummary | undefined;
21+
readonly onDidChangeCompileSummary: Event<void>;
22+
}
23+
1824
export type VerifySketchMode =
1925
/**
2026
* When the user explicitly triggers the verify command from the primary UI: menu, toolbar, or keybinding. The UI shows the output, updates the toolbar items state, etc.
@@ -46,13 +52,18 @@ export interface VerifySketchParams {
4652
type VerifyProgress = 'idle' | VerifySketchMode;
4753

4854
@injectable()
49-
export class VerifySketch extends CoreServiceContribution {
55+
export class VerifySketch
56+
extends CoreServiceContribution
57+
implements CompileSummaryProvider
58+
{
5059
@inject(CoreErrorHandler)
5160
private readonly coreErrorHandler: CoreErrorHandler;
5261

5362
private readonly onDidChangeEmitter = new Emitter<void>();
5463
private readonly onDidChange = this.onDidChangeEmitter.event;
64+
private readonly onDidChangeCompileSummaryEmitter = new Emitter<void>();
5565
private verifyProgress: VerifyProgress = 'idle';
66+
private _compileSummary: CompileSummary | undefined;
5667

5768
override registerCommands(registry: CommandRegistry): void {
5869
registry.registerCommand(VerifySketch.Commands.VERIFY_SKETCH, {
@@ -117,6 +128,14 @@ export class VerifySketch extends CoreServiceContribution {
117128
super.handleError(error);
118129
}
119130

131+
get compileSummary(): CompileSummary | undefined {
132+
return this._compileSummary;
133+
}
134+
135+
get onDidChangeCompileSummary(): Event<void> {
136+
return this.onDidChangeCompileSummaryEmitter.event;
137+
}
138+
120139
private async verifySketch(
121140
params?: VerifySketchParams
122141
): Promise<CoreService.Options.Compile | undefined> {
@@ -141,7 +160,7 @@ export class VerifySketch extends CoreServiceContribution {
141160
return options;
142161
}
143162

144-
await this.doWithProgress({
163+
const compileSummary = await this.doWithProgress({
145164
progressText: nls.localize(
146165
'arduino/sketch/compile',
147166
'Compiling sketch...'
@@ -160,6 +179,13 @@ export class VerifySketch extends CoreServiceContribution {
160179
nls.localize('arduino/sketch/doneCompiling', 'Done compiling.'),
161180
{ timeout: 3000 }
162181
);
182+
183+
this._compileSummary = compileSummary;
184+
this.onDidChangeCompileSummaryEmitter.fire();
185+
if (this._compileSummary) {
186+
this.fireBuildDidComplete(this._compileSummary);
187+
}
188+
163189
// Returns with the used options for the compilation
164190
// so that follow-up tasks (such as upload) can reuse the compiled code.
165191
// Note that the `fqbn` is already decorated with the board settings, if any.
@@ -201,6 +227,28 @@ export class VerifySketch extends CoreServiceContribution {
201227
compilerWarnings,
202228
};
203229
}
230+
231+
// Execute the a command contributed by the Arduino Tools VSIX to send the `ino/buildDidComplete` notification to the language server
232+
private fireBuildDidComplete(compileSummary: CompileSummary): void {
233+
const params = {
234+
...compileSummary,
235+
};
236+
console.info(
237+
`Executing 'arduino.languageserver.notifyBuildDidComplete' with ${JSON.stringify(
238+
params.buildOutputUri
239+
)}`
240+
);
241+
this.commandService
242+
.executeCommand('arduino.languageserver.notifyBuildDidComplete', params)
243+
.catch((err) =>
244+
console.error(
245+
`Unexpected error when firing event on build did complete. ${JSON.stringify(
246+
params.buildOutputUri
247+
)}`,
248+
err
249+
)
250+
);
251+
}
204252
}
205253

206254
export namespace VerifySketch {

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

+1-1
Original file line numberDiff line numberDiff line change
@@ -171,7 +171,7 @@ export interface CoreService {
171171
compile(
172172
options: CoreService.Options.Compile,
173173
cancellationToken?: CancellationToken
174-
): Promise<void>;
174+
): Promise<CompileSummary | undefined>;
175175
upload(
176176
options: CoreService.Options.Upload,
177177
cancellationToken?: CancellationToken

arduino-ide-extension/src/node/core-service-impl.ts

+14-43
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
import { type ClientReadableStream } from '@grpc/grpc-js';
22
import { ApplicationError } from '@theia/core/lib/common/application-error';
33
import type { CancellationToken } from '@theia/core/lib/common/cancellation';
4-
import { CommandService } from '@theia/core/lib/common/command';
54
import {
65
Disposable,
76
DisposableCollection,
@@ -69,15 +68,13 @@ export class CoreServiceImpl extends CoreClientAware implements CoreService {
6968
private readonly responseService: ResponseService;
7069
@inject(MonitorManager)
7170
private readonly monitorManager: MonitorManager;
72-
@inject(CommandService)
73-
private readonly commandService: CommandService;
7471
@inject(BoardDiscovery)
7572
private readonly boardDiscovery: BoardDiscovery;
7673

7774
async compile(
7875
options: CoreService.Options.Compile,
7976
cancellationToken?: CancellationToken
80-
): Promise<void> {
77+
): Promise<CompileSummary | undefined> {
8178
const coreClient = await this.coreClient;
8279
const { client, instance } = coreClient;
8380
const request = this.compileRequest(options, instance);
@@ -91,7 +88,7 @@ export class CoreServiceImpl extends CoreClientAware implements CoreService {
9188
);
9289
const toDisposeOnFinally = new DisposableCollection(handler);
9390

94-
return new Promise<void>((resolve, reject) => {
91+
return new Promise<CompileSummary | undefined>((resolve, reject) => {
9592
let hasRetried = false;
9693

9794
const handleUnexpectedError = (error: Error) => {
@@ -164,50 +161,24 @@ export class CoreServiceImpl extends CoreClientAware implements CoreService {
164161
call
165162
.on('data', handler.onData)
166163
.on('error', handleError)
167-
.on('end', resolve);
164+
.on('end', () => {
165+
if (isCompileSummary(compileSummary)) {
166+
resolve(compileSummary);
167+
} else {
168+
console.error(
169+
`Have not received the full compile summary from the CLI while running the compilation. ${JSON.stringify(
170+
compileSummary
171+
)}`
172+
);
173+
resolve(undefined);
174+
}
175+
});
168176
};
169177

170178
startCompileStream();
171-
}).finally(() => {
172-
toDisposeOnFinally.dispose();
173-
if (!isCompileSummary(compileSummary)) {
174-
if (cancellationToken && cancellationToken.isCancellationRequested) {
175-
// NOOP
176-
return;
177-
}
178-
console.error(
179-
`Have not received the full compile summary from the CLI while running the compilation. ${JSON.stringify(
180-
compileSummary
181-
)}`
182-
);
183-
} else {
184-
this.fireBuildDidComplete(compileSummary);
185-
}
186179
});
187180
}
188181

189-
// This executes on the frontend, the VS Code extension receives it, and sends an `ino/buildDidComplete` notification to the language server.
190-
private fireBuildDidComplete(compileSummary: CompileSummary): void {
191-
const params = {
192-
...compileSummary,
193-
};
194-
console.info(
195-
`Executing 'arduino.languageserver.notifyBuildDidComplete' with ${JSON.stringify(
196-
params.buildOutputUri
197-
)}`
198-
);
199-
this.commandService
200-
.executeCommand('arduino.languageserver.notifyBuildDidComplete', params)
201-
.catch((err) =>
202-
console.error(
203-
`Unexpected error when firing event on build did complete. ${JSON.stringify(
204-
params.buildOutputUri
205-
)}`,
206-
err
207-
)
208-
);
209-
}
210-
211182
private compileRequest(
212183
options: CoreService.Options.Compile & {
213184
exportBinaries?: boolean;

arduino-ide-extension/src/test/browser/update-arduino-state.test.ts

+14-4
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ import {
3131
UpdateArduinoState,
3232
UpdateStateParams,
3333
} from '../../browser/contributions/update-arduino-state';
34+
import { CompileSummaryProvider } from '../../browser/contributions/verify-sketch';
3435
import { NotificationCenter } from '../../browser/notification-center';
3536
import {
3637
CurrentSketch,
@@ -61,10 +62,12 @@ describe('update-arduino-state', function () {
6162
let currentSketchMock: CurrentSketch | undefined;
6263
let sketchDirUriMock: URI | undefined;
6364
let dataDirUriMock: URI | undefined;
65+
let compileSummary: CompileSummary | undefined;
6466
let onCurrentSketchDidChangeEmitter: Emitter<CurrentSketch>;
6567
let onDataDirDidChangeEmitter: Emitter<URI | undefined>;
6668
let onSketchDirDidChangeEmitter: Emitter<URI | undefined>;
6769
let onDataStoreDidChangeEmitter: Emitter<BoardsDataStoreChangeEvent>;
70+
let compileSummaryDidChangeEmitter: Emitter<void>;
6871

6972
beforeEach(async () => {
7073
toDisposeAfterEach = new DisposableCollection();
@@ -76,15 +79,18 @@ describe('update-arduino-state', function () {
7679
currentSketchMock = undefined;
7780
sketchDirUriMock = undefined;
7881
dataDirUriMock = undefined;
82+
compileSummary = undefined;
7983
onCurrentSketchDidChangeEmitter = new Emitter();
8084
onDataDirDidChangeEmitter = new Emitter();
8185
onSketchDirDidChangeEmitter = new Emitter();
8286
onDataStoreDidChangeEmitter = new Emitter();
87+
compileSummaryDidChangeEmitter = new Emitter();
8388
toDisposeAfterEach.pushAll([
8489
onCurrentSketchDidChangeEmitter,
8590
onDataDirDidChangeEmitter,
8691
onSketchDirDidChangeEmitter,
8792
onDataStoreDidChangeEmitter,
93+
compileSummaryDidChangeEmitter,
8894
]);
8995

9096
const container = createContainer();
@@ -418,10 +424,8 @@ describe('update-arduino-state', function () {
418424
buildPlatform: undefined,
419425
buildOutputUri: 'file:///path/to/build',
420426
};
421-
await commandRegistry.executeCommand(
422-
'arduino.languageserver.notifyBuildDidComplete',
423-
summary
424-
);
427+
compileSummary = summary;
428+
compileSummaryDidChangeEmitter.fire();
425429
await wait(50);
426430

427431
const params = stateUpdateParams.filter(
@@ -585,6 +589,12 @@ describe('update-arduino-state', function () {
585589
new ContainerModule((bind, unbind, isBound, rebind) => {
586590
bindSketchesContribution(bind, unbind, isBound, rebind);
587591
bind(UpdateArduinoState).toSelf().inSingletonScope();
592+
bind(CompileSummaryProvider).toConstantValue(<CompileSummaryProvider>{
593+
get compileSummary(): CompileSummary | undefined {
594+
return compileSummary;
595+
},
596+
onDidChangeCompileSummary: compileSummaryDidChangeEmitter.event,
597+
});
588598
rebind(BoardsService).toConstantValue(<BoardsService>{
589599
getDetectedPorts() {
590600
return {};

0 commit comments

Comments
 (0)