code review: help implementing clock_montonic

[Gedare Bloom]

On Wed, May 12, 2021 at 2:42 PM zack_on_the_speed_chanel
<zack_on_the_speed_chanel at protonmail.ch> wrote:
>
> > This ticket mostly references the need for a test. Have you tried to
> > write a test for the missing functionality?
> >
> How I made a test for it was to create the timer using timer_create() . It also said to use a previous test with timer_realtime and i tested it with  sometime similar to PSXtimer02 test. I  got a invalid argument error when I tried it. Joel told me i also have to impliment the monotonic clock
>
>
> >his math doesn't make sense to me. explain what you're trying to do?
>
> I wanted calculate the remaining time with the timespec. I assumed that the timespec contained the total time seconds+miliseconds. I knew that a cpu tick is an uint32. So i converted to timspec
>
>       remaining = (uint32_t) ( ptimer->Timer.expire - now );
>
> to something like
> expire->tv.nsec+expire->tv.sec  - spec->tv.nsec+spec->tv.sec);
>

This doesn't work. Example:
now = 0s 999999999ns
expire = 1s 0ns

expire - now = 1ns
expire.nsec + expire.sec - now.nsec + now.sec = -999999999 cast to
uint32_t is 3294967297ns or so.

Use helper functions in rtems/timespec.h or score/timespec.h
(depending where you implement your code, in this case, you probably
use the score services).

>
> > > Yes, settime is an important function to distinguish between MONOTONIC
> > and REALTIME cases. That is going to be tricky to think through.
>
> Where do you want to check the clock type. I though I only need to distinguish between both when getting the value of the clock. I don't exactly know what needs to be done me being really new to posix and operating system development.
>
The monotonic clock cannot be set.

>
>
> Zack
>
>
> ‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐
> On Tuesday, May 11, 2021 6:23 AM, Gedare Bloom <gedare at rtems.org> wrote:
>
> > On Fri, May 7, 2021 at 12:53 PM zack_on_the_speed_chanel
> > zack_on_the_speed_chanel at protonmail.ch wrote:
> >
> > > hello,
> > > Currenttly i'm trying to implement the clock_monotonic which was part of ticket #3889. So far these are the changes are as follows
> >
> > This ticket mostly references the need for a test. Have you tried to
> > write a test for the missing functionality?
> >
> > > ptimer->clock_type= &clock_id;
> > > in the timer create i added a field for the timer id and saved it when the timer was made. Then in getttime I used the appropriate timers to get the time.
> > > for example.
> > > if (ptimer->clock_type ==CLOCK_REALTIME) {
> > > Per_CPU_Control *cpu;
> > > struct timespec spec;
> > > struct timespec expire;
> > >
> > >     cpu = _POSIX_Timer_Acquire_critical( ptimer, &lock_context );
> > >
> >
> > Maybe move the clock_type check to here?
> >
> > >     _TOD_Get(spec);
> > >       _Timespec_From_ticks( expire, &ptimer->Timer.expire);
> > >
> > >
> > >     if (spec->tv.nsec+spec->tv.sec > expire->tv.nsec+expire->tv.sec ) {
> > >
> > >       remaining = (uint32_t) (expire->tv.nsec+expire->tv.sec  - spec->tv.nsec+spec->tv.sec);
> > >
> >
> > this math doesn't make sense to me. explain what you're trying to do?
> >
> > >     } else {
> > >       remaining = 0;
> > >     }
> > >
> > >     _Timespec_From_ticks( remaining, &value->it_value );
> > >     value->it_interval = ptimer->timer_data.it_interval;
> > >
> > >     _POSIX_Timer_Release( cpu, &lock_context );
> > >     return 0;
> > >
> > >
> > > }
> > > Here i made two separate cases for getting the time. Also joel told me to I feel that it is a bit redundant to call everything using timespec to calculating remaining time and then just use timespec_From_ticks to get the time. It was a recommendation from Joel. Did I make the correct change. Also I assume that Delete does not need any changes because It does not require any measuring of time. Is there anything i need to change for timer_settime?
> >
> > Yes, settime is an important function to distinguish between MONOTONIC
> > and REALTIME cases. That is going to be tricky to think through.
> >
> > > Thanks
> > > Zack
> > >
> > > devel mailing list
> > > devel at rtems.org
> > > http://lists.rtems.org/mailman/listinfo/devel
>
>