8000 Avoid resetting serial terminal when possible by microbit-matt-hillsdon · Pull Request #1117 · microbit-foundation/python-editor-v3 · GitHub
[go: up one dir, main page]
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.

Sign up for GitHub

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

Avoid resetting serial terminal when possible #1117

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
30 changes: 20 additions & 10 deletions src/device/webusb.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import {
FlashDataSource,
HexGenerationError,
MicrobitWebUSBConnectionOptions,
SerialOption,
WebUSBError,
} from "./device";

Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -113,7 +118,7 @@ export class MicrobitWebUSBConnection
setTimeout(() => {
if (this.status === ConnectionStatus.CONNECTED) {
this.unloading = false;
this.startSerialInternal();
this.startSerialInternal(SerialOption.NoReset);
}
}, assumePageIsStayingOpenDelay);
},
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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");
Expand Down Expand Up @@ -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.
Expand All @@ -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.s 8000 erialReadInProgress = this.connection
.startSerial(this.serialListener)
Expand All @@ -278,7 +291,6 @@ export class MicrobitWebUSBConnection
this.connection.stopSerial(this.serialListener);
await this.serialReadInProgress;
this.serialReadInProgress = undefined;
this.emit(EVENT_SERIAL_RESET, {});
}
}

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

Expand Down
8 changes: 7 additions & 1 deletion src/project/project-actions.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import {
DeviceConnection,
EVENT_END_USB_SELECT,
HexGenerationError,
SerialOption,
WebUSBError,
WebUSBErrorCode,
} from "../device/device";
Expand Down Expand Up @@ -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
);
Expand Down
50 changes: 25 additions & 25 deletions src/serial/SerialArea.tsx
View file
Open in desktop
6D47
Original file line number Diff line number Diff line change
Expand Up @@ -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
@microbit-matt-hillsdon microbit-matt-hillsdon Mar 31, 2023 •
edited
Loading

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>
);
Expand Down
0