楼主所吐槽的是“正确性”的问题。
一个嵌入式系统里面,什么是“正确”的行为,如果没有事先定义标准,大家都按照自己的理解去写,必然会变成这样。
在缺乏标准的领域,确保正确性主要靠工程师本身经验丰富、思维清晰。
在国内,能写出基本“正确”行为的嵌入式软件工程师,通常没有时间和心情做免费开源的东西;而免费开源的东西,往往跟山寨字幕组的翻译一样,醉翁之意不在酒。大环境如此,不能要求每个人都是linus。
老生常谈:多看《时间触发嵌入式系统设计模式》。
国产操作系统行业内了解的人都是会心一笑。本人不信邪,选了rt-thread系统开发控制器固件,于是踩到了以下的雷。
该设备的功能:在指定的时间(精确到50us),主机通过串口与从机通信,并在指定的时间检查有无回复。同时,通过usb接收电脑端指令,并通过usb上报设备状态。
现在我们先测试串口通信,很简单不是嘛?不就是定时收发数据么,来试试看就知道了。
*
备注:rt-thread作者已联系我,正在修复这些问题。我会在这里更新后续使用和测试情况
*
不想看代码的可以直接跳到最后。
====================================================
讨论的代码基于aeff91b。我还没有做fork,修正问题可以下载此压缩包,覆盖同名文件。
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框架开发类似产品。请各位用国产系统之前请先做评估,看看别人这些功能用得怎么样,为解决潜在问题留出时间余量。
[修改于 4年2个月前 - 2020/10/22 22:52:05]
楼主所吐槽的是“正确性”的问题。
一个嵌入式系统里面,什么是“正确”的行为,如果没有事先定义标准,大家都按照自己的理解去写,必然会变成这样。
在缺乏标准的领域,确保正确性主要靠工程师本身经验丰富、思维清晰。
在国内,能写出基本“正确”行为的嵌入式软件工程师,通常没有时间和心情做免费开源的东西;而免费开源的东西,往往跟山寨字幕组的翻译一样,醉翁之意不在酒。大环境如此,不能要求每个人都是linus。
老生常谈:多看《时间触发嵌入式系统设计模式》。
时段 | 个数 |
---|---|
{{f.startingTime}}点 - {{f.endTime}}点 | {{f.fileCount}} |
200字以内,仅用于支线交流,主线讨论请采用回复功能。