[正在解决]rt-thread系统的几个很大的bug
关键词
国产操作系统
rt-thread

国产操作系统行业内了解的人都是会心一笑。本人不信邪,选了rt-thread系统开发控制器固件,于是踩到了以下的雷。

该设备的功能:在指定的时间(精确到50us),主机通过串口与从机通信,并在指定的时间检查有无回复。同时,通过usb接收电脑端指令,并通过usb上报设备状态。

现在我们先测试串口通信,很简单不是嘛?不就是定时收发数据么,来试试看就知道了。


*

备注:rt-thread作者已联系我,正在修复这些问题。我会在这里更新后续使用和测试情况

*

不想看代码的可以直接跳到最后。

====================================================

讨论的代码基于aeff91b。我还没有做fork,修正问题可以下载此压缩包,覆盖同名文件。

attachment icon rt-thread-uart-fix.zip 31.94KB ZIP 16次下载


1.    任务延时函数 rt_thread_delay_until实现居然是错的

这么重要的api,上传之前难道不检查一下么,还是不懂怎么写test case?

我想作者一定是简单的测试了:

ts = rt_tick_get(); rt_thread_delay_until(&tick, 100);

这样当然测不出问题啦,你要这样测才知道行不行:

RT_ASSERT(RT_TICK_PER_SECOND == 1000);
ledon(); ts = rt_tick_get(); for(int i=0;i<99;i++) nop_loop_1ms(); rt_thread_delay_until(&tick, 100); ledoff();
ledon(); ts = rt_tick_get(); for(int i=0;i<100;i++) nop_loop_1ms(); rt_thread_delay_until(&tick, 100); ledoff();
ledon(); ts = rt_tick_get(); for(int i=0;i<101;i++) nop_loop_1ms(); rt_thread_delay_until(&tick, 100); ledoff();

别人要用delay until,就是因为不确定从tick_get到delay中间过了多久,不然直接delay(100-99)就好了。

实际测出来是299ms, 100ms, 100ms

我们看看作者怎么写的,文件thread.c 581行:

/* disable interrupt */  
level = rt_hw_interrupt_disable();    
if (rt_tick_get() - *tick < inc_tick)    
{    
    *tick = rt_tick_get() - *tick + inc_tick;    
    /* suspend thread */    
    rt_thread_suspend(thread);    
    /* reset the timeout of thread timer and start it */    
    rt_timer_control(&(thread->thread_timer), RT_TIMER_CTRL_SET_TIME, tick);    
    rt_timer_start(&(thread->thread_timer));
...
/* get the wakeup tick */    
*tick = rt_tick_get();    
return RT_EOK;    
}

作者原意是根据当前的tick,设置实际的定时器超时时间(把inc_tick扣掉从tick_get到delay的时间)。既然最后tick改写为函数返回前的时间,那中间为什么要更新tick的值?这有什么用?访问tick指针的速度肯定没有访问局部变量的速度快,而且容易把人搞糊涂。

我花了好一会才完全确定RT_TIMER_CTRL_SET_TIME是设置定时器的时间长度,不信你看下变量名和注释:

rtdef.h

struct rt_timer {
   struct rt_object parent;            /**< inherit from rt_object */    
    rt_list_t        row[RT_TIMER_SKIP_LIST_LEVEL];    
    void (*timeout_func)(void *parameter);     /**< timeout function */    
    void            *parameter;      /**< timeout function's parameter */    
    rt_tick_t        init_tick;        /**< timer timeout tick */    
    rt_tick_t        timeout_tick;       /**< timeout tick */    
};

谁能告诉我timer timeout tick和timeout tick什么区别?init_tick又是什么意思?

看了文档之后才确定,原来init_tick是定时器时间长度,是duration或者period的意思。timer.c里面是这样的:

