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

2107 diff mbox series

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@ideasonboard.com>
---
 include/libcamera/timer.h | 1 +
 src/libcamera/timer.cpp   | 8 ++++----
 2 files changed, 5 insertions(+), 4 deletions(-)

Hi Laurent,

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@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()

Anyway
Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>

Thanks
  j

> +	return running_;
>  }
>
>  /**
> --
> Regards,
>
> Laurent Pinchart
>
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel@lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel

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@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@jmondi.org>
> 
> > +	return running_;
> >  }
> >
> >  /**