code review: help implementing clock_montonic

[zack_on_the_speed_chanel]

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).


I did it using rtems subtract function.


 rtems_timespec_subtract (now, expire, result);

where now expire and  result are all timespec

_TOD_Get(now); or _Timecounter_Nanouptime(now );

and expire is
rtems_timespec_from_ticks (ptimer->Timer.expire,expire );


also i'm getting this compiler warning I don't know if i have to resolve it, I get that my timespec variable may not be initialized.
 warning: 'result' may be used uninitialized in this function [-Wmaybe-uninitialized]

>The monotonic clock cannot be set.

Were you talking about the  fact that the monotonic clock only can go up in time?
Yes, settime is an important function to distinguish between MONOTONIC
> > > and REALTIME cases. That is going to be tricky to think through.
> > >

i don't understand what you mean since i have added a clock type field to the POSIX_TIMER_CONTROL


Thanks
Zack


‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐
On Thursday, May 13, 2021 3:05 PM, Gedare Bloom <gedare at rtems.org> wrote:

> 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