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

[Henrik Maier]

Not a solution to the problem. This code also suffers from the same rounding
issue as the AVR code. The only difference is that is deals better with the
overflow.

Example: 
   ms = 2
   NutGetTickClock = 997

Result is: 1, a 50 % error.

And this error is avoidable by applying standard rounding procedures.

Henrik

> -----Original Message-----
> From: en-nut-discussion-bounces at egnite.de [mailto:en-nut-discussion-
> bounces at egnite.de] On Behalf Of ennut at thezachariasgroup.com
> Sent: Tuesday, 15 July 2008 10:49 PM
> To: Ethernut User Chat (English)
> Subject: Re: [En-Nut-Discussion] NutGetTickClock rounding (again)
> 
>    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
> 
> _______________________________________________
> http://lists.egnite.de/mailman/listinfo/en-nut-discussion