You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by GitBox <gi...@apache.org> on 2022/02/05 20:49:04 UTC
[GitHub] [incubator-nuttx] pkarashchenko opened a new pull request #5424: timer: fix operation of periodic timers
pkarashchenko opened a new pull request #5424:
URL: https://github.com/apache/incubator-nuttx/pull/5424
## Summary
The POSIX timers and timerfd are based on wdog. The wd_start() API adds 1 tick to a delay parameter so periodic timers have accumulative error. Subtract 1 tick from delay before calling wd_start() from wdentry callback to remove accumulative error.
Remove 'pt_last' member from 'struct posix_timer_s' because it is not used anywhere.
## Impact
POSIX timers and timerfd users
## Testing
WIP
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pkarashchenko commented on pull request #5424: timer: fix operation of periodic timers
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on pull request #5424:
URL: https://github.com/apache/incubator-nuttx/pull/5424#issuecomment-1031832927
Closing in favor of https://github.com/apache/incubator-nuttx/pull/5432
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on pull request #5424: timer: fix operation of periodic timers
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #5424:
URL: https://github.com/apache/incubator-nuttx/pull/5424#issuecomment-1030875859
> @xiaoxiang781216 without my patch I'm getting 40ms always (not between 30ms-40ms) and with my patch I'm getting 30ms always (not between 20ms-30ms).
It's just because you start the POSIX timer at the same time of schedule timer interrupt(30*n in above example).
> If you can describe an situation even theoretical when I can have less than 30ms with my patch, then please describe it.
You can try to start your POSIX timer in a GPIO interrupt handler, toggle the GPIO by hand, and measure the result. In most case, you will see what I describe, because GPIO can happen randomly in one tick period.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 edited a comment on pull request #5424: timer: fix operation of periodic timers
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 edited a comment on pull request #5424:
URL: https://github.com/apache/incubator-nuttx/pull/5424#issuecomment-1030882331
If you are familiar with Linux kernel, two set of API exist:
timer: http://www.makelinux.net/ldd3/chp-7-sect-4.shtml
hrtimer: https://www.kernel.org/doc/html/latest/timers/hrtimers.html
NuttX's wdog is same as Linux's timer from the concept.
If you want to achieve the accuracy as the hardware, the solution I suggest is:
1. Implement the high resolution timer like Linux kernel
2. Implement wdog on top of hrtimer(then you will understand what I said in this thread).
3. Implement POSIX timer and timer fd on top of hrtimer
This is exactly how Linux kernel switch from the tick resolution to the high resolution.
Before this huge work is done, the fast fix is set CONFIG_USEC_PER_TICK to the accuracy you want. Of course, you need to enable tickless(CONFIG_SCHED_TICKLESS) to reduce the schedule timer interrupt loading.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on pull request #5424: timer: fix operation of periodic timers
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #5424:
URL: https://github.com/apache/incubator-nuttx/pull/5424#issuecomment-1030873038
> @xiaoxiang781216 I'm familiar with FreeRTOS delay interface and delays are not a part of this PR. I think that periodic POSIX timer use case that I'm trying to fix is more similar to https://freertos.org/RTOS-software-timer.html than to https://www.freertos.org/vtaskdelayuntil.html But in a proposed example
>
> ```
> clock_t last_time = current_time;
> for (; ; )
> {
> clock_t next_time = last_time + x;
> clock_t current_time = clock_systime_ticks();
> int error= next_time - current_time;
> last_time = next_time;
> delay(x + error);
Here should be `error` not `x + error`.
> }
> ```
>
> if the task is preempted (for example for 1-2 system ticks) right before a `delay` call, then app periodic logic and compensation calculation will be screwed-up. I've been working for a years with mission critical systems and learned that lesson well.
No, in this case you will delay with few ticks(even skip the sleep in case of error <= 0) and then catch up the sched jitter. So you get one tick error at most in each callback for a long run(assume you don't get error < 0).
>
> The only reliable way is to have a timer that posts a periodic event and task that is waiting for that event. Now the question is should it be a MCU HW timer or OS SW timer based on internal OS clock.
It is no difference(of course, I assume that both implementation is right and accuracy) since the internal OS clock is also built on top of one HW timer.
> @xiaoxiang781216 @patacongo please undesrtand me write, the question is not about can I or can not I code a periodic processing in a write way, but about how timers are operating. I could easily implement `board_timerhook()` and post an event to my task each third system tick and get the reliable operation. What I'm trying to say is that if there is not blockers for periodic POSIX timer to expire in time then it should expire in time (for example if I have 10ms system tick and configure POSIX timer to have periodic expiration with 30ms period then timer should expire each 30ms and not each 40ms. I'm not configuring 35ms or 37ms, but 3 multiples of system tick).
If you config your system with 10ms tick and set a 30ms expiration, you will get it fire between 30ms-40ms(any values between them is possible). Even with your patch(#5421), you will get it fire between 20ms-30ms, no difference from the accuracy view. If you set a 30ms period POSIX timer, and always get the callback at 30+10, 60+10, 90+10... 30*n+10, it's a good implementation.
> If you trying to convince me that we should have crappy timers, then I'm very disappointed.
>
If you want to achieve the more accuracy, the only choice is to reduce the tick length. For example, if you set tick to 1ms, you get the sequence as 30+1, 60+1, 90+1...30*n+1.
> How the POSIX timers are implemented, that is another question. Currently they are relying on `wdog` that has it's implemented is a way as it is designed, but if we need to change that to get reliable and well expected operation then I'm fine with that. I feel that we mixed in a discussion timers and `wdog`, so there are multiple threads in this discussion that I want to separate.
>
> I'm porting an application that is running reliably with RT Linux that have same configuration for a system clock and talking honestly I do not see any reason (strong argument) why it should not run the same with NuttX.
>
> Let's focus discussion on POSIX timers only and leave the `wdog` alone. We can have a separate discussion why we need to add one extra tick if `wd_start` is called from `wdog` expiration handler that is actually called from the timer ISR (exactly at the beginning of a phase). I agree with @patacongo that in general `wd_start` can be called at any time and we do not know are we at the beginning or in the middle or an the end of the phase, so `+1` 100% needed in general case, but if we are building a reoccurring event with `wdog` and `wd_start` is called from `wd_timer` (eventually from the system tick ISR) then adding one tick is a bit strange and IMO we just introduce the extra delay in the place where it is really not needed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] patacongo commented on pull request #5424: timer: fix operation of periodic timers
Posted by GitBox <gi...@apache.org>.
patacongo commented on pull request #5424:
URL: https://github.com/apache/incubator-nuttx/pull/5424#issuecomment-1030908195
One thing that I especially dislike about this solution is that introduces a very ugly couple, external logic that uses the wdogs would have special internal knowledge of the implementation of the wdog. That is the worst form of coupling and terrible modular thinking.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 edited a comment on pull request #5424: timer: fix operation of periodic timers
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 edited a comment on pull request #5424:
URL: https://github.com/apache/incubator-nuttx/pull/5424#issuecomment-1030875859
> @xiaoxiang781216 without my patch I'm getting 40ms always (not between 30ms-40ms) and with my patch I'm getting 30ms always (not between 20ms-30ms).
It's just because you start the POSIX timer at the same time of schedule timer interrupt(10*n in above example).
> If you can describe an situation even theoretical when I can have less than 30ms with my patch, then please describe it.
You can try to start your POSIX timer in a GPIO interrupt handler, toggle the GPIO by hand, and measure the result. In most case, you will see what I describe, because GPIO can happen randomly in one tick period.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] patacongo commented on pull request #5424: timer: fix operation of periodic timers
Posted by GitBox <gi...@apache.org>.
patacongo commented on pull request #5424:
URL: https://github.com/apache/incubator-nuttx/pull/5424#issuecomment-1030712544
Subtracting one is not always the right solution. It only works in your case, again because your specific test case is running synchronously with the system time. But that might not always be the case. I do not recommend using this solution.
I provided you with a better, adaptive solution in #5421 that will work in ALL cases. this case, basically one:
- given the last time and desired period, it gets the desired time of the next timer expiration:
`clock_t next_time= last_time + period`
- At the time of the timer expiration, it calculates the error between the desired time and the actual time:
`clock_t error = next_time - clock_systime_ticks();`
- Then it uses that error to correct the time for the next tick:
`clock_t tick = period + error`
This will give you perfect timing regardless of the clock phase.
You cannot defeat quantization errors by adding or subtracting one. That just doesn't work in a robust way.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on pull request #5424: timer: fix operation of periodic timers
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #5424:
URL: https://github.com/apache/incubator-nuttx/pull/5424#issuecomment-1030779882
@pkarashchenko @patacongo 's suggestion is the right direction to avoid the cumulative error in a long run. You can't avoid the cumulative error with the relative timeout, because the code from the timer interrupt happen to the next wd_start need time to finish the execution. You must use the absolute time like above to compensate this gap, otherwise the cumulative error will become bigger and bigger.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pkarashchenko edited a comment on pull request #5424: timer: fix operation of periodic timers
Posted by GitBox <gi...@apache.org>.
pkarashchenko edited a comment on pull request #5424:
URL: https://github.com/apache/incubator-nuttx/pull/5424#issuecomment-1030887123
> If you are familiar with Linux kernel, two set of API exist: timer: http://www.makelinux.net/ldd3/chp-7-sect-4.shtml hrtimer: https://www.kernel.org/doc/html/latest/timers/hrtimers.html NuttX's wdog is same as Linux's timer from the concept. If you want to achieve the accuracy as the hardware, the solution I suggest is:
>
> 1. Implement the high resolution timer like Linux kernel
>
> 2. Implement wdog on top of hrtimer(then you will understand what I said in this thread).
>
> 3. Implement POSIX timer and timer fd on top of hrtimer
>
>
> This is exactly how Linux kernel switch from the tick resolution to the high resolution. Before this huge work is done, the fast fix is set CONFIG_USEC_PER_TICK to the accuracy you want. Of course, you need to enable tickless(CONFIG_SCHED_TICKLESS) to reduce the schedule timer interrupt loading.
No matter what accuracy do I want I'm still having +1 tick with current code. So if I configure `CONFIG_USEC_PER_TICK` to `1000`, then I'm having 11ms on a 10ms timer and that is 10% overhit (this is huge) no matter if I use `CONFIG_SCHED_TICKLESS` or not.
Here is a picture when I start timer from the button ISR:
Here is toggle GPIO as a periodic action. The pulses are exactly 30ms length
I do not understand why proposed changes that improve accuracy almost at 0 cost are pushed back so hard.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] hartmannathan commented on pull request #5424: timer: fix operation of periodic timers
Posted by GitBox <gi...@apache.org>.
hartmannathan commented on pull request #5424:
URL: https://github.com/apache/incubator-nuttx/pull/5424#issuecomment-1031812180
> Do not substract 1 from the delays. It is very bad engineering for one module to make assumptions about the internal design of another module. Please don't do this.
I think there should be a thread on the dev@ mailing list to design the right solution. Once a design is agreed, then let's come back to PRs.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pkarashchenko commented on pull request #5424: timer: fix operation of periodic timers
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on pull request #5424:
URL: https://github.com/apache/incubator-nuttx/pull/5424#issuecomment-1030880794
> It's just because you start the POSIX timer at the same time of schedule timer interrupt(10*n in above example).
It does not matter when I'm starting a timer. This change is not about timer start, but about timer reschedule that is done in SYS Tick ISR handler, so it is always in sync with timer interrupt.
> You can try to start your POSIX timer in a GPIO interrupt handler, toggle the GPIO by hand, and measure the result. In most case, you will see what I describe here, because GPIO can happen randomly in one tick period(0ms-9.9999ms).
I will try, but I can ensure you that it will be the same. But I will make a test with real HW and send you a picture from the logic analyzer.
BTW my programs look like:
```
int periodic_main(int argc, char *argv[])
{
int ret;
timer_t timerid;
struct itimerspec time_value;
struct sigevent ev;
struct siginfo value;
sigset_t set;
ret = timer_create(CLOCK_REALTIME, &ev, &timerid);
if (ret < 0)
{
syslog(LOG_ALERT, "ERROR: timer_create() failed: %d\n", errno);
return EXIT_FAILURE;
}
/* Ignore the default signal action */
signal(SIGALRM, SIG_IGN);
/* Set up a signal to wake-up periodically */
time_value.it_value.tv_sec = 0;
time_value.it_value.tv_nsec = 30000000;
time_value.it_interval.tv_sec = 0;
time_value.it_interval.tv_nsec = 30000000;
ret = timer_settime(timerid, 0, &time_value, NULL);
if (ret < 0)
{
syslog(LOG_ALERT, "ERROR: timer_settime() failed: %d\n", errno);
return EXIT_FAILURE;
}
for (; ; )
{
/* Wait for a signal */
sigemptyset(&set);
sigaddset(&set, SIGALRM);
ret = sigwaitinfo(&set, &value);
if (ret < 0)
{
}
else
{
/* Perform periodic action here */
}
}
return 0;
}
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pkarashchenko commented on pull request #5424: timer: fix operation of periodic timers
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on pull request #5424:
URL: https://github.com/apache/incubator-nuttx/pull/5424#issuecomment-1030874866
If you can describe an situation even theoretical when I can have less than 30ms with my patch, then please describe it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pkarashchenko edited a comment on pull request #5424: timer: fix operation of periodic timers
Posted by GitBox <gi...@apache.org>.
pkarashchenko edited a comment on pull request #5424:
URL: https://github.com/apache/incubator-nuttx/pull/5424#issuecomment-1030713282
I do not fully follow you point. In POSIX timer use case `wd_start` is called from `wd_timer` that is called from `nxsched_process_timer` that is called from systick interrupt, so IMO all phases are synchronized and we do not have quantization error and in this case we do not need extra tick.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on pull request #5424: timer: fix operation of periodic timers
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #5424:
URL: https://github.com/apache/incubator-nuttx/pull/5424#issuecomment-1030882331
If you are familiar with Linux kernel, two set of API exist:
timer: http://www.makelinux.net/ldd3/chp-7-sect-4.shtml
hrtimer: https://www.kernel.org/doc/html/latest/timers/hrtimers.html
NuttX's wdog is same as Linux's timer from the concept.
If you want to achieve the accuracy as the hardware, the solution I suggest is:
1. Implement the high resolution timer like Linux kernel
2. Implement wdog on top of hrtimer(then you will understand what I said in this thread).
3. Implement POSIX timer and timer fd on top of hrtimer
This is exactly how Linux kernel switch from the tick resolution to the high resolution.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pkarashchenko closed pull request #5424: timer: fix operation of periodic timers
Posted by GitBox <gi...@apache.org>.
pkarashchenko closed pull request #5424:
URL: https://github.com/apache/incubator-nuttx/pull/5424
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] patacongo edited a comment on pull request #5424: timer: fix operation of periodic timers
Posted by GitBox <gi...@apache.org>.
patacongo edited a comment on pull request #5424:
URL: https://github.com/apache/incubator-nuttx/pull/5424#issuecomment-1030908195
One thing that I especially dislike about this solution is that introduces a very ugly couple, external logic that uses the wdogs would have special internal knowledge of the implementation of the wdog. That is the worst form of coupling and terrible modular thinking.
This is very bad quality engineering and should not be permitted into the repository.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 edited a comment on pull request #5424: timer: fix operation of periodic timers
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 edited a comment on pull request #5424:
URL: https://github.com/apache/incubator-nuttx/pull/5424#issuecomment-1030873038
> @xiaoxiang781216 I'm familiar with FreeRTOS delay interface and delays are not a part of this PR. I think that periodic POSIX timer use case that I'm trying to fix is more similar to https://freertos.org/RTOS-software-timer.html than to https://www.freertos.org/vtaskdelayuntil.html But in a proposed example
>
> ```
> clock_t last_time = current_time;
> for (; ; )
> {
> clock_t next_time = last_time + x;
> clock_t current_time = clock_systime_ticks();
> int error= next_time - current_time;
> last_time = next_time;
> delay(x + error);
Here should be `error` not `x + error`.
> }
> ```
>
> if the task is preempted (for example for 1-2 system ticks) right before a `delay` call, then app periodic logic and compensation calculation will be screwed-up. I've been working for a years with mission critical systems and learned that lesson well.
No, in this case you will delay with few ticks(even skip the sleep in case of error <= 0) and then catch up the sched jitter. So you get one tick error at most in each callback for a long run(assume you don't get error < 0).
>
> The only reliable way is to have a timer that posts a periodic event and task that is waiting for that event. Now the question is should it be a MCU HW timer or OS SW timer based on internal OS clock.
It is no difference(of course, I assume that both implementation is right and accuracy) since the internal OS clock is also built on top of one HW timer.
> @xiaoxiang781216 @patacongo please undesrtand me write, the question is not about can I or can not I code a periodic processing in a write way, but about how timers are operating. I could easily implement `board_timerhook()` and post an event to my task each third system tick and get the reliable operation. What I'm trying to say is that if there is not blockers for periodic POSIX timer to expire in time then it should expire in time (for example if I have 10ms system tick and configure POSIX timer to have periodic expiration with 30ms period then timer should expire each 30ms and not each 40ms. I'm not configuring 35ms or 37ms, but 3 multiples of system tick).
If you config your system with 10ms tick and set a 30ms expiration, you will get it fire between 30ms-40ms(any values between them is possible). Even with your patch(#5421), you will get it fire between 20ms-30ms, no difference from the accuracy view. If you set a 30ms period POSIX timer, and always get the callback at 30+10, 60+10, 90+10... 30*n+10, it's a good implementation.
> If you trying to convince me that we should have crappy timers, then I'm very disappointed.
>
If you want to achieve the more accuracy, the only choice is to reduce the tick length. For example, if you set tick to 1ms, you get the sequence as 30+1, 60+1, 90+1...30*n+1.
> How the POSIX timers are implemented, that is another question. Currently they are relying on `wdog` that has it's implemented is a way as it is designed
If POSIX timer can achieve that each callback happen between `[n*period, n*period+tick)`, you can't do better since wdog can only achieve this accuracy.
>, but if we need to change that to get reliable and well expected operation then I'm fine with that. I feel that we mixed in a discussion timers and `wdog`, so there are multiple threads in this discussion that I want to separate.
If the accuracy of wdog doesn't match your expectation, we have to redesign the wdog(the interface may even change to not use the tick as unit), but it's impossible to fix the problem by simply add or sub one tick.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 edited a comment on pull request #5424: timer: fix operation of periodic timers
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 edited a comment on pull request #5424:
URL: https://github.com/apache/incubator-nuttx/pull/5424#issuecomment-1030873038
> @xiaoxiang781216 I'm familiar with FreeRTOS delay interface and delays are not a part of this PR. I think that periodic POSIX timer use case that I'm trying to fix is more similar to https://freertos.org/RTOS-software-timer.html than to https://www.freertos.org/vtaskdelayuntil.html But in a proposed example
>
> ```
> clock_t last_time = current_time;
> for (; ; )
> {
> clock_t next_time = last_time + x;
> clock_t current_time = clock_systime_ticks();
> int error= next_time - current_time;
> last_time = next_time;
> delay(x + error);
Here should be `error` not `x + error`.
> }
> ```
>
> if the task is preempted (for example for 1-2 system ticks) right before a `delay` call, then app periodic logic and compensation calculation will be screwed-up. I've been working for a years with mission critical systems and learned that lesson well.
No, in this case you will delay with few ticks(even skip the sleep in case of error <= 0) and then catch up the sched jitter. So you get one tick error at most in each callback for a long run(assume you don't get error < 0).
>
> The only reliable way is to have a timer that posts a periodic event and task that is waiting for that event. Now the question is should it be a MCU HW timer or OS SW timer based on internal OS clock.
It is no difference(of course, I assume that both implementation is right and accuracy) since the internal OS clock is also built on top of one HW timer.
> @xiaoxiang781216 @patacongo please undesrtand me write, the question is not about can I or can not I code a periodic processing in a write way, but about how timers are operating. I could easily implement `board_timerhook()` and post an event to my task each third system tick and get the reliable operation. What I'm trying to say is that if there is not blockers for periodic POSIX timer to expire in time then it should expire in time (for example if I have 10ms system tick and configure POSIX timer to have periodic expiration with 30ms period then timer should expire each 30ms and not each 40ms. I'm not configuring 35ms or 37ms, but 3 multiples of system tick).
If you config your system with 10ms tick and set a 30ms expiration, you will get it fire between 30ms-40ms(any values between them is possible). Even with your patch(#5421), you will get it fire between 20ms-30ms, no difference from the accuracy view. If you set a 30ms period POSIX timer, and always get the callback at 30+10, 60+10, 90+10... 30*n+10, it's a good implementation.
> If you trying to convince me that we should have crappy timers, then I'm very disappointed.
>
If you want to achieve the more accuracy, the only choice is to reduce the tick length. For example, if you set tick to 1ms, you get the sequence as 30+1, 60+1, 90+1...30*n+1.
> How the POSIX timers are implemented, that is another question. Currently they are relying on `wdog` that has it's implemented is a way as it is designed
If POSIX timer can achieve that each callback happen between [n*period, n*period+tick), you can't do better since wdog can only achieve this accuracy.
>, but if we need to change that to get reliable and well expected operation then I'm fine with that. I feel that we mixed in a discussion timers and `wdog`, so there are multiple threads in this discussion that I want to separate.
If the accuracy of wdog doesn't match your expectation, we have to redesign the wdog(the interface may even change to not use the tick as unit). It's impossible to fix the problem by add or sub one tick.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pkarashchenko commented on pull request #5424: timer: fix operation of periodic timers
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on pull request #5424:
URL: https://github.com/apache/incubator-nuttx/pull/5424#issuecomment-1030863068
@xiaoxiang781216 I'm familiar with FreeRTOS delay interface and delays are not a part of this PR. I think that periodic POSIX timer use case that I'm trying to fix is more similar to https://freertos.org/RTOS-software-timer.html than to https://www.freertos.org/vtaskdelayuntil.html
But in a proposed example
```
clock_t last_time = current_time;
for (; ; )
{
clock_t next_time = last_time + x;
clock_t current_time = clock_systime_ticks();
int error= next_time - current_time;
last_time = next_time;
delay(x + error);
}
```
if the task is preempted (for example for 1-2 system ticks) right before a `delay` call, then app periodic logic and compensation calculation will be screwed-up. I've been working for a years with mission critical systems and learned that lesson well.
The only reliable way is to have a timer that posts a periodic event and task that is waiting for that event. Now the question is should it be a MCU HW timer or OS SW timer based on internal OS clock. @xiaoxiang781216 @patacongo please undesrtand me write, the question is not about can I or can not I code a periodic processing in a write way, but about how timers are operating. I could easily implement `board_timerhook()` and post an event to my task each third system tick and get the reliable operation. What I'm trying to say is that if there is not blockers for periodic POSIX timer to expire in time then it should expire in time (for example if I have 10ms system tick and configure POSIX timer to have periodic expiration with 30ms period then timer should expire each 30ms and not each 40ms. I'm not configuring 35ms or 37ms, but 3 multiples of system tick). If you trying to convince me that we should have crappy timers, then I'm very disappointed.
How the POSIX timers are implemented, that is another question. Currently they are relying on `wdog` that has it's implemented is a way as it is designed, but if we need to change that to get reliable and well expected operation then I'm fine with that. I feel that we mixed in a discussion timers and `wdog`, so there are multiple threads in this discussion that I want to separate.
I'm porting an application that is running reliably with RT Linux that have same configuration for a system clock and talking honestly I do not see any reason (strong argument) why it should not run the same with NuttX.
Let's focus discussion on POSIX timers only and leave the `wdog` alone. We can have a separate discussion why we need to add one extra tick if `wd_start` is called from `wdog` expiration handler that is actually called from the timer ISR (exactly at the beginning of a phase). I agree with @patacongo that in general `wd_start` can be called at any time and we do not know are we at the beginning or in the middle or an the end of the phase, so `+1` 100% needed in general case, but if we are building a reoccurring event with `wdog` and `wd_start` is called from `wd_timer` (eventually from the system tick ISR) then adding one tick is a bit strange and IMO we just introduce the extra delay in the place where it is really not needed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pkarashchenko commented on pull request #5424: timer: fix operation of periodic timers
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on pull request #5424:
URL: https://github.com/apache/incubator-nuttx/pull/5424#issuecomment-1030888258
I'm looking for arguments like: "This change leads to such bug (inconsistent behavior) under next circumstances".
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pkarashchenko commented on pull request #5424: timer: fix operation of periodic timers
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on pull request #5424:
URL: https://github.com/apache/incubator-nuttx/pull/5424#issuecomment-1030714389
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 edited a comment on pull request #5424: timer: fix operation of periodic timers
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 edited a comment on pull request #5424:
URL: https://github.com/apache/incubator-nuttx/pull/5424#issuecomment-1030875859
> @xiaoxiang781216 without my patch I'm getting 40ms always (not between 30ms-40ms) and with my patch I'm getting 30ms always (not between 20ms-30ms).
It's just because you start the POSIX timer at the same time of schedule timer interrupt(10*n in above example).
> If you can describe an situation even theoretical when I can have less than 30ms with my patch, then please describe it.
You can try to start your POSIX timer in a GPIO interrupt handler, toggle the GPIO by hand, and measure the result. In most case, you will see what I describe, because GPIO can happen randomly in one tick period(0ms-9.9999ms).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pkarashchenko commented on pull request #5424: timer: fix operation of periodic timers
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on pull request #5424:
URL: https://github.com/apache/incubator-nuttx/pull/5424#issuecomment-1030874096
@xiaoxiang781216 without my patch I'm getting 40ms always (not between 30ms-40ms) and with my patch I'm getting 30ms always (not between 20ms-30ms).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pkarashchenko commented on pull request #5424: timer: fix operation of periodic timers
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on pull request #5424:
URL: https://github.com/apache/incubator-nuttx/pull/5424#issuecomment-1030901325
> I'm not pushing back; I am thinking about what @patacongo wrote (also at [here](https://cwiki.apache.org/confluence/display/NUTTX/Short+Time+Delays)) about the API contract being to wait at least the given time and not knowing whether the (in that case delay, but in other cases non-delay usage) is starting in phase with the tick timer or not. I also see what you're saying about the timing being off by (in this case) 10%, which also seems wrong. I'm not sure what to suggest here. All I know if what I've always done: since software execution is sequential in nature and (with compiled code + multiple threads + CPU caches + other factors) it is not possible to predict what its timing will be, so my go-to solution is a hardware timer when I care about timing and software timers when all I want is to ensure that some time passes without caring too much about accuracy. I appreciate that this is not a good solution for everyone.
I really believe that this change does not contradict with anything that is written in https://cwiki.apache.org/confluence/display/NUTTX/Short+Time+Delays
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 edited a comment on pull request #5424: timer: fix operation of periodic timers
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 edited a comment on pull request #5424:
URL: https://github.com/apache/incubator-nuttx/pull/5424#issuecomment-1030875859
> @xiaoxiang781216 without my patch I'm getting 40ms always (not between 30ms-40ms) and with my patch I'm getting 30ms always (not between 20ms-30ms).
It's just because you start the POSIX timer at the same time of schedule timer interrupt(10*n in above example).
> If you can describe an situation even theoretical when I can have less than 30ms with my patch, then please describe it.
You can try to start your POSIX timer in a GPIO interrupt handler, toggle the GPIO by hand, and measure the result. In most case, you will see what I describe here, because GPIO can happen randomly in one tick period(0ms-9.9999ms).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 closed pull request #5424: timer: fix operation of periodic timers
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 closed pull request #5424:
URL: https://github.com/apache/incubator-nuttx/pull/5424
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] patacongo edited a comment on pull request #5424: timer: fix operation of periodic timers
Posted by GitBox <gi...@apache.org>.
patacongo edited a comment on pull request #5424:
URL: https://github.com/apache/incubator-nuttx/pull/5424#issuecomment-1030908195
One thing that I especially dislike about this solution is that introduces a very ugly couple, external logic that uses the wdogs would have special internal knowledge of the implementation of the wdog. That is the worst form of coupling and terrible modular thinking.
This is very bad quality engineering and should not be permitted into the repository. It ONLY works because of the particular timing you are using. it is not a general solution.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] patacongo commented on a change in pull request #5424: timer: fix operation of periodic timers
Posted by GitBox <gi...@apache.org>.
patacongo commented on a change in pull request #5424:
URL: https://github.com/apache/incubator-nuttx/pull/5424#discussion_r800914044
##########
File path: fs/vfs/fs_timerfd.c
##########
@@ -531,7 +531,11 @@ static void timerfd_timeout(wdparm_t idev)
if (dev->delay)
{
- wd_start(&dev->wdog, dev->delay, timerfd_timeout, idev);
+ /* We have to subtract 1 tick because wd_start() will add 1 tick
+ * unconditionally
+ */
+
+ wd_start(&dev->wdog, dev->delay - 1, timerfd_timeout, idev);
}
Review comment:
Same comment as before. This is horrible. You cannot use internal knowledge of the implementation inside of wd_start.(). This is not proper modular design.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] hartmannathan commented on pull request #5424: timer: fix operation of periodic timers
Posted by GitBox <gi...@apache.org>.
hartmannathan commented on pull request #5424:
URL: https://github.com/apache/incubator-nuttx/pull/5424#issuecomment-1030898525
I'm not pushing back; I am thinking about what @patacongo wrote (also at [here](https://cwiki.apache.org/confluence/display/NUTTX/Short+Time+Delays)) about the API contract being to wait at least the given time and not knowing whether the (in that case delay, but in other cases non-delay usage) is starting in phase with the tick timer or not. I also see what you're saying about the timing being off by (in this case) 10%, which also seems wrong. I'm not sure what to suggest here. All I know if what I've always done: since software execution is sequential in nature and (with compiled code + multiple threads + CPU caches + other factors) it is not possible to predict what its timing will be, so my go-to solution is a hardware timer when I care about timing and software timers when all I want is to ensure that some time passes without caring too much about accuracy. I appreciate that this is not a good solution for everyone.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 edited a comment on pull request #5424: timer: fix operation of periodic timers
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 edited a comment on pull request #5424:
URL: https://github.com/apache/incubator-nuttx/pull/5424#issuecomment-1030873038
> @xiaoxiang781216 I'm familiar with FreeRTOS delay interface and delays are not a part of this PR. I think that periodic POSIX timer use case that I'm trying to fix is more similar to https://freertos.org/RTOS-software-timer.html than to https://www.freertos.org/vtaskdelayuntil.html But in a proposed example
>
> ```
> clock_t last_time = current_time;
> for (; ; )
> {
> clock_t next_time = last_time + x;
> clock_t current_time = clock_systime_ticks();
> int error= next_time - current_time;
> last_time = next_time;
> delay(x + error);
Here should be `error` not `x + error`.
> }
> ```
>
> if the task is preempted (for example for 1-2 system ticks) right before a `delay` call, then app periodic logic and compensation calculation will be screwed-up. I've been working for a years with mission critical systems and learned that lesson well.
No, in this case you will delay with few ticks(even skip the sleep in case of error <= 0) and then catch up the sched jitter. So you get one tick error at most in each callback for a long run(assume you don't get error < 0).
>
> The only reliable way is to have a timer that posts a periodic event and task that is waiting for that event. Now the question is should it be a MCU HW timer or OS SW timer based on internal OS clock.
It is no difference(of course, I assume that both implementation is right and accuracy) since the internal OS clock is also built on top of one HW timer.
> @xiaoxiang781216 @patacongo please undesrtand me write, the question is not about can I or can not I code a periodic processing in a write way, but about how timers are operating. I could easily implement `board_timerhook()` and post an event to my task each third system tick and get the reliable operation. What I'm trying to say is that if there is not blockers for periodic POSIX timer to expire in time then it should expire in time (for example if I have 10ms system tick and configure POSIX timer to have periodic expiration with 30ms period then timer should expire each 30ms and not each 40ms. I'm not configuring 35ms or 37ms, but 3 multiples of system tick).
If you config your system with 10ms tick and set a 30ms expiration, you will get it fire between 30ms-40ms(any values between them is possible). Even with your patch(#5421), you will get it fire between 20ms-30ms, no difference from the accuracy view. If you set a 30ms period POSIX timer, and always get the callback at 30+10, 60+10, 90+10... 30*n+10, it's a good implementation.
> If you trying to convince me that we should have crappy timers, then I'm very disappointed.
>
If you want to achieve the more accuracy, the only choice is to reduce the tick length. For example, if you set tick to 1ms, you get the sequence as 30+1, 60+1, 90+1...30*n+1.
> How the POSIX timers are implemented, that is another question. Currently they are relying on `wdog` that has it's implemented is a way as it is designed
If POSIX timer can achieve that each callback happen between `[n*period, n*period+tick)`, you can't do better since wdog can only achieve this accuracy.
>, but if we need to change that to get reliable and well expected operation then I'm fine with that. I feel that we mixed in a discussion timers and `wdog`, so there are multiple threads in this discussion that I want to separate.
If the accuracy of wdog doesn't match your expectation, we have to redesign the wdog(the interface may even change to not use the tick as unit). It's impossible to fix the problem by add or sub one tick.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pkarashchenko commented on pull request #5424: timer: fix operation of periodic timers
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on pull request #5424:
URL: https://github.com/apache/incubator-nuttx/pull/5424#issuecomment-1030887123
> If you are familiar with Linux kernel, two set of API exist: timer: http://www.makelinux.net/ldd3/chp-7-sect-4.shtml hrtimer: https://www.kernel.org/doc/html/latest/timers/hrtimers.html NuttX's wdog is same as Linux's timer from the concept. If you want to achieve the accuracy as the hardware, the solution I suggest is:
>
> 1. Implement the high resolution timer like Linux kernel
>
> 2. Implement wdog on top of hrtimer(then you will understand what I said in this thread).
>
> 3. Implement POSIX timer and timer fd on top of hrtimer
>
>
> This is exactly how Linux kernel switch from the tick resolution to the high resolution. Before this huge work is done, the fast fix is set CONFIG_USEC_PER_TICK to the accuracy you want. Of course, you need to enable tickless(CONFIG_SCHED_TICKLESS) to reduce the schedule timer interrupt loading.
No matter what accuracy do I want I'm still having +1 tick with current code. So if I configure `CONFIG_USEC_PER_TICK` to `1000`, then I'm having 11ms on a 10ms timer and that is 10% overhit (this is huge) no matter if I use `CONFIG_SCHED_TICKLESS` or not.
Here is a picture when I start timer from the button ISR:
Here is toggle GPIO as a periodic action.
I do not understand why proposed changes that improve accuracy almost as 0 cost are pushed back so hard.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pkarashchenko edited a comment on pull request #5424: timer: fix operation of periodic timers
Posted by GitBox <gi...@apache.org>.
pkarashchenko edited a comment on pull request #5424:
URL: https://github.com/apache/incubator-nuttx/pull/5424#issuecomment-1030887123
> If you are familiar with Linux kernel, two set of API exist: timer: http://www.makelinux.net/ldd3/chp-7-sect-4.shtml hrtimer: https://www.kernel.org/doc/html/latest/timers/hrtimers.html NuttX's wdog is same as Linux's timer from the concept. If you want to achieve the accuracy as the hardware, the solution I suggest is:
>
> 1. Implement the high resolution timer like Linux kernel
>
> 2. Implement wdog on top of hrtimer(then you will understand what I said in this thread).
>
> 3. Implement POSIX timer and timer fd on top of hrtimer
>
>
> This is exactly how Linux kernel switch from the tick resolution to the high resolution. Before this huge work is done, the fast fix is set CONFIG_USEC_PER_TICK to the accuracy you want. Of course, you need to enable tickless(CONFIG_SCHED_TICKLESS) to reduce the schedule timer interrupt loading.
No matter what accuracy do I want I'm still having +1 tick with current code. So if I configure `CONFIG_USEC_PER_TICK` to `1000`, then I'm having 11ms on a 10ms timer and that is 10% overhit (this is huge) no matter if I use `CONFIG_SCHED_TICKLESS` or not.
Here is a picture when I start timer from the button ISR:
Here is toggle GPIO as a periodic action. The pulses are exactly 30ms lenght
I do not understand why proposed changes that improve accuracy almost as 0 cost are pushed back so hard.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pkarashchenko edited a comment on pull request #5424: timer: fix operation of periodic timers
Posted by GitBox <gi...@apache.org>.
pkarashchenko edited a comment on pull request #5424:
URL: https://github.com/apache/incubator-nuttx/pull/5424#issuecomment-1030887123
> If you are familiar with Linux kernel, two set of API exist: timer: http://www.makelinux.net/ldd3/chp-7-sect-4.shtml hrtimer: https://www.kernel.org/doc/html/latest/timers/hrtimers.html NuttX's wdog is same as Linux's timer from the concept. If you want to achieve the accuracy as the hardware, the solution I suggest is:
>
> 1. Implement the high resolution timer like Linux kernel
>
> 2. Implement wdog on top of hrtimer(then you will understand what I said in this thread).
>
> 3. Implement POSIX timer and timer fd on top of hrtimer
>
>
> This is exactly how Linux kernel switch from the tick resolution to the high resolution. Before this huge work is done, the fast fix is set CONFIG_USEC_PER_TICK to the accuracy you want. Of course, you need to enable tickless(CONFIG_SCHED_TICKLESS) to reduce the schedule timer interrupt loading.
No matter what accuracy do I want I'm still having +1 tick with current code. So if I configure `CONFIG_USEC_PER_TICK` to `1000`, then I'm having 11ms on a 10ms timer and that is 10% overhit (this is huge) no matter if I use `CONFIG_SCHED_TICKLESS` or not.
Here is a picture when I start timer from the button ISR:
Here is toggle GPIO as a periodic action. The pulses are exactly 30ms length
I do not understand why proposed changes that improve accuracy almost as 0 cost are pushed back so hard.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on pull request #5424: timer: fix operation of periodic timers
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #5424:
URL: https://github.com/apache/incubator-nuttx/pull/5424#issuecomment-1030781813
Maybe, you can read vTaskDelay and vTaskDelayUntil from FreeRTOS to get the ideal:
https://www.freertos.org/a00127.html
https://www.freertos.org/vtaskdelayuntil.html
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pkarashchenko edited a comment on pull request #5424: timer: fix operation of periodic timers
Posted by GitBox <gi...@apache.org>.
pkarashchenko edited a comment on pull request #5424:
URL: https://github.com/apache/incubator-nuttx/pull/5424#issuecomment-1030880794
> It's just because you start the POSIX timer at the same time of schedule timer interrupt(10*n in above example).
It does not matter when I'm starting a timer. This change is not about timer start, but about timer reschedule that is done in SYS Tick ISR handler, so it is always in sync with timer interrupt.
> You can try to start your POSIX timer in a GPIO interrupt handler, toggle the GPIO by hand, and measure the result. In most case, you will see what I describe here, because GPIO can happen randomly in one tick period(0ms-9.9999ms).
I will try, but I can ensure you that it will be the same. But I will make a test with real HW and send you a picture from the logic analyzer.
BTW my programs look like:
```
int periodic_main(int argc, char *argv[])
{
int ret;
timer_t timerid;
struct itimerspec time_value;
struct sigevent ev;
struct siginfo value;
sigset_t set;
ret = timer_create(CLOCK_REALTIME, &ev, &timerid);
if (ret < 0)
{
syslog(LOG_ALERT, "ERROR: timer_create() failed: %d\n", errno);
return EXIT_FAILURE;
}
/* Ignore the default signal action */
signal(SIGALRM, SIG_IGN);
/* Set up a signal to wake-up periodically */
time_value.it_value.tv_sec = 0;
time_value.it_value.tv_nsec = 30000000;
time_value.it_interval.tv_sec = 0;
time_value.it_interval.tv_nsec = 30000000;
ret = timer_settime(timerid, 0, &time_value, NULL);
if (ret < 0)
{
syslog(LOG_ALERT, "ERROR: timer_settime() failed: %d\n", errno);
return EXIT_FAILURE;
}
for (; ; )
{
/* Wait for a signal */
sigemptyset(&set);
sigaddset(&set, SIGALRM);
ret = sigwaitinfo(&set, &value);
if (ret < 0)
{
}
else
{
/* Perform periodic action here */
}
}
return 0;
}
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pkarashchenko commented on pull request #5424: timer: fix operation of periodic timers
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on pull request #5424:
URL: https://github.com/apache/incubator-nuttx/pull/5424#issuecomment-1030908414
> One thing that I especially dislike about this solution is that introduces a very ugly couple, external logic that uses the wdogs would have special internal knowledge of the implementation of the wdog. That is the worst form of coupling and terrible modular thinking.
I will do an alternative PR with other solution in wdog
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org