[libcamera-devel] [PATCH v8 2/4] cam: event_loop: Rename addEvent to addFDEvent

[Laurent Pinchart]

On Wed, May 18, 2022 at 12:22:03AM +0100, Kieran Bingham via libcamera-devel wrote:
> Quoting Eric Curtin via libcamera-devel (2022-05-05 16:18:49)
> > Since we now have two types of events addFDEvent and addTimerEvent.
> 
> I think this could be expanded a little:
> 
> """
> With the addition of addTimerEvent, the naming of addEvent is specific
> to the management of an fd, while the naming is generic.
> 
> Update the name to make the naming scheme consistent in specifying the
> type of event to be added.
> """

Looks good to me.

I would actually have moved this patch first in the series, but that
requires rewording the commit message, it's not worth the effort at this
point.

> But that's optional, and could be adapted as best fits.
> 
> It looks like the change is fine so:
> 
> 
> Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> 
> 
> > Signed-off-by: Eric Curtin <ecurtin at redhat.com>
> > ---
> >  src/cam/drm.cpp        | 4 ++--
> >  src/cam/event_loop.cpp | 4 ++--
> >  src/cam/event_loop.h   | 4 ++--
> >  3 files changed, 6 insertions(+), 6 deletions(-)
> > 
> > diff --git a/src/cam/drm.cpp b/src/cam/drm.cpp
> > index 46e34eb5..421cd61a 100644
> > --- a/src/cam/drm.cpp
> > +++ b/src/cam/drm.cpp
> > @@ -432,8 +432,8 @@ int Device::init()
> >         if (ret < 0)
> >                 return ret;
> >  
> > -       EventLoop::instance()->addEvent(fd_, EventLoop::Read,
> > -                                       std::bind(&Device::drmEvent, this));
> > +       EventLoop::instance()->addFDEvent(fd_, EventLoop::Read,
> > +                                         std::bind(&Device::drmEvent, this));
> >  
> >         return 0;
> >  }
> > diff --git a/src/cam/event_loop.cpp b/src/cam/event_loop.cpp
> > index 181c971c..0c2176c7 100644
> > --- a/src/cam/event_loop.cpp
> > +++ b/src/cam/event_loop.cpp
> > @@ -64,8 +64,8 @@ void EventLoop::callLater(const std::function<void()> &func)
> >         event_base_once(base_, -1, EV_TIMEOUT, dispatchCallback, this, nullptr);
> >  }
> >  
> > -void EventLoop::addEvent(int fd, EventType type,
> > -                        const std::function<void()> &callback)
> > +void EventLoop::addFDEvent(int fd, EventType type,

The function should be named addFdEvent().

With this changed and the commit message updated,

Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

> > +                          const std::function<void()> &callback)
> >  {
> >         std::unique_ptr<Event> event = std::make_unique<Event>(callback);
> >         short events = (type & Read ? EV_READ : 0)
> > diff --git a/src/cam/event_loop.h b/src/cam/event_loop.h
> > index 89215dce..d921cd8f 100644
> > --- a/src/cam/event_loop.h
> > +++ b/src/cam/event_loop.h
> > @@ -36,8 +36,8 @@ public:
> >  
> >         void callLater(const std::function<void()> &func);
> >  
> > -       void addEvent(int fd, EventType type,
> > -                     const std::function<void()> &handler);
> > +       void addFDEvent(int fd, EventType type,
> > +                       const std::function<void()> &handler);
> >  
> >         void addTimerEvent(const std::chrono::milliseconds d,
> >                            const std::function<void()> &handler);

-- 
Regards,

Laurent Pinchart