[component][drivers][serial_v2] 优化serial_v2 #10603

Ryan-CW-Code · 2025-08-12T06:46:21Z

拉取/合并请求描述：(PR description)

[

为什么提交这份PR (why to submit this PR)

你的解决方案是什么 (what is your solution)

请提供验证的bsp和config (provide the config and bsp)

DMA模式写满丢弃和写满覆盖和中断方式都跑过utest

]

当前拉取/合并请求的状态 Intent for your PR

必须选择一项 Choose one (Mandatory):

本拉取/合并请求是一个草稿版本 This PR is for a code-review and is intended to get feedback
本拉取/合并请求是一个成熟版本 This PR is mature, and ready to be integrated into the repo

代码质量 Code Quality：

我在这个拉取/合并请求中已经考虑了 As part of this pull request, I've considered the following:

已经仔细查看过代码改动的对比 Already check the difference between PR and old code
代码风格正确，包括缩进空格，命名及其他风格 Style guide is adhered to, including spacing, naming and other styles
没有垃圾代码，代码尽量精简，不包含#if 0代码，不包含已经被注释了的代码 All redundant code is removed and cleaned up
所有变更均有原因及合理的，并且不会影响到其他软件组件代码或BSP All modifications are justified and not affect other components or BSP
对难懂代码均提供对应的注释 I've commented appropriately where code is tricky
代码是高质量的 Code in this PR is of high quality
已经使用formatting 等源码格式化工具确保格式符合RT-Thread代码规范 This PR complies with RT-Thread code specification
如果是新增bsp, 已经添加ci检查到.github/workflows/bsp_buildings.yml 详细请参考链接BSP自查

…部的时间绕回判断，写满丢弃策略rx接收数据错乱修复，clang-tidy和cppcheck审查优化

github-actions · 2025-08-12T06:46:34Z

📌 Code Review Assignment

🏷️ Tag: bsp_stm32

Reviewers: Liang1795 hamburger-os wdfk-prog

Changed Files (Click to expand)

bsp/stm32/libraries/HAL_Drivers/drivers/drv_usart_v2.c

🏷️ Tag: components

Reviewers: Maihuanyi

Changed Files (Click to expand)

components/drivers/include/drivers/dev_serial_v2.h
components/drivers/serial/dev_serial_v2.c

🏷️ Tag: components_driver_serial_v2

Reviewers: Ryan-CW-Code

Changed Files (Click to expand)

components/drivers/serial/dev_serial_v2.c

📊 Current Review Status (Last Updated: 2025-08-14 11:23 CST)

⌛ Liang1795 Pending Review
⌛ Maihuanyi Pending Review
⌛ Ryan-CW-Code Pending Review
⌛ hamburger-os Pending Review
⌛ wdfk-prog Pending Review

📝 Review Instructions

维护者可以通过单击此处来刷新审查状态: 🔄 刷新状态
Maintainers can refresh the review status by clicking here: 🔄 Refresh Status
确认审核通过后评论 LGTM/lgtm
Comment LGTM/lgtm after confirming approval
PR合并前需至少一位维护者确认
PR must be confirmed by at least one maintainer before merging

ℹ️ 刷新CI状态操作需要具备仓库写入权限。
ℹ️ Refresh CI status operation requires repository Write permission.

wdfk-prog · 2025-08-12T13:36:09Z

rt-thread/components/drivers/serial/dev_serial_v2.c

Lines 390 to 391 in cb5bacf

    
           getc_element = serial->ops->getc(serial); 
        
           if (getc_element == -1) break;

这里不止-1要break吧?负数都得break;rt_ssize_t _serial_poll_rx函数返回值可以负值,可以将错误码也返回

rt-thread/components/drivers/serial/dev_serial_v2.c

Line 443 in cb5bacf

serial->ops->putc(serial, *putc_buffer);

这里应该也一样,可以判断返回值

rt-thread/components/drivers/serial/dev_serial_v2.c

Lines 579 to 582 in cb5bacf

    
           if (rt_atomic_flag_test_and_set(&tx_fifo->activated)) 
        
           { 
        
               return 0; 
        
           }

