[drv]a new serial driver - serialX #5798

thewon86 · 2022-04-10T08:17:38Z

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

[
经过一段时间的使用，serialX 有非常优异的表现。
几个串口驱动版本的对比测试请移步论坛 https://club.rt-thread.org/ask/article/3676.html
]

以下的内容不应该在提交PR时的message修改，修改下述message，PR会被直接关闭。请在提交PR后，浏览器查看PR并对以下检查项逐项check，没问题后逐条在页面上打钩。
The following content must not be changed in the submitted PR message. Otherwise, the PR will be closed immediately. After submitted PR, please use a web browser to visit PR, and check items one by one, and ticked them if no problem.

当前拉取/合并请求的状态 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

mysterywolf · 2022-04-10T10:26:23Z

https://gitee.com/thewon/serialX

mysterywolf · 2022-04-10T10:32:14Z

components/drivers/Kconfig

+ depends on RT_USING_SERIAL_V1
+ default 64
+
+ config RT_SERIAL_FIFO_BUFSZ
+ int "Set SerialX FIFO buffer size"
+ depends on RT_USING_SERIAL_X
+ default 128
+
+ config RT_SERIAL_HARD_FIFO
+ int "Set Hard FIFO buffer size, useful only if the chip supported"
+ depends on RT_USING_SERIAL_X
+ default 64
+
+ config RT_SERIAL_DMA_BUFSZ
+ int "Set SerialX DMA buffer size"
+ depends on RT_USING_SERIAL_X && RT_SERIAL_USING_DMA


多个config depends on RT_USING_SERIAL_X就不要用depends on了
用
if RT_USING_SERIAL_X
config xxxx

endif
的句式，更清晰

Guozhanxin · 2022-04-11T09:24:59Z

大佬，有没有可能和V2版本融合起来啊，目前V2版本历史包袱还没那么重，应该比较容易调整的。如果再加一个版本的串口框架进来，如果我是用户，我都不知道该用哪个了😂

a1012112796 · 2022-04-11T10:08:44Z

src/device.c

+ {
+ rt_set_errno(-RT_ERROR);
+ return 0;
+ }


Suggested change

{

rt_set_errno(-RT_ERROR);

return 0;

}

{

return -RT_ERROR;

}

为何不直接返回异常呢?

保持和 read write 一致的代码逻辑

read write 的返回值是实际读写成功的字节数, 没有成功自然是应该为0了，这里返回的的是 err, 0 == RT_EOK? 感觉还是不太一样的。参照标准库的 fflush()

在设备未打开、或者底层未实现 flush 接口的情况下返回什么值？

a1012112796 · 2022-04-11T10:09:45Z

src/device.c

+ /* set error code */
+ rt_set_errno(-RT_ENOSYS);
+
+ return 0;


Suggested change

/* set error code */

rt_set_errno(-RT_ENOSYS);

return 0;

return -RT_ENOSYS;

thewon86 · 2022-04-14T05:30:26Z

大佬，有没有可能和V2版本融合起来啊，目前V2版本历史包袱还没那么重，应该比较容易调整的。如果再加一个版本的串口框架进来，如果我是用户，我都不知道该用哪个了😂

交给使用者去选择吧。usb 协议栈那么多，总不能只留一个其它都不支持了吧，modbus 库那么多也不能只让用一个吧。

aozima · 2022-04-14T06:30:16Z

选择太多的话，做成软件包比较合适。

mysterywolf · 2022-04-14T06:32:59Z

选择太多的话，做成软件包比较合适。

别做软件包了吧，这个是关键性核心功能，如果谁都不妥协的话，就先这样放着吧，先百家争鸣。经过一段时间的收敛，再最终汇总成一个新版本的串口框架。16年了，RT-Thread串口框架还是有问题，没有收敛下来，串口框架是个大问题呀。

aozima · 2022-04-14T06:36:19Z

选择太多的话，做成软件包比较合适。

别做软件包了吧，这个是关键性核心功能，如果谁都不妥协的话，就先这样放着吧，先百家争鸣。经过一段时间的收敛，再最终汇总成一个新版本的串口框架。16年了，RT-Thread串口框架还是有问题，没有收敛下来，串口框架是个大问题呀。

