[IGSTK-Developers] Clock and TimeStamp problem

[Patrick Cheng]

Hi Luis,

Adding type check is a good idea for this issue, I totally agree on that.

Patrick

Luis Ibanez wrote:
> 
> 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.
>>
>>
> 
> 
>