楼主所吐槽的是“正确性”的问题。
一个嵌入式系统里面,什么是“正确”的行为,如果没有事先定义标准,大家都按照自己的理解去写,必然会变成这样。
在缺乏标准的领域,确保正确性主要靠工程师本身经验丰富、思维清晰。
在国内,能写出基本“正确”行为的嵌入式软件工程师,通常没有时间和心情做免费开源的东西;而免费开源的东西,往往跟山寨字幕组的翻译一样,醉翁之意不在酒。大环境如此,不能要求每个人都是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。
老生常谈:多看《时间触发嵌入式系统设计模式》。
200字以内,仅用于支线交流,主线讨论请采用回复功能。