支持在内置的V2上继续改进，直到能解决所有已知问题。

armink · 2022-04-14T08:05:50Z

选择太多的话，做成软件包比较合适。

别做软件包了吧，这个是关键性核心功能，如果谁都不妥协的话，就先这样放着吧，先百家争鸣。经过一段时间的收敛，再最终汇总成一个新版本的串口框架。16年了，RT-Thread串口框架还是有问题，没有收敛下来，串口框架是个大问题呀。

作为 RT-Thread 上游软件更多要考虑长期的兼容性和可维护性，发散、灵活、自由的组件选择交给软件包。

另外，现在串口 V2 还有什么问题？

thewon86 · 2022-04-15T00:59:49Z

选择太多的话，做成软件包比较合适。

别做软件包了吧，这个是关键性核心功能，如果谁都不妥协的话，就先这样放着吧，先百家争鸣。经过一段时间的收敛，再最终汇总成一个新版本的串口框架。16年了，RT-Thread串口框架还是有问题，没有收敛下来，串口框架是个大问题呀。

作为 RT-Thread 上游软件更多要考虑长期的兼容性和可维护性，发散、灵活、自由的组件选择交给软件包。

另外，现在串口 V2 还有什么问题？

阻塞非阻塞概念不明确。
做不到 0 等待 write
有一个匪夷所思的限制要求：“rt_device_read(dev, -1, buf, len);” 要求这个 len 必须 <= 驱动内的缓存大小。
非 poll 模式不支持 stream 。控制台只能使用 poll 输出。降低甚至影响某些调试过程业务时序。最严重的就是对外产生超时。
驱动和框架之间接口定义已经变了模糊，推广到其它芯片变得困难。（说实话，如果没有 DMA 接口也不会这么复杂）

armink · 2022-04-15T01:25:43Z

选择太多的话，做成软件包比较合适。

别做软件包了吧，这个是关键性核心功能，如果谁都不妥协的话，就先这样放着吧，先百家争鸣。经过一段时间的收敛，再最终汇总成一个新版本的串口框架。16年了，RT-Thread串口框架还是有问题，没有收敛下来，串口框架是个大问题呀。

作为 RT-Thread 上游软件更多要考虑长期的兼容性和可维护性，发散、灵活、自由的组件选择交给软件包。
另外，现在串口 V2 还有什么问题？

阻塞非阻塞概念不明确。

这个具体指的是什么？举个例子？

做不到 0 等待 write

这个具体指的是什么，你期望做成什么样子？举个例子？

有一个匪夷所思的限制要求：“rt_device_read(dev, -1, buf, len);” 要求这个 len 必须 <= 驱动内的缓存大小。

@Guozhanxin 这个请确认一下

非 poll 模式不支持 stream 。控制台只能使用 poll 输出。降低甚至影响某些调试过程业务时序。最严重的就是对外产生超时。

这个可以加

驱动和框架之间接口定义已经变了模糊，推广到其它芯片变得困难。（说实话，如果没有 DMA 接口也不会这么复杂）

可否举个例子？

thewon86 · 2022-04-15T03:06:18Z

@armink 看论坛文章，有更详细的说明。

BernardXiong · 2022-04-15T09:29:19Z

不建议分裂，这样做整体都乱掉了

mysterywolf · 2022-04-15T21:05:47Z

如果框架本身没有什么重大问题，仅仅是因为结构上的问题的话，可以在下次社区会议上讨论一下，商量出一个解决方案来。直接关掉人家的pr太打击人了。可以投-1票，但是别直接关PR.

BernardXiong · 2022-04-16T08:23:15Z

如果框架本身没有什么重大问题，仅仅是因为结构上的问题的话，可以在下次社区会议上讨论一下，商量出一个解决方案来。直接关掉人家的pr太打击人了。可以投-1票，但是别直接关PR.

不会通过的，上面已经写得很清楚，之前几位都是一致性的投反对票

mysterywolf · 2022-08-04T23:28:01Z

