Pass rt_tick_t for RT_TIMER_CTRL_SET_TIME and RT_TIMER_CTRL_GET_TIME #10717
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
lygstate
requested review from
BernardXiong,
Rbb666 and
mysterywolf
as code owners
|
|
github-actions
bot
added
BSP
Kernel
📌 Code Review Assignment🏷️ Tag: componentsReviewers: Maihuanyi Changed Files (Click to expand)
🏷️ Tag: components_libcReviewers: GorrayLi mysterywolf Changed Files (Click to expand)
🏷️ Tag: components_lwpReviewers: xu18838022837 Changed Files (Click to expand)
🏷️ Tag: kernelReviewers: GorrayLi ReviewSun hamburger-os lianux-mm wdfk-prog xu18838022837 Changed Files (Click to expand)
📊 Current Review Status (Last Updated: 2025-09-23 13:59 CST)
📝 Review Instructions
|
There was a problem hiding this comment.
Pull Request Overview
Standardize timer control APIs to use rt_tick_t for RT_TIMER_CTRL_SET_TIME/GET_TIME and fix unit mismatches.
- Replace int/rt_int32_t timeout arguments with rt_tick_t when calling rt_timer_control for SET_TIME across kernel, drivers, and components.
- Update tests to use rt_tick_t for timer control.
- Fix OSAL timer period change to convert milliseconds to ticks before RT_TIMER_CTRL_SET_TIME.
Reviewed Changes
Copilot reviewed 22 out of 22 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/signal.c | Use rt_tick_t when setting thread timer timeout. |
| src/mempool.c | Use rt_tick_t for memory pool allocation timeout timer. |
| src/ipc.c | Use rt_tick_t for sem/mutex/event/mailbox/message queue timeout timers. |
| examples/utest/testcases/kernel/timer_tc.c | Update test variables to rt_tick_t for timer control. |
| components/vbus/watermark_queue.h | Use rt_tick_t for watermark queue timeout. |
| components/vbus/vbus.c | Use rt_tick_t for vbus post timeout. |
| components/vbus/prio_queue.c | Use rt_tick_t for priority queue pop timeout. |
| components/utilities/rt-link/src/rtlink.c | Use rt_tick_t for multiple RT-Link timeouts. |
| components/lwp/lwp_ipc.c | Use rt_tick_t for LWP IPC timeouts and pass to RT_TIMER_CTRL_SET_TIME. |
| components/libc/posix/pthreads/pthread_cond.c | Use rt_tick_t for pthread_cond timed wait. |
| components/libc/posix/io/poll/poll.c | Use rt_tick_t for poll timeout. |
| components/libc/posix/io/epoll/epoll.c | Use rt_tick_t for epoll timeout. |
| components/drivers/ipc/waitqueue.c | Use rt_tick_t for internal waitqueue ticks and compare against RT_WAITING_FOREVER with cast. |
| components/drivers/ipc/dataqueue.c | Use rt_tick_t for dataqueue push/pop timeouts. |
| components/drivers/ipc/condvar.c | Compare timeout against (rt_tick_t)RT_WAITING_FOREVER before starting timer. |
| components/drivers/ipc/completion_up.c | Use rt_tick_t for completion wait timeout. |
| components/drivers/ipc/completion_mp.c | Use rt_tick_t for completion timeout in MP variant. |
| bsp/at91/at91sam9g45/drivers/at91_mci.c | Use rt_tick_t for request timeout. |
| bsp/at91/at91sam9260/drivers/at91_mci.c | Use rt_tick_t for request timeout. |
| bsp/allwinner/libraries/sunxi-hal/hal/source/usb/host/ehci.h | Change hr_timeouts[] to rt_tick_t. |
| bsp/allwinner/libraries/sunxi-hal/hal/source/usb/host/ehci-timer.c | Update local timeout variables to rt_tick_t. |
| bsp/allwinner/libraries/sunxi-hal/hal/source/sdmmc/osal/os/RT-Thread/os_timer.c | Convert ms to ticks and pass rt_tick_t to RT_TIMER_CTRL_SET_TIME. |
bsp/allwinner/libraries/sunxi-hal/hal/source/sdmmc/osal/os/RT-Thread/os_timer.c
Outdated
Show resolved
Hide resolved
bsp/allwinner/libraries/sunxi-hal/hal/source/sdmmc/osal/os/RT-Thread/os_timer.c
Show resolved
Hide resolved
lygstate
force-pushed
the
use_rt_tick_t
branch
2 times, most recently
from
4d90295 to
3e2e1d7
Compare
Rbb666
requested changes
Sep 23, 2025
| /* start timer */ | ||
| if (timeout > 0) | ||
| { | ||
| rt_tick_t timeout_tick = timeout; |
There was a problem hiding this comment.
涉及到内核相关的，尽量不要用c99的语法吧。局部变量的声明需要放在函数的开头。
There was a problem hiding this comment.
这也是C89语法，C89语法的意思就是，变量声明必须在{之后的第一行，不一定非要在函数的第一行，有CI保证C89语法吗？
There was a problem hiding this comment.
https://stackoverflow.com/a/9513651/321938
你可以用C89编译器试一下一下代码
include <stdio.h>
int main()
{
int i = 22;
printf("%d\n", i);
{
int j = 42;
printf("%d\n", j);
}
return 0;
}| /* start timer */ | ||
| if (timeout > 0) | ||
| { | ||
| rt_tick_t timeout_tick = timeout; |
| /* start timer */ | ||
| if (timeout > 0) | ||
| { | ||
| rt_tick_t timeout_tick = timeout; |
Signed-off-by: Yonggang Luo <luoyonggang@gmail.com>
…hread/os_timer.c call to RT_TIMER_CTRL_SET_TIME RT_TIMER_CTRL_SET_TIME expects a pointer to rt_tick_t (ticks). period_tick is derived from periodMS (milliseconds) via rt_tick_from_millisecond to ensure correct units are passed.
lygstate
force-pushed
the
use_rt_tick_t
branch
from
3e2e1d7 to
6736d29
Compare
Rbb666
approved these changes
Sep 23, 2025
| /* start timer */ | ||
| if (timeout > 0) | ||
| { | ||
| rt_tick_t timeout_tick = timeout; |
|
@lygstate 这个PR需要rebase方式合并好一些？ |
我看到你把ci修正了，所以rebase一下来通过ci |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Pass rt_tick_t for RT_TIMER_CTRL_SET_TIME and RT_TIMER_CTRL_GET_TIME
为什么提交这份PR (why to submit this PR)
RT_TIMER_CTRL_SET_TIME and RT_TIMER_CTRL_GET_TIME only accept rt_tick_t, but the current code base pass int/rt_int32_t
randomly.
你的解决方案是什么 (what is your solution)
Check all place that use RT_TIMER_CTRL_SET_TIME /RT_TIMER_CTRL_GET_TIME and fixes it accordingly
当前拉取/合并请求的状态 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