rt_err_t rt_timer_control(rt_timer_t timer, int cmd, void *arg)    
{    
...
case RT_TIMER_CTRL_SET_TIME:    
timer->init_tick = *(rt_tick_t *)arg;    
...

rt_err_t rt_timer_start(rt_timer_t timer)
{
...
RT_ASSERT(timer->init_tick < RT_TICK_MAX / 2);    
timer->timeout_tick = rt_tick_get() + timer->init_tick;

那就是说,这里是错误的:

*tick = rt_tick_get() - *tick + inc_tick;

加减不分,数学是体育老师教的?inc_tick是定时的长度,应该是inc_tick - (rt_tick_get() - *tick)才对。

改写成这样才正常:

level = rt_hw_interrupt_disable();    
rt_tick_t elapsed = rt_tick_get() - *tick;
if (elapsed < inc_tick)    
{    
    inc_tick = inc_tick - elapsed ;
    /* suspend thread */    
    rt_thread_suspend(thread);    
    /* reset the timeout of thread timer and start it */    
    rt_timer_control(&(thread->thread_timer), RT_TIMER_CTRL_SET_TIME, &inc_tick);


2.    串口dma发送无规律错误,可稳定重现

test1() {
   char buf[] = {"hello"};
   rt_device_write(serial, buf, sizeof(buf));
}
test2() {
   char buf[] = {"world"};
   rt_device_write(serial, buf, sizeof(buf));
}

while(1) {test1(); test2();}

发出去的是herld类似这种,不会正常发helloworld。后来发现是驱动框架直接把buf提交给dma engine了。

usb框架的作者比较聪明,知道函数一旦返回用户指针就无效了,所以驱动层都是把buf内容复制到内部的缓存,没有直接用buf的地址。


执行路径是这样的:在serial.c,rt_device_write实际上是调用了rt_serial_write,检测到dma模式开启,会调用_serial_dma_tx,内部是调用dma_transmit,即为stm32_dma_transmit

rt_err_t rt_hw_serial_register(struct rt_serial_device *serial,
...
    device->write       = rt_serial_write;

static rt_size_t rt_serial_write(struct rt_device *dev,
...

rt_inline int _serial_dma_tx(struct rt_serial_device *serial, const rt_uint8_t *data, int length)
{
...
            serial->ops->dma_transmit(serial, (rt_uint8_t *)data, length, RT_SERIAL_DMA_TX);
...
}

在drv_usart.c的291行

static rt_size_t stm32_dma_transmit(struct rt_serial_device *serial, rt_uint8_t *buf, rt_size_t size, int direction)       
...
        if (HAL_UART_Transmit_DMA(&uart->handle, buf, size) == HAL_OK)

总之buf指针被直接推给HAL库,HAL把buf设置为DMA起始地址。问题是buf在栈上,rt_device_write返回之后,buf已经销毁了,这时候buf的地址可能已经挪作别用了。到test2又拿来放另一个buf,发送的内容就变了。

我没看到任何文档提到这个特性,device框架没有关于api是同步或者异步,驱动有或者无缓存的说明。我想任何正常的程序员逻辑上都会认为:已经write的buf再修改,不会影响前面的write操作。

如果这个算作是个人理解问题,建议看


3.    串口中断发送实质上是轮询发送

dma发送不好用,得确认地址能支持dma,得锁住缓存区确认发完之前没修改,那我用中断发送行不行?答案是坑爹的不行。

rt-thread提供的stm32 bsp,是这样实现串口发送的:

rt_inline int _serial_int_tx(struct rt_serial_device *serial, const rt_uint8_t *data, int length)    
{    
int size;    
struct rt_serial_tx_fifo *tx;    
RT_ASSERT(serial != RT_NULL);    
size = length;    
tx = (struct rt_serial_tx_fifo*) serial->serial_tx;    
RT_ASSERT(tx != RT_NULL);    
while (length)    
{    
/*    
        * to be polite with serial console add a line feed    
        * to the carriage return character    
        */    
if (*data == '\n' && (serial->parent.open_flag & RT_DEVICE_FLAG_STREAM))    
{    
if (serial->ops->putc(serial, '\r') == -1)    
{    
rt_completion_wait(&(tx->completion), RT_WAITING_FOREVER);    
continue;    
}    
}    
if (serial->ops->putc(serial, *(char*)data) == -1)    
{    
rt_completion_wait(&(tx->completion), RT_WAITING_FOREVER);    
continue;    
}    
data ++; length --;    
}    
return size - length;    
}

然后我们看看这个putc是怎么回事,在drv_usart.c:

static int stm32_putc(struct rt_serial_device *serial, char c)    
{    
struct stm32_uart *uart;    
RT_ASSERT(serial != RT_NULL);    
uart = rt_container_of(serial, struct stm32_uart, serial);    
UART_INSTANCE_CLEAR_FUNCTION(&(uart->handle), UART_FLAG_TC);    
。。。
uart->handle.Instance->DR = c;    
#endif    
while (__HAL_UART_GET_FLAG(&(uart->handle), UART_FLAG_TC) == RESET);    
return 1;    
}

啥,这不是轮询发送么?行啊真厉害,cpu占用率100%,这要系统干什么。

而且,stm32是有1字节的发送缓存的,写入之后不用Data register发完。只要tx empty为1(1个外设时钟周期之后),马上可以写下一个字节。如果等待tx complete(TC)标志,发出来串口数据是不连续的,字节之间有很小的间隙。

这个等待tx->completion也是聋子的耳朵,发送的时候都在while里面等待TC标志,那返回出来肯定是完成了发送啊。

作者会不会写串口中断发送啊,天哪。

只好自己修改serial.h,增加发送缓存队列。看这个名字叫做tx_fifo,原来某个作者应该是知道该加发送缓存的哦,怎么最后又搞成这样,真是乱七八糟。

struct rt_serial_tx_fifo
{
    /* software fifo */
    rt_uint8_t *buffer;

    rt_uint16_t put_index, get_index;

    rt_bool_t is_full;
};

修改drv_usart.c,增加忙状态查询。这个putc跟stdlib的putc略有区别,既然能返回int我们就借用它检测忙状态,轮询和中断发送都可以用它了。

static int stm32_putc(struct rt_serial_device *serial, char c)
{
    struct stm32_uart *uart;
    RT_ASSERT(serial != RT_NULL);

    uart = rt_container_of(serial, struct stm32_uart, serial);
    if (__HAL_UART_GET_FLAG(&(uart->handle), UART_FLAG_TC) == RESET) return -1;
    UART_INSTANCE_CLEAR_FUNCTION(&(uart->handle), UART_FLAG_TC);
    #if defined(SOC_SERIES_STM32L4) || defined(SOC_SERIES_STM32F7) || defined(SOC_SERIES_STM32F0) \
        || defined(SOC_SERIES_STM32L0) || defined(SOC_SERIES_STM32G0) || defined(SOC_SERIES_STM32H7) \
        || defined(SOC_SERIES_STM32G4)
        uart->handle.Instance->TDR = c;
    #else
        uart->handle.Instance->DR = c;
    #endif
    return 1;
}

修改serial.c:

rt_inline int _serial_poll_tx(struct rt_serial_device *serial, const rt_uint8_t *data, int length)
{
    int size;
    RT_ASSERT(serial != RT_NULL);

    size = length;
    while (length)
    {
        /*
         * to be polite with serial console add a line feed
         * to the carriage return character
         */
        if (*data == '\n' && (serial->parent.open_flag & RT_DEVICE_FLAG_STREAM))
        {
            while(serial->ops->putc(serial, '\r') < 0);
        }

        while(serial->ops->putc(serial, *data) < 0);

        ++ data;
        -- length;
    }

    return size - length;
}

rt_inline int _serial_int_tx(struct rt_serial_device *serial, const rt_uint8_t *data, int length)
{
    int size;
    struct rt_serial_tx_fifo *tx_fifo;

    RT_ASSERT(serial != RT_NULL);

    size = length;
    tx_fifo = (struct rt_serial_tx_fifo*) serial->serial_tx;
    RT_ASSERT(tx_fifo != RT_NULL);

    while (length)
    {
        rt_base_t level;

        /* disable interrupt */
        level = rt_hw_interrupt_disable();

        rt_uint16_t tmp = tx_fifo->put_index + 1;
        if (tmp >= serial->config.bufsz) tmp = 0;
        if(tmp == tx_fifo->get_index) {
            /* buffer is full */
            rt_hw_interrupt_enable(level);
            break;
        }

        if (*data == '\n' && (serial->parent.open_flag & RT_DEVICE_FLAG_STREAM)) {
            tx_fifo->buffer[tx_fifo->put_index] = '\r';
        } else {
            tx_fifo->buffer[tx_fifo->put_index] = *data;
            data ++; length --;
        }

        tx_fifo->put_index = tmp;
        serial->ops->control(serial, RT_DEVICE_CTRL_SET_INT, (void *)RT_DEVICE_FLAG_INT_TX);

        /* enable interrupt */
        rt_hw_interrupt_enable(level);

    }

    return size - length;
}

serial.c初始化串口那里,为啥要memset你不知道这完全没必要而且很慢的吗?没写过缓存空间根本不会被读出,写入的时候自然有正确值了,为什么要清零?看来维护串口驱动的人逻辑能力确实不太行。我把它注释掉了,并且增加了设置tx中断的代码:

static rt_err_t rt_serial_open(struct rt_device *dev, rt_uint16_t oflag) {
...
/* initialize the Rx/Tx structure according to open flag */
    if (serial->serial_rx == RT_NULL)
    { 
        if (oflag & RT_DEVICE_FLAG_INT_RX)
        {
            struct rt_serial_rx_fifo* rx_fifo;

            rx_fifo = (struct rt_serial_rx_fifo*) rt_malloc (sizeof(struct rt_serial_rx_fifo) +
                serial->config.bufsz);
            RT_ASSERT(rx_fifo != RT_NULL);
            rx_fifo->buffer = (rt_uint8_t*) (rx_fifo + 1);
            //rt_memset(rx_fifo->buffer, 0, serial->config.bufsz);
            rx_fifo->put_index = 0;
            rx_fifo->get_index = 0;
            rx_fifo->is_full = RT_FALSE;

            serial->serial_rx = rx_fifo;
            dev->open_flag |= RT_DEVICE_FLAG_INT_RX;
            /* configure low level device */
            serial->ops->control(serial, RT_DEVICE_CTRL_SET_INT, 0);
            serial->ops->control(serial, RT_DEVICE_CTRL_SET_INT, (void *)RT_DEVICE_FLAG_INT_RX);
        }
...
    if (serial->serial_tx == RT_NULL)
        {
            if (oflag & RT_DEVICE_FLAG_INT_TX)
            {
                struct rt_serial_tx_fifo* tx_fifo;
    
                tx_fifo = (struct rt_serial_tx_fifo*) rt_malloc (sizeof(struct rt_serial_tx_fifo) +
                    serial->config.bufsz);
                RT_ASSERT(tx_fifo != RT_NULL);
                tx_fifo->buffer = (rt_uint8_t*) (tx_fifo + 1);
                //rt_memset(tx_fifo->buffer, 0, serial->config.bufsz);
                tx_fifo->put_index = 0;
                tx_fifo->get_index = 0;
                tx_fifo->is_full = RT_FALSE;
    
                serial->serial_tx = tx_fifo;
                dev->open_flag |= RT_DEVICE_FLAG_INT_TX;
                serial->ops->control(serial, RT_DEVICE_CTRL_SET_INT, 0);
            }

针对问题(3)还有几处其他修改,具体看代码吧。


说明一下通常的串口中断发送操作流程:

a)    待发送的数据压入环形缓存,如果缓存快满,返回实际压入的字节数。

b)    开启发送寄存器空(TXE)中断(如果当前发送缓存空,会触发一次中断)

c)    (如果mcu设计没有tx empty中断,就得用tx complete中断。这时候需要从缓存取1字节,写入DR)

d)    中断里面检测缓存剩余数据量,如果没有剩余数据就关闭tx中断。有数据就从缓存取1字节,写入DR。


从以上问题分析,开发此系统的人,软件工程能力是不及格的。它这不是粗心大意错误,是由于对基本概念的不理解。这都能出问题,后面高级功能还不知道藏了多少雷。

我认为这是典型的中国学生行为模式:死读书,表面功夫做得像,实质上不理解事物本质。这类人适合机械堆砌工作量,不能干任何品质要求高的活,走前人没走过的路更不行。

我在前几年招人的过程中见多了这种程序员。他工作一周埋下的雷,得你自己用两周排除掉。就算免费发工资也不能用这类人,他会让你倒亏进去的。你可以借着国产化融资烧钱,雇一大堆这样的人搞人海战术,发挥人力成本低的“优势”。但是每次搞出一个坑,就要几个人填,最后泡沫被自身重量压垮。


人的偏见是节约成本的必然选择。以后我会用arduino框架开发类似产品。请各位用国产系统之前请先做评估,看看别人这些功能用得怎么样,为解决潜在问题留出时间余量。



[修改于 1 年前 - 2020-10-22 22:52:05]

novakon
1年0个月前 修改于 1年0个月前
1楼

楼主所吐槽的是“正确性”的问题。

一个嵌入式系统里面,什么是“正确”的行为,如果没有事先定义标准,大家都按照自己的理解去写,必然会变成这样。

在缺乏标准的领域,确保正确性主要靠工程师本身经验丰富、思维清晰。

在国内,能写出基本“正确”行为的嵌入式软件工程师,通常没有时间和心情做免费开源的东西;而免费开源的东西,往往跟山寨字幕组的翻译一样,醉翁之意不在酒。大环境如此,不能要求每个人都是linus。

老生常谈:多看《时间触发嵌入式系统设计模式》。

回复
评论
1
加载评论中,请稍候...
200字以内,仅用于支线交流,主线讨论请采用回复功能。
折叠评论
大仙
1年0个月前
2楼

attachment icon 时间触发嵌入式系统设计模式.pdf 72.67MB PDF 200次下载 预览



回复
评论(1)
加载评论中,请稍候...
200字以内,仅用于支线交流,主线讨论请采用回复功能。
折叠评论

想参与大家的讨论?现在就 登录 或者 注册

%7B%22isDisplay%22%3Atrue%7D

仅供内部学术交流或培训使用,请先保存到本地。本内容不代表科创观点,未经原作者同意,请勿转载。

下载

插入资源
全部
图片
视频
音频
附件
全部
未使用
已使用
正在上传
空空如也~
上传中..{{f.progress}}%
处理中..
上传失败,点击重试
等待中...
{{f.name}}
空空如也~
(视频){{r.oname}}
{{selectedResourcesId.indexOf(r.rid) + 1}}
处理中..
处理失败
插入表情
我的表情
共享表情
Emoji
上传
注意事项
最大尺寸100px,超过会被压缩。为保证效果,建议上传前自行处理。
建议上传自己DIY的表情,严禁上传侵权内容。
点击重试等待上传{{s.progress}}%处理中...已上传
空空如也~
草稿箱
加载中...
此处只插入正文,如果要使用草稿中的其余内容,请点击继续创作。
{{fromNow(d.toc)}}
{{getDraftInfo(d)}}
标题:{{d.t}}
内容:{{d.c}}
继续创作
删除插入插入
{{forum.displayName}}
{{forum.countThreads}}
篇文章,
{{forum.countPosts}}
条回复
{{forum.description || "暂无简介"}}
ID: {{user.uid}}
学术分隐藏
投诉或举报
加载中...
{{tip}}
请选择违规类型:
{{reason.type}}

空空如也

支持的图片格式:jpg, jpeg, png
插入公式
分享回复:{{shareId}}
加载中...
评论控制
加载中...
文号:{{pid}}
投诉或举报
加载中...
{{tip}}
请选择违规类型:
{{reason.type}}

空空如也

加载中...
详情
详情
推送到专栏从专栏移除
设为匿名取消匿名
查看作者
回复
只看作者
加入收藏取消收藏
加入关注取消关注
折叠回复
置顶取消置顶
评学术分
鼓励
设为精选取消精选
建议修改
编辑
通过审核
评论控制
退修或删除
历史版本
违规记录
投诉或举报
加入黑名单移除黑名单
查看IP
{{format('YYYY/MM/DD HH:mm:ss', toc)}}
投诉或举报
加载中...
{{tip}}
请选择违规类型:
{{reason.type}}

空空如也

下载资料
{{fileName}}
大小:{{size}}
下载当前附件将花费 {{costMessage}}
{{description}}
你当前剩余 {{holdMessage}}
{{fileName}}
大小:{{size}}
当前附件免费。
你已购买过此附件,下载当前附件不需要花费积分。
加载中...
{{errorInfo}}
附件已丢失
当前账号的附件下载数量限制如下:
时段 个数
{{f.startingTime}}点 - {{f.endTime}}点 {{f.fileCount}}