[libcamera-devel] [PATCH 1/9] libcamera: timer: Remove the interval() method

[Jacopo Mondi]

Hi Laurent,

On Sun, Oct 06, 2019 at 08:32:18AM +0300, Laurent Pinchart wrote:
> The libcamera timers are single-shot timers. They are started with a
> duration, but fire once only, not based on an interval. Remove the
> interval concept by removing the interval() method, and rename other
> occurences of interval to duration.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> ---
>  include/libcamera/timer.h        |  4 +---
>  src/libcamera/timer.cpp          | 19 ++++++-------------
>  src/qcam/qt_event_dispatcher.cpp |  7 ++++++-
>  3 files changed, 13 insertions(+), 17 deletions(-)
>
> diff --git a/include/libcamera/timer.h b/include/libcamera/timer.h
> index 476ae45f1e53..09f426a59993 100644
> --- a/include/libcamera/timer.h
> +++ b/include/libcamera/timer.h
> @@ -24,11 +24,10 @@ public:
>  	~Timer();
>
>  	void start(unsigned int msec) { start(std::chrono::milliseconds(msec)); }
> -	void start(std::chrono::milliseconds interval);
> +	void start(std::chrono::milliseconds duration);
>  	void stop();
>  	bool isRunning() const;
>
> -	std::chrono::milliseconds interval() const { return interval_; }
>  	std::chrono::steady_clock::time_point deadline() const { return deadline_; }
>
>  	Signal<Timer *> timeout;
> @@ -40,7 +39,6 @@ private:
>  	void registerTimer();
>  	void unregisterTimer();
>
> -	std::chrono::milliseconds interval_;
>  	std::chrono::steady_clock::time_point deadline_;
>  };
>
> diff --git a/src/libcamera/timer.cpp b/src/libcamera/timer.cpp
> index b3cea3dadb49..34410bab0fb0 100644
> --- a/src/libcamera/timer.cpp
> +++ b/src/libcamera/timer.cpp
> @@ -61,19 +61,18 @@ Timer::~Timer()
>   */
>
>  /**
> - * \brief Start or restart the timer with a timeout of \a interval
> - * \param[in] interval The timer duration in milliseconds
> + * \brief Start or restart the timer with a timeout of \a duration
> + * \param[in] duration The timer duration in milliseconds
>   *
>   * If the timer is already running it will be stopped and restarted.
>   */
> -void Timer::start(std::chrono::milliseconds interval)
> +void Timer::start(std::chrono::milliseconds duration)
>  {
> -	interval_ = interval;
> -	deadline_ = utils::clock::now() + interval;
> +	deadline_ = utils::clock::now() + duration;
>
>  	LOG(Timer, Debug)
> -		<< "Starting timer " << this << " with interval "
> -		<< interval.count() << ": deadline "
> +		<< "Starting timer " << this << " with duration "
> +		<< duration.count() << ": deadline "
>  		<< utils::time_point_to_string(deadline_);

Shouldn't you print the duration instead of the expected deadline
here?

>
>  	registerTimer();
> @@ -113,12 +112,6 @@ bool Timer::isRunning() const
>  	return deadline_ != utils::time_point();
>  }
>
> -/**
> - * \fn Timer::interval()
> - * \brief Retrieve the timer interval
> - * \return The timer interval in milliseconds
> - */
> -
>  /**
>   * \fn Timer::deadline()
>   * \brief Retrieve the timer deadline
> diff --git a/src/qcam/qt_event_dispatcher.cpp b/src/qcam/qt_event_dispatcher.cpp
> index 994af3ead82a..9e989bef7d53 100644
> --- a/src/qcam/qt_event_dispatcher.cpp
> +++ b/src/qcam/qt_event_dispatcher.cpp
> @@ -5,6 +5,7 @@
>   * qt_event_dispatcher.cpp - qcam - Qt-based event dispatcher
>   */
>
> +#include <chrono>
>  #include <iostream>
>
>  #include <QAbstractEventDispatcher>
> @@ -112,7 +113,11 @@ void QtEventDispatcher::exceptionNotifierActivated(int socket)
>
>  void QtEventDispatcher::registerTimer(Timer *timer)
>  {
> -	int timerId = startTimer(timer->interval());
> +	std::chrono::steady_clock::time_point now = std::chrono::steady_clock::now();
> +	std::chrono::steady_clock::duration duration = timer->deadline() - now;

Would it make sense an helper to get this from the timer ? (the
deadline_ - now part). Nothing urgent though

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

Thanks
   j

> +	std::chrono::milliseconds msec =
> +		std::chrono::duration_cast<std::chrono::milliseconds>(duration);
> +	int timerId = startTimer(msec);
>  	timers_[timerId] = timer;
>  	timerIds_[timer] = timerId;
>  }
> --
> Regards,
>
> Laurent Pinchart
>
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://lists.libcamera.org/pipermail/libcamera-devel/attachments/20191006/d53ba305/attachment.sig>