TimeStamp::ok()

Since all getTimeStamp are not returning a boolean of success, some wrong TimeStamp should be defined to allow getters to provide a "wrong" output. Now it is 0. But it is used in many tests. Maybe it is still ok. Let's discuss it.

added Agreement required label

mentioned in issue #344 (closed)

created merge request !381 (merged) to address this issue

mentioned in merge request !381 (merged)

assigned to @jsola

closed via merge request !381 (merged)

mentioned in commit 9189a5ee

I added some assertions to ensure that provided timestamp is ok and several tests failed. Maybe the constructor from double should include 0 as valid value: (>= instead of the current >)

TimeStamp::TimeStamp(const double& _ts) :
        time_stamp_nano_(_ts > 0 ? (unsigned long int)(_ts*1e9) : 0),
        is_valid_(_ts >= 0)
{
    //
}

reopened

We have implementations Xxx.getTimeStamp(), in:

• Problem
• Processor
• Frame
• Capture

Protecting all calls to all these functions, I mean check t.ok() after the call, seems like a lot of work... and in many cases it is maybe not necessary.

Instead, maybe we can start by putting an assert in each one of these getTimeStamp() with a clear message,

TimeStamp Xxx::getTimeStamp() const
{
  assert (this->time_stamp_.ok() && "Xxx: Invalid timestamp has been requested!");
  return time_stamp_;
}

At least this will help debugging situations with invalid timestamps.

Also, we can add a warning (not an assert) in TimeStamp class each time a time stamp is set in a way that becomes invalid. This affects constructors, operator =, and function set() in class TimeStamp, e.g.

void TimeStamp::set(double _ts)
{
  WOLF_WARN(ts>=0, "TimeStamp: setting value results in invalid TimeStamp!");
  this->ts_ = _ts; // dummy implementation only for illustration purposes
}

With the assert combined with the warning, I think it can be relatively powerful to track possible bugs related to invalid timestamps.

Later on, and in view of the experience of cases of assert violation, we can then think about better strategies.

OK maybe the assert should also be a WOLF_WARN just to allow the execution to proceed. Maybe the assert is too severe. TBD

In my opinion, all functions that take a TimeStamp as an input should either have an assertion or handle an invalid input. Maybe it is a lot of work, but it is not super-urgent. I would leave this issue opened as a TODO(release) issue.