[componnents][finsh] 适配serial_v2提高效率，减少内存占用 #10607
Conversation
|
之前shell使用serial_v2也没问题，此PR仅优化效率和内存占用。 |
📌 Code Review Assignment🏷️ Tag: componentsReviewers: Maihuanyi Changed Files (Click to expand)
📊 Current Review Status (Last Updated: 2025-08-14 16:04 CST)
📝 Review Instructions
|
There was a problem hiding this comment.
Pull Request Overview
This PR adapts the finsh shell component to support serial_v2 to improve efficiency and reduce memory usage. The changes introduce conditional compilation to handle differences between the traditional serial interface and the newer serial_v2 interface.
Key changes:
- Conditional compilation for semaphore-based vs blocking I/O approaches
- Different device opening flags and control mechanisms for serial_v2
- Removal of interrupt-based RX indication in favor of blocking reads for serial_v2
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| components/finsh/shell.h | Conditionally excludes rx_sem semaphore when using serial_v2 |
| components/finsh/shell.c | Implements serial_v2 support with blocking I/O and timeout control |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| return -1; /* EOF */ | ||
| } | ||
|
|
||
| /* Temporarily retain the distinction */ |
There was a problem hiding this comment.
The comment should be in English for consistency with the codebase. Consider changing to 'Temporarily retain the distinction' or a more descriptive comment explaining why this distinction exists.
| /* Temporarily retain the distinction */ | |
| /* Retain distinction between serial device versions for compatibility; refactoring pending. */ |
| } | ||
| } | ||
| #else | ||
| while (rt_device_read(device, -1, &ch, 1) != 1) |
There was a problem hiding this comment.
The serial_v2 implementation uses a tight loop without any yield or delay mechanism. This could lead to high CPU usage when waiting for input. Consider adding a small delay or yield to prevent excessive CPU consumption.
| rt_device_set_rx_indicate(dev, finsh_rx_ind); | ||
| #else | ||
| rt_int32_t rx_timeout = RT_WAITING_FOREVER; | ||
| rt_device_control(dev, RT_SERIAL_CTRL_SET_RX_TIMEOUT, (void *)&rx_timeout); |
There was a problem hiding this comment.
[nitpick] The variable declaration inside the conditional block reduces code clarity. Consider declaring rx_timeout at the beginning of the function scope or using a more descriptive variable name to indicate its purpose.
| rt_device_control(dev, RT_SERIAL_CTRL_SET_RX_TIMEOUT, (void *)&rx_timeout); | |
| rt_device_control(dev, RT_SERIAL_CTRL_SET_RX_TIMEOUT, (void *)&serial_rx_timeout); |
Ryan-CW-Code
force-pushed
the
shell_serial_v2
branch
2 times, most recently
from
41be059 to
c5595bd
Compare
Ryan-CW-Code
force-pushed
the
shell_serial_v2
branch
from
c5595bd to
fe18afe
Compare
|
不清除要怎么调节头文件顺序才能编译通过了，在我几个工程里面都可以，这个pr先关闭吧，也没什么影响 |
1 participant
拉取/合并请求描述：(PR description)
[
为什么提交这份PR (why to submit this PR)
你的解决方案是什么 (what is your solution)
请提供验证的bsp和config (provide the config and bsp)
]
当前拉取/合并请求的状态 Intent for your PR
必须选择一项 Choose one (Mandatory):
代码质量 Code Quality：
我在这个拉取/合并请求中已经考虑了 As part of this pull request, I've considered the following:
#if 0代码，不包含已经被注释了的代码 All redundant code is removed and cleaned up