[En-Nut-Discussion] NutGetTickClock rounding (again)

[ennut at thezachariasgroup.com]

   In the ARM arch the following code works well:

/*!
  * \brief Calculate system ticks for a given number of milliseconds.
  */
u_long NutTimerMillisToTicks(u_long ms) {
     if (ms >= 0x3E8000) {
       return (ms / 1000) * NutGetTickClock();
     }
     else {
       return (ms * NutGetTickClock()) / 1000;
     }
}


At 07:11 AM 7/15/2008, you wrote:
>Bernhard,
>
> > - If 'ms' is 'big', and if the compiler evaluates the multiplication
>first,
> > then an overflow can happen.
>
>The problem with the current implementation is not an overflow issue but a
>rounding issue. It is simply a matter of applying proper rounding to an
>integer division. An overflow issue only occurs if "ms" is > 49.7 days (for
>a typical NutGetTickClock of ~1000).
>
>However it is probably a good suggestion to document the potential overflow
>for NutTimerMillisToTicks and add a caveat to its documentation.
>
>However my concern with function NutTimerMillisToTicks is small values for
>the "ms" parameter, values between 1 and 10 ms for example.
>
> > - since NutGetTickClock() gives a result in 'ticks per Hertz' then it
>seems
> > more appropriate to reduce 'ms' to a value in seconds before the next
> > calculation.
>
>In theory this is correct, but as we don't have floating point math,
>impracticable.
>
>Your implementation would return 1 for all ms values < 1000 as anything
>smaller than 1000 divided by 1000 equals to 0 in integer land.
>
>Henrik
>
> > -----Original Message-----
> > From: en-nut-discussion-bounces at egnite.de [mailto:en-nut-discussion-
> > bounces at egnite.de] On Behalf Of Bernard Fouché
> > Sent: Tuesday, 15 July 2008 7:04 PM
> > To: Ethernut User Chat (English)
> > Subject: Re: [En-Nut-Discussion] NutGetTickClock rounding (again)
> >
> > What about:
> >
> > u_long NutTimerMillisToTicks(u_long ms)
> > {
> >     u_long x;
> >
> >     x = ( ms/1000UL ) * NutGetTickClock();
> >     if (x == 0) {
> >         x = 1;
> >     }
> >
> >     return (x);
> > }
> >
> > This because:
> >
> > - If 'ms' is 'big', and if the compiler evaluates the multiplication
>first,
> > then an overflow can happen.
> >
> > - since NutGetTickClock() gives a result in 'ticks per Hertz' then it
>seems
> > more appropriate to reduce 'ms' to a value in seconds before the next
> > calculation.
> >
> >     Bernard
> >
> > Henrik Maier wrote:
> > > There has be some discussion about a better implementation and bug-fix
>of
> > > NutTimerMillisToTicks (refer to posts in 2006, subject "Request for
> > Comment:
> > > NutTimerMillisToTicks()").
> > >
> > > This has been applied in recent Nut/OS releases (>4.2.1 onwards).
> > >
> > > However the current implementation does only round for the special case
> > when
> > > the result would be 0 (see if clause below). However for other cases
>with
> > > small ms values it does not round at all and we end up with large errors
> > > (50%) for small ms values in cases where NutGetTickClock() is just below
> > > 1000, for example 997. For clock frequencies where NutGetTickClock() is
> > > greater than 1000 it does not matter but for cases where it is less than
> > > 1000 it results in unnecessary inaccuracies.
> > >
> > > I came across this issue when recently upgrading from Nut/OS 4.2.1 to
> > 4.4.1
> > > and some of my time-out calculations did not work anymore as they were
> > too
> > > short.
> > >
> > >
> > > Current implementation:
> > > -----------------------
> > >
> > > u_long NutTimerMillisToTicks(u_long ms)
> > > {
> > >     u_long x;
> > >
> > >     x = ms * NutGetTickClock() / 1000UL;
> > >     if (x == 0) {
> > >         x = 1;
> > >     }
> > >
> > >     return (x);
> > > }
> > >
> > > [..snip..]
> > >
> > > Suggested implementation:
> > > -------------------------
> > > u_long NutTimerMillisToTicks(u_long ms)
> > > {
> > >     return (ms * NutGetTickClock() + 500) / 1000UL;
> > > }
> > >
> > _______________________________________________
> > http://lists.egnite.de/mailman/listinfo/en-nut-discussion
>
>_______________________________________________
>http://lists.egnite.de/mailman/listinfo/en-nut-discussion