请再考虑一下这份PR，
首先，上面投的反对票存疑，因为这些反对不是站在技术角度上针对该框架哪里有什么缺陷或者bug而投的反对票，整个讨论下来，反对的原因只有一个，一个非技术的原因，就是serial_v2框架比serialx诞生的早。基于这种理由而投所谓反对票是站不住脚的。

经过几个月的观察，在本PR关闭后，作者出出 @thewon86 依然在社区和论坛上致力于维护这个框架，而且反馈也很好，应该予以考虑合并进去。serial_v2是官方写的，根据目前的维护经验来说，官方维护存在一个后续维护问题。炸弹哥写的这个框架，请问后续发现这个框架有问题的时候，炸弹哥项目这么多，还能抽出时间来维护吗？从炸弹哥写完这个框架之后，这个框架就一直在没有被维护过。

反观Serialx框架，一直在作者自己的仓库中不断的维护。 https://gitee.com/thewon/serialX
因此基于谁先出生谁后出生这种反对逻辑我认为是无效的。
关于serial_v2和serialx至于使用谁的问题，出出已经给出了一部分对比文章，用户完全可以根据自己的需求来选择，没有必要强制用户一定要使用serial_v2.
https://club.rt-thread.org/ask/article/bfd92159ba11aef6.html
https://club.rt-thread.org/ask/article/0ee3da5b6a9c347d.html
https://club.rt-thread.org/ask/article/bf21ccfb437e1c5d.html
https://club.rt-thread.org/ask/article/2460fcd7db4821ae.html

BernardXiong · 2022-08-06T11:55:23Z

之前提到过，既然是这样，是否可以融入到v2中呢？

这个是发展的原则性问题，一些点上需要收敛，如果都是各种版本，还都合并到主干中，那么主干变成了大杂烩吗。

如果觉得v2不好，也可以是v3，当出现v3的时候，依然一样的需要考虑到所有的情况，所有BSP的情况。

aozima · 2022-08-06T14:04:47Z

最近刚好在项目中用到了serial_v2，也发现了一些问题，正在梳理看怎么改能适用于更多的项目并有利于后面的维护。

我的理解，V2目前还在初级阶段，只能在很小一部分BSP和其应用中测试，不说覆盖所有的场景，连大部分场景都不够。
所以需要继续改进。

所以我认为：serial_v2不一定只能是当前仓库中这个版本，而是下一个更好的版本。
如果能有更合适的，可以在其更加完善后，能作为通用框架，删除当前代码取而代之。
而不建议再增加一个了。

mysterywolf · 2022-08-06T14:52:25Z

开会的时候再讨论一下吧，反正这个PR不会在4.1.1中合并的。

mysterywolf · 2023-02-21T04:27:30Z

由于社区对待这个PR是否进主线存在分歧，但是社区上很多小伙伴都愿意使用serialX，基于此，这个PR我先合并到serialX-dev分支中进行孵化。此外，作者出出一直在持续优化和完善serialX框架并适配了多款BSP，甚至超过了serial2的适配bsp数量。在社区论坛也输出了多篇高质量serialX讲解文章。

thewon86 force-pushed the serialX branch 2 times, most recently from 730f239 to e55208c Compare April 10, 2022 09:35

mysterywolf requested review from BernardXiong, armink, Guozhanxin and whj4674672 April 10, 2022 10:29

mysterywolf requested changes Apr 10, 2022

View reviewed changes

thewon86 force-pushed the serialX branch from e55208c to e1f44d2 Compare April 11, 2022 02:05

a1012112796 reviewed Apr 11, 2022

View reviewed changes

thewon86 force-pushed the serialX branch from e1f44d2 to bf256f4 Compare April 13, 2022 01:18

a new serial driver

12a3bcf

thewon86 force-pushed the serialX branch from bf256f4 to 12a3bcf Compare April 14, 2022 05:31

BernardXiong closed this Apr 15, 2022

mysterywolf reopened this Apr 15, 2022

mysterywolf added the discussion This PR/issue needs to be discussed later label Apr 15, 2022

BernardXiong closed this Apr 16, 2022

mysterywolf reopened this Aug 4, 2022

Merge branch 'master' into serialX

7694078

mysterywolf added the v5.0.0 label Aug 6, 2022

