[IGSTK-Developers] Clock and TimeStamp problem

[Luis Ibanez]

Hi Patrick,

Thanks for listing the modifications on the time units.

A bug was added to the bugtracker regarding the need
for adding the string "InMilliseconds" to the names
of methods that expect time values.

Since we still have some concerns regarding the risk
of mixing up the concepts, the solution is to use the
natural type safety of C++. We should replace the use
of plain "doubles" for representing Time, and should
replace it with a "Time" type and a "TimeLapse" types.

TimeLapse will represent the difference between two
"Time", just like "vectors" are the difference between
two "Points".  By using this type mechanism, we can
mitigate the risk that Patrick pointed out:

    transform1.SetToIdentity(
         transform2.GetExpirationTime() )


This operation will become impossible, because the
compiler will be expecting a type "TimeLapse" as the
argument of the "SetToIdentity()" method, while the
method "GetExpirationTime()" will be returning a type
"Time".


Counting on application developers to do the right
thing is not safe enough:

          "Hope is not a strategy"


We should enforce the concepts by using C++ mechanisms,
and by taking applying "safety by design".



    Luis



----------------------
Patrick Cheng wrote:
> This is the major changes:
> 
> IGSTKSandbox: IGSTK/Source/igstkRealTimeClock.cxx
> Change the returned time unit to milliseconds
> http://public.kitware.com/cgi-bin/viewcvs.cgi/IGSTK/Source/igstkRealTimeClock.cxx?root=IGSTKSandbox&r1=1.1&r2=1.2 
> 
> 
> IGSTKSandbox: IGSTK/Source/igstkPulseGenerator.cxx
> The time period unit for time out is changed into milliseconds
> http://public.kitware.com/cgi-bin/viewcvs.cgi/IGSTK/Source/igstkPulseGenerator.cxx?root=IGSTKSandbox&r1=1.1&r2=1.2 
> 
> 
> IGSTKSandbox: IGSTK/Testing/igstkViewRefreshRateTest.cxx
> http://public.kitware.com/cgi-bin/viewcvs.cgi/IGSTK/Testing/igstkViewRefreshRateTest.cxx?root=IGSTKSandbox&r1=1.1&r2=1.2 
> 
> The problem with this test is: The testing frequency is too high 
> (30&60Hz) for it to pass (I lower it to 20&40Hz)
> 
> One place we need to pay attention is: When we use time stamp directly 
> with the frequency.
> IGSTKSandbox: IGSTK/Source/igstkView.cxx
> Line 420
> -------------------------------------------------------------------
> TimeStamp renderTime;
> double frequency = m_PulseGenerator->GetFrequency();
> // Frequency is in milliseconds
> renderTime.SetStartTimeNowAndExpireAfter( 1000.0 / frequency );
>                                           ^^^^^^
> -------------------------------------------------------------------
> 
> 
> 
> The reason for changing the unit is to avoid confusion and being 
> consistent.
> Please refer to this function in TimeStamp:
> --------------------------------------------------------------------
> TimeStamp
> ::SetStartTimeNowAndExpireAfter(double millisecondsToExpire)
> {
>   this->m_StartTime      = RealTimeClock::GetTimeStamp();
>   this->m_ExpirationTime = this->m_StartTime + millisecondsToExpire;
> }
> ---------------------------------------------------------------------
> 
> 
> One more place we need to pay attention to is:
> Every time we set the new transform, for instance:
> 
> transform.SetToIdentity( ValidTimePeriod ), the 'ValidTimePeriod' is a 
> duration starting from this moment.
> 
> But programmer might mistakenly program it like:
> transform1.SetToIdentity( transform2.GetExpirationTime() ), Which uses a 
> absolute time, this will be a big mistake.
> 
> We should try to avoid that.
> 
>