Skip to content

Commit 23db8f4

Browse files
committed
terminal: fix input handler loop getting stuck
When no hub is connected, the terminal input handler loop would get stuck waiting for didWrite or didFailToWrite which would never come because handleWriteUart in ble/sagas only runs when a hub is connected. This is fixed by only dispatching a write action if a user program is running on the hub. By using this state, it also fixes characters being buffered before the user program starts, e.g. - connect hub - type into terminal - no echo - start the repl - previously typed characters are echoed after the repl prompt Now, anything typed before the user program is just ignored. Fixes: pybricks/support#865 Also properly fixes pybricks/support#303
1 parent a12ae33 commit 23db8f4

File tree

3 files changed

+32
-2
lines changed

3 files changed

+32
-2
lines changed

CHANGELOG.md

+2
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,9 @@
66

77
### Fixed
88
- Fixed terminal scroll not working ([support#866]).
9+
- Fixed terminal breaks if input is given before hub is connected ([support#865]).
910

11+
[support#865]: https://github.com/pybricks/support/issues/865
1012
[support#866]: https://github.com/pybricks/support/issues/866
1113

1214
## [2.0.0] - 2022-12-20

src/terminal/sagas.test.ts

+15
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,7 @@ describe('Terminal data source responds to receive data actions', () => {
7171

7272
test('basic function works', async () => {
7373
const saga = new AsyncSaga(terminal, { nextMessageId: createCountFunc() });
74+
saga.updateState({ hub: { runtime: HubRuntimeState.Running } });
7475

7576
saga.put(receiveData('test1234'));
7677

@@ -82,6 +83,7 @@ describe('Terminal data source responds to receive data actions', () => {
8283

8384
test('messages are queued until previous has completed', async () => {
8485
const saga = new AsyncSaga(terminal, { nextMessageId: createCountFunc() });
86+
saga.updateState({ hub: { runtime: HubRuntimeState.Running } });
8587

8688
saga.put(receiveData('test1234'));
8789
await delay(50); // without delay, messages are combined
@@ -108,6 +110,7 @@ describe('Terminal data source responds to receive data actions', () => {
108110

109111
test('messages are queued until previous has failed', async () => {
110112
const saga = new AsyncSaga(terminal, { nextMessageId: createCountFunc() });
113+
saga.updateState({ hub: { runtime: HubRuntimeState.Running } });
111114

112115
saga.put(receiveData('test1234'));
113116
await delay(50); // without delay, messages are combined
@@ -134,6 +137,7 @@ describe('Terminal data source responds to receive data actions', () => {
134137

135138
test('small messages are combined', async () => {
136139
const saga = new AsyncSaga(terminal, { nextMessageId: createCountFunc() });
140+
saga.updateState({ hub: { runtime: HubRuntimeState.Running } });
137141

138142
saga.put(receiveData('test1234'));
139143
saga.put(receiveData('test1234'));
@@ -148,6 +152,7 @@ describe('Terminal data source responds to receive data actions', () => {
148152
const testData = '012345678901234567890123456789';
149153

150154
const saga = new AsyncSaga(terminal, { nextMessageId: createCountFunc() });
155+
saga.updateState({ hub: { runtime: HubRuntimeState.Running } });
151156

152157
saga.put(receiveData(testData));
153158

@@ -163,4 +168,14 @@ describe('Terminal data source responds to receive data actions', () => {
163168

164169
await saga.end();
165170
});
171+
172+
test('if user program is not running, do not dispatch write', async () => {
173+
const saga = new AsyncSaga(terminal, { nextMessageId: createCountFunc() });
174+
saga.updateState({ hub: { runtime: HubRuntimeState.Disconnected } });
175+
176+
saga.put(receiveData('test1234'));
177+
178+
// line below will fail with unhandled pending dispatches if not working correctly
179+
await saga.end();
180+
});
166181
});

src/terminal/sagas.ts

+15-2
Original file line numberDiff line numberDiff line change
@@ -49,8 +49,10 @@ function* receiveUartData(action: ReturnType<typeof didNotify>): Generator {
4949
}
5050

5151
function* receiveTerminalData(): Generator {
52+
const nextMessageId = yield* getContext<() => number>('nextMessageId');
5253
const channel = yield* actionChannel(receiveData);
53-
while (true) {
54+
55+
for (;;) {
5456
// wait for input from terminal
5557
const action = yield* take(channel);
5658
let value = action.value;
@@ -68,10 +70,21 @@ function* receiveTerminalData(): Generator {
6870
value += action.value;
6971
}
7072

71-
const nextMessageId = yield* getContext<() => number>('nextMessageId');
73+
const isUserProgramRunning = yield* select(
74+
(s: RootState) => s.hub.runtime === HubRuntimeState.Running,
75+
);
76+
77+
// REVISIT: this test is a bit dangerous since it is not actually
78+
// testing that handleWriteUart in ble/sagas is running. In theory
79+
// it should be fine as long a the logic for the state doesn't change.
80+
if (!isUserProgramRunning) {
81+
// if no user program is running, input goes to /dev/null
82+
continue;
83+
}
7284

7385
// stdin gets piped to BLE connection
7486
const data = encoder.encode(value);
87+
7588
for (let i = 0; i < data.length; i += nordicUartSafeTxCharLength) {
7689
const { id } = yield* put(
7790
write(nextMessageId(), data.slice(i, i + nordicUartSafeTxCharLength)),

0 commit comments

Comments
 (0)