mysterywolf marked this pull request as draft September 29, 2022 05:36

mysterywolf changed the base branch from master to sensor-dev February 21, 2023 04:15

mysterywolf changed the base branch from sensor-dev to serialX-dev February 21, 2023 04:15

Merge branch 'serialX-dev' into serialX

05c3ad0

mysterywolf marked this pull request as ready for review February 21, 2023 04:21

mysterywolf merged commit 67bc7e8 into RT-Thread:serialX-dev Feb 21, 2023

thewon86 deleted the serialX branch February 23, 2023 06:33

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[drv]a new serial driver - serialX #5798

[drv]a new serial driver - serialX #5798

thewon86 commented Apr 10, 2022 •

edited

Loading

mysterywolf commented Apr 10, 2022

mysterywolf Apr 10, 2022

thewon86 Apr 11, 2022

Guozhanxin commented Apr 11, 2022

a1012112796 Apr 11, 2022

thewon86 Apr 12, 2022

a1012112796 Apr 12, 2022 •

edited

Loading

thewon86 Apr 13, 2022

a1012112796 Apr 11, 2022

thewon86 commented Apr 14, 2022

aozima commented Apr 14, 2022

mysterywolf commented Apr 14, 2022 •

edited

Loading

aozima commented Apr 14, 2022 •

edited

Loading

armink commented Apr 14, 2022

thewon86 commented Apr 15, 2022 •

edited

Loading

armink commented Apr 15, 2022

thewon86 commented Apr 15, 2022

BernardXiong commented Apr 15, 2022

mysterywolf commented Apr 15, 2022 •

edited

Loading

BernardXiong commented Apr 16, 2022 •

edited

Loading

mysterywolf commented Aug 4, 2022 •

edited

Loading

BernardXiong commented Aug 6, 2022

aozima commented Aug 6, 2022

mysterywolf commented Aug 6, 2022

mysterywolf commented Feb 21, 2023 •

edited

Loading

[drv]a new serial driver - serialX #5798

[drv]a new serial driver - serialX #5798

Conversation

thewon86 commented Apr 10, 2022 • edited Loading

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

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

代码质量 Code Quality：

mysterywolf commented Apr 10, 2022

mysterywolf Apr 10, 2022

Choose a reason for hiding this comment

thewon86 Apr 11, 2022

Choose a reason for hiding this comment

Guozhanxin commented Apr 11, 2022

a1012112796 Apr 11, 2022

Choose a reason for hiding this comment

thewon86 Apr 12, 2022

Choose a reason for hiding this comment

a1012112796 Apr 12, 2022 • edited Loading

Choose a reason for hiding this comment

thewon86 Apr 13, 2022

Choose a reason for hiding this comment

a1012112796 Apr 11, 2022

Choose a reason for hiding this comment

thewon86 commented Apr 14, 2022

aozima commented Apr 14, 2022

mysterywolf commented Apr 14, 2022 • edited Loading

aozima commented Apr 14, 2022 • edited Loading

armink commented Apr 14, 2022

thewon86 commented Apr 15, 2022 • edited Loading

armink commented Apr 15, 2022

thewon86 commented Apr 15, 2022

BernardXiong commented Apr 15, 2022

mysterywolf commented Apr 15, 2022 • edited Loading

BernardXiong commented Apr 16, 2022 • edited Loading

mysterywolf commented Aug 4, 2022 • edited Loading

BernardXiong commented Aug 6, 2022

aozima commented Aug 6, 2022

mysterywolf commented Aug 6, 2022

mysterywolf commented Feb 21, 2023 • edited Loading

thewon86 commented Apr 10, 2022 •

edited

Loading

a1012112796 Apr 12, 2022 •

edited

Loading

mysterywolf commented Apr 14, 2022 •

edited

Loading

aozima commented Apr 14, 2022 •

edited

Loading

thewon86 commented Apr 15, 2022 •

edited

Loading

mysterywolf commented Apr 15, 2022 •

edited

Loading

BernardXiong commented Apr 16, 2022 •

edited

Loading

mysterywolf commented Aug 4, 2022 •

edited

Loading

mysterywolf commented Feb 21, 2023 •

edited

Loading