这里可以返回负值错误码

rt-thread/components/drivers/serial/dev_serial_v2.c

Lines 648 to 651 in cb5bacf

    
           if (rt_atomic_flag_test_and_set(&tx_fifo->activated)) 
        
           { 
        
               return 0; 
        
           }

这里可以返回负值错误码

rt-thread/components/drivers/serial/dev_serial_v2.c

Lines 602 to 607 in cb5bacf

    
           if (ret == -RT_ETIMEOUT) 
        
           { 
        
               return ret; 
        
           } 
        
           return 0;

这里直接返回ret既可吧

rt-thread/components/drivers/serial/dev_serial_v2.c

Line 851 in cb5bacf

serial->ops->control(serial,

serial->ops->control的调用处都可以有返回值判断

rt-thread/components/drivers/serial/dev_serial_v2.c

Lines 1193 to 1195 in cb5bacf

    
           while (serial->ops->getc(serial) != -1) 
        
           { 
        
           }

这里没有超时退出机制,可以考虑加一个

rt-thread/components/drivers/serial/dev_serial_v2.c

Lines 1213 to 1229 in cb5bacf

    
           static void _serial_tx_flush(struct rt_serial_device *serial) 
        
           { 
        
               struct rt_serial_tx_fifo *tx_fifo; 
        
               RT_ASSERT(serial != RT_NULL); 
        
               if (serial->config.tx_bufsz != 0) 
        
               { 
        
                   tx_fifo = (struct rt_serial_tx_fifo *)serial->serial_tx; 
        
                   RT_ASSERT(tx_fifo != RT_NULL); 
        
                   if (rt_atomic_load(&tx_fifo->activated)) 
        
                   { 
        
                       rt_completion_wait(&tx_fifo->tx_cpt, RT_WAITING_FOREVER); 
        
                       return; 
        
                   } 
        
               } 
        
           }

这个return是否没有必要

Ryan-CW-Code · 2025-08-12T13:47:35Z

返回值这里我还是按规范统一返回接收或者读取的数据大小吧，不再返回-RT_ETIMEOUT，按照文档来实现更合理点。
小细节我再优化下

…字节数，细节优化

Ryan-CW-Code · 2025-08-13T01:54:51Z

中断下19个测试例程，dma下18个测试例程，都跑过了

Copilot

Pull Request Overview

This PR optimizes the serial_v2 driver component by adding enhanced timeout handling, POSIX mode support, and improving error handling mechanisms. The changes focus on making the serial driver more robust and compliant with POSIX standards.

Enhanced timeout handling for both RX and TX operations with proper timeout calculation
Added POSIX mode support for better compatibility with standard I/O operations
Improved error handling and return value validation across the driver

Reviewed Changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 8 comments.

File	Description
components/drivers/serial/dev_serial_v2.c	Core driver optimizations including timeout handling, POSIX mode support, and error handling improvements
components/drivers/include/drivers/dev_serial_v2.h	Added new control command and POSIX mode flag to device structure
bsp/stm32/libraries/HAL_Drivers/drivers/drv_usart_v2.c	STM32-specific driver improvements with better interrupt handling and error checking
examples/utest/testcases/drivers/serial_v2/*.c	Updated test cases with improved validation and reduced delay times for faster testing

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

Copilot · 2025-08-13T02:00:13Z

components/drivers/serial/dev_serial_v2.c

+    rt_err_t                 ret   = 0;
+    rt_uint16_t              flags = 0;
+    rt_device_t              device;
+    struct rt_serial_device *serial;


[nitpick] Variable 'serial' is declared but only used in a conditional block much later. Consider declaring it closer to its usage or within the conditional block to improve code readability.

Suggested change

struct rt_serial_device *serial;

Copilot · 2025-08-13T02:00:14Z

components/drivers/serial/dev_serial_v2.c

    {
        getc_element = serial->ops->getc(serial);
-        if (getc_element == -1) break;
+        if (getc_element < 0) break;


Changed condition from '== -1' to '< 0', but this may break compatibility with drivers that return specific negative error codes other than -1. The original check for -1 was likely intentional as it's a common convention for 'no data available'.

Suggested change

if (getc_element < 0) break;

if (getc_element == -1) break;

实际串口的数据是uint8_t类型，getc和putc的返回值是int类型，所以这里判断是否为负数是可行的

Copilot · 2025-08-13T02:00:14Z

components/drivers/serial/dev_serial_v2.c

        }
-        serial->ops->putc(serial, *putc_buffer);
+        putc_result = serial->ops->putc(serial, *putc_buffer);
+        if (putc_result < 0) break;


The putc operation break on error may leave the transmission incomplete. Consider logging the error or implementing proper error handling to inform the caller about partial transmission.

Suggested change

if (putc_result < 0) break;

if (putc_result < 0)

{

LOG_E("Serial putc failed at buffer offset %d, error code: %d", (int)(putc_buffer - (rt_uint8_t *)buffer), putc_result);

break;

}

Copilot · 2025-08-13T02:00:14Z

components/drivers/serial/dev_serial_v2.c

    struct rt_serial_device  *serial;
    struct rt_serial_tx_fifo *tx_fifo;
-    rt_uint8_t               *put_ptr;
+    rt_uint8_t               *put_ptr = RT_NULL;


[nitpick] Initializing 'put_ptr' to RT_NULL is unnecessary since it's assigned immediately after declaration in the function logic.

Suggested change

rt_uint8_t *put_ptr = RT_NULL;

rt_uint8_t *put_ptr;

这里是为了抑制clang-tidy的[clang-analyzer-core.CallAndMessage]的警告

Copilot · 2025-08-13T02:00:15Z

bsp/stm32/libraries/HAL_Drivers/drivers/drv_usart_v2.c

-        char chr = UART_GET_RDR(&uart->handle, stm32_uart_get_mask(uart->handle.Init.WordLength, uart->handle.Init.Parity));
-        rt_hw_serial_control_isr(serial, RT_HW_SERIAL_CTRL_PUTC, &chr);
-        rt_hw_serial_isr(serial, RT_SERIAL_EVENT_RX_IND);
+        do {


[nitpick] The do-while loop structure for reading RXNE flag should have proper spacing. Consider adding a space after 'do' for consistency with coding standards.

这里是误判吗？检查了代码格式是带空格的，使用clang-format格式化

Copilot · 2025-08-13T02:00:15Z

bsp/stm32/libraries/HAL_Drivers/drivers/drv_usart_v2.c

    {
-        HAL_UART_Transmit_DMA(&uart->handle, buf, size);
+        if (HAL_UART_Transmit_DMA(&uart->handle, buf, size) != HAL_OK)
+            return 0;


When HAL_UART_Transmit_DMA fails, returning 0 may not properly indicate the error condition to the caller. Consider returning a negative error code or logging the failure reason.

Suggested change

return 0;

{

LOG_E("HAL_UART_Transmit_DMA failed in stm32_transmit");

return -1;

}

stm32_transmit的返回值是传输成功的字节数，不需要负数进行判断

stm32_transmit的返回值是传输成功的字节数，不需要负数进行判断

rt_ssize_t类型支持返回负数.正数就是返回长度,负数就是错误码.rt_ssize_t的定义就是这么用的.最好可以都将rt_ssize_t的负值利用起来^_^

examples/utest/testcases/drivers/serial_v2/uart_flush_txnb.c

Copilot · 2025-08-13T02:00:16Z

examples/utest/testcases/drivers/serial_v2/uart_flush_txb.c

-    rt_int32_t  tx_timeout = 1;
+    rt_int32_t  tx_timeout = 10 * 1000;
    uart_write_buffer      = (rt_uint8_t *)rt_malloc(sizeof(rt_uint8_t) * (RT_SERIAL_TC_TXBUF_SIZE * 5 + 10));
+    for (rt_uint32_t count = 0; count < sizeof(rt_uint8_t) * (RT_SERIAL_TC_TXBUF_SIZE * 5 + 10); count++)


Similar to uart_flush_txnb.c, this loop uses 'RT_SERIAL_TC_TXBUF_SIZE' but the buffer allocation uses 'RT_SERIAL_TC_TXBUF_SIZE * 5 + 10'. The sizeof(rt_uint8_t) multiplication is redundant since rt_uint8_t is always 1 byte.

Suggested change

for (rt_uint32_t count = 0; count < sizeof(rt_uint8_t) * (RT_SERIAL_TC_TXBUF_SIZE * 5 + 10); count++)

uart_write_buffer = (rt_uint8_t *)rt_malloc(RT_SERIAL_TC_TXBUF_SIZE * 5 + 10);

for (rt_uint32_t count = 0; count < (RT_SERIAL_TC_TXBUF_SIZE * 5 + 10); count++)

wdfk-prog

LGTM

wdfk-prog · 2025-08-14T00:34:20Z

bsp/stm32/libraries/HAL_Drivers/drivers/drv_usart_v2.c

    {
-        HAL_UART_Transmit_DMA(&uart->handle, buf, size);
+        if (HAL_UART_Transmit_DMA(&uart->handle, buf, size) != HAL_OK)
+            return 0;


stm32_transmit的返回值是传输成功的字节数，不需要负数进行判断

rt_ssize_t类型支持返回负数.正数就是返回长度,负数就是错误码.rt_ssize_t的定义就是这么用的.最好可以都将rt_ssize_t的负值利用起来^_^

wdfk-prog · 2025-08-14T00:36:11Z

bsp/stm32/libraries/HAL_Drivers/drivers/drv_usart_v2.c

+            char chr = UART_GET_RDR(&uart->handle, stm32_uart_get_mask(uart->handle.Init.WordLength, uart->handle.Init.Parity));
+            rt_hw_serial_control_isr(serial, RT_HW_SERIAL_CTRL_PUTC, (void *)&chr);
+            rt_hw_serial_isr(serial, RT_SERIAL_EVENT_RX_IND);
+        } while (__HAL_UART_GET_FLAG(&uart->handle, UART_FLAG_RXNE));


这种while循环没有超时退出机制的总是有点担心.

虽然也没啥就是了,锦上添花罢了

认知中这里应该不会有问题，UART_GET_RDR会清除UART_FLAG_RXNE标志。

这里还是增加一个超时吧，最多处理1024字节就强制退出

wdfk-prog · 2025-08-14T00:38:42Z

components/drivers/serial/dev_serial_v2.c

+    rt_memset(&serial->rx_notify, RT_NULL, sizeof(struct rt_device_notify));
+
+    /* Call the control() API to close the serial device. disable ignore return value */
+    serial->ops->control(serial, RT_DEVICE_CTRL_CLOSE, RT_NULL);


这个control没有做返回值判断

这里是关闭设备，就算失败了也需要继续执行清理操作。

获取control失败后没有什么好的处理措施

Ryan-CW-Code · 2025-08-14T02:07:00Z

跟改完后，dma和中断的utest都跑过

…shell

wdfk-prog · 2025-08-15T05:19:45Z

components/drivers/serial/dev_serial_v2.c

                                      size,
                                      RT_SERIAL_TX_BLOCKING);
