[libcamera-devel,5/9] libcamera: timer: Allow restarting a timer before expiration

Message ID 20191006053226.8976-6-laurent.pinchart@ideasonboard.com
State Accepted
Headers show
Series
  • Timer fixes and enhancements
Related show

Commit Message

Laurent Pinchart Oct. 6, 2019, 5:32 a.m. UTC
The Timer API allows restarting a timer before expiration. This isn't
correctly implemented, fix it.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 src/libcamera/timer.cpp | 6 ++++++
 1 file changed, 6 insertions(+)

Comments

Jacopo Mondi Oct. 6, 2019, 6:59 p.m. UTC | #1
Hi Laurent

On Sun, Oct 06, 2019 at 08:32:22AM +0300, Laurent Pinchart wrote:
> The Timer API allows restarting a timer before expiration. This isn't
> correctly implemented, fix it.

How come the timer.cpp test which test restarts was not failing ?
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
>  src/libcamera/timer.cpp | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/src/libcamera/timer.cpp b/src/libcamera/timer.cpp
> index 8c74e1015e43..5d4e52713e6e 100644
> --- a/src/libcamera/timer.cpp
> +++ b/src/libcamera/timer.cpp
> @@ -75,6 +75,9 @@ void Timer::start(std::chrono::milliseconds duration)
>  		<< duration.count() << ": deadline "
>  		<< utils::time_point_to_string(deadline_);
>
> +	if (isRunning())
> +		unregisterTimer();
> +
>  	registerTimer();
>  }
>
> @@ -88,6 +91,9 @@ void Timer::start(std::chrono::milliseconds duration)
>   */
>  void Timer::stop()
>  {
> +	if (!isRunning())
> +		return;
> +
>  	unregisterTimer();
>  }
>
> --
> Regards,
>
> Laurent Pinchart
>
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel@lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel
Laurent Pinchart Oct. 7, 2019, 3:01 a.m. UTC | #2
Hi Jacopo,

On Sun, Oct 06, 2019 at 08:59:19PM +0200, Jacopo Mondi wrote:
> On Sun, Oct 06, 2019 at 08:32:22AM +0300, Laurent Pinchart wrote:
> > The Timer API allows restarting a timer before expiration. This isn't
> > correctly implemented, fix it.
> 
> How come the timer.cpp test which test restarts was not failing ?

Because it was testing restart after expiration, not before it.

> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > ---
> >  src/libcamera/timer.cpp | 6 ++++++
> >  1 file changed, 6 insertions(+)
> >
> > diff --git a/src/libcamera/timer.cpp b/src/libcamera/timer.cpp
> > index 8c74e1015e43..5d4e52713e6e 100644
> > --- a/src/libcamera/timer.cpp
> > +++ b/src/libcamera/timer.cpp
> > @@ -75,6 +75,9 @@ void Timer::start(std::chrono::milliseconds duration)
> >  		<< duration.count() << ": deadline "
> >  		<< utils::time_point_to_string(deadline_);
> >
> > +	if (isRunning())
> > +		unregisterTimer();
> > +
> >  	registerTimer();
> >  }
> >
> > @@ -88,6 +91,9 @@ void Timer::start(std::chrono::milliseconds duration)
> >   */
> >  void Timer::stop()
> >  {
> > +	if (!isRunning())
> > +		return;
> > +
> >  	unregisterTimer();
> >  }
> >

Patch

diff --git a/src/libcamera/timer.cpp b/src/libcamera/timer.cpp
index 8c74e1015e43..5d4e52713e6e 100644
--- a/src/libcamera/timer.cpp
+++ b/src/libcamera/timer.cpp
@@ -75,6 +75,9 @@  void Timer::start(std::chrono::milliseconds duration)
 		<< duration.count() << ": deadline "
 		<< utils::time_point_to_string(deadline_);
 
+	if (isRunning())
+		unregisterTimer();
+
 	registerTimer();
 }
 
@@ -88,6 +91,9 @@  void Timer::start(std::chrono::milliseconds duration)
  */
 void Timer::stop()
 {
+	if (!isRunning())
+		return;
+
 	unregisterTimer();
 }