[RFC][utest] Add audio driver test framework #10243
Conversation
| int fd = -1; | ||
| int res = 0; | ||
| uint8_t *buffer = NULL; | ||
| void* player_buffer = NULL;struct rt_audio_caps caps = {0}; |
2655d6a to
71847bc
Compare
|
@1078249029 这个PR准备好了吗？ |
准备好了，麻烦您review下。 |
|
@unicornx 老师，上次跟您说的音频测试框架已经写好了，您有空的话也请review？ |
📌 Code Review Assignment🏷️ Tag: bsp_cvitekPath: Changed Files (Click to expand)
📊 Current Review Status (Last Updated: 2025-05-15 12:43 UTC)
📝 Review Instructions
|
There was a problem hiding this comment.
我记得上次开会满老师 @mysterywolf 说建议后面 ut cases 都移到各个模块自己的目录下去，不要放到 examples 下了，本身这个 examples 就很让人迷惑。可以参考 src/klibc/utest 这个是满老师最新作的 utest 例子，貌似就是每个模块下自己维护一个 utest 目录然后在里面加测试代码。然后就是模仿 src/klibc/utest 下的代码写，包括 SConscript 里要 denpend RT_UTEST_USING_ALL_CASES ，还有配置选项名字统一加 'RT_UTEST_TC_USING_' 前缀，参考 'RT_UTEST_TC_USING_KLIBC'。
还有一个问题就是建议在测试代码中以注释的方式写一下测试的设计思想，直接 review 代码一下子不知道你的设计思路理解起来很费劲。这个测试代码需要依赖硬件吗？看注释有模拟的部分，可以说明一下。另外我理解这里测试的对象是 driver audio 的 API，所以 drv_mic.c 和 drv_player.c 应该是测试的主体，对吧。可以在注释中列一下你覆盖的 API 吗？
代码上，所有全局变量和函数，如果不是 extern 的请全部 static。
There was a problem hiding this comment.
请每次更新后都和最新的 master 同步一下。
此外，几个问题先提一下，请一并和上面的同步问题一起先改掉。
- commit 的标题中不要出现 RFC 的字样，PR 可以是 RFC。如果 commit 中含有 RFC，一旦 merge 这个 commit 看上去就怪怪的，除非提交人会及时改 commit，但实践上看做不到这个。PR 里写 RFC 或者设置 draft 状态都很方便，以后要改 PR 的标题也可以在网页上直接操作，但是 commit 一旦合入就很难改了。
- commit 的标题前缀我觉得不是 utest，但是应该是属于 components: driver: audio 的因为按照现在的做法，utest 是属于子模块的，不再是单独的 utest，除非是涉及 utest 的 framework 的改动。
- 文件名，我建议有 TC 前缀就够了，不用加 rt 前缀，所有文件默认都是 RTT 的，加 rt 前缀属于多余，而且导致文件名分段太多。另外我比较建议不要大写 TC，文件名全部小写比较好。
- 只在内部使用的全局变量都要 static，好像没有改全。
两个flag变量是用于同步驱动与应用层状态机的状态的，只能放在全局。其他方法例如使用全局信号量或者手动延时的效果都不是很好，前者既麻烦又与其他测试模块有依赖，后者太死板，所以这里就没有改。 |
| #define thread_simulate_intr_create_stacksize 1024 | ||
| static rt_thread_t thread_simulate_intr_handle; | ||
|
|
||
| struct mic_device mic_dev; |
There was a problem hiding this comment.
这个文件名为啥前后都有 tc，后面那个有什么含义吗？要不要改一个更有意义的名字？
看了你的代码，感觉 tc_audio_tc.c 是测试的主程序，譬如叫 tc_audio_main.c 或者直接叫 tc_audio.c?
|
|
||
| struct mic_device mic_dev; | ||
|
|
||
| rt_uint8_t mic_flag = 0; |
There was a problem hiding this comment.
为啥 mic_flag 定义在 tc_audio_drv_mic.c 里，而 player_flag 定义在 tc_audio_tc.c 里？
我建议统一定义在 tc_audio_tc.c ，甚至我觉得如果 player_test 和 mic_test 不会同时跑的话，只要一个 flag 就好了, 因为这两个变量的作用应该是一回事且不会同时被使用。
另外我理解 mic_flag/player_flag 你是想表达一个类似测试过程中状态机的用于同步主程序和驱动之间的作用吧，flag 这个名字我觉得不能很清楚地表达意思，或许叫 step 更好？
| * rt_audio_register or rt_audio_rx_done existing bugs. | ||
| */ | ||
|
|
||
| #include "TC_rt_audio_common.h" |
|
另外这个 PR 的状态为何还是 Draft，可以改成正常的了。 |
0270c43 to
977e108
Compare
There was a problem hiding this comment.
Pull Request Overview
This PR introduces a test framework for audio drivers by simulating audio peripherals using memory operations.
- Added unit tests for audio player and microphone drivers.
- Integrated simulation threads to trigger DMA-like behavior.
- Updated build scripts and configuration options for audio driver tests.
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| components/drivers/audio/utest/tc_audio_main.c | Added unit tests for audio framework and memory verification logic. |
| components/drivers/audio/utest/tc_audio_drv_player.c | Added player driver simulation and interrupt thread for DMA behavior. |
| components/drivers/audio/utest/tc_audio_drv_mic.c | Added microphone driver simulation and interrupt thread for DMA behavior. |
| components/drivers/audio/utest/tc_audio_common.h | New header with shared definitions and constants for driver tests. |
| components/drivers/audio/utest/SConscript | Updated SConscript to include audio test cases. |
| components/drivers/audio/SConscript | Modified build script to recursively include SConscript files. |
| components/drivers/audio/Kconfig | Added new configuration option for audio driver test cases. |
Comments suppressed due to low confidence (1)
components/drivers/audio/utest/tc_audio_drv_player.c:15
- [nitpick] Consider renaming 'thread_simulate_intr_create_stacksize' to uppercase (e.g., THREAD_SIMULATE_INTR_CREATE_STACKSIZE) for consistency with other macro names.
#define thread_simulate_intr_create_stacksize 1024
| while (1) | ||
| { | ||
| if(fsm_step == 2) | ||
| { | ||
| break; | ||
| } | ||
| rt_thread_mdelay(10); | ||
| } | ||
|
|
There was a problem hiding this comment.
[nitpick] The busy-wait loop here could be improved by using an event-driven mechanism or proper signaling to avoid unnecessary CPU usage during test execution.
| while (1) | |
| { | |
| if(fsm_step == 2) | |
| { | |
| break; | |
| } | |
| rt_thread_mdelay(10); | |
| } | |
| rt_sem_t fsm_sem = rt_sem_create("fsm_sem", 0, RT_IPC_FLAG_FIFO); | |
| if (fsm_sem == RT_NULL) | |
| { | |
| rt_kprintf("Failed to create semaphore\n"); | |
| uassert_true(0); | |
| goto __exit; | |
| } | |
| /* Wait for fsm_step to be updated to 2 */ | |
| rt_sem_take(fsm_sem, RT_WAITING_FOREVER); | |
| /* Clean up semaphore */ | |
| rt_sem_delete(fsm_sem); |
Copilot uses AI. Check for mistakes.
| if(fsm_step == 2) | ||
| { | ||
| /* Send the the data(0x55) from DMA buffer to kernel */ | ||
| rt_memset((void*)&mic_dev.rx_fifo[RX_DMA_BLOCK_SIZE], 0x55, RX_DMA_BLOCK_SIZE); | ||
| rt_audio_rx_done(&mic_dev.audio, &mic_dev.rx_fifo[RX_DMA_BLOCK_SIZE], RX_DMA_BLOCK_SIZE); | ||
| fsm_step = 3; | ||
| break; | ||
| } | ||
| if(fsm_step == 4) | ||
| { | ||
| rt_thread_mdelay(10); | ||
| rt_audio_rx_done(&mic_dev.audio, &mic_dev.rx_fifo[RX_DMA_BLOCK_SIZE], RX_DMA_BLOCK_SIZE); | ||
| break; | ||
| } | ||
| rt_thread_mdelay(10); | ||
| } | ||
|
|
||
| while(1) | ||
| { | ||
| rt_thread_mdelay(10); | ||
| } | ||
| } | ||
|
|
||
| static void thread_simulate_intr_create(void) | ||
| { |
There was a problem hiding this comment.
[nitpick] Consider replacing the busy-wait loop with an event-driven or signaling mechanism to improve efficiency in the test thread.
| if(fsm_step == 2) | |
| { | |
| /* Send the the data(0x55) from DMA buffer to kernel */ | |
| rt_memset((void*)&mic_dev.rx_fifo[RX_DMA_BLOCK_SIZE], 0x55, RX_DMA_BLOCK_SIZE); | |
| rt_audio_rx_done(&mic_dev.audio, &mic_dev.rx_fifo[RX_DMA_BLOCK_SIZE], RX_DMA_BLOCK_SIZE); | |
| fsm_step = 3; | |
| break; | |
| } | |
| if(fsm_step == 4) | |
| { | |
| rt_thread_mdelay(10); | |
| rt_audio_rx_done(&mic_dev.audio, &mic_dev.rx_fifo[RX_DMA_BLOCK_SIZE], RX_DMA_BLOCK_SIZE); | |
| break; | |
| } | |
| rt_thread_mdelay(10); | |
| } | |
| while(1) | |
| { | |
| rt_thread_mdelay(10); | |
| } | |
| } | |
| static void thread_simulate_intr_create(void) | |
| { | |
| rt_sem_take(fsm_step_sem, RT_WAITING_FOREVER); // Wait for state change signal | |
| if (fsm_step == 2) | |
| { | |
| /* Send the data(0x55) from DMA buffer to kernel */ | |
| rt_memset((void*)&mic_dev.rx_fifo[RX_DMA_BLOCK_SIZE], 0x55, RX_DMA_BLOCK_SIZE); | |
| rt_audio_rx_done(&mic_dev.audio, &mic_dev.rx_fifo[RX_DMA_BLOCK_SIZE], RX_DMA_BLOCK_SIZE); | |
| fsm_step = 3; | |
| } | |
| else if (fsm_step == 4) | |
| { | |
| rt_audio_rx_done(&mic_dev.audio, &mic_dev.rx_fifo[RX_DMA_BLOCK_SIZE], RX_DMA_BLOCK_SIZE); | |
| } | |
| } | |
| } | |
| static void thread_simulate_intr_create(void) | |
| { | |
| fsm_step_sem = rt_sem_create("fsm_step_sem", 0, RT_IPC_FLAG_FIFO); // Create semaphore | |
| if (fsm_step_sem == RT_NULL) | |
| { | |
| rt_kprintf("Error: Failed to create semaphore!\n"); | |
| return; | |
| } |
Copilot uses AI. Check for mistakes.
Achieve driver framework by operating memory to simulate audio peripheral drivers. And it could be as a draft standrad for other drivers test framework. Signed-off-by: 1078249029 <1078249029@qq.com>
拉取/合并请求描述：(PR description)
[
为什么提交这份PR (why to submit this PR)
Fixed #10046
你的解决方案是什么 (what is your solution)
Achieve driver framework by operating memory to simulate audio peripheral drivers. And it could be as a draft standrad for other drivers test framework.
请提供验证的bsp和config (provide the config and bsp)
]
当前拉取/合并请求的状态 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