[utest][driver][serial_v2] 优化flush判断逻辑 #10841
Conversation
📌 Code Review Assignment🏷️ Tag: componentsReviewers: Maihuanyi Changed Files (Click to expand)
📊 Current Review Status (Last Updated: 2025-10-27 18:40 CST)
📝 Review Instructions
|
|
👋 感谢您对 RT-Thread 的贡献！Thank you for your contribution to RT-Thread! 为确保代码符合 RT-Thread 的编码规范，请在你的仓库中执行以下步骤运行代码格式化工作流（如果格式化CI运行失败）。 🛠 操作步骤 | Steps
完成后，提交将自动更新至 如有问题欢迎联系我们，再次感谢您的贡献！💐 |
|
|
There was a problem hiding this comment.
Pull Request Overview
This PR improves the test logic for serial_v2 driver unit tests, specifically targeting flush operation timing validation and configuration management. The main focus is on making the timing checks more robust and consolidating TX DMA configuration.
Key changes:
- Introduces a configurable lower bound for flush timing validation to handle timing variations
- Replaces BSP-specific DMA macro with a generic Kconfig option for better portability
- Fixes code style issues with while loop formatting
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| components/drivers/serial/utest/Kconfig | Adds new Kconfig option for TX DMA configuration |
| components/drivers/serial/utest/v2/uart_timeout_txb.c | Updates DMA macro reference and fixes while loop formatting |
| components/drivers/serial/utest/v2/uart_flush_txnb.c | Improves flush timing validation logic and fixes while loop formatting |
| components/drivers/serial/utest/v2/uart_flush_txb.c | Improves flush timing validation logic and fixes while loop formatting |
| tick_diff = rt_tick_get() - old_tick; | ||
|
|
||
| if (tick_diff < expect_time || tick_diff > (expect_time + 10)) | ||
| rt_uint32_t lower_bound = (expect_time > 2) ? (expect_time - 2) : 0; |
There was a problem hiding this comment.
Magic number alert / 魔术数字警告: The constant value 2 used in the lower bound calculation is a magic number that lacks explanation. Consider defining it as a named constant (e.g., TIMING_TOLERANCE_MARGIN) to improve code readability and maintainability.
English: The hardcoded value 2 should be extracted to a constant with a meaningful name explaining why this specific tolerance is needed for the lower bound calculation.
中文: 硬编码的值 2 应该提取为一个有意义名称的常量,解释为什么下限计算需要这个特定的容差值。
Example/示例:
#define TIMING_TOLERANCE_MARGIN 2
rt_uint32_t lower_bound = (expect_time > TIMING_TOLERANCE_MARGIN) ? (expect_time - TIMING_TOLERANCE_MARGIN) : 0;| tick_diff = rt_tick_get() - old_tick; | ||
|
|
||
| if (tick_diff < expect_time || tick_diff > (expect_time + 10)) | ||
| rt_uint32_t lower_bound = (expect_time > 2) ? (expect_time - 2) : 0; |
There was a problem hiding this comment.
Magic number alert / 魔术数字警告: The constant value 2 used in the lower bound calculation is a magic number that lacks explanation. Consider defining it as a named constant (e.g., TIMING_TOLERANCE_MARGIN) to improve code readability and maintainability.
English: The hardcoded value 2 should be extracted to a constant with a meaningful name explaining why this specific tolerance is needed for the lower bound calculation.
中文: 硬编码的值 2 应该提取为一个有意义名称的常量,解释为什么下限计算需要这个特定的容差值。
Example/示例:
#define TIMING_TOLERANCE_MARGIN 2
rt_uint32_t lower_bound = (expect_time > TIMING_TOLERANCE_MARGIN) ? (expect_time - TIMING_TOLERANCE_MARGIN) : 0;|
|
||
| if (tick_diff < expect_time || tick_diff > (expect_time + 10)) | ||
| rt_uint32_t lower_bound = (expect_time > 2) ? (expect_time - 2) : 0; | ||
| if (tick_diff < lower_bound || tick_diff > (expect_time + 10)) |
There was a problem hiding this comment.
Magic number inconsistency / 魔术数字不一致: The upper bound uses a hardcoded value 10 for tolerance, which is inconsistent with the lower bound tolerance of 2. Both tolerance values should be defined as named constants for consistency and maintainability.
English: The upper bound tolerance (10) and lower bound tolerance (2) should both be defined as named constants to ensure consistency and make the timing validation logic more maintainable.
中文: 上限容差(10)和下限容差(2)都应该定义为命名常量,以确保一致性并使时间验证逻辑更易于维护。
Example/示例:
#define TIMING_LOWER_TOLERANCE 2
#define TIMING_UPPER_TOLERANCE 10
rt_uint32_t lower_bound = (expect_time > TIMING_LOWER_TOLERANCE) ? (expect_time - TIMING_LOWER_TOLERANCE) : 0;
if (tick_diff < lower_bound || tick_diff > (expect_time + TIMING_UPPER_TOLERANCE))|
|
||
| if (tick_diff < expect_time || tick_diff > (expect_time + 10)) | ||
| rt_uint32_t lower_bound = (expect_time > 2) ? (expect_time - 2) : 0; | ||
| if (tick_diff < lower_bound || tick_diff > (expect_time + 10)) |
There was a problem hiding this comment.
Magic number inconsistency / 魔术数字不一致: The upper bound uses a hardcoded value 10 for tolerance, which is inconsistent with the lower bound tolerance of 2. Both tolerance values should be defined as named constants for consistency and maintainability.
English: The upper bound tolerance (10) and lower bound tolerance (2) should both be defined as named constants to ensure consistency and make the timing validation logic more maintainable.
中文: 上限容差(10)和下限容差(2)都应该定义为命名常量,以确保一致性并使时间验证逻辑更易于维护。
Example/示例:
#define TIMING_LOWER_TOLERANCE 2
#define TIMING_UPPER_TOLERANCE 10
rt_uint32_t lower_bound = (expect_time > TIMING_LOWER_TOLERANCE) ? (expect_time - TIMING_LOWER_TOLERANCE) : 0;
if (tick_diff < lower_bound || tick_diff > (expect_time + TIMING_UPPER_TOLERANCE))
There was a problem hiding this comment.
@Ryan-CW-Code AI的这个提示好像还可以，可以给两个宏说明下
There was a problem hiding this comment.
感觉意义不大，是计算发送 n 个字节大约需要多少毫秒，然后在这个值的基础上再加一些冗余。
因为上面计算的时候默认向下取整的，如果过于严格，很容易导致测试不通过，这个测试主要测试flush有没有生效。
您再评估一下，依然感觉有必要的话我就加上。
3 participants
拉取/合并请求描述：(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