[libcamera-devel] [PATCH v8 1/4] cam: event_loop: Add timer events to event loop

[Kieran Bingham]

Quoting Eric Curtin via libcamera-devel (2022-05-05 16:18:48)
> Extend the EventLoop class to support periodic timer events. This can be
> used to run tasks periodically, such as handling the event loop of SDL.
> 
> Also delete all events in the list, before we event_base_loopbreak.
> 
> Signed-off-by: Eric Curtin <ecurtin at redhat.com>
> ---
>  src/cam/event_loop.cpp | 33 +++++++++++++++++++++++++++++++++
>  src/cam/event_loop.h   |  6 ++++++
>  2 files changed, 39 insertions(+)
> 
> diff --git a/src/cam/event_loop.cpp b/src/cam/event_loop.cpp
> index e25784c0..181c971c 100644
> --- a/src/cam/event_loop.cpp
> +++ b/src/cam/event_loop.cpp
> @@ -47,6 +47,10 @@ int EventLoop::exec()
>  void EventLoop::exit(int code)
>  {
>         exitCode_ = code;
> +       for (auto const &e : events_) {
> +               event_del(e->event_);
> +       }

No need for { } on a single line statement.

> +
>         event_base_loopbreak(base_);
>  }
>  
> @@ -84,6 +88,35 @@ void EventLoop::addEvent(int fd, EventType type,
>         events_.push_back(std::move(event));
>  }
>  
> +void EventLoop::toTimeval(const std::chrono::milliseconds d, struct timeval &tv)
> +{
> +       tv.tv_sec = std::chrono::duration_cast<std::chrono::seconds>(d).count();

Do you need to subtract the seconds from here now?

> +       tv.tv_usec = std::chrono::duration_cast<std::chrono::microseconds>(d).count();

Or does this wrap accordingly?

I.e. would 1.5 seconds end up as :
	  tv.tv_sec = 1;
	  tv.tv_usec = 1500000;

With that resolved, I think that's a 


Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>

> +}
> +
> +void EventLoop::addTimerEvent(const std::chrono::milliseconds d,
> +                             const std::function<void()> &callback)
> +{
> +       std::unique_ptr<Event> event = std::make_unique<Event>(callback);
> +       event->event_ = event_new(base_, -1, EV_PERSIST, &EventLoop::Event::dispatch,
> +                                 event.get());
> +       if (!event->event_) {
> +               std::cerr << "Failed to create timer event" << std::endl;
> +               return;
> +       }
> +
> +       struct timeval tv;
> +       toTimeval(d, tv);

It's a shame that it's frowned upon to override std namespace to have
custom constructors or having a construction to allow:
	struct timeval tv(d);

would be nice ... (but I don't think it's something we should do here).

--
Kieran


> +
> +       int ret = event_add(event->event_, &tv);
> +       if (ret < 0) {
> +               std::cerr << "Failed to add timer event" << std::endl;
> +               return;
> +       }
> +
> +       events_.push_back(std::move(event));
> +}
> +
>  void EventLoop::dispatchCallback([[maybe_unused]] evutil_socket_t fd,
>                                  [[maybe_unused]] short flags, void *param)
>  {
> diff --git a/src/cam/event_loop.h b/src/cam/event_loop.h
> index a4613eb2..89215dce 100644
> --- a/src/cam/event_loop.h
> +++ b/src/cam/event_loop.h
> @@ -14,6 +14,8 @@
>  
>  #include <event2/util.h>
>  
> +using namespace std::chrono_literals;
> +
>  struct event_base;
>  
>  class EventLoop
> @@ -37,6 +39,9 @@ public:
>         void addEvent(int fd, EventType type,
>                       const std::function<void()> &handler);
>  
> +       void addTimerEvent(const std::chrono::milliseconds d,
> +                          const std::function<void()> &handler);
> +
>  private:
>         struct Event {
>                 Event(const std::function<void()> &callback);
> @@ -57,6 +62,7 @@ private:
>         std::list<std::unique_ptr<Event>> events_;
>         std::mutex lock_;
>  
> +       void toTimeval(const std::chrono::milliseconds d, struct timeval &tv);
>         static void dispatchCallback(evutil_socket_t fd, short flags,
>                                      void *param);
>         void dispatchCall();
> -- 
> 2.35.1
>