[libcamera-devel] [PATCH 2/9] libcamera: timer: Don't reset deadline after time out

[Laurent Pinchart]

Hi Jacopo,

On Sun, Oct 06, 2019 at 08:25:11PM +0200, Jacopo Mondi wrote:
> On Sun, Oct 06, 2019 at 08:32:19AM +0300, Laurent Pinchart wrote:
> > Users of the Timer class may benefit from retrieving the timer deadline
> > after it times out. This is currently not possible as the deadline is
> > reset to 0 when the timer times out or is stopped. Fix this by not
> > resetting the deadline, and adding a new running_ field to the Timer
> > class to implement isRunning().
> >
> > Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> > ---
> >  include/libcamera/timer.h | 1 +
> >  src/libcamera/timer.cpp   | 8 ++++----
> >  2 files changed, 5 insertions(+), 4 deletions(-)
> >
> > diff --git a/include/libcamera/timer.h b/include/libcamera/timer.h
> > index 09f426a59993..3540efb41b6f 100644
> > --- a/include/libcamera/timer.h
> > +++ b/include/libcamera/timer.h
> > @@ -39,6 +39,7 @@ private:
> >  	void registerTimer();
> >  	void unregisterTimer();
> >
> > +	bool running_;
> >  	std::chrono::steady_clock::time_point deadline_;
> >  };
> >
> > diff --git a/src/libcamera/timer.cpp b/src/libcamera/timer.cpp
> > index 34410bab0fb0..8c74e1015e43 100644
> > --- a/src/libcamera/timer.cpp
> > +++ b/src/libcamera/timer.cpp
> > @@ -43,7 +43,7 @@ LOG_DEFINE_CATEGORY(Timer)
> >   * \param[in] parent The parent Object
> >   */
> >  Timer::Timer(Object *parent)
> > -	: Object(parent)
> > +	: Object(parent), running_(false)
> >  {
> >  }
> >
> > @@ -89,17 +89,17 @@ void Timer::start(std::chrono::milliseconds duration)
> >  void Timer::stop()
> >  {
> >  	unregisterTimer();
> > -
> > -	deadline_ = utils::time_point();
> >  }
> >
> >  void Timer::registerTimer()
> >  {
> >  	thread()->eventDispatcher()->registerTimer(this);
> > +	running_ = true;
> >  }
> >
> >  void Timer::unregisterTimer()
> >  {
> > +	running_ = false;
> >  	thread()->eventDispatcher()->unregisterTimer(this);
> >  }
> >
> > @@ -109,7 +109,7 @@ void Timer::unregisterTimer()
> >   */
> >  bool Timer::isRunning() const
> >  {
> > -	return deadline_ != utils::time_point();
> 
> weird, i would have expected this to be deadline < utils::time_point()

utils::timer_point() is a "zero" time point, corresponding to the epoch.
The Timer class use this as a magic value to denote a non-running timer.
That's what this patch removes.

> Anyway
> Reviewed-by: Jacopo Mondi <jacopo at jmondi.org>
> 
> > +	return running_;
> >  }
> >
> >  /**

-- 
Regards,

Laurent Pinchart