Skip to content

Avoid resetting serial terminal when possible #1117

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

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 16 additions & 1 deletion src/device/device.ts
Original file line number Diff line number Diff line change
@@ -129,8 +129,23 @@ export interface FlashDataSource {
files(): Promise<Record<string, Uint8Array>>;
}

export const enum SerialOption {
/**
* Don't read serial data.
*/
None,
/**
* Emit a reset to clear any listeners, then read serial data.
*/
Reset,
/**
* Read serial data. No reset is emitted.
*/
NoReset,
}

export interface ConnectOptions {
serial?: boolean;
serial: SerialOption;
}

export interface DeviceConnection extends EventEmitter {
30 changes: 20 additions & 10 deletions src/device/webusb.ts
Original file line number Diff line number Diff line change
@@ -23,6 +23,7 @@ import {
FlashDataSource,
HexGenerationError,
MicrobitWebUSBConnectionOptions,
SerialOption,
WebUSBError,
} from "./device";

@@ -78,7 +79,11 @@ export class MicrobitWebUSBConnection
this.visibilityReconnect = false;
if (!this.flashing) {
this.log("Reconnecting visible tab");
this.connect();
this.connect({
// If any other connection status change occurs then visibilitReconnect is set to false, so
// it's likely the same program at this point.
serial: SerialOption.NoReset,
});
}
}
} else {
@@ -113,7 +118,7 @@ export class MicrobitWebUSBConnection
setTimeout(() => {
if (this.status === ConnectionStatus.CONNECTED) {
this.unloading = false;
this.startSerialInternal();
this.startSerialInternal(SerialOption.NoReset);
}
}, assumePageIsStayingOpenDelay);
},
@@ -165,7 +170,9 @@ export class MicrobitWebUSBConnection
}
}

async connect(options: ConnectOptions = {}): Promise<ConnectionStatus> {
async connect(
options: ConnectOptions = { serial: SerialOption.Reset }
): Promise<ConnectionStatus> {
return this.withEnrichedErrors(async () => {
await this.connectInternal(options);
return this.status;
@@ -217,7 +224,7 @@ export class MicrobitWebUSBConnection
await this.stopSerialInternal();
this.log("Reconnecting before flash");
await this.connectInternal({
serial: false,
serial: SerialOption.None,
});
if (!this.connection) {
throw new Error("Must be connected now");
@@ -249,13 +256,13 @@ export class MicrobitWebUSBConnection
this.log("Reinstating serial after flash");
if (this.connection.daplink) {
await this.connection.daplink.connect();
await this.startSerialInternal();
await this.startSerialInternal(SerialOption.Reset);
}
}
}
}

private async startSerialInternal() {
private async startSerialInternal(option: SerialOption) {
if (!this.connection) {
// As connecting then starting serial are async we could disconnect between them,
// so handle this gracefully.
@@ -264,6 +271,12 @@ export class MicrobitWebUSBConnection
if (this.serialReadInProgress) {
await this.stopSerialInternal();
}
if (option === SerialOption.None || option === SerialOption.Reset) {
this.emit(EVENT_SERIAL_RESET, {});
}
if (option === SerialOption.None) {
return;
}
// This is async but won't return until we stop serial so we error handle with an event.
this.serialReadInProgress = this.connection
.startSerial(this.serialListener)
@@ -278,7 +291,6 @@ export class MicrobitWebUSBConnection
this.connection.stopSerial(this.serialListener);
await this.serialReadInProgress;
this.serialReadInProgress = undefined;
this.emit(EVENT_SERIAL_RESET, {});
}
}

@@ -384,9 +396,7 @@ export class MicrobitWebUSBConnection
this.connection = new DAPWrapper(device, this.logging);
}
await withTimeout(this.connection.reconnectAsync(), 10_000);
if (options.serial === undefined || options.serial) {
this.startSerialInternal();
}
this.startSerialInternal(options.serial);
this.setStatus(ConnectionStatus.CONNECTED);
}

8 changes: 7 additions & 1 deletion src/project/project-actions.tsx
Original file line number Diff line number Diff line change
@@ -24,6 +24,7 @@ import {
DeviceConnection,
EVENT_END_USB_SELECT,
HexGenerationError,
SerialOption,
WebUSBError,
WebUSBErrorCode,
} from "../device/device";
@@ -131,7 +132,12 @@ export class ProjectActions {
} else {
if (await this.showConnectHelp(forceConnectHelp, finalFocusRef)) {
return this.connectInternal(
{ serial: userAction !== ConnectionAction.FLASH },
{
serial:
userAction === ConnectionAction.FLASH
? SerialOption.None
: SerialOption.Reset,
},
userAction,
finalFocusRef
);
50 changes: 25 additions & 25 deletions src/serial/SerialArea.tsx
Original file line number Diff line number Diff line change
@@ -53,31 +53,31 @@ const SerialArea = ({
position="relative"
overflow="hidden"
>
{!connected ? null : (
<Box
alignItems="stretch"
backgroundColor={backgroundColorTerm}
height="100%"
>
<SerialBar
height={12}
compact={compact}
onSizeChange={onSizeChange}
showSyncStatus={showSyncStatus}
expandDirection={expandDirection}
hideExpandTextOnTraceback={hideExpandTextOnTraceback}
showHintsAndTips={showHintsAndTips}
/>
<XTerm
visibility={compact ? "hidden" : undefined}
height={`calc(100% - ${SerialArea.compactSize}px)`}
ml={1}
mr={1}
fontSizePt={terminalFontSizePt}
tabOutRef={tabOutRef}
/>
</Box>
)}
<Box
// Need to render this when not connected as we need it to maintain scrollback across disconnect/reconnect in some cases.
display={connected ? undefined : "none"}
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Until this change, reset wasn't really what was resetting the terminal—we were also discarding it entirely.

Need to review for unexpected consequences here.

alignItems="stretch"
backgroundColor={backgroundColorTerm}
height="100%"
>
<SerialBar
height={12}
compact={compact}
onSizeChange={onSizeChange}
showSyncStatus={showSyncStatus}
expandDirection={expandDirection}
hideExpandTextOnTraceback={hideExpandTextOnTraceback}
showHintsAndTips={showHintsAndTips}
/>
<XTerm
visibility={compact ? "hidden" : undefined}
height={`calc(100% - ${SerialArea.compactSize}px)`}
ml={1}
mr={1}
fontSizePt={terminalFontSizePt}
tabOutRef={tabOutRef}
/>
</Box>
</Flex>
</TerminalContext>
);