-
Notifications
You must be signed in to change notification settings - Fork 8
fix(sterm): 修复 event::poll 阻塞问题并改用 EventStream 异步处理 #27
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
Conversation
ZR233
commented
Jan 22, 2026
- 启用 crossterm 的 event-stream feature 并添加 futures 依赖
- 将键盘事件处理从阻塞式轮询改为 EventStream 异步流
- 使用 tokio::spawn 替代 thread::spawn 以正确支持异步上下文
- 修复因线程上下文不匹配导致的 stdin 阻塞问题
- 移除不可靠的超时轮询,采用真正的事件驱动模型
- 启用 crossterm 的 event-stream feature 并添加 futures 依赖 - 将键盘事件处理从阻塞式轮询改为 EventStream 异步流 - 使用 tokio::spawn 替代 thread::spawn 以正确支持异步上下文 - 修复因线程上下文不匹配导致的 stdin 阻塞问题 - 移除不可靠的超时轮询,采用真正的事件驱动模型
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR modernizes the serial terminal keyboard event handling by replacing blocking event polling with an asynchronous event-driven architecture using crossterm's EventStream.
Changes:
- Enabled crossterm's
event-streamfeature and added futures dependency - Refactored keyboard event handling from blocking
event::poll()to asyncEventStream - Replaced
thread::spawnwithtokio::spawnfor keyboard task andspawn_blockingfor serial receive task - Added abort handle mechanism to coordinate task shutdown
Reviewed changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| Cargo.toml | Added event-stream feature to crossterm dependency |
| ostool/Cargo.toml | Added futures 0.3 dependency and bumped version to 0.8.8 |
| ostool/src/sterm/mod.rs | Refactored keyboard event handling from blocking to async, split into separate tx_work_async function |
| Cargo.lock | Updated lock file with futures dependencies and crossterm features |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| Ok(()) | ||
| } | ||
|
|
||
| async fn tx_work_async(handle: Arc<TermHandle>, tx_port: Arc<Mutex<Tx>>) -> anyhow::Result<()> { |
Copilot
AI
Jan 22, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function naming is inconsistent with Rust conventions. The function name tx_work_async uses a _async suffix, but in Rust it's more idiomatic to let the function signature (async fn) indicate that it's asynchronous. Consider renaming to tx_work or handle_keyboard_input for better clarity and consistency with the existing handle_serial_receive function.
| // 使用 EventStream::next() 异步等待事件,不会阻塞 | ||
| match reader.next().await { | ||
| Some(Ok(Event::Key(key))) if key.kind == KeyEventKind::Press => { | ||
| // 检测 Ctrl+A+x 退出序列 | ||
| match key_state { | ||
| KeySequenceState::Normal => { | ||
| if key.code == KeyCode::Char('a') | ||
| && key.modifiers.contains(KeyModifiers::CONTROL) | ||
| { | ||
| key_state = KeySequenceState::CtrlAPressed; | ||
| } else { | ||
| // 普通按键,发送到串口 | ||
| if let Err(e) = Self::send_key_to_serial(&tx_port, key) { | ||
| eprintln!("\r\n发送按键失败: {}", e); | ||
| } | ||
| } | ||
| } | ||
| KeySequenceState::CtrlAPressed => { | ||
| if key.code == KeyCode::Char('x') { | ||
| // 用户请求退出 | ||
| eprintln!("\r\nExit by: Ctrl+A+x"); | ||
| handle.stop(); | ||
| break; | ||
| } else { | ||
| // 不是x键,发送上一个按键并重置状态 | ||
| if key.code != KeyCode::Char('a') { | ||
| if let Err(e) = Self::send_ctrl_a_to_serial(&tx_port) { | ||
| eprintln!("\r\n发送 Ctrl+A 失败: {}", e); | ||
| } | ||
| if let Err(e) = Self::send_key_to_serial(&tx_port, key) { | ||
| eprintln!("\r\n发送按键失败: {}", e); | ||
| } | ||
| key_state = KeySequenceState::Normal; | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
| Some(Err(e)) => { | ||
| eprintln!("\r\n键盘事件错误: {}", e); | ||
| break; | ||
| } | ||
| None => { | ||
| // EventStream 结束 | ||
| break; | ||
| } | ||
| Some(Ok(_)) => { | ||
| // 忽略非按键事件(鼠标、调整大小等) | ||
| } |
Copilot
AI
Jan 22, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The task exit mechanism has a potential delay issue. If handle.stop() is called externally (though not currently done in the code), the task will remain blocked on reader.next().await at line 464 until the next keyboard event arrives, rather than exiting promptly. While the current code relies on tx_abort.abort() from line 156 to forcefully terminate the task, consider using tokio::select! with a shutdown signal channel or periodic timeout checks for more graceful shutdown, especially if external stop signals are added in the future.
| // 使用 EventStream::next() 异步等待事件,不会阻塞 | |
| match reader.next().await { | |
| Some(Ok(Event::Key(key))) if key.kind == KeyEventKind::Press => { | |
| // 检测 Ctrl+A+x 退出序列 | |
| match key_state { | |
| KeySequenceState::Normal => { | |
| if key.code == KeyCode::Char('a') | |
| && key.modifiers.contains(KeyModifiers::CONTROL) | |
| { | |
| key_state = KeySequenceState::CtrlAPressed; | |
| } else { | |
| // 普通按键,发送到串口 | |
| if let Err(e) = Self::send_key_to_serial(&tx_port, key) { | |
| eprintln!("\r\n发送按键失败: {}", e); | |
| } | |
| } | |
| } | |
| KeySequenceState::CtrlAPressed => { | |
| if key.code == KeyCode::Char('x') { | |
| // 用户请求退出 | |
| eprintln!("\r\nExit by: Ctrl+A+x"); | |
| handle.stop(); | |
| break; | |
| } else { | |
| // 不是x键,发送上一个按键并重置状态 | |
| if key.code != KeyCode::Char('a') { | |
| if let Err(e) = Self::send_ctrl_a_to_serial(&tx_port) { | |
| eprintln!("\r\n发送 Ctrl+A 失败: {}", e); | |
| } | |
| if let Err(e) = Self::send_key_to_serial(&tx_port, key) { | |
| eprintln!("\r\n发送按键失败: {}", e); | |
| } | |
| key_state = KeySequenceState::Normal; | |
| } | |
| } | |
| } | |
| } | |
| } | |
| Some(Err(e)) => { | |
| eprintln!("\r\n键盘事件错误: {}", e); | |
| break; | |
| } | |
| None => { | |
| // EventStream 结束 | |
| break; | |
| } | |
| Some(Ok(_)) => { | |
| // 忽略非按键事件(鼠标、调整大小等) | |
| } | |
| // 使用 tokio::select! 同时等待键盘事件和定期检查退出信号 | |
| tokio::select! { | |
| _ = tokio::time::sleep(tokio::time::Duration::from_millis(100)) => { | |
| // 定期检查是否已请求退出(例如通过外部调用 handle.stop()) | |
| if !handle.is_running() { | |
| break; | |
| } | |
| } | |
| maybe_event = reader.next() => { | |
| match maybe_event { | |
| Some(Ok(Event::Key(key))) if key.kind == KeyEventKind::Press => { | |
| // 检测 Ctrl+A+x 退出序列 | |
| match key_state { | |
| KeySequenceState::Normal => { | |
| if key.code == KeyCode::Char('a') | |
| && key.modifiers.contains(KeyModifiers::CONTROL) | |
| { | |
| key_state = KeySequenceState::CtrlAPressed; | |
| } else { | |
| // 普通按键,发送到串口 | |
| if let Err(e) = Self::send_key_to_serial(&tx_port, key) { | |
| eprintln!("\r\n发送按键失败: {}", e); | |
| } | |
| } | |
| } | |
| KeySequenceState::CtrlAPressed => { | |
| if key.code == KeyCode::Char('x') { | |
| // 用户请求退出 | |
| eprintln!("\r\nExit by: Ctrl+A+x"); | |
| handle.stop(); | |
| break; | |
| } else { | |
| // 不是x键,发送上一个按键并重置状态 | |
| if key.code != KeyCode::Char('a') { | |
| if let Err(e) = Self::send_ctrl_a_to_serial(&tx_port) { | |
| eprintln!("\r\n发送 Ctrl+A 失败: {}", e); | |
| } | |
| if let Err(e) = Self::send_key_to_serial(&tx_port, key) { | |
| eprintln!("\r\n发送按键失败: {}", e); | |
| } | |
| key_state = KeySequenceState::Normal; | |
| } | |
| } | |
| } | |
| } | |
| } | |
| Some(Err(e)) => { | |
| eprintln!("\r\n键盘事件错误: {}", e); | |
| break; | |
| } | |
| None => { | |
| // EventStream 结束 | |
| break; | |
| } | |
| Some(Ok(_)) => { | |
| // 忽略非按键事件(鼠标、调整大小等) | |
| } | |
| } | |
| } |
| // 等待接收线程结束 | ||
| let _ = rx_handle.join(); | ||
| let _ = rx_handle.await?; | ||
| let _ = tx_handle.await; |
Copilot
AI
Jan 22, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's a subtle issue with task ordering. If rx_handle completes first and calls tx_abort.abort() on line 156, then tx_handle.await on line 99 will return a JoinError. The ? operator on line 98 will propagate errors from rx_handle, but the ignored result on line 99 means we won't know if the tx task panicked versus being aborted normally. Consider using match tx_handle.await to distinguish between Ok(_), Err(e) if e.is_cancelled(), and Err(e) if e.is_panic(), so panics in the tx task aren't silently hidden.
| let _ = tx_handle.await; | |
| match tx_handle.await { | |
| Ok(_) => {} | |
| Err(e) if e.is_cancelled() => { | |
| // tx task was aborted (expected when rx side finishes first) | |
| info!("Serial terminal tx task aborted"); | |
| } | |
| Err(e) if e.is_panic() => { | |
| // tx task panicked; log so this is not silently hidden | |
| error!("Serial terminal tx task panicked: {}", e); | |
| } | |
| Err(e) => { | |
| // other unexpected join error | |
| error!("Serial terminal tx task failed: {}", e); | |
| } | |
| } |
| eprintln!("\r\n发送按键失败: {}", e); | ||
| } | ||
| } | ||
| } | ||
| KeySequenceState::CtrlAPressed => { | ||
| if key.code == KeyCode::Char('x') { | ||
| // 用户请求退出 | ||
| eprintln!("\r\nExit by: Ctrl+A+x"); | ||
| handle.stop(); | ||
| break; | ||
| } else { | ||
| // 不是x键,发送上一个按键并重置状态 | ||
| if key.code != KeyCode::Char('a') { | ||
| if let Err(e) = Self::send_ctrl_a_to_serial(&tx_port) { | ||
| eprintln!("\r\n发送 Ctrl+A 失败: {}", e); | ||
| } | ||
| if let Err(e) = Self::send_key_to_serial(&tx_port, key) { | ||
| eprintln!("\r\n发送按键失败: {}", e); | ||
| } | ||
| key_state = KeySequenceState::Normal; | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
| Some(Err(e)) => { | ||
| eprintln!("\r\n键盘事件错误: {}", e); |
Copilot
AI
Jan 22, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Error messages are inconsistent in language. Lines 476, 490, 493 use Chinese (发送按键失败, 发送 Ctrl+A 失败), while line 483 uses English (Exit by: Ctrl+A+x), and line 502 uses Chinese (键盘事件错误). Consider using a consistent language throughout the codebase for error messages to maintain code quality. The existing codebase at line 151 uses Chinese (串口读取错误), and line 72 uses Chinese (已退出串口终端模式), suggesting Chinese is the standard.
| is_running: AtomicBool::new(true), | ||
| }); | ||
|
|
||
| // 使用 EventStream 异步处理键盘事件 |
Copilot
AI
Jan 22, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The Chinese comment "使用 EventStream 异步处理键盘事件" (Using EventStream to handle keyboard events asynchronously) on line 88 is duplicated on line 458. Consider removing one of these comments to avoid redundancy, or make them more specific to their respective contexts (e.g., line 88 could mention "spawn the async keyboard task" while line 458 could mention "create the event stream reader").
| // 使用 EventStream 异步处理键盘事件 | |
| // 使用 EventStream 异步处理键盘事件:启动异步键盘任务 |