Repository navigation
Conversation
RT_SERIAL_EVENT_RX_TIMEOUT has been defined in dev_serial.h, but
rt_hw_serial_isr() never had a case for it and the switch has no default
branch, so a driver emitting the event was silently ignored.
Two in-tree drivers rely on it to delimit a frame by the hardware idle-line
timeout:
bsp/wch/arm/ch579m/board/drv_uart.c UART_II_RECV_TOUT
bsp/synwit/libraries/SWM341_drivers/drv_uart.c UART_IT_RX_TOUT
Both BSPs build against the v1 framework, so the "frame finished"
notification never reached the application.
Handle the timeout like a receive indication: drain the hardware FIFO and
report the bytes buffered in the software FIFO, which is what a driver using
the timeout as a frame delimiter expects.
The v2 framework is deliberately untouched. dev_serial_v2.h does not define
the event at all (only 0x01..0x04) and its interrupt handler drops unknown
events through "default:", so a v2 driver cannot emit it in the first place.
Refs RT-Thread#8143
|
👋 感谢您对 RT-Thread 的贡献!Thank you for your contribution to RT-Thread! 为确保代码符合 RT-Thread 的编码规范,请在你的仓库中执行以下步骤运行代码格式化工作流(如果格式化CI运行失败)。 🛠 操作步骤 | Steps
完成后,提交将自动更新至 如有问题欢迎联系我们,再次感谢您的贡献!💐 |
📌 Code Review Assignment🏷️ Tag: componentsReviewers: @Maihuanyi Changed Files (Click to expand)
📊 Current Review Status (Last Updated: 2026-10-21 13:55 CST)
📝 Review Instructions
|
|
@Rbb666 同样需要批准 workflow(首次从 fork 贡献): 本改动只有 V1 的一行 case。已用 若认可"超时事件等价于一次 RX_IND"这个语义,我可以补一个不依赖硬件的 utest(伪 serial 设备直接调 |
Cover the event handled by the previous commit with a unit test that does not need any real hardware. The test registers a fake serial device and opens it with RT_DEVICE_FLAG_INT_RX, so the v1 framework allocates the software RX FIFO itself. The fake getc() serves bytes from a scripted buffer, standing in for the hardware FIFO, and rx_indicate records the reported length. Three scenarios: 1. RX_IND with 4 bytes -> one callback, length 4 2. RX_TIMEOUT with 5 more bytes -> one callback, length 9 (4 + 5 buffered) 3. RX_TIMEOUT with an empty FIFO -> no callback, nothing to report Scenario 3 documents the boundary decision: a timeout with no buffered data stays silent, matching the RX_IND path which only calls rx_indicate when the buffered length is non-zero. Verified on bsp/qemu-vexpress-a9 (RT_USING_SERIAL_V1): master 1c801ef : scenario 2 fails, callbacks=0 (event dropped) this branch : all three scenarios pass The v1 test group is new; components/drivers/serial/utest only had bypass/ and v2/ before. It is registered through a new RT_UTEST_SERIAL_V1 Kconfig option that depends on RT_USING_SERIAL_V1. Refs RT-Thread#8143 Refs RT-Thread#11909
|
已补上不依赖硬件的 utest(commit 用例设计不接真实串口、不需要输入激励:注册一个伪 serial 设备并用
三个场景与边界结论
边界结论:缓冲区为空时,超时事件不回调。因为实现是并入 验证环境
这个用例不依赖任何硬件,理论上可以直接进 PR 正文里"仅编译验证、未做行为验证"的说法已经过时,我同步更新。 |
|
V2版本已经推出很长时间了,长时间并行维护V1和V2版本是个坑,个人感觉应该冻结V1版本仅作为历史兼容,串口重心往V2靠,否则V2的意义是什么呢 |
|
@Ryan-CW-Code 这个问题确实决定了我这个 PR 该怎么落地,先补两个可能影响判断的事实(都在当前 master 上核对过): 1. 这个事件宏只存在于 V1 的头文件。
V2 下驱动引用它会直接编译失败,所以不存在"V2 里被静默丢弃"这回事。V2 处理接收超时用的是另一套机制: 2. 树内发出该事件的两个驱动,都不是 V2 的。
所以这个 PR 的性质不是"给 V1 加新功能",而是"让 V1 里已经声明的 API 名副其实"——宏在那儿,两个驱动的调用也在那儿,行为却是空操作。 不过你的顾虑我认同:即便只是补一个 case,也是给 V1 增加了一份要长期维护的行为。 如果按"冻结 V1、只作历史兼容"的方向,反向的方案其实更自洽:把这个从未实现过的宏从 V1 头文件里删掉,同时改掉 ch579m / swm341-mini 那两处调用(或者标注为需要迁移到 V2)。这样 V1 会更薄,也不必新增我那 210 行的 utest。我个人倾向这个方向更贴合你说的路线——前提是这两个 BSP 是否还有人在维护,这一点我判断不了。 顺带一个 V2 侧的同类隐患,供一起讨论:V2 的 最终怎么落地请维护者定:保留当前实现(V1 补 case + utest),还是改成"删宏 + 改驱动",两种我都能执行,给结论我就动。 |
保留V1是因为BSP中还有很多BSP是依赖并使用V1的,因为有些BSP只是为了把串口当成MSH功能即可(可以满足最基础的需求了),并没有很深度的使用串口,所以是有历史遗留的。 我认为后续的BSP可以主动引导贡献者去优先适配V2版本的,像瑞萨这些BSP已经适配了V2 |
拉取/合并请求描述:(PR description)
[
为什么提交这份PR (why to submit this PR)
关联 issue #8143。
RT_SERIAL_EVENT_RX_TIMEOUT(0x05)在components/drivers/include/drivers/dev_serial.h里已经定义,但rt_hw_serial_isr()的 switch 只处理 0x01~0x04,且没有 default 分支。所以驱动发出这个事件时,框架不做任何事——事件被静默丢弃。仓库里正好有两个驱动依赖它,都是拿芯片的接收空闲超时中断来切分数据帧:
这两个 BSP(
bsp/wch/arm/ch579m、bsp/synwit/swm341-mini)都用 V1 框架,结果是"一帧收完"的通知永远到不了应用层,帧边界实际丢失,只能退化成按 FIFO 水位由RX_IND上报。你的解决方案是什么 (what is your solution)
把超时事件并入
RX_IND分支处理:排空硬件 FIFO,然后按软件 FIFO 中已缓存的长度回调rx_indicate。这正是"用超时作为帧结束标志"的驱动所期望的语义,也与我之前在 issue 里建议的方案 A 一致。只改了
components/drivers/serial/dev_serial.c一行 case(+5 行含注释),没有新增字段、不改 ABI。关于 V2
V2 框架未做改动,原因是我在编译验证时确认:
components/drivers/include/drivers/dev_serial_v2.h只定义了 0x01~0x04,没有RT_SERIAL_EVENT_RX_TIMEOUT'RT_SERIAL_EVENT_RX_TIMEOUT' undeclared,已撤回)也就是说这个坑纯粹存在于 V1。V2 若将来需要该语义,应当走它自己的读侧超时机制(
RT_SERIAL_CTRL_SET_RX_TIMEOUT),那是另一个话题。请提供验证的bsp和config (provide the config and bsp)
RT_USING_SERIAL_V1);另用 bsp/renesas/ra6m3-hmi-board(V2)做回归,确认未影响 V2 编译验证情况:
行为验证一开始做不了:这个事件由芯片的接收空闲超时中断触发,QEMU 的 pl011 没有该中断,我也没有 ch579m / SWM341 硬件。
后续改为用伪 serial 设备绕开硬件依赖,新增 utest
components.drivers.serial.v1.uart_rx_timeout:注册伪设备并用RT_DEVICE_FLAG_INT_RX打开,让框架自己分配软件 FIFO,伪getc()从脚本缓冲区模拟硬件 FIFO,直接调用rt_hw_serial_isr注入事件。同一个用例在 master 与本分支上的结果相反:
边界结论:缓冲区为空时超时事件不回调(实现并入
RX_IND分支,该分支仅在rx_length != 0时上报)。这个用例已经补上了(commit
dcadd964),不依赖硬件、可直接进 CI。如果维护者认为"超时事件等价于一次 RX_IND"这个语义方向不对(比如希望超时走独立回调、或空超时也应回调),请直接说明,我按结论调整或撤回。]
当前拉取/合并请求的状态 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