-    if (send_size == 0)
+    if (send_size <= 0)


这里应该返回send_size既可?函数返回值类型也是rt_ssize_t的

rt_device_read和write函数返回实际读或者写的字节数。不返回错误值吧

wdfk-prog · 2025-08-15T07:15:08Z

components/drivers/serial/dev_serial_v2.c

+                                              RT_SERIAL_TX_BLOCKING);
+        if (transmit_size <= 0)
+        {
+            return send_size;


这里应该返回transmit_size既可?函数返回值类型也是rt_ssize_t的

这里还必须返回send_size因为是循环调用的嘛

Ryan-CW-Code added 3 commits August 12, 2025 14:19

[bsp][stm32][drv_usart_v2] uart_isr不再使用链式调用，RXNE内循环读空，一些细节优化

3826031

[componnents][drivers][serial_v2] 增加获取config接口，使用rt_tick_get_delta替代内…

bf558ac

…部的时间绕回判断，写满丢弃策略rx接收数据错乱修复，clang-tidy和cppcheck审查优化

[utest][drivers][serial_v2] 测试例程进行优化

e1ce867

Ryan-CW-Code requested a review from Rbb666 as a code owner August 12, 2025 06:46

github-actions bot added BSP: STM32 BSP related with ST/STM32 BSP testcase Component component: drivers/serial component: drivers labels Aug 12, 2025

[bsp][stm32][drv_usart_v2] 撤回DMAMUX1修改

cb5bacf

Ryan-CW-Code added 2 commits August 13, 2025 09:53

[componnents][drivers][serial_v2] read和write不再返回-RT_ETIMEOUT改为已发送或读取的…

3f50c4f

…字节数，细节优化

[utest][drivers][serial_v2] 增加单独的timeout测试，减少测试中的硬延时

1055b11

[utest][drivers][serial_v2] 漏提交的posix下标志位

5217a83

Rbb666 requested a review from Copilot August 13, 2025 01:58

Copilot AI reviewed Aug 13, 2025

View reviewed changes

[utest][drivers][serial_v2] 优化测试例程

7be8b69

wdfk-prog approved these changes Aug 14, 2025

View reviewed changes

Ryan-CW-Code added 2 commits August 14, 2025 10:03

[bsp][stm32][drv_usart_v2] transmit返回错误值，RXNE增加超时逻辑

b95a88a

[componnents][drivers][serial_v2] 增加transmit错误处理，增加超时退出机制显式的避免死循环

d7d0f5f

[componnents][drivers][serial_v2] 适配serial_v2和posix非libc标准库下的console和…

fa15a7a

…shell

Ryan-CW-Code mentioned this pull request Aug 14, 2025

[componnents][finsh] 适配serial_v2提高效率，减少内存占用 #10607

Closed

10 tasks

wdfk-prog reviewed Aug 15, 2025

View reviewed changes

Rbb666 approved these changes Aug 16, 2025

View reviewed changes

Rbb666 merged commit e0243e8 into RT-Thread:master Aug 16, 2025
63 checks passed

Ryan-CW-Code deleted the serial_v2 branch August 20, 2025 02:39

Rbb666 added this to the v5.2.2 milestone Sep 23, 2025

-            return 0;
+        {
+            LOG_E("HAL_UART_Transmit_DMA failed in stm32_transmit");
+            return -1;
+        }

	for (rt_uint32_t count = 0; count < sizeof(rt_uint8_t) * (RT_SERIAL_TC_TXBUF_SIZE * 5 + 10); count++)
	uart_write_buffer = (rt_uint8_t )rt_malloc(RT_SERIAL_TC_TXBUF_SIZE 5 + 10);
	for (rt_uint32_t count = 0; count < (RT_SERIAL_TC_TXBUF_SIZE * 5 + 10); count++)

[component][drivers][serial_v2] 优化serial_v2 #10603

[component][drivers][serial_v2] 优化serial_v2 #10603

Uh oh!

Conversation

Ryan-CW-Code commented Aug 12, 2025

拉取/合并请求描述：(PR description)

为什么提交这份PR (why to submit this PR)

你的解决方案是什么 (what is your solution)

请提供验证的bsp和config (provide the config and bsp)

当前拉取/合并请求的状态 Intent for your PR

代码质量 Code Quality：

Uh oh!

github-actions bot commented Aug 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

📌 Code Review Assignment

🏷️ Tag: bsp_stm32

🏷️ Tag: components

🏷️ Tag: components_driver_serial_v2

📊 Current Review Status (Last Updated: 2025-08-14 11:23 CST)

📝 Review Instructions

Uh oh!

wdfk-prog commented Aug 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Ryan-CW-Code commented Aug 12, 2025

Uh oh!

Ryan-CW-Code commented Aug 13, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Aug 13, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Aug 13, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Aug 13, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Aug 13, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Aug 13, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Aug 13, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI Aug 13, 2025

Choose a reason for hiding this comment

Uh oh!

wdfk-prog left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Aug 12, 2025 •

edited

Loading

wdfk-prog commented Aug 12, 2025 •

edited

Loading